On ti, 2015-11-17 at 13:46 +0100, Patrik Jakobsson wrote: > On Wed, Nov 11, 2015 at 07:03:54PM +0200, Imre Deak wrote: > > When this option is 0 (so the power well support is disabled) we are > > supposed to enable all power wells once and don't disable them unless we > > system suspend the device. Currently if the option is 0, we can call the > > power well enable handlers multiple times, whenever their refcount > > changes from 0->1. This may not be a problem for the HW, but it's not > > logical and may trigger some warnings in the power well code which > > doesn't expect this. So simply keep around a reference while we are > > not system suspended to solve this. For simplicity mark the module > > option read only, so we don't need to deal with re-enabling the feature > > during runtime. If someone really needs that it could be added later in > > a more proper way. > > > > Signed-off-by: Imre Deak <[email protected]> > > --- > > drivers/gpu/drm/i915/i915_params.c | 2 +- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 16 +++++++++++++++- > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > b/drivers/gpu/drm/i915/i915_params.c > > index 7cf7474..2b36e67 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -131,7 +131,7 @@ module_param_named_unsafe(preliminary_hw_support, > > i915.preliminary_hw_support, i > > MODULE_PARM_DESC(preliminary_hw_support, > > "Enable preliminary hardware support."); > > > > -module_param_named_unsafe(disable_power_well, i915.disable_power_well, > > int, 0600); > > +module_param_named_unsafe(disable_power_well, i915.disable_power_well, > > int, 0400); > > MODULE_PARM_DESC(disable_power_well, > > "Disable display power wells when possible " > > "(-1=auto [default], 0=power wells always on, 1=power wells disabled > > when possible)"); > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index b33969b..d167a45 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -1427,7 +1427,7 @@ void intel_display_power_put(struct drm_i915_private > > *dev_priv, > > for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { > > WARN_ON(!power_well->count); > > > > - if (!--power_well->count && i915.disable_power_well) > > + if (!--power_well->count) > > intel_power_well_disable(dev_priv, power_well); > > } > > > > @@ -1899,6 +1899,10 @@ void intel_power_domains_fini(struct > > drm_i915_private *dev_priv) > > * the power well is not enabled, so just enable it in case > > * we're going to unload/reload. */ > > intel_display_set_init_power(dev_priv, true); > > + > > + /* Remove the refcount we took to keep power well support disabled. */ > > + if (!i915.disable_power_well) > > + intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > } > > > > static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) > > @@ -2099,6 +2103,9 @@ void intel_power_domains_init_hw(struct > > drm_i915_private *dev_priv, bool resume) > > > > /* For now, we need the power well to be always enabled. */ > > intel_display_set_init_power(dev_priv, true); > > + /* Disable power support if the user asked so. */ > > + if (!i915.disable_power_well) > > + intel_display_power_get(dev_priv, POWER_DOMAIN_INIT); > > intel_power_domains_sync_hw(dev_priv); > > power_domains->initializing = false; > > } > > @@ -2114,6 +2121,13 @@ void intel_power_domains_suspend(struct > > drm_i915_private *dev_priv) > > { > > if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) > > skl_display_core_uninit(dev_priv); > > + > > + /* > > + * Even if power well support was disabled we still want to disabled > > + * we want to disable the power wells while we are system suspended. > > + */ > > This comment needs fixing. With that done: > > Reviewed-by: Patrik Jakobsson <[email protected]>
Pushed to dinq, thanks for the review. > > > + if (!i915.disable_power_well) > > + intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > } > > > > /** _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
