Since the intel_display_power_domain_set struct, currently its current
size close to 1kB, can be allocated on the stack, it's better to
allocate the per-domain wakeref pointer array - used for debugging -
within the struct dynamically, so do this.

The memory freeing is guaranteed by the fact that the acquired domain
references tracked by the struct can't be leaked either.

v2:
- Don't use fetch_and_zero() when freeing the wakerefs array. (Jani)
- Simplify intel_display_power_get/put_in_set(). (Jani)
- Check in intel_crtc_destroy() that the wakerefs array has been freed.
v3:
- Add intel_display_power_set_disabled() and a separate assert
  function instead of open coding these. (Jani)

Cc: Jani Nikula <jani.nik...@intel.com>
Signed-off-by: Imre Deak <imre.d...@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c     |  11 ++
 .../drm/i915/display/intel_display_power.c    | 109 ++++++++++++++----
 .../drm/i915/display/intel_display_power.h    |   6 +-
 3 files changed, 104 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c 
b/drivers/gpu/drm/i915/display/intel_crtc.c
index 037fc140b585c..c18d98bfe1a7c 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -21,6 +21,7 @@
 #include "intel_crtc.h"
 #include "intel_cursor.h"
 #include "intel_display_debugfs.h"
+#include "intel_display_power.h"
 #include "intel_display_trace.h"
 #include "intel_display_types.h"
 #include "intel_drrs.h"
@@ -37,6 +38,14 @@ static void assert_vblank_disabled(struct drm_crtc *crtc)
                drm_crtc_vblank_put(crtc);
 }
 
+static void assert_power_domains_disabled(struct intel_crtc *crtc)
+{
+       struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+       drm_WARN_ON(&i915->drm,
+                   !intel_display_power_set_disabled(i915, 
&crtc->enabled_power_domains));
+}
+
 struct intel_crtc *intel_first_crtc(struct drm_i915_private *i915)
 {
        return to_intel_crtc(drm_crtc_from_index(&i915->drm, 0));
@@ -204,6 +213,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc)
 
        cpu_latency_qos_remove_request(&crtc->vblank_pm_qos);
 
+       assert_power_domains_disabled(crtc);
+
        drm_crtc_cleanup(&crtc->base);
        kfree(crtc);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c 
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 4c1de91e56ff9..ca63b4f1af41b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -830,20 +830,85 @@ void intel_display_power_put_unchecked(struct 
drm_i915_private *dev_priv,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+static void
+add_domain_to_set(struct drm_i915_private *i915,
+                 struct intel_display_power_domain_set *power_domain_set,
+                 enum intel_display_power_domain domain,
+                 intel_wakeref_t wf)
+{
+       drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
+
+       if (!power_domain_set->wakerefs)
+               power_domain_set->wakerefs = kcalloc(POWER_DOMAIN_NUM,
+                                                    
sizeof(*power_domain_set->wakerefs),
+                                                    GFP_KERNEL);
+
+       if (power_domain_set->wakerefs)
+               power_domain_set->wakerefs[domain] = wf;
+
+       set_bit(domain, power_domain_set->mask.bits);
+}
+
+static intel_wakeref_t
+remove_domain_from_set(struct drm_i915_private *i915,
+                      struct intel_display_power_domain_set *power_domain_set,
+                      enum intel_display_power_domain domain)
+{
+       intel_wakeref_t wf;
+
+       drm_WARN_ON(&i915->drm, !test_bit(domain, power_domain_set->mask.bits));
+
+       clear_bit(domain, power_domain_set->mask.bits);
+
+       if (!power_domain_set->wakerefs)
+               return -1;
+
+       wf = fetch_and_zero(&power_domain_set->wakerefs[domain]);
+
+       if (bitmap_empty(power_domain_set->mask.bits, POWER_DOMAIN_NUM)) {
+               kfree(power_domain_set->wakerefs);
+               power_domain_set->wakerefs = NULL;
+       }
+
+       return wf;
+
+}
+#else
+static void
+add_domain_to_set(struct drm_i915_private *i915,
+                 struct intel_display_power_domain_set *power_domain_set,
+                 enum intel_display_power_domain domain,
+                 intel_wakeref_t wf)
+{
+       drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
+
+       set_bit(domain, power_domain_set->mask.bits);
+}
+
+static intel_wakeref_t
+remove_domain_from_set(struct drm_i915_private *i915,
+                      struct intel_display_power_domain_set *power_domain_set,
+                      enum intel_display_power_domain domain)
+{
+       drm_WARN_ON(&i915->drm, !test_bit(domain, power_domain_set->mask.bits));
+
+       clear_bit(domain, power_domain_set->mask.bits);
+
+       return -1;
+}
+#endif
+
 void
 intel_display_power_get_in_set(struct drm_i915_private *i915,
                               struct intel_display_power_domain_set 
*power_domain_set,
                               enum intel_display_power_domain domain)
 {
-       intel_wakeref_t __maybe_unused wf;
-
-       drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
+       intel_wakeref_t wf;
 
        wf = intel_display_power_get(i915, domain);
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-       power_domain_set->wakerefs[domain] = wf;
-#endif
-       set_bit(domain, power_domain_set->mask.bits);
+
+       add_domain_to_set(i915, power_domain_set, domain, wf);
 }
 
 bool
@@ -853,16 +918,11 @@ intel_display_power_get_in_set_if_enabled(struct 
drm_i915_private *i915,
 {
        intel_wakeref_t wf;
 
-       drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
-
        wf = intel_display_power_get_if_enabled(i915, domain);
        if (!wf)
                return false;
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-       power_domain_set->wakerefs[domain] = wf;
-#endif
-       set_bit(domain, power_domain_set->mask.bits);
+       add_domain_to_set(i915, power_domain_set, domain, wf);
 
        return true;
 }
@@ -874,20 +934,27 @@ intel_display_power_put_mask_in_set(struct 
drm_i915_private *i915,
 {
        enum intel_display_power_domain domain;
 
-       drm_WARN_ON(&i915->drm,
-                   !bitmap_subset(mask->bits, power_domain_set->mask.bits, 
POWER_DOMAIN_NUM));
-
        for_each_power_domain(domain, mask) {
-               intel_wakeref_t __maybe_unused wf = -1;
+               intel_wakeref_t wf = remove_domain_from_set(i915, 
power_domain_set, domain);
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-               wf = fetch_and_zero(&power_domain_set->wakerefs[domain]);
-#endif
                intel_display_power_put(i915, domain, wf);
-               clear_bit(domain, power_domain_set->mask.bits);
        }
 }
 
+bool
+intel_display_power_set_disabled(struct drm_i915_private *i915,
+                                struct intel_display_power_domain_set 
*power_domain_set)
+{
+       if (!bitmap_empty(power_domain_set->mask.bits, POWER_DOMAIN_NUM))
+               return false;
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+       drm_WARN_ON(&i915->drm, power_domain_set->wakerefs);
+#endif
+
+       return true;
+}
+
 static int
 sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv,
                                   int disable_power_well)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h 
b/drivers/gpu/drm/i915/display/intel_display_power.h
index 1e77e52c87fec..31b0e9ae863c3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -147,7 +147,7 @@ struct i915_power_domains {
 struct intel_display_power_domain_set {
        struct intel_power_domain_mask mask;
 #ifdef CONFIG_DRM_I915_DEBUG_RUNTIME_PM
-       intel_wakeref_t wakerefs[POWER_DOMAIN_NUM];
+       intel_wakeref_t *wakerefs;
 #endif
 };
 
@@ -243,6 +243,10 @@ intel_display_power_put_all_in_set(struct drm_i915_private 
*i915,
        intel_display_power_put_mask_in_set(i915, power_domain_set, 
&power_domain_set->mask);
 }
 
+bool
+intel_display_power_set_disabled(struct drm_i915_private *i915,
+                                struct intel_display_power_domain_set 
*power_domain_set);
+
 void intel_display_power_debug(struct drm_i915_private *i915, struct seq_file 
*m);
 
 enum intel_display_power_domain
-- 
2.37.1

Reply via email to