| Message ID | 0b0cc2971b2abc8fe0fc55a9ae652a7b8a7235cd.1687423204.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: renesas: shmobile: Atomic conversion + DT support | expand |
Hi Geert, Thank you for the patch. On Thu, Jun 22, 2023 at 11:21:42AM +0200, Geert Uytterhoeven wrote: > Turning a CRTC off will prevent a queued page flip from ever completing, > potentially confusing userspace. Wait for queued page flips to complete > before turning the CRTC off to avoid this. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Given that you're duplicating the rcar-du page flip handling code, I have a feeling core helpers would be handy. It's not a blocker for this series though, so Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 37 +++++++++++++++++++ > .../gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 3 ++ > 2 files changed, 40 insertions(+) > > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c > index 1d7fcf64bf2aab80..d2a0ac5f9368c11c 100644 > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c > @@ -285,11 +285,40 @@ void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc) > scrtc->event = NULL; > if (event) { > drm_crtc_send_vblank_event(&scrtc->base, event); > + wake_up(&scrtc->flip_wait); > drm_crtc_vblank_put(&scrtc->base); > } > spin_unlock_irqrestore(&dev->event_lock, flags); > } > > +static bool shmob_drm_crtc_page_flip_pending(struct shmob_drm_crtc *scrtc) > +{ > + struct drm_device *dev = scrtc->base.dev; > + unsigned long flags; > + bool pending; > + > + spin_lock_irqsave(&dev->event_lock, flags); > + pending = scrtc->event != NULL; > + spin_unlock_irqrestore(&dev->event_lock, flags); > + > + return pending; > +} > + > +static void shmob_drm_crtc_wait_page_flip(struct shmob_drm_crtc *scrtc) > +{ > + struct drm_crtc *crtc = &scrtc->base; > + struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); > + > + if (wait_event_timeout(scrtc->flip_wait, > + !shmob_drm_crtc_page_flip_pending(scrtc), > + msecs_to_jiffies(50))) > + return; > + > + dev_warn(sdev->dev, "page flip timeout\n"); > + > + shmob_drm_crtc_finish_page_flip(scrtc); > +} > + > /* ----------------------------------------------------------------------------- > * CRTC Functions > */ > @@ -302,6 +331,12 @@ static void shmob_drm_crtc_stop(struct shmob_drm_crtc *scrtc) > if (!scrtc->started) > return; > > + /* > + * Wait for page flip completion before stopping the CRTC as userspace > + * expects page flips to eventually complete. > + */ > + shmob_drm_crtc_wait_page_flip(scrtc); > + > /* Stop the LCDC. */ > shmob_drm_crtc_start_stop(scrtc, false); > > @@ -515,6 +550,8 @@ int shmob_drm_crtc_create(struct shmob_drm_device *sdev) > unsigned int i; > int ret; > > + init_waitqueue_head(&sdev->crtc.flip_wait); > + > sdev->crtc.dpms = DRM_MODE_DPMS_OFF; > > primary = shmob_drm_plane_create(sdev, 0); > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h > index 2c6d7541427581a6..b9863e026e8a9b83 100644 > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h > @@ -14,6 +14,8 @@ > #include <drm/drm_connector.h> > #include <drm/drm_encoder.h> > > +#include <linux/wait.h> > + > #include <video/videomode.h> > > struct drm_pending_vblank_event; > @@ -24,6 +26,7 @@ struct shmob_drm_crtc { > struct drm_crtc base; > > struct drm_pending_vblank_event *event; > + wait_queue_head_t flip_wait; > int dpms; > > const struct shmob_drm_format_info *format;
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index 1d7fcf64bf2aab80..d2a0ac5f9368c11c 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -285,11 +285,40 @@ void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc) scrtc->event = NULL; if (event) { drm_crtc_send_vblank_event(&scrtc->base, event); + wake_up(&scrtc->flip_wait); drm_crtc_vblank_put(&scrtc->base); } spin_unlock_irqrestore(&dev->event_lock, flags); } +static bool shmob_drm_crtc_page_flip_pending(struct shmob_drm_crtc *scrtc) +{ + struct drm_device *dev = scrtc->base.dev; + unsigned long flags; + bool pending; + + spin_lock_irqsave(&dev->event_lock, flags); + pending = scrtc->event != NULL; + spin_unlock_irqrestore(&dev->event_lock, flags); + + return pending; +} + +static void shmob_drm_crtc_wait_page_flip(struct shmob_drm_crtc *scrtc) +{ + struct drm_crtc *crtc = &scrtc->base; + struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); + + if (wait_event_timeout(scrtc->flip_wait, + !shmob_drm_crtc_page_flip_pending(scrtc), + msecs_to_jiffies(50))) + return; + + dev_warn(sdev->dev, "page flip timeout\n"); + + shmob_drm_crtc_finish_page_flip(scrtc); +} + /* ----------------------------------------------------------------------------- * CRTC Functions */ @@ -302,6 +331,12 @@ static void shmob_drm_crtc_stop(struct shmob_drm_crtc *scrtc) if (!scrtc->started) return; + /* + * Wait for page flip completion before stopping the CRTC as userspace + * expects page flips to eventually complete. + */ + shmob_drm_crtc_wait_page_flip(scrtc); + /* Stop the LCDC. */ shmob_drm_crtc_start_stop(scrtc, false); @@ -515,6 +550,8 @@ int shmob_drm_crtc_create(struct shmob_drm_device *sdev) unsigned int i; int ret; + init_waitqueue_head(&sdev->crtc.flip_wait); + sdev->crtc.dpms = DRM_MODE_DPMS_OFF; primary = shmob_drm_plane_create(sdev, 0); diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h index 2c6d7541427581a6..b9863e026e8a9b83 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h @@ -14,6 +14,8 @@ #include <drm/drm_connector.h> #include <drm/drm_encoder.h> +#include <linux/wait.h> + #include <video/videomode.h> struct drm_pending_vblank_event; @@ -24,6 +26,7 @@ struct shmob_drm_crtc { struct drm_crtc base; struct drm_pending_vblank_event *event; + wait_queue_head_t flip_wait; int dpms; const struct shmob_drm_format_info *format;
Turning a CRTC off will prevent a queued page flip from ever completing, potentially confusing userspace. Wait for queued page flips to complete before turning the CRTC off to avoid this. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 37 +++++++++++++++++++ .../gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 3 ++ 2 files changed, 40 insertions(+)