Hi Maxime On Thu, 17 Sep 2020 at 13:16, Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > The vc4 display engine has a first controller called the HVS that will > perform the composition of the planes. That HVS has 3 FIFOs and can > therefore compose planes for up to three outputs. The timings part is > generated through a component called the Pixel Valve, and the BCM2711 has 6 > of them. > > Thus, the HVS has some bits to control which FIFO gets output to which > Pixel Valve. The current code supports that muxing by looking at all the > CRTCs in a new DRM atomic state in atomic_check, and given the set of > contraints that we have, assigns FIFOs to CRTCs or reject the mode s/contraints/constraints > entirely. The actual muxing will occur during atomic_commit. > > However, that doesn't work if only a fraction of the CRTCs' state is > updated in that state, since it will ignore the CRTCs that are kept running > unmodified, and will thus unassign its associated FIFO, and later disable > it. Yup, that would surely mess things up :) > In order to make the code work as expected, let's pull the CRTC state of > all the enabled CRTC in our atomic_check so that we can operate on all the > running CRTCs, no matter whether they are affected by the new state or not. > > Cc: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> > Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> Tested-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> Thanks Dave > --- > drivers/gpu/drm/vc4/vc4_kms.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c > index 16e233e1406e..af3ee3dcdab6 100644 > --- a/drivers/gpu/drm/vc4/vc4_kms.c > +++ b/drivers/gpu/drm/vc4/vc4_kms.c > @@ -620,6 +620,23 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) > struct drm_crtc *crtc; > int i, ret; > > + /* > + * Since the HVS FIFOs are shared across all the pixelvalves and > + * the TXP (and thus all the CRTCs), we need to pull the current > + * state of all the enabled CRTCs so that an update to a single > + * CRTC still keeps the previous FIFOs enabled and assigned to > + * the same CRTCs, instead of evaluating only the CRTC being > + * modified. > + */ > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > + if (!crtc->state->enable) > + continue; > + > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + } > + > for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > struct vc4_crtc_state *vc4_crtc_state = > to_vc4_crtc_state(crtc_state); > -- > 2.26.2 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel