[Intel-gfx] [PATCH 2/5] drm/i915/gt: Push engine stopping into reset-prepare

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-17 14:42:15)
> 
> On 17/07/2019 14:30, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-07-17 14:21:50)
> >>
> >> On 17/07/2019 14:08, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-07-17 14:04:34)
> >>>>
> >>>> On 16/07/2019 13:49, Chris Wilson wrote:
> >>>>> -             if (retry)
> >>>>> -                     stop_engines(gt, engine_mask);
> >>>>> -
> >>>>
> >>>> Only other functional change I see is that we stop retrying to stop the
> >>>> engines before reset attempts. I don't know if that is a concern or not.
> >>>
> >>> Ah, but we do stop the engine before resets in *reset_prepare. The other
> >>> path to arrive is in sanitize where we don't know enough state to safely
> >>> tweak the engines. For those, I claim it shouldn't matter as the engines
> >>> should be idle and we only need the reset to clear stale context state.
> >>
> >> Yes I know that we do call stop in prepare, just not on the reset retry
> >> path. It's the above loop, if reset was failing and needed retries
> >> before we would re-retried stopping engines and now we would not.
> > 
> > The engines are still stopped. The functional change is to remove the
> > dangerous clearing of RING_HEAD/CTL.
> 
> Okay for execlists, but for ringbuffer I was simply asking if _one_ of 
> the reasons for failed reset could be failure to stop cs. In which case 
> removal of stop_engines from the retry loop might be detrimental for 
> ringbuffer.

For ringbuffer, we do the full shebang in prepare_reset, with a nice
splat if we fail to clear the head. iirc, that has never been an issue,
although one should always reserve judgment for g4x to randomly fail
with head updates. If it helps, we can remove the loop here as I don't
think it accomplishes anything -- the examples I have where it times out
are followed by a hard machine hang.
-Chris