On 10/03/2017 08:50, Chris Wilson wrote:
On Fri, Mar 10, 2017 at 07:32:51AM +0000, Tvrtko Ursulin wrote:From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> In commit 003342a50021 ("drm/i915: Keep track of active forcewake domains in a bitmask") I forgot to adjust the newly introduce fw_domains_active state across reset. This caused the assert_forcewakes_inactive to trigger during suspend and resume if there were user held forcewakes. v2: Bitmask checks are required since vfuncs are not always present. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Fixes: 003342a50021 ("drm/i915: Keep track of active forcewake domains in a bitmask") Testcase: igt/drv_suspend/forcewake Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: "Paneri, Praveen" <praveen.paneri@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: v4.10+ <stable@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/intel_uncore.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 2a3f35c30501..7efae77faca0 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -304,12 +304,14 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, fw = dev_priv->uncore.fw_domains_active; if (fw) dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); + dev_priv->uncore.fw_domains_active = 0;Hmm, I think we would be happier with (think of a good name) static void __intel_uncore_force_wake_get(i915, fw_domains) { i915->uncore.funcs.force_wake_get(i915, fw_domains); i915->uncore.funcs.fw_domains_active |= fw_domain; } static void __intel_uncore_force_wake_put(i915, fw_domains) { i915->uncore.funcs.force_wake_put(i915, fw_domains); i915->uncore.funcs.fw_domains_active &= ~fw_domain; } Another alternative would be to move the bitops down to the callbacks. gcc might be happier, at the expense of some duplication and risk of forgetting. Anyway worth the effort?
Yes I agree, was considering the second option myself. Think I still prefer that one to keep the inseparable together.
Regards, Tvrtko