[Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification

[Daniel Vetter]

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