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@xxxxxxxxxxxxxxx wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxx> > > > -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@xxxxxxxxxxxxxxxxxxxxx > > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx