[Intel-gfx] [PATCH] drm/i915: Clear stop-engine for a pardoned reset

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2018-08-15 09:52:18)
>> Chris Wilson <chris at chris-wilson.co.uk> writes:
>> 
>> > If we pardon a per-engine reset, we may leave the STOP_RING bit asserted
>> > in RING_MI_MODE resulting in the engine hanging. Unconditionally clear
>> > it on the per-engine exit path as we know that either we skipped the
>> > reset and so need the cancellation, or the reset was successful and the
>> > cancellation is a no-op, or there was an error and we will follow up
>> > with a full-reset or wedging (both of which will stop the engines again
>> > as required).
>> >
>> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.c         |  1 +
>> >  drivers/gpu/drm/i915/intel_engine_cs.c  | 10 ++++++++++
>> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>> >  3 files changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > index 9dce55182c3a..41111f2a9c39 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -2079,6 +2079,7 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
>> >               goto out;
>> >  
>> >  out:
>> > +     intel_engine_cancel_stop_cs(engine);
>> >       i915_gem_reset_finish_engine(engine);
>> 
>> Should we just lift the whole stop/start dance into
>> gem_reset_prepare|finish_engine()s?
>
> No, because it is also used by wedging where we do want the asymmetry.
> Been there, done that, have the gem_eio bruises.

On wedge the submission path is blocked but yeah,
gpu can be in any state and enabling the engine at that
point is asking for trouble.

Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

> -Chris