[Intel-gfx] [PATCH] drm/i915: Always retire residual requests before suspend

[Tvrtko Ursulin]

On 17/07/2018 09:41, Chris Wilson wrote:
> If the driver is wedged, we skip idling the GPU. However, we may still
> have a few requests still not retired following the wedging (since they
> will be waiting for a background worker trying to acquire struct_mutex).
> As we hold the struct_mutex, always do a quick request retirement in
> order to flush the wedged path.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107257
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 42d24410a98c..cc875e1dc7f6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5074,6 +5074,8 @@ int i915_gem_suspend(struct drm_i915_private *i915)
>   
>   		assert_kernel_context_is_current(i915);
>   	}
> +	i915_retire_requests(i915); /* ensure we flush after wedging */
> +

We cannot do this in i915_gem_set_wedged due not having the mutex?

I think it should go in an else block of the !terminally_wedged block to 
signify the alternative idling method for that case. And also to make 
sure the !terminally_wedged case does not start relying on this extra 
retire pass.

Or alternative teach i915_gem_wait_for_idle how to handle the wedged 
case and only make switching to kernel context dependant on 
terminally_wedged status in i915_gem_suspend?

Simple else block sounds good enough to me. For that option:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko

>   	mutex_unlock(&i915->drm.struct_mutex);
>   
>   	intel_uc_suspend(i915);
>