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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
