Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

From: Daniel Vetter <daniel@ffwll.ch>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on
Date: Fri, 6 Jun 2014 11:36:37 +0200	[thread overview]
Message-ID: <20140606093637.GD7416@phenom.ffwll.local> (raw)
In-Reply-To: <20140606085710.GG27580@intel.com>

On Fri, Jun 06, 2014 at 11:57:10AM +0300, Ville Syrjälä wrote:
> On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
> > Otherwise we incur an unsightly WARNING. The mutex locking is a bit
> > overkill, but it curbs races and eventially we might grow a locking
> > check in the vblank wait code to make sure the right crtc lock is
> > held.
> > 
> > This is fallout from
> > 
> > commit 9393707190194eb8b42e412b444a03331db6862f
> > Author:     Jesse Barnes <jbarnes@virtuousgeek.org>
> > AuthorDate: Fri Apr 4 16:12:09 2014 -0700
> > 
> >     drm/i915: warn when a vblank wait times out
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79612
> > Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 601caa88c092..cf94e9640627 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2927,11 +2927,16 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> >  	/* real source -> none transition */
> >  	if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
> >  		struct intel_pipe_crc_entry *entries;
> > +		struct intel_crtc *crtc =
> > +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>                         ^^^^^^
> 
> What's that?

Oops. Luckily patch still worked, C rules are bendy enough ;-)

> 
> Otherwise:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks,
-Daniel

> 
> >  
> >  		DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
> >  				 pipe_name(pipe));
> >  
> > -		intel_wait_for_vblank(dev, pipe);
> > +		mutex_lock(&crtc->base.mutex);
> > +		if (crtc->active)
> > +			intel_wait_for_vblank(dev, pipe);
> > +		mutex_unlock(&crtc->base.mutex);
> >  
> >  		spin_lock_irq(&pipe_crc->lock);
> >  		entries = pipe_crc->entries;
> > -- 
> > 2.0.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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