[Intel-gfx] [PATCH 15/16] drm/i915: Power down any power well left on by BIOS

[Souza, Jose]

On Mon, 2018-10-15 at 14:06 +0300, Imre Deak wrote:
> On Fri, Oct 12, 2018 at 02:52:17PM -0700, José Roberto de Souza
> wrote:
> > Just not enable power wells is not enough as BIOS/firmware can turn
> > on some power wells during boot, so is needed disable those to save
> > power and to avoid mismatch state errors in
> > intel_power_domains_verify_state().
> > So here disabling every non-real power well first as it could have
> > some dependency in a real power well and then disabling all power
> > wells in reverse(power well 2 depends on power well 1 and so on)
> > other as required by spec.
> 
> You can't disable a power well while the function depending on it is
> still on, that would lead to a hang. Also some power wells can only
> be
> enabled/disabled at a specific place in the modeset sequence, so
> disabling them here would lead to timeouts. The correct way to get
> power
> wells disbled is to disable the corresponding functions by doing a
> modeset disabling any active outputs.

Do a modeset disabling would require initialize some display stuff and
that is what this PR is avoiding when display is disabled by parameter.
Also I did not reproduced any hang and CI did not too as somes IGT
tests load i915 with display off.

> 
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 59
> > +++++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 56c65d921acd..0f5016b74228 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3785,6 +3785,61 @@ static void vlv_cmnlane_wa(struct
> > drm_i915_private *dev_priv)
> >  
> >  static void intel_power_domains_verify_state(struct
> > drm_i915_private *dev_priv);
> >  
> > +static void
> > +intel_power_domains_disable_leftovers(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> > +	struct i915_power_well *power_well;
> > +	int i;
> > +
> > +	mutex_lock(&power_domains->lock);
> > +
> > +	/* Disable everything that is enabled and is not a HW
> > power_well */
> > +	for_each_power_well(dev_priv, power_well) {
> > +		WARN_ON(power_well->count);
> > +
> > +		/*
> > +		 * Power wells not belonging to any domain (like the
> > MISC_IO
> > +		 * and PW1 power wells) are under FW control, so ignore
> > them,
> > +		 * since their state can change asynchronously.
> > +		 */
> > +		if (!power_well->desc->domains || power_well->desc-
> > >always_on)
> > +			continue;
> > +
> > +		if (power_well->desc->id != DISP_PW_ID_NONE)
> > +			continue;
> > +
> > +		if (!power_well->hw_enabled)
> > +			continue;
> > +
> > +		intel_power_well_disable(dev_priv, power_well);
> > +	}
> > +
> > +	/* Disabled HW power wells in reverse order, so power well 2 is
> > +	 * disabled before power well 1 and so on as required by spec.
> > +	 */
> > +	for (i = power_domains->power_well_count - 1; i >= 0; i--) {
> > +		power_well = &power_domains->power_wells[i];
> > +
> > +		WARN_ON(power_well->count);
> > +
> > +		if (!power_well->desc->domains || power_well->desc-
> > >always_on)
> > +			continue;
> > +
> > +		if (power_well->desc->id == DISP_PW_ID_NONE)
> > +			continue;
> > +
> > +		if (!power_well->hw_enabled)
> > +			continue;
> > +
> > +		intel_power_well_disable(dev_priv, power_well);
> > +	}
> > +
> > +	mutex_unlock(&power_domains->lock);
> > +
> > +	intel_power_domains_verify_state(dev_priv);
> > +}
> > +
> >  /**
> >   * intel_power_domains_init_hw - initialize hardware power domain
> > state
> >   * @dev_priv: i915 device instance
> > @@ -3838,6 +3893,10 @@ 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);
> >  
> > +	/* Disable everything left enabled by BIOS/firmware */
> > +	if (!INTEL_INFO(dev_priv)->num_pipes)
> > +		intel_power_domains_disable_leftovers(dev_priv);
> > +
> >  	power_domains->initializing = false;
> >  }
> >  
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx