From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Flush the pending flips on the CRTC before modification Date: Fri, 28 Sep 2012 08:37:20 +0200 Message-ID: <20120928063720.GI2098@bremse> References: <1348777558-17881-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-bk0-f49.google.com (mail-bk0-f49.google.com [209.85.214.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 296859E950 for ; Thu, 27 Sep 2012 23:37:25 -0700 (PDT) Received: by bkwj4 with SMTP id j4so2724848bkw.36 for ; Thu, 27 Sep 2012 23:37:25 -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 Can I haz testcase plz? Shouldn't be too hard to race a few pageflips with dpms off and crtc disabling, maybe we need to add some busy load onto the gpu first to delay things for long enough. Thanks, Daniel > --- > 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); > + > + return pending; > +} > + > static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > > flush_work_sync(&to_intel_crtc(crtc)->vblank_work.work); > > if (crtc->fb == 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_device *dev, > > atomic_clear_mask(1 << intel_crtc->plane, > &obj->pending_flip.counter); > - if (atomic_read(&obj->pending_flip) == 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); > -- > 1.7.10.4 > > _______________________________________________ > 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