[Intel-gfx] [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay

[Chris Wilson]

On Wed, Dec 07, 2016 at 07:28:05PM +0200, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We need an uninterruptible wait for the overlay off request during
> modeset. Restore the operation of dev_priv->mm.interruptible
> sufficiently for that.
> 
> Toss in a WARN_ON() to make sure the request succeeds.
> 
> Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling to its one caller")
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 36 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_request.h | 35 ++------------------------------
>  drivers/gpu/drm/i915/intel_display.c    |  2 +-
>  3 files changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index fcf22b0e2967..1a7b88166c51 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
>  	for_each_engine(engine, dev_priv, id)
>  		engine_retire_requests(engine);
>  }
> +
> +/**
> + * i915_gem_active_retire - waits until the request is retired
> + * @active - the active request on which to wait
> + *
> + * i915_gem_active_retire() waits until the request is completed,
> + * and then ensures that at least the retirement handler for this
> + * @active tracker is called before returning. If the @active
> + * tracker is idle, the function returns immediately.
> + */
> +int __must_check
> +i915_gem_active_retire(struct i915_gem_active *active,
> +		       struct mutex *mutex)
> +{
> +	struct drm_i915_gem_request *request;
> +	long ret;
> +
> +	request = i915_gem_active_raw(active, mutex);
> +	if (!request)
> +		return 0;
> +
> +	ret = i915_wait_request(request,
> +				(request->i915->mm.interruptible ?
> +				 I915_WAIT_INTERRUPTIBLE : 0) |
> +				I915_WAIT_LOCKED,
> +				MAX_SCHEDULE_TIMEOUT);

At least pass in the flags.

> +	if (ret < 0)
> +		return ret;
> +
> +	list_del_init(&active->link);
> +	RCU_INIT_POINTER(active->request, NULL);
> +
> +	active->retire(active, request);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index e2b077df2da0..09add6b9cfc7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active *active, unsigned int flags)
>  	return ret < 0 ? ret : 0;
>  }
>  
> -/**
> - * i915_gem_active_retire - waits until the request is retired
> - * @active - the active request on which to wait
> - *
> - * i915_gem_active_retire() waits until the request is completed,
> - * and then ensures that at least the retirement handler for this
> - * @active tracker is called before returning. If the @active
> - * tracker is idle, the function returns immediately.
> - */
> -static inline int __must_check
> -i915_gem_active_retire(struct i915_gem_active *active,
> -		       struct mutex *mutex)
> -{
> -	struct drm_i915_gem_request *request;
> -	long ret;
> -
> -	request = i915_gem_active_raw(active, mutex);
> -	if (!request)
> -		return 0;
> -
> -	ret = i915_wait_request(request,
> -				I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> -				MAX_SCHEDULE_TIMEOUT);
> -	if (ret < 0)
> -		return ret;
> -
> -	list_del_init(&active->link);
> -	RCU_INIT_POINTER(active->request, NULL);
> -
> -	active->retire(active, request);
> -
> -	return 0;
> -}
> +int __must_check i915_gem_active_retire(struct i915_gem_active *active,
> +					struct mutex *mutex);
>  
>  #define for_each_active(mask, idx) \
>  	for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a41082e2750e..5a74991854e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
>  
>  		mutex_lock(&dev->struct_mutex);
>  		dev_priv->mm.interruptible = false;
> -		(void) intel_overlay_switch_off(intel_crtc->overlay);
> +		WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
>  		dev_priv->mm.interruptible = true;
>  		mutex_unlock(&dev->struct_mutex);

Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
(For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always
be a no-op.) And then we are down to just the shrinker running
uninterruptibly, just one last place to fix.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre