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 <ramalinga...@intel.com>
> > Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.d...@intel.com>
> > ---
> >  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.syrj...@linux.intel.com>
> 
> >             .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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to