On Mon, Nov 12, 2018 at 07:19:44PM +0200, Ville Syrjälä wrote: > On Fri, Nov 09, 2018 at 04:58:22PM +0200, Imre Deak wrote: > > Even though PW#1 and the MISC_IO power wells are managed by the > > DMC firmware (toggled dynamically if conditions allow it) from the > > driver's POV they are always on if the display core is initialized > > (always restored by DMC to the enabled state after exiting from DC5/6 > > for instance b/c of MMIO access). Accordingly we can just mark them as > > always-on and remove the special casing for them during state > > verification (thus enabling verification for these power wells too). > > > > Cc: Ramalingam C <ramalingam.c@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++-------- > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 44a77de439f2..6b1576ae778f 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -2358,6 +2358,7 @@ static const struct i915_power_well_desc skl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > First I was wondering if this somehow changes the behaviour of > __intel_display_power_is_enabled(), but it won't since all these > wells have domains==0 and so would be skipped there anyway. Yes, it should only affect the verification, otherwise doesn't change the behaviour. > > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2370,6 +2371,7 @@ static const struct i915_power_well_desc skl_power_wells[] = { > > { > > .name = "MISC IO power well", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_MISC_IO, > > @@ -2449,6 +2451,8 @@ static const struct i915_power_well_desc bxt_power_wells[] = { > > }, > > { > > .name = "power well 1", > > + /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2508,6 +2512,7 @@ static const struct i915_power_well_desc glk_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2636,6 +2641,7 @@ static const struct i915_power_well_desc cnl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2803,6 +2809,7 @@ static const struct i915_power_well_desc icl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = &hsw_power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -3936,14 +3943,6 @@ static void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) > > int domains_count; > > bool enabled; > > > > - /* > > - * 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) > > - continue; > > - > > enabled = power_well->desc->ops->is_enabled(dev_priv, > > power_well); > > if ((power_well->count || power_well->desc->always_on) != > > -- > > 2.13.2 > > -- > Ville Syrjälä > Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx