[Intel-gfx] [PATCH v2 1/4] drm/i915: Wait for flips to complete before returning.

[Daniel Vetter]

On Tue, May 24, 2016 at 01:24:08PM +0200, Maarten Lankhorst wrote:
> Doing a page flip right after setcrtc would fail because part of the update is run
> asynchronously. This is a regression and is causing the kms_flip to fails after
> reverting the atomic page flip patch.

How exactly does it fail? What's the error output from igt? Which testcase
exactly goes boom? What else is on fire when running full kms_flip?

I know you don't like typing docs and commit message, and I'm not too
happy with the terseness in general from your commits. But when the
fireworks show is happening, it really needs to be a full in-depth story
and not the just 2 sentence summary from the bestseller list.
-Daniel

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Fixes: a6747b7304a9 ("drm/i915: Make unpin async.")
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a500f08ec0ac..21c0a2f3102b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3788,7 +3788,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip
>  	queue_work(dev_priv->wq, &work->unpin_work);
>  }
>  
> -static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> +static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc, bool intr)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3796,10 +3796,15 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>  
>  	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
>  
> -	ret = wait_event_interruptible_timeout(
> -					dev_priv->pending_flip_queue,
> -					!intel_crtc_has_pending_flip(crtc),
> -					60*HZ);
> +	if (intr)
> +		ret = wait_event_interruptible_timeout(
> +						dev_priv->pending_flip_queue,
> +						!intel_crtc_has_pending_flip(crtc),
> +						60*HZ);
> +	else
> +		ret = wait_event_timeout(dev_priv->pending_flip_queue,
> +					 !intel_crtc_has_pending_flip(crtc),
> +					 60*HZ);
>  
>  	if (ret < 0)
>  		return ret;
> @@ -12736,7 +12741,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
>  		struct intel_flip_work *work;
>  
>  		if (!state->legacy_cursor_update) {
> -			ret = intel_crtc_wait_for_pending_flips(crtc);
> +			ret = intel_crtc_wait_for_pending_flips(crtc, true);
>  			if (ret)
>  				return ret;
>  
> @@ -13005,6 +13010,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	struct drm_crtc_state *old_crtc_state;
>  	struct drm_crtc *crtc;
>  	int ret = 0, i;
> +	unsigned crtc_mask = 0;
>  
>  	ret = intel_atomic_prepare_commit(dev, state, nonblock);
>  	if (ret) {
> @@ -13075,6 +13081,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		bool modeset = needs_modeset(crtc->state);
>  
> +		crtc_mask |= drm_crtc_mask(crtc);
> +
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
>  			dev_priv->display.crtc_enable(crtc);
> @@ -13105,6 +13113,12 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		intel_schedule_update(crtc, intel_state, work, nonblock);
>  	}
>  
> +	if (!nonblock && !state->legacy_cursor_update) {
> +		drm_for_each_crtc(crtc, dev)
> +			if (crtc_mask & drm_crtc_mask(crtc))
> +				intel_crtc_wait_for_pending_flips(crtc, false);
> +	}
> +
>  	/* FIXME: add subpixel order */
>  
>  	drm_atomic_state_free(state);
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch