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

[Chris Wilson]

On Mon, May 15, 2017 at 03:25:27PM -0700, Michel Thierry wrote:
> 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.

Aye, but in the long run I was thinking of capturing this request in
engine->hangcheck.active_request and then we reuse that info in the later
phases.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre