| Message ID | 1637680811-90510-1-git-send-email-mrodin@de.adit-jv.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: rcar-du: do not restart rcar-du groups on gen3 | expand |
Hi Michael, Sorry for getting back to you so late, your patch got burried in my inbox. On Tue, Nov 23, 2021 at 04:20:11PM +0100, Michael Rodin wrote: > 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: I can confirm the symptoms. > 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. I've tested this patch, and it breaks the HDMI output on my Salvator-XS M3N board. My test setup has a panel connected to LVDS0 and an HDMI monitor connected to HDMI0. The kernel is configured with fbdev emulation enabled. When the system boots, I get two penguins on the LVDS panel and the HDMI monitor. I then run $ modetest -M rcar-du -s HDMI-A-1@60:1024x768@XR24 (you may need to change the CRTC number depending on your setup) Without this patch, I see a brief flicker on the LVDS panel, and a test pattern on the HDMI monitor. With this patch, the flicker is gone, but my HDMI monitor show an error message that complains about unsupported timings. The Gen3 documentation still documents many register bits as being updated on DRES. I don't think we can get rid of group restart like this. > Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com> > --- > drivers/gpu/drm/rcar-du/rcar_du_group.c | 5 ++++- > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 -- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c > index 8665a1d..ff0a1c8 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -250,7 +250,7 @@ void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) > * when the display controller will have to be restarted. > */ > if (start) { > - if (rgrp->used_crtcs++ != 0) > + if (rgrp->used_crtcs++ != 0 && rgrp->dev->info->gen != 3) > __rcar_du_group_start_stop(rgrp, false); > __rcar_du_group_start_stop(rgrp, true); > } else { > @@ -263,6 +263,9 @@ void rcar_du_group_restart(struct rcar_du_group *rgrp) > { > rgrp->need_restart = false; > > + if (rgrp->dev->info->gen == 3) > + return; > + > __rcar_du_group_start_stop(rgrp, false); > __rcar_du_group_start_stop(rgrp, true); > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > index b7fc5b0..a652c06 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > @@ -88,8 +88,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) > * 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; >
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 8665a1d..ff0a1c8 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -250,7 +250,7 @@ void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) * when the display controller will have to be restarted. */ if (start) { - if (rgrp->used_crtcs++ != 0) + if (rgrp->used_crtcs++ != 0 && rgrp->dev->info->gen != 3) __rcar_du_group_start_stop(rgrp, false); __rcar_du_group_start_stop(rgrp, true); } else { @@ -263,6 +263,9 @@ void rcar_du_group_restart(struct rcar_du_group *rgrp) { rgrp->need_restart = false; + if (rgrp->dev->info->gen == 3) + return; + __rcar_du_group_start_stop(rgrp, false); __rcar_du_group_start_stop(rgrp, true); } diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index b7fc5b0..a652c06 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -88,8 +88,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) * 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;
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. Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com> --- drivers/gpu/drm/rcar-du/rcar_du_group.c | 5 ++++- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-)