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

Daniel Vetter
2014-06-17T08:06:58Z

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