From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 1/3] drm/i915: Wait for pending flips in intel_pipe_set_base() Date: Fri, 02 Nov 2012 15:25:59 +0000 Message-ID: <275ffc$785jr7@fmsmga002.fm.intel.com> References: <1351793163-8542-1-git-send-email-ville.syrjala@linux.intel.com> <1351793163-8542-2-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0948919231==" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id B6FA89F307 for ; Fri, 2 Nov 2012 08:26:45 -0700 (PDT) In-Reply-To: <1351793163-8542-2-git-send-email-ville.syrjala@linux.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 To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0948919231== Content-Type: text/plain On Thu, 1 Nov 2012 20:06:00 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä > > intel_pipe_set_base() never actually waited for any pending page flips > on the CRTC. It looks like it tried to, by calling intel_finish_fb() on > the current front buffer. But the pending flips were actually tracked > in the BO of the previous front buffer, so the call to intel_finish_fb() > never did anything useful. > > Now even the pending_flip counter is gone, so we should just > use intel_crtc_wait_for_pending_flips(), but since we're already holding > struct_mutex when we would call that function, we need another version > of it, that itself doesn't lock struct_mutex. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++++++++------------ > 1 files changed, 32 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1a38267..7bf4749 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2228,6 +2228,37 @@ static void intel_crtc_update_sarea_pos(struct drm_crtc *crtc, int x, int y) > } > } > > +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); > + > + return pending; > +} > + > +static void intel_crtc_wait_for_pending_flips_locked(struct drm_crtc *crtc) > +{ Can we rearrange this such that the waiting logic is inside _locked() and then intel_crtc_wiat_for_pending_flips() becomes a wrapper that acquires the struct_mutex and then calls _locked()? Just to keep the code simpler at the expense of the pathological case. -Chris -- Chris Wilson, Intel Open Source Technology Centre --===============0948919231== 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 --===============0948919231==--