| Message ID | 20190626154549.10066-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Add a wakeref getter for iff the wakeref is already active | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Since the reset path wants to recover the engines itself, it only wants > to reinitialise the hardware using i915_gem_init_hw(). Pull the call to > intel_engines_resume() to the module init/resume path so we can avoid it > during reset. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_pm.c | 7 ++- > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 24 -------- > drivers/gpu/drm/i915/gt/intel_engine_pm.h | 2 - > drivers/gpu/drm/i915/gt/intel_gt_pm.c | 21 ++++++- > drivers/gpu/drm/i915/gt/intel_gt_pm.h | 2 +- > drivers/gpu/drm/i915/gt/intel_reset.c | 21 ++++++- > drivers/gpu/drm/i915/i915_gem.c | 71 +++++++---------------- > 7 files changed, 65 insertions(+), 83 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c > index 6b730bd4d72f..4d774376f5b8 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c > @@ -254,14 +254,15 @@ void i915_gem_resume(struct drm_i915_private *i915) > i915_gem_restore_gtt_mappings(i915); > i915_gem_restore_fences(i915); > > + if (i915_gem_init_hw(i915)) > + goto err_wedged; > + > /* > * As we didn't flush the kernel context before suspend, we cannot > * guarantee that the context image is complete. So let's just reset > * it and start again. > */ > - intel_gt_resume(&i915->gt); > - > - if (i915_gem_init_hw(i915)) > + if (intel_gt_resume(&i915->gt)) > goto err_wedged; > > intel_uc_resume(i915); > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > index 5253c382034d..84e432abe8e0 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > @@ -142,27 +142,3 @@ void intel_engine_init__pm(struct intel_engine_cs *engine) > { > intel_wakeref_init(&engine->wakeref); > } > - > -int intel_engines_resume(struct drm_i915_private *i915) > -{ > - struct intel_engine_cs *engine; > - enum intel_engine_id id; > - int err = 0; > - > - intel_gt_pm_get(&i915->gt); > - for_each_engine(engine, i915, id) { > - intel_engine_pm_get(engine); > - engine->serial++; /* kernel context lost */ > - err = engine->resume(engine); > - intel_engine_pm_put(engine); > - if (err) { > - dev_err(i915->drm.dev, > - "Failed to restart %s (%d)\n", > - engine->name, err); > - break; > - } > - } > - intel_gt_pm_put(&i915->gt); > - > - return err; > -} > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h > index 7d057cdcd919..015ac72d7ad0 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h > @@ -31,6 +31,4 @@ void intel_engine_park(struct intel_engine_cs *engine); > > void intel_engine_init__pm(struct intel_engine_cs *engine); > > -int intel_engines_resume(struct drm_i915_private *i915); > - > #endif /* INTEL_ENGINE_PM_H */ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > index ec6b69d014b6..36ba80e6a0b7 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > @@ -5,6 +5,7 @@ > */ > > #include "i915_drv.h" > +#include "intel_engine_pm.h" > #include "intel_gt_pm.h" > #include "intel_pm.h" > #include "intel_wakeref.h" > @@ -122,10 +123,11 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force) > intel_engine_reset(engine, false); > } > > -void intel_gt_resume(struct intel_gt *gt) > +int intel_gt_resume(struct intel_gt *gt) > { > struct intel_engine_cs *engine; > enum intel_engine_id id; > + int err = 0; > > /* > * After resume, we may need to poke into the pinned kernel > @@ -133,9 +135,12 @@ void intel_gt_resume(struct intel_gt *gt) > * Only the kernel contexts should remain pinned over suspend, > * allowing us to fixup the user contexts on their first pin. > */ > + intel_gt_pm_get(gt); > for_each_engine(engine, gt->i915, id) { > struct intel_context *ce; > > + intel_engine_pm_get(engine); > + > ce = engine->kernel_context; > if (ce) > ce->ops->reset(ce); > @@ -143,5 +148,19 @@ void intel_gt_resume(struct intel_gt *gt) > ce = engine->preempt_context; > if (ce) > ce->ops->reset(ce); > + > + engine->serial++; /* kernel context lost */ > + err = engine->resume(engine); > + > + intel_engine_pm_put(engine); > + if (err) { > + dev_err(gt->i915->drm.dev, > + "Failed to restart %s (%d)\n", > + engine->name, err); > + break; > + } > } > + intel_gt_pm_put(gt); > + > + return err; > } > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > index 4dbb92cf58d7..ba960e1fc209 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > @@ -22,6 +22,6 @@ void intel_gt_pm_put(struct intel_gt *gt); > void intel_gt_pm_init_early(struct intel_gt *gt); > > void intel_gt_sanitize(struct intel_gt *gt, bool force); > -void intel_gt_resume(struct intel_gt *gt); > +int intel_gt_resume(struct intel_gt *gt); > > #endif /* INTEL_GT_PM_H */ > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index e7cbd9cf85c1..adfdb908587f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -949,6 +949,21 @@ static int do_reset(struct drm_i915_private *i915, > return gt_reset(i915, stalled_mask); > } > > +static int resume(struct drm_i915_private *i915) > +{ > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > + int ret; > + > + for_each_engine(engine, i915, id) { > + ret = engine->resume(engine); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > /** > * i915_reset - reset chip after a hang > * @i915: #drm_i915_private to reset > @@ -1023,9 +1038,13 @@ void i915_reset(struct drm_i915_private *i915, > if (ret) { > DRM_ERROR("Failed to initialise HW following reset (%d)\n", > ret); > - goto error; > + goto taint; > } > > + ret = resume(i915); > + if (ret) > + goto taint; > + > i915_queue_hangcheck(i915); > > finish: > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index deecbe128e5b..b7f290b77f8f 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -46,7 +46,6 @@ > #include "gem/i915_gem_ioctls.h" > #include "gem/i915_gem_pm.h" > #include "gem/i915_gemfs.h" > -#include "gt/intel_engine_pm.h" > #include "gt/intel_gt.h" > #include "gt/intel_gt_pm.h" > #include "gt/intel_mocs.h" > @@ -1192,12 +1191,17 @@ static void init_unused_rings(struct intel_gt *gt) > } > } > > -static int init_hw(struct intel_gt *gt) > +int i915_gem_init_hw(struct drm_i915_private *i915) > { > - struct drm_i915_private *i915 = gt->i915; > - struct intel_uncore *uncore = gt->uncore; > + struct intel_uncore *uncore = &i915->uncore; > + struct intel_gt *gt = &i915->gt; > int ret; > > + BUG_ON(!i915->kernel_context); > + ret = i915_terminally_wedged(i915); > + if (ret) > + return ret; > + > gt->last_init_time = ktime_get(); > > /* Double layer security blanket, see i915_gem_init() */ > @@ -1248,51 +1252,10 @@ static int init_hw(struct intel_gt *gt) > > intel_mocs_init_l3cc_table(gt); > > - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); > - > - return 0; > - > -out: > - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); > - > - return ret; > -} > - > -int i915_gem_init_hw(struct drm_i915_private *i915) > -{ > - struct intel_uncore *uncore = &i915->uncore; > - int ret; > - > - BUG_ON(!i915->kernel_context); > - ret = i915_terminally_wedged(i915); > - if (ret) > - return ret; > - > - /* Double layer security blanket, see i915_gem_init() */ > - intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL); > - > - ret = init_hw(&i915->gt); > - if (ret) > - goto err_init; > - > - /* Only when the HW is re-initialised, can we replay the requests */ > - ret = intel_engines_resume(i915); > - if (ret) > - goto err_engines; > - > - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); > - > intel_engines_set_scheduler_caps(i915); > > - return 0; > - > -err_engines: > - intel_uc_fini_hw(i915); > -err_init: > +out: > intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); > - > - intel_engines_set_scheduler_caps(i915); > - > return ret; > } > > @@ -1524,6 +1487,11 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > if (ret) > goto err_uc_init; > > + /* Only when the HW is re-initialised, can we replay the requests */ > + ret = intel_gt_resume(&dev_priv->gt); > + if (ret) > + goto err_init_hw; > + > /* > * Despite its name intel_init_clock_gating applies both display > * clock gating workarounds; GT mmio workarounds and the occasional > @@ -1537,20 +1505,20 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > > ret = intel_engines_verify_workarounds(dev_priv); > if (ret) > - goto err_init_hw; > + goto err_gt; > > ret = __intel_engines_record_defaults(dev_priv); > if (ret) > - goto err_init_hw; > + goto err_gt; > > if (i915_inject_load_failure()) { > ret = -ENODEV; > - goto err_init_hw; > + goto err_gt; > } > > if (i915_inject_load_failure()) { > ret = -EIO; > - goto err_init_hw; > + goto err_gt; > } > > intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL); > @@ -1564,7 +1532,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > * HW as irrevisibly wedged, but keep enough state around that the > * driver doesn't explode during runtime. > */ > -err_init_hw: > +err_gt: > mutex_unlock(&dev_priv->drm.struct_mutex); > > i915_gem_set_wedged(dev_priv); > @@ -1574,6 +1542,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > i915_gem_drain_workqueue(dev_priv); > > mutex_lock(&dev_priv->drm.struct_mutex); > +err_init_hw: > intel_uc_fini_hw(dev_priv); > err_uc_init: > intel_uc_fini(dev_priv); > -- > 2.20.1
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index 6b730bd4d72f..4d774376f5b8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -254,14 +254,15 @@ void i915_gem_resume(struct drm_i915_private *i915) i915_gem_restore_gtt_mappings(i915); i915_gem_restore_fences(i915); + if (i915_gem_init_hw(i915)) + goto err_wedged; + /* * As we didn't flush the kernel context before suspend, we cannot * guarantee that the context image is complete. So let's just reset * it and start again. */ - intel_gt_resume(&i915->gt); - - if (i915_gem_init_hw(i915)) + if (intel_gt_resume(&i915->gt)) goto err_wedged; intel_uc_resume(i915); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index 5253c382034d..84e432abe8e0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -142,27 +142,3 @@ void intel_engine_init__pm(struct intel_engine_cs *engine) { intel_wakeref_init(&engine->wakeref); } - -int intel_engines_resume(struct drm_i915_private *i915) -{ - struct intel_engine_cs *engine; - enum intel_engine_id id; - int err = 0; - - intel_gt_pm_get(&i915->gt); - for_each_engine(engine, i915, id) { - intel_engine_pm_get(engine); - engine->serial++; /* kernel context lost */ - err = engine->resume(engine); - intel_engine_pm_put(engine); - if (err) { - dev_err(i915->drm.dev, - "Failed to restart %s (%d)\n", - engine->name, err); - break; - } - } - intel_gt_pm_put(&i915->gt); - - return err; -} diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h index 7d057cdcd919..015ac72d7ad0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h @@ -31,6 +31,4 @@ void intel_engine_park(struct intel_engine_cs *engine); void intel_engine_init__pm(struct intel_engine_cs *engine); -int intel_engines_resume(struct drm_i915_private *i915); - #endif /* INTEL_ENGINE_PM_H */ diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index ec6b69d014b6..36ba80e6a0b7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -5,6 +5,7 @@ */ #include "i915_drv.h" +#include "intel_engine_pm.h" #include "intel_gt_pm.h" #include "intel_pm.h" #include "intel_wakeref.h" @@ -122,10 +123,11 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force) intel_engine_reset(engine, false); } -void intel_gt_resume(struct intel_gt *gt) +int intel_gt_resume(struct intel_gt *gt) { struct intel_engine_cs *engine; enum intel_engine_id id; + int err = 0; /* * After resume, we may need to poke into the pinned kernel @@ -133,9 +135,12 @@ void intel_gt_resume(struct intel_gt *gt) * Only the kernel contexts should remain pinned over suspend, * allowing us to fixup the user contexts on their first pin. */ + intel_gt_pm_get(gt); for_each_engine(engine, gt->i915, id) { struct intel_context *ce; + intel_engine_pm_get(engine); + ce = engine->kernel_context; if (ce) ce->ops->reset(ce); @@ -143,5 +148,19 @@ void intel_gt_resume(struct intel_gt *gt) ce = engine->preempt_context; if (ce) ce->ops->reset(ce); + + engine->serial++; /* kernel context lost */ + err = engine->resume(engine); + + intel_engine_pm_put(engine); + if (err) { + dev_err(gt->i915->drm.dev, + "Failed to restart %s (%d)\n", + engine->name, err); + break; + } } + intel_gt_pm_put(gt); + + return err; } diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index 4dbb92cf58d7..ba960e1fc209 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -22,6 +22,6 @@ void intel_gt_pm_put(struct intel_gt *gt); void intel_gt_pm_init_early(struct intel_gt *gt); void intel_gt_sanitize(struct intel_gt *gt, bool force); -void intel_gt_resume(struct intel_gt *gt); +int intel_gt_resume(struct intel_gt *gt); #endif /* INTEL_GT_PM_H */ diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index e7cbd9cf85c1..adfdb908587f 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -949,6 +949,21 @@ static int do_reset(struct drm_i915_private *i915, return gt_reset(i915, stalled_mask); } +static int resume(struct drm_i915_private *i915) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + int ret; + + for_each_engine(engine, i915, id) { + ret = engine->resume(engine); + if (ret) + return ret; + } + + return 0; +} + /** * i915_reset - reset chip after a hang * @i915: #drm_i915_private to reset @@ -1023,9 +1038,13 @@ void i915_reset(struct drm_i915_private *i915, if (ret) { DRM_ERROR("Failed to initialise HW following reset (%d)\n", ret); - goto error; + goto taint; } + ret = resume(i915); + if (ret) + goto taint; + i915_queue_hangcheck(i915); finish: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index deecbe128e5b..b7f290b77f8f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -46,7 +46,6 @@ #include "gem/i915_gem_ioctls.h" #include "gem/i915_gem_pm.h" #include "gem/i915_gemfs.h" -#include "gt/intel_engine_pm.h" #include "gt/intel_gt.h" #include "gt/intel_gt_pm.h" #include "gt/intel_mocs.h" @@ -1192,12 +1191,17 @@ static void init_unused_rings(struct intel_gt *gt) } } -static int init_hw(struct intel_gt *gt) +int i915_gem_init_hw(struct drm_i915_private *i915) { - struct drm_i915_private *i915 = gt->i915; - struct intel_uncore *uncore = gt->uncore; + struct intel_uncore *uncore = &i915->uncore; + struct intel_gt *gt = &i915->gt; int ret; + BUG_ON(!i915->kernel_context); + ret = i915_terminally_wedged(i915); + if (ret) + return ret; + gt->last_init_time = ktime_get(); /* Double layer security blanket, see i915_gem_init() */ @@ -1248,51 +1252,10 @@ static int init_hw(struct intel_gt *gt) intel_mocs_init_l3cc_table(gt); - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); - - return 0; - -out: - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); - - return ret; -} - -int i915_gem_init_hw(struct drm_i915_private *i915) -{ - struct intel_uncore *uncore = &i915->uncore; - int ret; - - BUG_ON(!i915->kernel_context); - ret = i915_terminally_wedged(i915); - if (ret) - return ret; - - /* Double layer security blanket, see i915_gem_init() */ - intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL); - - ret = init_hw(&i915->gt); - if (ret) - goto err_init; - - /* Only when the HW is re-initialised, can we replay the requests */ - ret = intel_engines_resume(i915); - if (ret) - goto err_engines; - - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); - intel_engines_set_scheduler_caps(i915); - return 0; - -err_engines: - intel_uc_fini_hw(i915); -err_init: +out: intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL); - - intel_engines_set_scheduler_caps(i915); - return ret; } @@ -1524,6 +1487,11 @@ int i915_gem_init(struct drm_i915_private *dev_priv) if (ret) goto err_uc_init; + /* Only when the HW is re-initialised, can we replay the requests */ + ret = intel_gt_resume(&dev_priv->gt); + if (ret) + goto err_init_hw; + /* * Despite its name intel_init_clock_gating applies both display * clock gating workarounds; GT mmio workarounds and the occasional @@ -1537,20 +1505,20 @@ int i915_gem_init(struct drm_i915_private *dev_priv) ret = intel_engines_verify_workarounds(dev_priv); if (ret) - goto err_init_hw; + goto err_gt; ret = __intel_engines_record_defaults(dev_priv); if (ret) - goto err_init_hw; + goto err_gt; if (i915_inject_load_failure()) { ret = -ENODEV; - goto err_init_hw; + goto err_gt; } if (i915_inject_load_failure()) { ret = -EIO; - goto err_init_hw; + goto err_gt; } intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL); @@ -1564,7 +1532,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) * HW as irrevisibly wedged, but keep enough state around that the * driver doesn't explode during runtime. */ -err_init_hw: +err_gt: mutex_unlock(&dev_priv->drm.struct_mutex); i915_gem_set_wedged(dev_priv); @@ -1574,6 +1542,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) i915_gem_drain_workqueue(dev_priv); mutex_lock(&dev_priv->drm.struct_mutex); +err_init_hw: intel_uc_fini_hw(dev_priv); err_uc_init: intel_uc_fini(dev_priv);
Since the reset path wants to recover the engines itself, it only wants to reinitialise the hardware using i915_gem_init_hw(). Pull the call to intel_engines_resume() to the module init/resume path so we can avoid it during reset. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 7 ++- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 24 -------- drivers/gpu/drm/i915/gt/intel_engine_pm.h | 2 - drivers/gpu/drm/i915/gt/intel_gt_pm.c | 21 ++++++- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 2 +- drivers/gpu/drm/i915/gt/intel_reset.c | 21 ++++++- drivers/gpu/drm/i915/i915_gem.c | 71 +++++++---------------- 7 files changed, 65 insertions(+), 83 deletions(-)