drm/i915: Provide an assert for when we expect forcewake to be held
authorChris Wilson <chris@chris-wilson.co.uk>
Mon, 9 Oct 2017 11:03:01 +0000 (12:03 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Mon, 9 Oct 2017 16:07:29 +0000 (17:07 +0100)
Add assert_forcewakes_active() (the complementary function to
assert_forcewakes_inactive) that documents the requirement of a
function for its callers to be holding the forcewake ref (i.e. the
function is part of a sequence over which RC6 must be prevented).

One such example is during ringbuffer reset, where RC6 must be held
across the whole reinitialisation sequence.

v2: Include debug information in the WARN so we know which fw domain is
missing.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> #v1
Link: https://patchwork.freedesktop.org/patch/msgid/20171009110301.21705-5-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/intel_ringbuffer.c
drivers/gpu/drm/i915/intel_uncore.c
drivers/gpu/drm/i915/intel_uncore.h

index 05c08b0bc172fea7fcdaf02cbd909097345c8f25..4285f09ff8b843793fd521798ac180d1407db137 100644 (file)
@@ -579,7 +579,16 @@ out:
 static void reset_ring_common(struct intel_engine_cs *engine,
                              struct drm_i915_gem_request *request)
 {
-       /* Try to restore the logical GPU state to match the continuation
+       /*
+        * RC6 must be prevented until the reset is complete and the engine
+        * reinitialised. If it occurs in the middle of this sequence, the
+        * state written to/loaded from the power context is ill-defined (e.g.
+        * the PP_BASE_DIR may be lost).
+        */
+       assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
+
+       /*
+        * Try to restore the logical GPU state to match the continuation
         * of the request queue. If we skip the context/PD restore, then
         * the next request may try to execute assuming that its context
         * is valid and loaded on the GPU and so may try to access invalid
index b3c3f94fc7e46ae196c89522036fcbd27058eded..983617b5b338d3da3a9ae1a69a812a6de4068a36 100644 (file)
@@ -626,7 +626,23 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
        if (!dev_priv->uncore.funcs.force_wake_get)
                return;
 
-       WARN_ON(dev_priv->uncore.fw_domains_active);
+       WARN(dev_priv->uncore.fw_domains_active,
+            "Expected all fw_domains to be inactive, but %08x are still on\n",
+            dev_priv->uncore.fw_domains_active);
+}
+
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+                             enum forcewake_domains fw_domains)
+{
+       if (!dev_priv->uncore.funcs.force_wake_get)
+               return;
+
+       assert_rpm_wakelock_held(dev_priv);
+
+       fw_domains &= dev_priv->uncore.fw_domains;
+       WARN(fw_domains & ~dev_priv->uncore.fw_domains_active,
+            "Expected %08x fw_domains to be active, but %08x are off\n",
+            fw_domains, fw_domains & ~dev_priv->uncore.fw_domains_active);
 }
 
 /* We give fast paths for the really cool registers */
index 66eae2ce2f29fa1e556affc1573aecb2a639fddf..582771251b57a28122f98a8092bfd1a3211c0590 100644 (file)
@@ -137,6 +137,8 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv);
 
 u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
 void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+void assert_forcewakes_active(struct drm_i915_private *dev_priv,
+                             enum forcewake_domains fw_domains);
 const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
 
 enum forcewake_domains