Commit 4055dc75 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Stop touching forcewake following a gen6+ engine reset

Forcewake is not affected by the engine reset on gen6+. Indeed the
reason why we added intel_uncore_forcewake_reset() to
gen6_reset_engines() was to keep the bookkeeping intact because the
reset did not touch the forcewake bit (yet we cancelled the forcewake
consumers)!  This was done in commit 521198a2:
    Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
    Date:   Fri Aug 23 16:52:30 2013 +0300

	drm/i915: sanitize forcewake registers on reset

	In reset we try to restore the forcewake state to
	pre reset state, using forcewake_count. The reset
	doesn't seem to clear the forcewake bits so we
	get warn on forcewake ack register not clearing.

That futzing of the forcewake bookkeeping was dropped in commit
0294ae7b ("drm/i915: Consolidate forcewake resetting to a single
function"), but it did not make the realisation that the remaining
intel_uncore_forcewake_reset() was redundant.

The new danger with using intel_uncore_forcewake_reset() with per-engine
resets is that the driver and hw are still in an active state as we
perform the reset. We may be using the forcewake to read protected
registers elsewhere and those results may be clobbered by the concurrent
dropping of forcewake.
Reported-by: default avatarMichel Thierry <michel.thierry@intel.com>
Fixes: 142bc7d9 ("drm/i915: Modify error handler for per engine hang recovery")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170817173229.20324-1-chris@chris-wilson.co.ukReviewed-by: default avatarMichel Thierry <michel.thierry@intel.com>
Acked-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
parent 7bc66c6a
...@@ -1497,7 +1497,6 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, ...@@ -1497,7 +1497,6 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
[VECS] = GEN6_GRDOM_VECS, [VECS] = GEN6_GRDOM_VECS,
}; };
u32 hw_mask; u32 hw_mask;
int ret;
if (engine_mask == ALL_ENGINES) { if (engine_mask == ALL_ENGINES) {
hw_mask = GEN6_GRDOM_FULL; hw_mask = GEN6_GRDOM_FULL;
...@@ -1509,11 +1508,7 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, ...@@ -1509,11 +1508,7 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
hw_mask |= hw_engine_mask[engine->id]; hw_mask |= hw_engine_mask[engine->id];
} }
ret = gen6_hw_domain_reset(dev_priv, hw_mask); return gen6_hw_domain_reset(dev_priv, hw_mask);
intel_uncore_forcewake_reset(dev_priv, true);
return ret;
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment