[Intel-gfx] [PATCH] drm/i915: Cancel reset-engine if we couldn't find an active request

[Michel Thierry]

On 5/18/2017 12:56 AM, Chris Wilson wrote:
> On Wed, May 17, 2017 at 06:11:06PM -0700, Michel Thierry wrote:
>> On 17/05/17 13:52, Chris Wilson wrote:
>>> On Wed, May 17, 2017 at 01:41:34PM -0700, Michel Thierry wrote:
>>>> @@ -2827,21 +2829,35 @@ int i915_gem_reset_prepare_engine(struct intel_engine_cs *engine)
>>>>
>>>> 	if (engine_stalled(engine)) {
>>>> 		request = i915_gem_find_active_request(engine);
>>>> -		if (request && request->fence.error == -EIO)
>>>> -			err = -EIO; /* Previous reset failed! */
>>>> +
>>>> +		if (request) {
>>>> +			if (request->fence.error == -EIO)
>>>> +				return ERR_PTR(-EIO); /* Previous reset failed! */
>>>> +
>>>> +			if (i915_gem_request_completed(request))
>>>> +				return NULL; /* request completed, skip it */
>>>
>>> This check is pointless here. We are just a few cycles since it was
>>> known to be true. Both paths should be doing it just before the actual
>>> reset for symmetry.
>>
>> As you said, in gem_reset_request, 'guilty' should check for
>> i915_gem_request_completed instead of engine_stalled... but at that
>> point it's too late to cancel the reset (intel_gpu_reset has already
>> been called).
>
> Ok. At that point we are just deciding between skipping the request or
> replaying it. The motivation behind carrying forward the active_request
> was to avoid the repeated searches + engine_stalled() checks (since any
> future check can then just confirm the active_request is still
> incomplete).

Agreed, we'll still avoid the repeated searches + engine_stalled.
Let me send that.