Re: [PATCH 07/15] drm/i915: vlv: check port power domain instead of only D0 for eDP VDD on

[Imre Deak <imre.deak@intel.com>]

On Wed, 2014-04-09 at 11:32 -0300, Paulo Zanoni wrote:
> 2014-04-08 13:57 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > Some platforms need additional power domains to be on in addition to the
> > device D0 state to access the panel registers.
> >
> > Suggested by Daniel.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76987
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index e48d47c..c7a52d1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -313,8 +313,12 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
> >  {
> >         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > +       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +       struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > +       enum intel_display_power_domain power_domain;
> >
> > -       return !dev_priv->pm.suspended &&
> > +       power_domain = intel_display_port_power_domain(intel_encoder);
> > +       return intel_display_power_enabled(dev_priv, power_domain) &&
> >                (I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD) != 0;
> 
> This is an interesting problem. The patch looks like the correct fix
> for VLV, but it creates the problem that it will enable unneeded power
> wells on HSW/BDW.

I guess you mean port A here. In that case power_domain is
POWER_DOMAIN_PORT_DDI_A_4_LANES, which will map both for HSW and BDW to
the always-on power well, hence we only get an RPM ref for them?

> My crystal ball tells me we're gonna have many
> instances of this type of "problem" in the future. In this case,
> shouldn't we create a new power domain, e.g., POWER_DOMAIN_EDP_VDD,
> that grabs the appropriate power wells on VLV, but just gets runtime
> PM on HSW+?

Yea, well it's always a possibility that the current granularity is not
enough for a new platform and then we have no choice than to extend the
set of power domains. This is what happened now for the AUX stuff on
SKL, we need to add the new AUX domains. We can also decide that the
power saving doesn't justify the complexity and just grab the coarser
domain for a given functionality.

--Imre

> 
> >  }
> >
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
>