[Intel-gfx] [PATCH 2/2] drm/i915/gt: Only unwedge if we can reset first

[Daniele Ceraolo Spurio]

On 9/9/19 3:55 PM, Chris Wilson wrote:
> Unwedging the GPU requires a successful GPU reset before we restore the
> default submission, or else we may see residual context switch events
> that we were not expecting.
> 
> Reported-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_reset.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index fe57296b790c..5242496a893a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -809,6 +809,7 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
>   	struct intel_gt_timelines *timelines = &gt->timelines;
>   	struct intel_timeline *tl;
>   	unsigned long flags;
> +	bool ok;
>   
>   	if (!test_bit(I915_WEDGED, &gt->reset.flags))
>   		return true;
> @@ -854,7 +855,11 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
>   	}
>   	spin_unlock_irqrestore(&timelines->lock, flags);
>   
> -	intel_gt_sanitize(gt, false);
> +	ok = false;
> +	if (!reset_clobbers_display(gt->i915))
> +		ok = __intel_gt_reset(gt, ALL_ENGINES) == 0;

Of the thing we had in the gt_sanitize, we're ok skipping the 
uc_sanitize() because we take care of that during wedge (from 
intel_uc_reset_prepare), but what about the loop of 
__intel_engine_reset()? Is that safe to skip here?

Apart from that, the patch LGTM. Worth noting that with this change a 
successful reset is required to unwedge even after a suspend/resume 
cycle (in gem_sanitize), which is a good thing IMO.

Daniele

> +	if (!ok)
> +		return false;
>   
>   	/*
>   	 * Undo nop_submit_request. We prevent all new i915 requests from
>