[Intel-gfx] [PATCH 1/9] drm: Always reject drm_vblank_get() after drm_vblank_off()

[Daniel Vetter]

On Mon, May 26, 2014 at 04:32:02PM +0300, Ville Syrjälä wrote:
> On Mon, May 26, 2014 at 03:21:41PM +0200, Daniel Vetter wrote:
> > On Mon, May 26, 2014 at 02:46:24PM +0300, ville.syrjala at linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > Make sure drm_vblank_get() never succeeds when called between
> > > drm_vblank_off() and drm_vblank_on(). Borrow a trick from the
> > > old drm_vblank_{pre,post}_modeset() functions and just bump
> > > the refcount in drm_vblank_off() and drop it in drm_vblank_on().
> > > 
> > > Hopefully the use of inmodeset won't conflict badly with
> > > drm_vblank_{pre,post}_modeset().
> > > 
> > > For i915 there's a window between drm_vblank_off() and marking the
> > > crtc as inactive where the current code still allows drm_vblank_get().
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Afaics we never check anywhere for inmodeset in drm_vblank_get, so how
> > does this work?
> 
> If refcount is already >0 and vblank irq is disabled, drm_vblank_get() will
> return -EINVAL.

Oh right, I've missed that. drm_irq.c is never short of suprises. With
that enlightenment (and maybe a pimped commit message for blind people
like me) this is Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> > -Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/drm_irq.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > > index b565372..7b1e08c 100644
> > > --- a/drivers/gpu/drm/drm_irq.c
> > > +++ b/drivers/gpu/drm/drm_irq.c
> > > @@ -1039,6 +1039,15 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
> > >  	}
> > >  	spin_unlock(&dev->event_lock);
> > >  
> > > +	/*
> > > +	 * Prevent subsequent drm_vblank_get() from re-enabling
> > > +	 * the vblank interrupt by bumping the refcount.
> > > +	 */
> > > +	if (!dev->vblank[crtc].inmodeset) {
> > > +		atomic_inc(&dev->vblank[crtc].refcount);
> > > +		dev->vblank[crtc].inmodeset = 1;
> > > +	}
> > > +
> > >  	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> > >  }
> > >  EXPORT_SYMBOL(drm_vblank_off);
> > > @@ -1079,6 +1088,11 @@ void drm_vblank_on(struct drm_device *dev, int crtc)
> > >  	unsigned long irqflags;
> > >  
> > >  	spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > > +	/* Drop our private "prevent drm_vblank_get" refcount */
> > > +	if (dev->vblank[crtc].inmodeset) {
> > > +		atomic_dec(&dev->vblank[crtc].refcount);
> > > +		dev->vblank[crtc].inmodeset = 0;
> > > +	}
> > >  	/* re-enable interrupts if there's are users left */
> > >  	if (atomic_read(&dev->vblank[crtc].refcount) != 0)
> > >  		WARN_ON(drm_vblank_enable(dev, crtc));
> > > -- 
> > > 1.8.5.5
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch