[Intel-gfx] [PATCH 69/89] drm/i915/skl: Adding power domains for AUX controllers

[Imre Deak]

On Thu, 2014-09-18 at 14:56 +0100, Damien Lespiau wrote:
> Hi Imre,
> 
> I actually had some question there as well:
> 
> On Tue, Sep 16, 2014 at 03:35:15PM +0300, Imre Deak wrote:
> > > @@ -7685,24 +7689,32 @@ EXPORT_SYMBOL_GPL(i915_get_cdclk_freq);
> > >  	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |	\
> > >  	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |	\
> > >  	BIT(POWER_DOMAIN_PORT_CRT) |		\
> > > +	BIT(POWER_DOMAIN_AUX_A) |		\
> > > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > > +	BIT(POWER_DOMAIN_AUX_C) |		\
> > > +	BIT(POWER_DOMAIN_AUX_D) |		\
> > >  	BIT(POWER_DOMAIN_INIT))
> 
> That's the VLV_DPIO_CMN_BC_POWER_DOMAINS, shouldn't we move AUX_A out of
> this define?

Yes I haven't noticed this. Port A and D don't exist on VLV so we don't
need to add them here. It wouldn't be a problem to add them, since we
never ask for these domains on VLV, but I agree that it's confusing to
add them here.

> Where does the port A lanes/aux should be on VLV and CHV, I don't see a
> good fit nor where the POWER_DOMAIN_PORT_DDI_A_X_LANES is today

POWER_DOMAIN_PORT_DDI_A_X_LANES and POWER_DOMAIN_PORT_DDI_D_X_LANES is
part of VLV_DISPLAY_POWER_DOMAINS, but again they are only a place
holder there. The display power well is needed for all domains, so I
defined the mapping simply accordingly.

> > >  #define VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS (	\
> > >  	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |	\
> > >  	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
> > > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > >  	BIT(POWER_DOMAIN_INIT))
> > >  
> > >  #define VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS (	\
> > >  	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
> > > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > >  	BIT(POWER_DOMAIN_INIT))
> 
> Wouldn't it better to leave the AUX domain in the LANES_01 power domain,
> otherwise we'll power up the full 4 lanes for aux transactions?

With this patch I thought that we'd keep things as-is for all platforms
except SKL. I imagined that we'd get only the AUX power domains for AUX
functionality (in patch 72/89) and not take the port power domains for
full port functionality. That would provide the extra power saving on
SKL and would keep the other platforms unchanged.

You are right that we probably don't need the TX power wells for AUX
functionality on VLV, but I haven't checked this yet, so this would need
to be done separately as a follow-up.

--Imre

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140918/a653a775/attachment.sig>