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@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> (v2)
---
drivers/gpu/drm/i915/Kconfig.debug | 12 +++++++++++
drivers/gpu/drm/i915/intel_display.c | 2 --
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_runtime_pm.c | 36 +++++++++++++++++++++++++++------
4 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 459f8f88a34c..9e36ffb5eb7c 100644
--- 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/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 09d62b0c62cb..ad0f0e5389d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15907,8 +15907,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 d0402051925b..529e72bcc5ef 100644
--- 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 6153d5be5cf6..f1263f8f40ac 100644
--- 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,11 +3898,11 @@ 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);
}
static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv)
@@ -3919,7 +3933,9 @@ 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)
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+
+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
--
2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
