[Intel-gfx] [PATCH v2] drm/i915: Fix forcewake active domain tracking

[Chris Wilson]

On Fri, Mar 10, 2017 at 07:32:51AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> 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 at intel.com>
> Fixes: 003342a50021 ("drm/i915: Keep track of active forcewake domains in a bitmask")
> Testcase: igt/drv_suspend/forcewake
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: "Paneri, Praveen" <praveen.paneri at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Cc: v4.10+ <stable at vger.kernel.org>
> ---
>  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?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre