From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Flush the pending flips on the CRTC before modification Date: Fri, 28 Sep 2012 11:22:45 +0100 Message-ID: <275ffc$6pg4hg@fmsmga002.fm.intel.com> References: <1348777558-17881-1-git-send-email-chris@chris-wilson.co.uk> <20120928100551.GM19732@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1352369712==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 75B789E7BA for ; Fri, 28 Sep 2012 03:22:54 -0700 (PDT) In-Reply-To: <20120928100551.GM19732@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1352369712== Content-Type: text/plain On Fri, 28 Sep 2012 13:05:51 +0300, Ville Syrjälä wrote: > On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote: > > This was meant to be the purpose of the > > intel_crtc_wait_for_pending_flips() function which is called whilst > > preparing the CRTC for a modeset or before disabling. However, as Ville > > Syrjala pointed out, we set the pending flip notification on the old > > framebuffer that is no longer attached to the CRTC by the time we come > > to flush the pending operations. Instead, we can simply wait on the > > pending unpin work to be finished on this CRTC, knowning that the > > hardware has therefore finished modifying the registers, before proceeding > > with our direct access. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++++-- > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a262326..39df185 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc) > > udelay(100); > > } > > > > +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) > > +{ > > + struct drm_device *dev = crtc->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + unsigned long flags; > > + bool pending; > > + > > + if (atomic_read(&dev_priv->mm.wedged)) > > + return false; > > + > > + spin_lock_irqsave(&dev->event_lock, flags); > > + pending = to_intel_crtc(crtc)->unpin_work != NULL; > > + spin_unlock_irqrestore(&dev->event_lock, flags); > > The locking looks pointless here. It does rather. Being pedagogical we should probably leave a mb of some sort in there... pending = to_intel_crtc(crtc)->unpin_work != NULL; smp_rmb(); with the existing spin_lock providing the necessary barriers before the wake_up(); -Chris -- Chris Wilson, Intel Open Source Technology Centre --===============1352369712== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1352369712==--