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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx