| Message ID | 20180530150206.7798-1-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Mika Kuoppala (2018-05-30 16:02:05) > Our reset handling has a retry layer further up in the > chain. As we have told the engine to prepare for reset, > and failed it, make sure to remove that preparation so > that the next attempted reset has a clean slate by triggering > another full prepare cycle for the engines. Note that with > successful reset, there is nothing to cleanup. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index b36a3b5736a0..68fe4c16acfb 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -2092,22 +2092,26 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv, > unsigned engine_mask) > { > struct intel_engine_cs *engine; > - unsigned int tmp; > + unsigned int tmp, ret; int ret; Might as well keep the types clean. > - for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > - if (gen8_reset_engine_start(engine)) > + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { > + if (gen8_reset_engine_start(engine)) { > + ret = -EIO; > goto not_ready; > + } > + } > > if (INTEL_GEN(dev_priv) >= 11) > - return gen11_reset_engines(dev_priv, engine_mask); > + ret = gen11_reset_engines(dev_priv, engine_mask); > else > - return gen6_reset_engines(dev_priv, engine_mask); > + ret = gen6_reset_engines(dev_priv, engine_mask); > > not_ready: > - for_each_engine_masked(engine, dev_priv, engine_mask, tmp) > - gen8_reset_engine_cancel(engine); > + if (ret) Or we just always clear the bit here. On the success path, it will be just a nop. On the not_ready path we're already clearing untouched engines, so might as well go whole hog and overkill everything? Might as well throw in a References: https://bugs.freedesktop.org/show_bug.cgi?id=106684 -Chris
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index b36a3b5736a0..68fe4c16acfb 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2092,22 +2092,26 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv, unsigned engine_mask) { struct intel_engine_cs *engine; - unsigned int tmp; + unsigned int tmp, ret; - for_each_engine_masked(engine, dev_priv, engine_mask, tmp) - if (gen8_reset_engine_start(engine)) + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { + if (gen8_reset_engine_start(engine)) { + ret = -EIO; goto not_ready; + } + } if (INTEL_GEN(dev_priv) >= 11) - return gen11_reset_engines(dev_priv, engine_mask); + ret = gen11_reset_engines(dev_priv, engine_mask); else - return gen6_reset_engines(dev_priv, engine_mask); + ret = gen6_reset_engines(dev_priv, engine_mask); not_ready: - for_each_engine_masked(engine, dev_priv, engine_mask, tmp) - gen8_reset_engine_cancel(engine); + if (ret) + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) + gen8_reset_engine_cancel(engine); - return -EIO; + return ret; } typedef int (*reset_func)(struct drm_i915_private *, unsigned engine_mask);
Our reset handling has a retry layer further up in the chain. As we have told the engine to prepare for reset, and failed it, make sure to remove that preparation so that the next attempted reset has a clean slate by triggering another full prepare cycle for the engines. Note that with successful reset, there is nothing to cleanup. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)