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

Michel Thierry michel.thierry at intel.com
Mon May 15 22:25:27 UTC 2017


On 5/15/2017 2:47 PM, Chris Wilson wrote:
> On Mon, May 15, 2017 at 10:31:58PM +0100, Chris Wilson wrote:
>> On Mon, May 15, 2017 at 02:20:01PM -0700, Michel Thierry wrote:
>>> @@ -2827,21 +2830,34 @@ 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,
>>> +							 engine->hangcheck.seqno))
>>
>> This is not the seqno for the request, so this is incorrect. It will
>> judge that the request was preempted (as hangcheck.seqno must be less
>> thn request->global_seqno) and so conclude that the request was never
>> completed.
>>
>> You just want if (i915_gem_request_completed(request))

Thanks, I'll change the function.

>
> Also not here. This pardon check should be deferred to the caller just
> before commiting to thre reset. In the case of global reset, we want to
> gather up all the engines' active requests first, complete our
> preparations and then double check the engine was hung.

i915_reset_engine calls this directly, but 'full reset' [from 
i915_gem_reset_prepare()] would not be affected and it won't pardon 
anything... i915_gem_reset_engine is doing the double check you mention.

-Michel


More information about the Intel-gfx mailing list