From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Flush the pending flips on the CRTC before modification Date: Fri, 28 Sep 2012 13:05:51 +0300 Message-ID: <20120928100551.GM19732@intel.com> References: <1348777558-17881-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id CBF97A0BA2 for ; Fri, 28 Sep 2012 03:05:54 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1348777558-17881-1-git-send-email-chris@chris-wilson.co.uk> 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: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 =3D crtc->dev; > + struct drm_i915_private *dev_priv =3D 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 =3D to_intel_crtc(crtc)->unpin_work !=3D NULL; > + spin_unlock_irqrestore(&dev->event_lock, flags); The locking looks pointless here. > + > + return pending; > +} > + > static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) > { > struct drm_device *dev =3D crtc->dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > = > flush_work_sync(&to_intel_crtc(crtc)->vblank_work.work); > = > if (crtc->fb =3D=3D NULL) > return; > = > + wait_event(dev_priv->pending_flip_queue, > + !intel_crtc_has_pending_flip(crtc)); > + > mutex_lock(&dev->struct_mutex); > intel_finish_fb(crtc->fb); > mutex_unlock(&dev->struct_mutex); > @@ -6388,9 +6409,8 @@ static void do_intel_finish_page_flip(struct drm_de= vice *dev, > = > atomic_clear_mask(1 << intel_crtc->plane, > &obj->pending_flip.counter); > - if (atomic_read(&obj->pending_flip) =3D=3D 0) > - wake_up(&dev_priv->pending_flip_queue); > = > + wake_up(&dev_priv->pending_flip_queue); > queue_work(dev_priv->wq, &work->work); > = > trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); -- = Ville Syrj=E4l=E4 Intel OTC