drm/i915: Verify power domains after enabling them

After
commit 2cd9a689e97b ("drm/i915: Refactor intel_display_set_init_power() logic")
it makes more sense to check the power domain/well refcounts after
enabling the power domains functionality. Before that it's guaranteed
that most power wells (in the INIT domain) will have a reference held,
so not an interesting state.

While at it also add the check after the init_hw/fini_hw, disable and
suspend/resume steps. Make the test optional on a Kconfig option since
it may add substantial overhead: on VLV/CHV the corresponding PUNIT reg
access for each power well may take up to 20ms.

v2:
- Add the state check to more spots. (Chris)

v3:
- During suspend check the state before deiniting display core.
  Afterwards DC states are disabled (and so the dc_off power well is
  enabled) even though we don't hold a reference on it.
- Do the test conditionally based on a new Kconfig option. (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
[Add DRM_I915_DEBUG_RUNTIME_PM to welcome messages]
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180817145837.26592-1-imre.deak@intel.com

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 459f8f8..9e36ffb 100644 (file)
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -30,6 +30,7 @@ config DRM_I915_DEBUG
        select SW_SYNC # signaling validation framework (igt/syncobj*)
        select DRM_I915_SW_FENCE_DEBUG_OBJECTS
        select DRM_I915_SELFTEST
+       select DRM_I915_DEBUG_RUNTIME_PM
         default n
         help
           Choose this option to turn on extra driver debugging that may affect
@@ -167,3 +168,14 @@ config DRM_I915_DEBUG_VBLANK_EVADE
          the vblank.
 
          If in doubt, say "N".
+
+config DRM_I915_DEBUG_RUNTIME_PM
+       bool "Enable extra state checking for runtime PM"
+       depends on DRM_I915
+       default n
+       help
+         Choose this option to turn on extra state checking for the
+         runtime PM functionality. This may introduce overhead during
+         driver loading, suspend and resume operations.
+
+         If in doubt, say "N"

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 35a012f..77a4a01 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1331,6 +1331,8 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
                DRM_INFO("DRM_I915_DEBUG enabled\n");
        if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
                DRM_INFO("DRM_I915_DEBUG_GEM enabled\n");
+       if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM))
+               DRM_INFO("DRM_I915_DEBUG_RUNTIME_PM enabled\n");
 }
 
 /**

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 592b847..95e9cad 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15905,8 +15905,6 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 
        intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
-       intel_power_domains_verify_state(dev_priv);
-
        intel_fbc_init_pipe_state(dev_priv);
 }
 

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b2ce343..35dd72f 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1966,7 +1966,6 @@ enum i915_drm_suspend_mode {
 void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
                                 enum i915_drm_suspend_mode);
 void intel_power_domains_resume(struct drm_i915_private *dev_priv);
-void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
 void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
 void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6153d5b..ff3fd8d 100644 (file)
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3716,6 +3716,8 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
        cmn->desc->ops->disable(dev_priv, cmn);
 }
 
+static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
+
 /**
  * intel_power_domains_init_hw - initialize hardware power domain state
  * @dev_priv: i915 device instance
@@ -3767,6 +3769,8 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
                intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
        intel_power_domains_sync_hw(dev_priv);
        power_domains->initializing = false;
+
+       intel_power_domains_verify_state(dev_priv);
 }
 
 /**
@@ -3788,6 +3792,8 @@ void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
        /* Remove the refcount we took to keep power well support disabled. */
        if (!i915_modparams.disable_power_well)
                intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+       intel_power_domains_verify_state(dev_priv);
 }
 
 /**
@@ -3805,6 +3811,8 @@ void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
 void intel_power_domains_enable(struct drm_i915_private *dev_priv)
 {
        intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+       intel_power_domains_verify_state(dev_priv);
 }
 
 /**
@@ -3817,6 +3825,8 @@ void intel_power_domains_enable(struct drm_i915_private *dev_priv)
 void intel_power_domains_disable(struct drm_i915_private *dev_priv)
 {
        intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+
+       intel_power_domains_verify_state(dev_priv);
 }
 
 /**
@@ -3845,15 +3855,19 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
         * firmware was inactive.
         */
        if (!IS_GEN9_LP(dev_priv) && suspend_mode == I915_DRM_SUSPEND_IDLE &&
-           dev_priv->csr.dmc_payload != NULL)
+           dev_priv->csr.dmc_payload != NULL) {
+               intel_power_domains_verify_state(dev_priv);
                return;
+       }
 
        /*
         * Even if power well support was disabled we still want to disable
         * power wells if power domains must be deinitialized for suspend.
         */
-       if (!i915_modparams.disable_power_well)
+       if (!i915_modparams.disable_power_well) {
                intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+               intel_power_domains_verify_state(dev_priv);
+       }
 
        if (IS_ICELAKE(dev_priv))
                icl_display_core_uninit(dev_priv);
@@ -3884,13 +3898,15 @@ void intel_power_domains_resume(struct drm_i915_private *dev_priv)
        if (power_domains->display_core_suspended) {
                intel_power_domains_init_hw(dev_priv, true);
                power_domains->display_core_suspended = false;
-
-               return;
+       } else {
+               intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
        }
 
-       intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+       intel_power_domains_verify_state(dev_priv);
 }
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+
 static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv)
 {
        struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -3919,7 +3935,7 @@ static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv)
  * acquiring reference counts for any power wells in use and disabling the
  * ones left on by BIOS but not required by any active output.
  */
-void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
+static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
 {
        struct i915_power_domains *power_domains = &dev_priv->power_domains;
        struct i915_power_well *power_well;
@@ -3974,6 +3990,14 @@ void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
        mutex_unlock(&power_domains->lock);
 }
 
+#else
+
+static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv)
+{
+}
+
+#endif
+
 /**
  * intel_runtime_pm_get - grab a runtime pm reference
  * @dev_priv: i915 device instance