From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/9] drm: Always reject drm_vblank_get() after drm_vblank_off() Date: Mon, 26 May 2014 16:32:02 +0300 Message-ID: <20140526133202.GG27580@intel.com> References: <1401104792-26560-1-git-send-email-ville.syrjala@linux.intel.com> <1401104792-26560-2-git-send-email-ville.syrjala@linux.intel.com> <20140526132141.GI14357@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20140526132141.GI14357@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > 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=E4l=E4 > = > 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. > -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 =3D 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 c= rtc) > > 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 =3D 0; > > + } > > /* re-enable interrupts if there's are users left */ > > if (atomic_read(&dev->vblank[crtc].refcount) !=3D 0) > > WARN_ON(drm_vblank_enable(dev, crtc)); > > -- = > > 1.8.5.5 > > = > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@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=E4l=E4 Intel OTC