Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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

+44 -9
+12
drivers/gpu/drm/i915/Kconfig.debug
··· 30 30 select SW_SYNC # signaling validation framework (igt/syncobj*) 31 31 select DRM_I915_SW_FENCE_DEBUG_OBJECTS 32 32 select DRM_I915_SELFTEST 33 + select DRM_I915_DEBUG_RUNTIME_PM 33 34 default n 34 35 help 35 36 Choose this option to turn on extra driver debugging that may affect ··· 168 167 the vblank. 169 168 170 169 If in doubt, say "N". 170 + 171 + config DRM_I915_DEBUG_RUNTIME_PM 172 + bool "Enable extra state checking for runtime PM" 173 + depends on DRM_I915 174 + default n 175 + help 176 + Choose this option to turn on extra state checking for the 177 + runtime PM functionality. This may introduce overhead during 178 + driver loading, suspend and resume operations. 179 + 180 + If in doubt, say "N"
+2
drivers/gpu/drm/i915/i915_drv.c
··· 1331 1331 DRM_INFO("DRM_I915_DEBUG enabled\n"); 1332 1332 if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) 1333 1333 DRM_INFO("DRM_I915_DEBUG_GEM enabled\n"); 1334 + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)) 1335 + DRM_INFO("DRM_I915_DEBUG_RUNTIME_PM enabled\n"); 1334 1336 } 1335 1337 1336 1338 /**
-2
drivers/gpu/drm/i915/intel_display.c
··· 15905 15905 15906 15906 intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); 15907 15907 15908 - intel_power_domains_verify_state(dev_priv); 15909 - 15910 15908 intel_fbc_init_pipe_state(dev_priv); 15911 15909 } 15912 15910
-1
drivers/gpu/drm/i915/intel_drv.h
··· 1966 1966 void intel_power_domains_suspend(struct drm_i915_private *dev_priv, 1967 1967 enum i915_drm_suspend_mode); 1968 1968 void intel_power_domains_resume(struct drm_i915_private *dev_priv); 1969 - void intel_power_domains_verify_state(struct drm_i915_private *dev_priv); 1970 1969 void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume); 1971 1970 void bxt_display_core_uninit(struct drm_i915_private *dev_priv); 1972 1971 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
+30 -6
drivers/gpu/drm/i915/intel_runtime_pm.c
··· 3716 3716 cmn->desc->ops->disable(dev_priv, cmn); 3717 3717 } 3718 3718 3719 + static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv); 3720 + 3719 3721 /** 3720 3722 * intel_power_domains_init_hw - initialize hardware power domain state 3721 3723 * @dev_priv: i915 device instance ··· 3769 3767 intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); 3770 3768 intel_power_domains_sync_hw(dev_priv); 3771 3769 power_domains->initializing = false; 3770 + 3771 + intel_power_domains_verify_state(dev_priv); 3772 3772 } 3773 3773 3774 3774 /** ··· 3792 3788 /* Remove the refcount we took to keep power well support disabled. */ 3793 3789 if (!i915_modparams.disable_power_well) 3794 3790 intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); 3791 + 3792 + intel_power_domains_verify_state(dev_priv); 3795 3793 } 3796 3794 3797 3795 /** ··· 3811 3805 void intel_power_domains_enable(struct drm_i915_private *dev_priv) 3812 3806 { 3813 3807 intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); 3808 + 3809 + intel_power_domains_verify_state(dev_priv); 3814 3810 } 3815 3811 3816 3812 /** ··· 3825 3817 void intel_power_domains_disable(struct drm_i915_private *dev_priv) 3826 3818 { 3827 3819 intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); 3820 + 3821 + intel_power_domains_verify_state(dev_priv); 3828 3822 } 3829 3823 3830 3824 /** ··· 3855 3845 * firmware was inactive. 3856 3846 */ 3857 3847 if (!IS_GEN9_LP(dev_priv) && suspend_mode == I915_DRM_SUSPEND_IDLE && 3858 - dev_priv->csr.dmc_payload != NULL) 3848 + dev_priv->csr.dmc_payload != NULL) { 3849 + intel_power_domains_verify_state(dev_priv); 3859 3850 return; 3851 + } 3860 3852 3861 3853 /* 3862 3854 * Even if power well support was disabled we still want to disable 3863 3855 * power wells if power domains must be deinitialized for suspend. 3864 3856 */ 3865 - if (!i915_modparams.disable_power_well) 3857 + if (!i915_modparams.disable_power_well) { 3866 3858 intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); 3859 + intel_power_domains_verify_state(dev_priv); 3860 + } 3867 3861 3868 3862 if (IS_ICELAKE(dev_priv)) 3869 3863 icl_display_core_uninit(dev_priv); ··· 3898 3884 if (power_domains->display_core_suspended) { 3899 3885 intel_power_domains_init_hw(dev_priv, true); 3900 3886 power_domains->display_core_suspended = false; 3901 - 3902 - return; 3887 + } else { 3888 + intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); 3903 3889 } 3904 3890 3905 - intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); 3891 + intel_power_domains_verify_state(dev_priv); 3906 3892 } 3893 + 3894 + #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM) 3907 3895 3908 3896 static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv) 3909 3897 { ··· 3935 3919 * acquiring reference counts for any power wells in use and disabling the 3936 3920 * ones left on by BIOS but not required by any active output. 3937 3921 */ 3938 - void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) 3922 + static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) 3939 3923 { 3940 3924 struct i915_power_domains *power_domains = &dev_priv->power_domains; 3941 3925 struct i915_power_well *power_well; ··· 3989 3973 3990 3974 mutex_unlock(&power_domains->lock); 3991 3975 } 3976 + 3977 + #else 3978 + 3979 + static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) 3980 + { 3981 + } 3982 + 3983 + #endif 3992 3984 3993 3985 /** 3994 3986 * intel_runtime_pm_get - grab a runtime pm reference