Re: [PATCH 15/15] drm/i915: Fix up PSR frontbuffer tracking

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 17, 2014 at 08:02:27AM +0100, Chris Wilson wrote:
> On Mon, Jun 16, 2014 at 07:51:35PM +0200, Daniel Vetter wrote:
> >  static void intel_edp_psr_work(struct work_struct *work)
> >  {
> >  	struct drm_i915_private *dev_priv =
> >  		container_of(work, typeof(*dev_priv), psr.work.work);
> > -	struct drm_device *dev = dev_priv->dev;
> >  	struct intel_dp *intel_dp = dev_priv->psr.enabled;
> >  
> > -	drm_modeset_lock_all(dev);
> > -	mutex_lock(&dev->struct_mutex);
> >  	mutex_lock(&dev_priv->psr.lock);
> >  	intel_dp = dev_priv->psr.enabled;
> >  
> >  	if (!intel_dp)
> >  		goto unlock;
> >  
> > -	if (intel_edp_psr_match_conditions(intel_dp))
> > -		intel_edp_psr_do_enable(intel_dp);
> > +	if (dev_priv->psr.busy_frontbuffer_bits)
> > +		goto unlock;
> 
> Hmm, I requeued the work item out of paranoia. But I think a comment
> here about how invalidate *will* be called again if any of the
> busy_frontbuffer_bits change and so the work will be requeued on the
> next update.

Yeah this deserves a comment. It only happens if invalidate was called
since the flush that queued the work. Eventually a new flush will happen,
but since we're seeing a non-NULL busy bits that didn't happen yet. And
because flush clears the bits first before rescheduling we're guaranteed
to not miss it.
-Daniel
> 
> > +
> > +	intel_edp_psr_do_enable(intel_dp);
> >  unlock:
> >  	mutex_unlock(&dev_priv->psr.lock);
> > -	mutex_unlock(&dev->struct_mutex);
> > -	drm_modeset_unlock_all(dev);
> >  }
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> 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