[Intel-gfx] [PATCH 1/2] drm/i915: Stop engines around GPU reset preparations

[Chris Wilson]

Quoting Mika Kuoppala (2018-03-02 12:17:19)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2018-03-02 11:50:32)
> >> Chris Wilson <chris at chris-wilson.co.uk> writes:
> >> > +static void i915_engines_set_mode(struct drm_i915_private *dev_priv,
> >> > +                               unsigned engine_mask,
> >> > +                               u32 mode)
> >> > +{
> >> > +     struct intel_engine_cs *engine;
> >> > +     enum intel_engine_id id;
> >> > +
> >> > +     if (INTEL_GEN(dev_priv) < 3)
> >> > +             return;
> >> > +
> >> > +     for_each_engine_masked(engine, dev_priv, engine_mask, id)
> >> > +             I915_WRITE_FW(RING_MI_MODE(engine->mmio_base), mode);
> >> 
> >> Is there reason to not use gen3_stop_engine in this level?
> >
> > It clears HEAD/TAIL, so undoing it in the case of no reset is a bit more
> > tricky.
> 
> With this we now have 3 different flavours of stopping an engine.
> 
> I would like to see early on prepare reset to call engine->stop(),
> which would be unified way of bring engine to halt. And limit
> any further restoration of state if we can't really manage to reset it,
> leaving it as stopped and dormant as we possibly could get it.
> 
> Then only on successful reset and restoration of init state, we would
> have an engine->start().

Wire it up, and see how it looks. We do

	engine->stop # reset(early) or suspend
	engine->reset # reset or suspend
	engine->cancel # reset or wedge
	engine->init # reset or resume
	engine->start # reset(late) or resume

I agree adopting that scheme should help, if we can nail the code into
individual steps without repetition. (If we find we repeat ourselves,
the above scheme doesn't fit :)
-Chris