[PATCH] drm/shmobile: Fix race condition between page flip request and handler

[PATCH] drm/shmobile: Fix race condition between page flip request and handler

[Laurent Pinchart]

The page flip handler stores the page flip event pointer and then calls
drm_vblank_get() to enable the vblank interrupt. Due to the vblank off
delay, the vblank interrupt can be enabled in the hardware at that
point, even if the vblank reference count is equal to 0. If a vblank
interrupt is triggered between storing the event pointer and calling
drm_vblank_get(), the page flip completion handler will process the
event and call drm_vblank_put() with a reference count equal to 0. This
will result in a BUG_ON.

Fix the race condition by calling drm_vblank_get() before storing the
event pointer.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index d917a41..7dff49e 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -494,10 +494,10 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc,
 
 	if (event) {
 		event->pipe = 0;
+		drm_vblank_get(dev, 0);
 		spin_lock_irqsave(&dev->event_lock, flags);
 		scrtc->event = event;
 		spin_unlock_irqrestore(&dev->event_lock, flags);
-		drm_vblank_get(dev, 0);
 	}
 
 	return 0;
-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/shmobile: Fix race condition between page flip request and handler

[Laurent Pinchart]

Hi Dave,

Could you please pick this patch for v3.10 ?

On Tuesday 12 March 2013 15:38:43 Laurent Pinchart wrote:
> The page flip handler stores the page flip event pointer and then calls
> drm_vblank_get() to enable the vblank interrupt. Due to the vblank off
> delay, the vblank interrupt can be enabled in the hardware at that
> point, even if the vblank reference count is equal to 0. If a vblank
> interrupt is triggered between storing the event pointer and calling
> drm_vblank_get(), the page flip completion handler will process the
> event and call drm_vblank_put() with a reference count equal to 0. This
> will result in a BUG_ON.
> 
> Fix the race condition by calling drm_vblank_get() before storing the
> event pointer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index d917a41..7dff49e 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> @@ -494,10 +494,10 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc
> *crtc,
> 
>  	if (event) {
>  		event->pipe = 0;
> +		drm_vblank_get(dev, 0);
>  		spin_lock_irqsave(&dev->event_lock, flags);
>  		scrtc->event = event;
>  		spin_unlock_irqrestore(&dev->event_lock, flags);
> -		drm_vblank_get(dev, 0);
>  	}
> 
>  	return 0;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/shmobile: Fix race condition between page flip request and handler

[Laurent Pinchart]

On Thursday 28 March 2013 16:10:02 Laurent Pinchart wrote:
> Hi Dave,
> 
> Could you please pick this patch for v3.10 ?

Ping ?

> On Tuesday 12 March 2013 15:38:43 Laurent Pinchart wrote:
> > The page flip handler stores the page flip event pointer and then calls
> > drm_vblank_get() to enable the vblank interrupt. Due to the vblank off
> > delay, the vblank interrupt can be enabled in the hardware at that
> > point, even if the vblank reference count is equal to 0. If a vblank
> > interrupt is triggered between storing the event pointer and calling
> > drm_vblank_get(), the page flip completion handler will process the
> > event and call drm_vblank_put() with a reference count equal to 0. This
> > will result in a BUG_ON.
> > 
> > Fix the race condition by calling drm_vblank_get() before storing the
> > event pointer.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index d917a41..7dff49e 100644
> > --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> > @@ -494,10 +494,10 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc
> > *crtc,
> > 
> >  	if (event) {
> >  	
> >  		event->pipe = 0;
> > 
> > +		drm_vblank_get(dev, 0);
> > 
> >  		spin_lock_irqsave(&dev->event_lock, flags);
> >  		scrtc->event = event;
> >  		spin_unlock_irqrestore(&dev->event_lock, flags);
> > 
> > -		drm_vblank_get(dev, 0);
> > 
> >  	}
> >  	
> >  	return 0;
-- 
Regards,

Laurent Pinchart