It's useful to track runtime PM refs that don't guarantee a device
power-on state to the rest of the driver. One such case is holding a
reference that will be put asynchronously, during which normal users
without their own reference shouldn't access the HW. A follow-up patch
will add support for disabling display power domains asynchronously
which needs this.
For this we can split wakeref_count into a low half-word tracking
all references (raw-wakerefs) and a high half-word tracking
references guaranteeing a power-on state (wakelocks).
Follow-up patches will make use of the API added here.
While at it add the missing docbook header for the unchecked
display-power and runtime_pm put functions.
No functional changes, except for printing leaked raw-wakerefs
and wakelocks separately in intel_runtime_pm_cleanup().
v2:
- Track raw wakerefs/wakelocks in the low/high half-word of
wakeref_count, instead of adding a new counter. (Chris)
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_drv.h | 52 +++++++-
drivers/gpu/drm/i915/intel_runtime_pm.c | 150 ++++++++++++++++++------
2 files changed, 160 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 247893ed1543..772ed0fedb39 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1619,6 +1619,24 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
unsigned int rotation);
/* intel_runtime_pm.c */
+#define BITS_PER_WAKEREF \
+ BITS_PER_TYPE(((struct i915_runtime_pm *)NULL)->wakeref_count)
+#define INTEL_RPM_WAKELOCK_SHIFT (BITS_PER_WAKEREF / 2)
+#define INTEL_RPM_WAKELOCK_BIAS (1 << INTEL_RPM_WAKELOCK_SHIFT)
+#define INTEL_RPM_RAW_WAKEREF_MASK (INTEL_RPM_WAKELOCK_BIAS - 1)
+
+static inline int
+intel_rpm_raw_wakeref_count(int wakeref_count)
+{
+ return wakeref_count & INTEL_RPM_RAW_WAKEREF_MASK;
+}
+
+static inline int
+intel_rpm_wakelock_count(int wakeref_count)
+{
+ return wakeref_count >> INTEL_RPM_WAKELOCK_SHIFT;
+}
+
static inline void
assert_rpm_device_not_suspended(struct i915_runtime_pm *rpm)
{
@@ -1627,11 +1645,33 @@ assert_rpm_device_not_suspended(struct i915_runtime_pm *rpm)
}
static inline void
-__assert_rpm_wakelock_held(struct i915_runtime_pm *rpm)
+____assert_rpm_raw_wakeref_held(struct i915_runtime_pm *rpm, int wakeref_count)
{
assert_rpm_device_not_suspended(rpm);
- WARN_ONCE(!atomic_read(&rpm->wakeref_count),
- "RPM wakelock ref not held during HW access");
+ WARN_ONCE(!intel_rpm_raw_wakeref_count(wakeref_count),
+ "RPM raw-wakeref not held\n");
+}
+
+static inline void
+____assert_rpm_wakelock_held(struct i915_runtime_pm *rpm, int wakeref_count)
+{
+ ____assert_rpm_raw_wakeref_held(rpm, wakeref_count);
+ WARN_ONCE(!intel_rpm_wakelock_count(wakeref_count),
+ "RPM wakelock ref not held during HW access\n");
+}
+
+static inline void
+assert_rpm_raw_wakeref_held(struct drm_i915_private *i915)
+{
+ struct i915_runtime_pm *rpm = &i915->runtime_pm;
+
+ ____assert_rpm_raw_wakeref_held(rpm, atomic_read(&rpm->wakeref_count));
+}
+
+static inline void
+__assert_rpm_wakelock_held(struct i915_runtime_pm *rpm)
+{
+ ____assert_rpm_wakelock_held(rpm, atomic_read(&rpm->wakeref_count));
}
static inline void
@@ -1661,7 +1701,8 @@ assert_rpm_wakelock_held(struct drm_i915_private *i915)
static inline void
disable_rpm_wakeref_asserts(struct drm_i915_private *i915)
{
- atomic_inc(&i915->runtime_pm.wakeref_count);
+ atomic_add(INTEL_RPM_WAKELOCK_BIAS + 1,
+ &i915->runtime_pm.wakeref_count);
}
/**
@@ -1678,7 +1719,8 @@ disable_rpm_wakeref_asserts(struct drm_i915_private *i915)
static inline void
enable_rpm_wakeref_asserts(struct drm_i915_private *i915)
{
- atomic_dec(&i915->runtime_pm.wakeref_count);
+ atomic_sub(INTEL_RPM_WAKELOCK_BIAS + 1,
+ &i915->runtime_pm.wakeref_count);
}
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b8fadd1b685c..84a18d8b942c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -110,9 +110,6 @@ track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
depot_stack_handle_t stack, *stacks;
unsigned long flags;
- atomic_inc(&rpm->wakeref_count);
- assert_rpm_wakelock_held(i915);
-
if (!HAS_RUNTIME_PM(i915))
return -1;
@@ -140,7 +137,7 @@ track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
return stack;
}
-static void cancel_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
+static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
depot_stack_handle_t stack)
{
struct i915_runtime_pm *rpm = &i915->runtime_pm;
@@ -236,14 +233,13 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
}
static noinline void
-untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+__intel_wakeref_dec_and_check_tracking(struct drm_i915_private *i915)
{
struct i915_runtime_pm *rpm = &i915->runtime_pm;
struct intel_runtime_pm_debug dbg = {};
struct drm_printer p;
unsigned long flags;
- assert_rpm_wakelock_held(i915);
if (atomic_dec_and_lock_irqsave(&rpm->wakeref_count,
&rpm->debug.lock,
flags)) {
@@ -311,19 +307,51 @@ static void init_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
static depot_stack_handle_t
track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
{
- atomic_inc(&i915->runtime_pm.wakeref_count);
- assert_rpm_wakelock_held(i915);
return -1;
}
-static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
+ intel_wakeref_t wref)
+{
+}
+
+static void
+__intel_wakeref_dec_and_check_tracking(struct drm_i915_private *i915)
{
- assert_rpm_wakelock_held(i915);
atomic_dec(&i915->runtime_pm.wakeref_count);
}
#endif
+static void
+intel_runtime_pm_acquire(struct drm_i915_private *i915, bool wakelock)
+{
+ struct i915_runtime_pm *rpm = &i915->runtime_pm;
+
+ if (wakelock) {
+ atomic_add(1 + INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
+ assert_rpm_wakelock_held(i915);
+ } else {
+ atomic_inc(&rpm->wakeref_count);
+ assert_rpm_raw_wakeref_held(i915);
+ }
+}
+
+static void
+intel_runtime_pm_release(struct drm_i915_private *i915, int wakelock)
+{
+ struct i915_runtime_pm *rpm = &i915->runtime_pm;
+
+ if (wakelock) {
+ assert_rpm_wakelock_held(i915);
+ atomic_sub(INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
+ } else {
+ assert_rpm_raw_wakeref_held(i915);
+ }
+
+ __intel_wakeref_dec_and_check_tracking(i915);
+}
+
bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id);
@@ -1946,13 +1974,17 @@ static void __intel_display_power_put(struct drm_i915_private *dev_priv,
}
/**
- * intel_display_power_put - release a power domain reference
+ * intel_display_power_put_unchecked - release an unchecked power domain reference
* @dev_priv: i915 device instance
* @domain: power domain to reference
*
* This function drops the power domain reference obtained by
* intel_display_power_get() and might power down the corresponding hardware
* block right away if this is the last reference.
+ *
+ * This function exists only for historical reasons and should be avoided in
+ * new code, as the correctness of its use cannot be checked. Always use
+ * intel_display_power_put() instead.
*/
void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain)
@@ -1962,6 +1994,16 @@ void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
}
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+/**
+ * intel_display_power_put - release a power domain reference
+ * @dev_priv: i915 device instance
+ * @domain: power domain to reference
+ * @wakeref: wakeref acquired for the reference that is being released
+ *
+ * This function drops the power domain reference obtained by
+ * intel_display_power_get() and might power down the corresponding hardware
+ * block right away if this is the last reference.
+ */
void intel_display_power_put(struct drm_i915_private *dev_priv,
enum intel_display_power_domain domain,
intel_wakeref_t wakeref)
@@ -4579,19 +4621,8 @@ static void intel_power_domains_verify_state(struct drm_i915_private *i915)
#endif
-/**
- * intel_runtime_pm_get - grab a runtime pm reference
- * @i915: i915 device instance
- *
- * This function grabs a device-level runtime pm reference (mostly used for GEM
- * code to ensure the GTT or GT is on) and ensures that it is powered up.
- *
- * Any runtime pm reference obtained by this function must have a symmetric
- * call to intel_runtime_pm_put() to release the reference again.
- *
- * Returns: the wakeref cookie to pass to intel_runtime_pm_put()
- */
-intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
+static intel_wakeref_t __intel_runtime_pm_get(struct drm_i915_private *i915,
+ bool wakelock)
{
struct pci_dev *pdev = i915->drm.pdev;
struct device *kdev = &pdev->dev;
@@ -4600,9 +4631,28 @@ intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
ret = pm_runtime_get_sync(kdev);
WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+ intel_runtime_pm_acquire(i915, wakelock);
+
return track_intel_runtime_pm_wakeref(i915);
}
+/**
+ * intel_runtime_pm_get - grab a runtime pm reference
+ * @i915: i915 device instance
+ *
+ * This function grabs a device-level runtime pm reference (mostly used for GEM
+ * code to ensure the GTT or GT is on) and ensures that it is powered up.
+ *
+ * Any runtime pm reference obtained by this function must have a symmetric
+ * call to intel_runtime_pm_put() to release the reference again.
+ *
+ * Returns: the wakeref cookie to pass to intel_runtime_pm_put()
+ */
+intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
+{
+ return __intel_runtime_pm_get(i915, true);
+}
+
/**
* intel_runtime_pm_get_if_in_use - grab a runtime pm reference if device in use
* @i915: i915 device instance
@@ -4633,6 +4683,8 @@ intel_wakeref_t intel_runtime_pm_get_if_in_use(struct drm_i915_private *i915)
return 0;
}
+ intel_runtime_pm_acquire(i915, true);
+
return track_intel_runtime_pm_wakeref(i915);
}
@@ -4663,33 +4715,56 @@ intel_wakeref_t intel_runtime_pm_get_noresume(struct drm_i915_private *i915)
assert_rpm_wakelock_held(i915);
pm_runtime_get_noresume(kdev);
+ intel_runtime_pm_acquire(i915, true);
+
return track_intel_runtime_pm_wakeref(i915);
}
+static void __intel_runtime_pm_put(struct drm_i915_private *i915,
+ intel_wakeref_t wref,
+ bool wakelock)
+{
+ struct pci_dev *pdev = i915->drm.pdev;
+ struct device *kdev = &pdev->dev;
+
+ untrack_intel_runtime_pm_wakeref(i915, wref);
+
+ intel_runtime_pm_release(i915, wakelock);
+
+ pm_runtime_mark_last_busy(kdev);
+ pm_runtime_put_autosuspend(kdev);
+}
+
/**
- * intel_runtime_pm_put - release a runtime pm reference
+ * intel_runtime_pm_put_unchecked - release an unchecked runtime pm reference
* @i915: i915 device instance
*
* This function drops the device-level runtime pm reference obtained by
* intel_runtime_pm_get() and might power down the corresponding
* hardware block right away if this is the last reference.
+ *
+ * This function exists only for historical reasons and should be avoided in
+ * new code, as the correctness of its use cannot be checked. Always use
+ * intel_runtime_pm_put() instead.
*/
void intel_runtime_pm_put_unchecked(struct drm_i915_private *i915)
{
- struct pci_dev *pdev = i915->drm.pdev;
- struct device *kdev = &pdev->dev;
-
- untrack_intel_runtime_pm_wakeref(i915);
-
- pm_runtime_mark_last_busy(kdev);
- pm_runtime_put_autosuspend(kdev);
+ __intel_runtime_pm_put(i915, -1, true);
}
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+/**
+ * intel_runtime_pm_put - release a runtime pm reference
+ * @i915: i915 device instance
+ * @wref: wakeref acquired for the reference that is being released
+ *
+ * This function drops the device-level runtime pm reference obtained by
+ * intel_runtime_pm_get() and might power down the corresponding
+ * hardware block right away if this is the last reference.
+ */
void intel_runtime_pm_put(struct drm_i915_private *i915, intel_wakeref_t wref)
{
- cancel_intel_runtime_pm_wakeref(i915, wref);
- intel_runtime_pm_put_unchecked(i915);
+ __intel_runtime_pm_put(i915, wref, true);
}
#endif
@@ -4767,10 +4842,11 @@ void intel_runtime_pm_cleanup(struct drm_i915_private *i915)
count = atomic_fetch_inc(&rpm->wakeref_count); /* balance untrack */
WARN(count,
- "i915->runtime_pm.wakeref_count=%d on cleanup\n",
- count);
+ "i915 raw-wakerefs=%d wakelocks=%d on cleanup\n",
+ intel_rpm_raw_wakeref_count(count),
+ intel_rpm_wakelock_count(count));
- untrack_intel_runtime_pm_wakeref(i915);
+ intel_runtime_pm_release(i915, false);
}
void intel_runtime_pm_init_early(struct drm_i915_private *i915)
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
