Quoting Mika Kuoppala (2018-03-02 12:17:19)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>
> > Quoting Mika Kuoppala (2018-03-02 11:50:32)
> >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
