Re: [PATCH 2/9] drm/i915: Warn if drm_vblank_get() still works after drm_vblank_off()

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, May 26, 2014 at 04:36:26PM +0300, Ville Syrjälä wrote:
> On Mon, May 26, 2014 at 03:22:45PM +0200, Daniel Vetter wrote:
> > On Mon, May 26, 2014 at 02:46:25PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3da73ef..da318a7 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -1305,6 +1305,17 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
> > >  	}
> > >  }
> > >  
> > > +static void assert_vblank_disabled(struct intel_crtc *crtc)
> > > +{
> > > +	struct drm_device *dev = crtc->base.dev;
> > > +	enum pipe pipe = crtc->pipe;
> > > +
> > > +	if (WARN_ON(drm_vblank_get(dev, pipe) == 0)) {
> > > +		drm_vblank_put(dev, pipe);
> > > +		drm_vblank_off(dev, pipe);
> > 
> > Imo the _off is too much, since with that it's not just an assert but a
> > "... and please make it so if not". Imo better to drop that.
> 
> The idea was that if drm_vblank_get() managed to re-enable vblank
> interrupts when it wasn't supposed to, we should turn them off again.
> But the whole thing is a bug, and another drm_vblank_get() might
> happen just after the drm_vblank_off() anyway, so I guess it's a bit
> pointless.

Yeah, I guess we could also drop the _put, since something is clearly
amiss already and will likely go down in flames soonish.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch