drm/i915: Add support for tracking wakerefs w/o power-on guarantee

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)
v3:
- Add a struct_member(T, m) helper instead of open-coding it. (Chris)
- Checkpatch indentation formatting fix.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-2-imre.deak@intel.com

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index c849cfa7cb282a76d0d4dadf8b05d1c2f4a05550..5c94c7ab46071c131e40d09524b5d8aa0b0e9262 100644 (file)
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -102,6 +102,8 @@
 #define page_pack_bits(ptr, bits) ptr_pack_bits(ptr, bits, PAGE_SHIFT)
 #define page_unpack_bits(ptr, bits) ptr_unpack_bits(ptr, bits, PAGE_SHIFT)
 
+#define struct_member(T, member) (((T *)0)->member)
+
 #define ptr_offset(ptr, member) offsetof(typeof(*(ptr)), member)
 
 #define fetch_and_zero(ptr) ({                                         \
@@ -118,7 +120,7 @@
  */
 #define container_of_user(ptr, type, member) ({                                \
        void __user *__mptr = (void __user *)(ptr);                     \
-       BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
+       BUILD_BUG_ON_MSG(!__same_type(*(ptr), struct_member(type, member)) && \
                         !__same_type(*(ptr), void),                    \
                         "pointer type mismatch in container_of()");    \
        ((type __user *)(__mptr - offsetof(type, member))); })

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5258abb2ebb5d5549ca4c382cda47ebd2508fb79..30b2d6ed2d5311c97e98a578ef57d2c9cc50a4d4 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1620,6 +1620,24 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
 int bdw_get_pipemisc_bpp(struct intel_crtc *crtc);
 
 /* intel_runtime_pm.c */
+#define BITS_PER_WAKEREF       \
+       BITS_PER_TYPE(struct_member(struct i915_runtime_pm, 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)
 {
@@ -1628,11 +1646,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
@@ -1662,7 +1702,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);
 }
 
 /**
@@ -1679,7 +1720,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 b8fadd1b685c10d073edb17171f27b9079feb714..53a172094c6ae0e3a3ad20581d7dc17fe33baf51 100644 (file)
--- 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,8 +137,8 @@ track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
        return stack;
 }
 
-static void cancel_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
-                                           depot_stack_handle_t stack)
+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;
        unsigned long flags, n;
@@ -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,6 +4621,21 @@ static void intel_power_domains_verify_state(struct drm_i915_private *i915)
 
 #endif
 
+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;
+       int ret;
+
+       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
@@ -4593,14 +4650,7 @@ static void intel_power_domains_verify_state(struct drm_i915_private *i915)
  */
 intel_wakeref_t intel_runtime_pm_get(struct drm_i915_private *i915)
 {
-       struct pci_dev *pdev = i915->drm.pdev;
-       struct device *kdev = &pdev->dev;
-       int ret;
-
-       ret = pm_runtime_get_sync(kdev);
-       WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
-
-       return track_intel_runtime_pm_wakeref(i915);
+       return __intel_runtime_pm_get(i915, true);
 }
 
 /**
@@ -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)