From 6dfc4a8f134fe0fe4c77dd09906e7305ba7b3edc Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Thu, 16 Aug 2018 22:34:14 +0300 Subject: [PATCH] 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 Reviewed-by: Chris Wilson [Add DRM_I915_DEBUG_RUNTIME_PM to welcome messages] Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20180817145837.26592-1-imre.deak@intel.com --- drivers/gpu/drm/i915/Kconfig.debug | 12 +++++++++ drivers/gpu/drm/i915/i915_drv.c | 2 ++ 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 ++++++++++++++++++++----- 5 files changed, 44 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/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 35a012ffc03b..77a4a01ddc08 100644 --- 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 592b847db88e..95e9cad5b4de 100644 --- 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 b2ce343b6027..35dd72fd0152 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..ff3fd8dbd2b4 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,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