Quoting Michal Wajdeczko (2018-05-17 18:23:23)
> On Thu, 17 May 2018 18:56:41 +0200, Chris Wilson
> <chris@xxxxxxxxxxxxxxxxxx> wrote:
>
> > We make a decision at module load whether to use the GuC backend or not,
> > but lose that setup across set-wedge. Currently, the guc doesn't
> > override the engine->set_default_submission hook letting execlists sneak
> > back in temporarily on unwedging leading to an unbalanced park/unpark.
> >
> > Testcase: igt/gem_eio
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_guc_submission.c | 34 +++++++++++++++------
> > drivers/gpu/drm/i915/intel_lrc.c | 2 +-
> > drivers/gpu/drm/i915/intel_lrc.h | 2 ++
> > 3 files changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c
> > b/drivers/gpu/drm/i915/intel_guc_submission.c
> > index 637e852888ec..cbd8caffd271 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> > @@ -1264,6 +1264,29 @@ static void guc_submission_unpark(struct
> > intel_engine_cs *engine)
> > intel_engine_pin_breadcrumbs_irq(engine);
> > }
> > +static void guc_set_default_submission(struct intel_engine_cs *engine)
> > +{
> > + /*
> > + * We inherit a bunch of functions from execlists that we'd like
> > + * to keep using:
> > + *
> > + * engine->submit_request = execlists_submit_request;
> > + * engine->cancel_requests = execlists_cancel_requests;
> > + * engine->schedule = execlists_schedule;
> > + *
> > + * But we need to override the actual submission backend in order
> > + * to talk to the GuC.
> > + */
> > + execlists_set_default_submission(engine);
> > +
> > + engine->execlists.tasklet.func = guc_submission_tasklet;
> > +
> > + engine->park = guc_submission_park;
> > + engine->unpark = guc_submission_unpark;
> > +
> > + engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
> > +}
> > +
> > int intel_guc_submission_enable(struct intel_guc *guc)
> > {
> > struct drm_i915_private *dev_priv = guc_to_i915(guc);
> > @@ -1302,17 +1325,10 @@ int intel_guc_submission_enable(struct intel_guc
> > *guc)
> > guc_interrupts_capture(dev_priv);
> > for_each_engine(engine, dev_priv, id) {
> > - struct intel_engine_execlists * const execlists =
> > - &engine->execlists;
> > -
> > - execlists->tasklet.func = guc_submission_tasklet;
> > -
> > engine->reset.prepare = guc_reset_prepare;
> > + engine->set_default_submission = guc_set_default_submission;
> > - engine->park = guc_submission_park;
> > - engine->unpark = guc_submission_unpark;
> > -
> > - engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
> > + engine->set_default_submission(engine);
>
> While this part looks ok (and maybe will help with my [1])
> but what about intel_guc_submission_disable(), where we
> will call intel_engines_reset_default_submission() and
> 'revert' to GuC again... time for nop_submission() ?
If we apply the "once you go GuC, you won't go back" rule, then we might
as well just call i915_gem_set_wedged() (or leave it wedged) on disabling.
So long as any re-enable path goes through a i915_reset, we will be
fine.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
