This regression has been introduced in
commit 8232644ccf099548710843e97360a3fcd6d28e04
Author: Chris Wilson <[email protected]>
Date: Wed Mar 5 12:00:39 2014 +0000
drm/i915: Convert the forcewake worker into a timer func
which started to use the delayed forcewake put also for the register
I/O forcewake dance. But this conflicts functionally with the
(reviewed in parallel)
commit 6d88064edcfc5e5893371f7c06b9f3078dc1edf6
Author: Paulo Zanoni <[email protected]>
Date: Fri Feb 21 17:58:29 2014 -0300
drm/i915: put runtime PM only when we actually release force_wake
which moved the runtime pm put calls into the delayed forcewake put
function to avoid an inversion between these. The problem is that the
register I/O function do _not_ grab a runtime pm reference, hence
dropping it is a bug.
So split the timer into two to re-balance the runtime pm refcounting.
The tricky bit to ensure is that the _raw timer doesn't run after
we've runtime-suspended the device. After all it only has an implicit
runtime pm reference provided by its caller. But the del_timer_sync in
the runtime suspend code will ensure this.
Add a comment to document this all.
Cc: Chris Wilson <[email protected]>
Cc: Ben Widawsky <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Paulo Zanoni <[email protected]>
Cc: Jesse Barnes <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76151
Signed-off-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++++++++
drivers/gpu/drm/i915/intel_uncore.c | 14 ++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe904016f..c2789702b9a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -511,7 +511,16 @@ struct intel_uncore {
unsigned fw_rendercount;
unsigned fw_mediacount;
+ /*
+ * Delayed forcewake put timers. The _raw one is for register I/O
+ * functions which don't grab a runtime pm reference, instead presuming
+ * that someone else holds that already.
+ *
+ * Synchronization with runtime pm actually suspended the device happens
+ * in the runtime suspend path in intel_uncore_forcewake_reset.
+ */
struct timer_list force_wake_timer;
+ struct timer_list force_wake_timer_raw;
};
#define DEV_INFO_FOR_EACH_FLAG(func, sep) \
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index e6bb421a3dbd..8010e2caf821 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -293,7 +293,7 @@ void vlv_force_wake_put(struct drm_i915_private *dev_priv,
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
}
-static void gen6_force_wake_timer(unsigned long arg)
+static void gen6_force_wake_timer_raw(unsigned long arg)
{
struct drm_i915_private *dev_priv = (void *)arg;
unsigned long irqflags;
@@ -304,6 +304,13 @@ static void gen6_force_wake_timer(unsigned long arg)
if (--dev_priv->uncore.forcewake_count == 0)
dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void gen6_force_wake_timer(unsigned long arg)
+{
+ struct drm_i915_private *dev_priv = (void *)arg;
+
+ gen6_force_wake_timer_raw(arg);
intel_runtime_pm_put(dev_priv);
}
@@ -314,6 +321,7 @@ static void intel_uncore_forcewake_reset(struct drm_device
*dev, bool restore)
unsigned long irqflags;
del_timer_sync(&dev_priv->uncore.force_wake_timer);
+ del_timer_sync(&dev_priv->uncore.force_wake_timer_raw);
/* Hold uncore.lock across reset to prevent any register access
* with forcewake not set correctly
@@ -542,7 +550,7 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg,
bool trace) { \
dev_priv->uncore.funcs.force_wake_get(dev_priv, \
FORCEWAKE_ALL); \
dev_priv->uncore.forcewake_count++; \
- mod_timer_pinned(&dev_priv->uncore.force_wake_timer, \
+ mod_timer_pinned(&dev_priv->uncore.force_wake_timer_raw, \
jiffies + 1); \
} \
val = __raw_i915_read##x(dev_priv, reg); \
@@ -726,6 +734,8 @@ void intel_uncore_init(struct drm_device *dev)
setup_timer(&dev_priv->uncore.force_wake_timer,
gen6_force_wake_timer, (unsigned long)dev_priv);
+ setup_timer(&dev_priv->uncore.force_wake_timer_raw,
+ gen6_force_wake_timer_raw, (unsigned long)dev_priv);
if (IS_VALLEYVIEW(dev)) {
dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
--
1.8.5.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
