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 <chris at chris-wilson.co.uk> 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 at 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