Hi Maxime,
On 9/17/20 9:16 PM, Maxime Ripard 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
> 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.
>
> 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.k...@samsung.com>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard <max...@cerno.tech>
Thanks for the quick patch and detailed explanation.
Tested-by: Hoegeun Kwon <hoegeun.k...@samsung.com>
Reviewed-by: Hoegeun Kwon <hoegeun.k...@samsung.com>
Best regards,
Hoegeun
> ---
> 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);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
