Hi, A few suggestions below. But I'm not a native speaker. Am 05.11.20 um 14:56 schrieb Maxime Ripard:
We've had a number of muxing corner-cases with specific ways to reproduce them, so let's document them to make sure they aren't lost and introduce regressions later on. Reviewed-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> Tested-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> --- drivers/gpu/drm/vc4/vc4_kms.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index bb2efc5d2d01..499c6914fce4 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -662,6 +662,28 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4) return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL); }+/*+ * The BCM2711 HVS has up to 7 output connected to the pixelvalves and
'7 outputs' Is it 'pixelvalves' or 'pixel valves'?
+ * the TXP (and therefore all the CRTCs found on that platform). + * + * The naive (and our initial) implementation would just iterate over + * all the active CRTCs, try to find a suitable FIFO, and then remove it + * from the available FIFOs pool. However, there's a few corner cases
I'd write. 'and remove it from the pool of available FIFOs'. Sounds more natural to me.
'there are a few'
+ * that need to be considered: + * + * - When running in a dual-display setup (so with two CRTCs involved), + * we can update the state of a single CRTC (for example by changing + * its mode using xrandr under X11) without affecting the other. In + * this case, the other CRTC wouldn't be in the state at all, so we + * need to consider all the running CRTCs in the DRM device to assign + * a FIFO, not just the one in the state. + * + * - Since we need the pixelvalve to be disabled and enabled back when + * the FIFO is changed, we should keep the FIFO assigned for as long + * as the CRTC is enabled, only considering it free again once that + * CRTC has been disabled. This can be tested by booting X11 on a + * single display, and changing the resolution down and then back up. + */
With my suggestions considered, Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
OpenPGP_0x680DC11D530B7A23.asc
Description: application/pgp-keys
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
