Commit b7dc9395 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Check caller held wakerefs in assert_forcewakes_active

The intent of the assert is to document that the caller took the
appropriate wakerefs for the function. However, as Tvrtko pointed out,
we simply check whether the fw_domains are active and may be confused by
the auto wakeref which may be dropped between the check and use. Let's
be more careful in the assert and check that each fw_domain has an
explicit caller wakeref above and beyond the automatic wakeref.

v2: Fix spelling for config DRM_I915_DEBUG_RUNTIME_PM
v3: Timer may still be active after we drop the autowakeref, we need to
check domain->active instead.
v4: The timer checks domain->active, but we still need to check the
timer. (This is starting to look weird...)
Reported-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190704102048.6436-1-chris@chris-wilson.co.uk
parent 4fda44bf
...@@ -738,6 +738,12 @@ void assert_forcewakes_inactive(struct intel_uncore *uncore) ...@@ -738,6 +738,12 @@ void assert_forcewakes_inactive(struct intel_uncore *uncore)
void assert_forcewakes_active(struct intel_uncore *uncore, void assert_forcewakes_active(struct intel_uncore *uncore,
enum forcewake_domains fw_domains) enum forcewake_domains fw_domains)
{ {
struct intel_uncore_forcewake_domain *domain;
unsigned int tmp;
if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM))
return;
if (!uncore->funcs.force_wake_get) if (!uncore->funcs.force_wake_get)
return; return;
...@@ -747,6 +753,24 @@ void assert_forcewakes_active(struct intel_uncore *uncore, ...@@ -747,6 +753,24 @@ void assert_forcewakes_active(struct intel_uncore *uncore,
WARN(fw_domains & ~uncore->fw_domains_active, WARN(fw_domains & ~uncore->fw_domains_active,
"Expected %08x fw_domains to be active, but %08x are off\n", "Expected %08x fw_domains to be active, but %08x are off\n",
fw_domains, fw_domains & ~uncore->fw_domains_active); fw_domains, fw_domains & ~uncore->fw_domains_active);
/*
* Check that the caller has an explicit wakeref and we don't mistake
* it for the auto wakeref.
*/
local_irq_disable();
for_each_fw_domain_masked(domain, fw_domains, uncore, tmp) {
unsigned int expect = 1;
if (hrtimer_active(&domain->timer) && READ_ONCE(domain->active))
expect++; /* pending automatic release */
if (WARN(domain->wake_count < expect,
"Expected domain %d to be held awake by caller, count=%d\n",
domain->id, domain->wake_count))
break;
}
local_irq_enable();
} }
/* We give fast paths for the really cool registers */ /* We give fast paths for the really cool registers */
......
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