[Intel-gfx] [PATCH 72/89] drm/i915/skl: Enable/disable power well for aux transaction

[Damien Lespiau]

On Tue, Sep 16, 2014 at 04:19:07PM +0300, Imre Deak wrote:
> > @@ -1233,6 +1233,9 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> >  	power_domain = intel_display_port_power_domain(intel_encoder);
> >  	intel_display_power_get(dev_priv, power_domain);
> >  
> > +	power_domain = intel_display_aux_power_domain(intel_encoder);
> > +	intel_display_power_get(dev_priv, power_domain);
> > +
> 
> The AUX power domains were added to save power when only AUX
> functionality is needed, since then we don't need to power on the power
> domain needed for full port functionality.

Hum I'm not sure about this. It seems to me that the value of the AUX
power domain is to be able to shutdown the AUX hardware when AUX is not
needed. That's slightly different from what you're saying; The cases
where "only AUX functionality is needed" seem very transient to me,
while having the main lanes working and no need for AUX sounds like the
case where it's interesting to have the AUX hw powered down. Of course,
with PSR we can do both.

> With the above change and everywhere else below we'll end up enabling
> both power domains, though we only need AUX functionality.

If we're powering up the panel that's probably to use it very soon, so I
don't really see the value not powering the main lanes at the same time,
they are going to be used for training very soon? I'm probably missing
something.

> The power wells needed for AUX are a subset of those needed for full
> port functionality on all platforms (at least atm), so this patch won't
> change anything. The patch would make sense, if you requested only the
> AUX domains.

I think it's fine if this patch is not changing anything, at least for
now, until we get to use this power domain to good ends?

This patch still need the reworks you mentionned in the previous mail,
of course.

-- 
Damien