Hello,
This patch series avoids flicker in some scenarios related to dual output configuration.
The issue was originally reported by Michael Rodin in [1]. The problem is described in details there, and copied here to facilitate discussion:
-------- Restarting a display unit group can cause a visible flicker on the display. Particularly when a LVDS display is connected to a Salvator board and an HDMI display is (re)connected, then there will be 2 visible flickers on the LVDS display:
1. during atomic_flush (The need_restart flag is set in this case by rcar_du_vsp_enable.): rcar_du_crtc_atomic_flush rcar_du_crtc_update_planes ... ... /* Restart the group if plane sources have changed. */ if (rcrtc->group->need_restart) rcar_du_group_restart(rcrtc->group); 2. during atomic_enable: rcar_du_crtc_atomic_enable rcar_du_crtc_start rcar_du_group_start_stop(rcrtc->group, true);
To avoid flickers in all use cases, do not restart DU groups on the Gen3 SoCs at all, since it is not required any more. --------
The proposed patch unfortunately introduced a regression. This series fixes the issue in the first scenario described above. The second scenario still leads to flicker, and I don't think that can be fixed as the hardware requires the whole group of outputs to be stopped for some register changes to take effect.
[1] https://lore.kernel.org/dri-devel/1637680811-90510-1-git-send-email-mrodin@d...
Laurent Pinchart (2): drm: rcar-du: Don't select VSP1 sink on Gen3 drm: rcar-du: Don't restart group when enabling plane on Gen3
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 12 ++++++++++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 9 --------- 2 files changed, 10 insertions(+), 11 deletions(-)
The VSP1 sink selection through register DEFR8 is only available on Gen2 hardware. Skip it on Gen3.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 862197be1e01..9b058d6cb032 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -549,8 +549,10 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, rcar_du_plane_setup_format(rgrp, (state->hwindex + 1) % 8, state);
- if (rcdu->info->gen < 3) - rcar_du_plane_setup_scanout(rgrp, state); + if (rcdu->info->gen >= 3) + return; + + rcar_du_plane_setup_scanout(rgrp, state);
if (state->source == RCAR_DU_PLANE_VSPD1) { unsigned int vspd1_sink = rgrp->index ? 2 : 0;
Quoting Laurent Pinchart (2022-02-21 17:13:39)
The VSP1 sink selection through register DEFR8 is only available on Gen2 hardware. Skip it on Gen3.
aha, interesting, and I see how this leads into the second patch.
Looks fine on it's own too though:
Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 862197be1e01..9b058d6cb032 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -549,8 +549,10 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, rcar_du_plane_setup_format(rgrp, (state->hwindex + 1) % 8, state);
if (rcdu->info->gen < 3)rcar_du_plane_setup_scanout(rgrp, state);
if (rcdu->info->gen >= 3)return;rcar_du_plane_setup_scanout(rgrp, state); if (state->source == RCAR_DU_PLANE_VSPD1) { unsigned int vspd1_sink = rgrp->index ? 2 : 0;-- Regards,
Laurent Pinchart
On Gen3 hardware enabling a VSP plane doesn't change any register that requires DRES to take effect. Avoid a group restart in that case.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 9 --------- 2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9b058d6cb032..22aeeb1cc1fb 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -560,6 +560,12 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, if (rcdu->vspd1_sink != vspd1_sink) { rcdu->vspd1_sink = vspd1_sink; rcar_du_set_dpad0_vsp1_routing(rcdu); + + /* + * Changes to the VSP1 sink take effect on DRES and thus + * need a restart of the group. + */ + rgrp->need_restart = true; } } } diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index b7fc5b069cbc..32530d698e75 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -84,15 +84,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
__rcar_du_plane_setup(crtc->group, &state);
- /* - * Ensure that the plane source configuration takes effect by requesting - * a restart of the group. See rcar_du_plane_atomic_update() for a more - * detailed explanation. - * - * TODO: Check whether this is still needed on Gen3. - */ - crtc->group->need_restart = true; - vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, &cfg); }
Quoting Laurent Pinchart (2022-02-21 17:13:40)
On Gen3 hardware enabling a VSP plane doesn't change any register that requires DRES to take effect. Avoid a group restart in that case.
This also makes it clear that the need_restart is due to the change occuring in the VSP1 sink, so I think this is cleaner all round.
Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++++++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 9 --------- 2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9b058d6cb032..22aeeb1cc1fb 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -560,6 +560,12 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, if (rcdu->vspd1_sink != vspd1_sink) { rcdu->vspd1_sink = vspd1_sink; rcar_du_set_dpad0_vsp1_routing(rcdu);
/** Changes to the VSP1 sink take effect on DRES and thus* need a restart of the group.*/rgrp->need_restart = true; } }} diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index b7fc5b069cbc..32530d698e75 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -84,15 +84,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
__rcar_du_plane_setup(crtc->group, &state);
/** Ensure that the plane source configuration takes effect by requesting* a restart of the group. See rcar_du_plane_atomic_update() for a more* detailed explanation.** TODO: Check whether this is still needed on Gen3.*/crtc->group->need_restart = true;vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, &cfg);}
-- Regards,
Laurent Pinchart
dri-devel@lists.freedesktop.org
-
Kieran Bingham -
Laurent Pinchart