| Message ID | 20180717202932.1423-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-07-17 21:29:32) > 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. > > v2: Remove comment about switching back temporarily to execlists on > guc_submission_disable(). We currently only call disable on shutdown, > and plan to also call disable before suspend and reset, in which case we > will either restore guc submission or mark the driver as wedged, making > the reset back to execlists pointless. > v3: Move reset.prepare across Fixes: 63572937cebf ("drm/i915/execlists: Flush pending preemption events during reset") > Testcase: igt/drv_module_reload/basic-reload-inject > Testcase: igt/gem_eio > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Pretty please? I know there are series incorporating it, but this should help clear some regressing incompletes from the board. -Chris
On Tue, Jul 17, 2018 at 09:29:32PM +0100, Chris Wilson 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. > > v2: Remove comment about switching back temporarily to execlists on > guc_submission_disable(). We currently only call disable on shutdown, > and plan to also call disable before suspend and reset, in which case we > will either restore guc submission or mark the driver as wedged, making > the reset back to execlists pointless. > v3: Move reset.prepare across > > Testcase: igt/drv_module_reload/basic-reload-inject > Testcase: igt/gem_eio > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> -Michał > --- > drivers/gpu/drm/i915/intel_guc_submission.c | 41 ++++++++++++++------- > drivers/gpu/drm/i915/intel_lrc.c | 4 +- > drivers/gpu/drm/i915/intel_lrc.h | 2 + > 3 files changed, 31 insertions(+), 16 deletions(-)
Quoting Michał Winiarski (2018-07-19 10:10:22) > On Tue, Jul 17, 2018 at 09:29:32PM +0100, Chris Wilson 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. > > > > v2: Remove comment about switching back temporarily to execlists on > > guc_submission_disable(). We currently only call disable on shutdown, > > and plan to also call disable before suspend and reset, in which case we > > will either restore guc submission or mark the driver as wedged, making > > the reset back to execlists pointless. > > v3: Move reset.prepare across > > > > Testcase: igt/drv_module_reload/basic-reload-inject > > Testcase: igt/gem_eio > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Michał Winiarski <michal.winiarski@intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> Ta, and with everything that purports to fix the elusive basic-reload-inject, I now need to find an igt patch and victim. -Chris
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 0fa1eb0bfff5..4aa5e6463e7b 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -1269,6 +1269,31 @@ 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. + */ + intel_execlists_set_default_submission(engine); + + engine->execlists.tasklet.func = guc_submission_tasklet; + + engine->park = guc_submission_park; + engine->unpark = guc_submission_unpark; + + engine->reset.prepare = guc_reset_prepare; + + 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); @@ -1307,17 +1332,8 @@ 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->park = guc_submission_park; - engine->unpark = guc_submission_unpark; - - engine->flags &= ~I915_ENGINE_SUPPORTS_STATS; + engine->set_default_submission = guc_set_default_submission; + engine->set_default_submission(engine); } return 0; @@ -1331,9 +1347,6 @@ void intel_guc_submission_disable(struct intel_guc *guc) guc_interrupts_release(dev_priv); guc_clients_doorbell_fini(guc); - - /* Revert back to manual ELSP submission */ - intel_engines_reset_default_submission(dev_priv); } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index db5351e6a3a5..c64ed9090e29 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2295,7 +2295,7 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *engine) kfree(engine); } -static void execlists_set_default_submission(struct intel_engine_cs *engine) +void intel_execlists_set_default_submission(struct intel_engine_cs *engine) { engine->submit_request = execlists_submit_request; engine->cancel_requests = execlists_cancel_requests; @@ -2335,7 +2335,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) engine->emit_breadcrumb = gen8_emit_breadcrumb; engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz; - engine->set_default_submission = execlists_set_default_submission; + engine->set_default_submission = intel_execlists_set_default_submission; if (INTEL_GEN(engine->i915) < 11) { engine->irq_enable = gen8_logical_ring_enable_irq; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 1593194e930c..4dfb78e3ec7e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -104,4 +104,6 @@ struct i915_gem_context; void intel_lr_context_resume(struct drm_i915_private *dev_priv); +void intel_execlists_set_default_submission(struct intel_engine_cs *engine); + #endif /* _INTEL_LRC_H_ */
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. v2: Remove comment about switching back temporarily to execlists on guc_submission_disable(). We currently only call disable on shutdown, and plan to also call disable before suspend and reset, in which case we will either restore guc submission or mark the driver as wedged, making the reset back to execlists pointless. v3: Move reset.prepare across Testcase: igt/drv_module_reload/basic-reload-inject Testcase: igt/gem_eio Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> --- drivers/gpu/drm/i915/intel_guc_submission.c | 41 ++++++++++++++------- drivers/gpu/drm/i915/intel_lrc.c | 4 +- drivers/gpu/drm/i915/intel_lrc.h | 2 + 3 files changed, 31 insertions(+), 16 deletions(-)