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).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
