From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 15/15] drm/i915: Fix up PSR frontbuffer tracking Date: Tue, 17 Jun 2014 10:06:50 +0200 Message-ID: <20140617080650.GA5821@phenom.ffwll.local> References: <1402941095-27879-1-git-send-email-daniel.vetter@ffwll.ch> <1402941095-27879-16-git-send-email-daniel.vetter@ffwll.ch> <20140617070227.GJ17744@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f172.google.com (mail-we0-f172.google.com [74.125.82.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 7FF558A1DB for ; Tue, 17 Jun 2014 01:06:58 -0700 (PDT) Received: by mail-we0-f172.google.com with SMTP id u57so6887513wes.31 for ; Tue, 17 Jun 2014 01:06:57 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140617070227.GJ17744@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , Intel Graphics Development , Rodrigo Vivi List-Id: intel-gfx@lists.freedesktop.org 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