| Message ID | 1433408582-9828-42-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Thursday 04 June 2015 12:02:58 Tomi Valkeinen wrote: > omap_crtc_disable() waits for pending page flips to finish. However, > omap_crtc_disable() is always called via omap_atomic_complete() and we > never have page flips going on at that time. Why is that ? Because our omap_atomic_complete() implementation waits for vblanks before allowing the next atomic commit to run, and that our vblank IRQ handler completes all pending page flips ? If so, I believe we have a few corner cases that won't work properly. For instance, if the user performs a full mode set on a CRTC without change the framebuffer, an event can be queued but drm_atomic_helper_wait_for_vblanks() won't wait for vblank on that CRTC as framebuffer_changed() will return false. If the next commit then disables the CRTC the event might not have completed yet. > So remove the omap_crtc_wait_page_flip() and related code. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_crtc.c | 32 -------------------------------- > 1 file changed, 32 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c > b/drivers/gpu/drm/omapdrm/omap_crtc.c index abfafd1600b8..2c0d91a67418 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > @@ -51,7 +51,6 @@ struct omap_crtc { > > /* pending event */ > struct drm_pending_vblank_event *event; > - wait_queue_head_t flip_wait; > > struct completion completion; > > @@ -277,41 +276,12 @@ static void omap_crtc_complete_page_flip(struct > drm_crtc *crtc) else > event->base.destroy(&event->base); > > - wake_up(&omap_crtc->flip_wait); > drm_crtc_vblank_put(crtc); > } > > spin_unlock_irqrestore(&dev->event_lock, flags); > } > > -static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc) > -{ > - struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > - struct drm_device *dev = crtc->dev; > - unsigned long flags; > - bool pending; > - > - spin_lock_irqsave(&dev->event_lock, flags); > - pending = omap_crtc->event != NULL; > - spin_unlock_irqrestore(&dev->event_lock, flags); > - > - return pending; > -} > - > -static void omap_crtc_wait_page_flip(struct drm_crtc *crtc) > -{ > - struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > - > - if (wait_event_timeout(omap_crtc->flip_wait, > - !omap_crtc_page_flip_pending(crtc), > - msecs_to_jiffies(50))) > - return; > - > - dev_warn(crtc->dev->dev, "page flip timeout!\n"); > - > - omap_crtc_complete_page_flip(crtc); > -} > - > static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t > irqstatus) { > struct omap_crtc *omap_crtc = > @@ -404,7 +374,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc) > > DBG("%s", omap_crtc->name); > > - omap_crtc_wait_page_flip(crtc); > drm_crtc_vblank_off(crtc); > } > > @@ -541,7 +510,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, > > crtc = &omap_crtc->base; > > - init_waitqueue_head(&omap_crtc->flip_wait); > init_completion(&omap_crtc->completion); > > omap_crtc->channel = channel;
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index abfafd1600b8..2c0d91a67418 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -51,7 +51,6 @@ struct omap_crtc { /* pending event */ struct drm_pending_vblank_event *event; - wait_queue_head_t flip_wait; struct completion completion; @@ -277,41 +276,12 @@ static void omap_crtc_complete_page_flip(struct drm_crtc *crtc) else event->base.destroy(&event->base); - wake_up(&omap_crtc->flip_wait); drm_crtc_vblank_put(crtc); } spin_unlock_irqrestore(&dev->event_lock, flags); } -static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc) -{ - struct omap_crtc *omap_crtc = to_omap_crtc(crtc); - struct drm_device *dev = crtc->dev; - unsigned long flags; - bool pending; - - spin_lock_irqsave(&dev->event_lock, flags); - pending = omap_crtc->event != NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); - - return pending; -} - -static void omap_crtc_wait_page_flip(struct drm_crtc *crtc) -{ - struct omap_crtc *omap_crtc = to_omap_crtc(crtc); - - if (wait_event_timeout(omap_crtc->flip_wait, - !omap_crtc_page_flip_pending(crtc), - msecs_to_jiffies(50))) - return; - - dev_warn(crtc->dev->dev, "page flip timeout!\n"); - - omap_crtc_complete_page_flip(crtc); -} - static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus) { struct omap_crtc *omap_crtc = @@ -404,7 +374,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc) DBG("%s", omap_crtc->name); - omap_crtc_wait_page_flip(crtc); drm_crtc_vblank_off(crtc); } @@ -541,7 +510,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, crtc = &omap_crtc->base; - init_waitqueue_head(&omap_crtc->flip_wait); init_completion(&omap_crtc->completion); omap_crtc->channel = channel;
omap_crtc_disable() waits for pending page flips to finish. However, omap_crtc_disable() is always called via omap_atomic_complete() and we never have page flips going on at that time. So remove the omap_crtc_wait_page_flip() and related code. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/omap_crtc.c | 32 -------------------------------- 1 file changed, 32 deletions(-)