[Intel-gfx] [PATCH 09/10] drm/i915/cnl: Enable DDI-F on Cannonlake.

[Imre Deak]

On Fri, Jan 26, 2018 at 02:06:23PM -0800, Rodrigo Vivi wrote:
> On Fri, Jan 26, 2018 at 09:39:42PM +0000, Pandiyan, Dhinakaran wrote:
> > 
> > On Thu, 2018-01-25 at 14:03 -0800, Rodrigo Vivi wrote:
> > > Now let's finish the Port-F support by adding the
> > > proper port F detection, irq and power well support.
> > > 
> > > v2: Rebase
> > > v3: Use BIT_ULL
> > > v4: Cover missed case on ddi init.
> > > v5: Update commit message.
> > > v6: Rebase on top of display headers rework.
> > > v7: Squash power-well handling related to DDI F to this
> > >     patch to avoid warns as pointed out by DK.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Cc: Manasi Navare <manasi.d.navare at intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
> > >  drivers/gpu/drm/i915/intel_ddi.c        |  4 ++++
> > >  drivers/gpu/drm/i915/intel_display.c    |  6 +++++-
> > >  drivers/gpu/drm/i915/intel_display.h    |  2 ++
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 19 ++++++++++++++++---
> > >  5 files changed, 29 insertions(+), 4 deletions(-)
> > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index 30fa2041a45f..c4042e342f50 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -157,11 +157,13 @@ enum intel_display_power_domain {
> > >  	POWER_DOMAIN_PORT_DDI_C_LANES,
> > >  	POWER_DOMAIN_PORT_DDI_D_LANES,
> > >  	POWER_DOMAIN_PORT_DDI_E_LANES,
> > > +	POWER_DOMAIN_PORT_DDI_F_LANES,
> > 
> > What well does this need? {B,C,D}_LANES all enable/disable power well 2
> > from what I can tell. I don't see a
> > BIT_ULL(POWER_DOMAIN_PORT_DDI_F_LANES) in this patch.
> 
> indeed. They need to be added along with other *DDI_{B,C,D}_LANES on
> CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS.
> 
> > 
> > >  	POWER_DOMAIN_PORT_DDI_A_IO,
> > >  	POWER_DOMAIN_PORT_DDI_B_IO,
> > >  	POWER_DOMAIN_PORT_DDI_C_IO,
> > >  	POWER_DOMAIN_PORT_DDI_D_IO,
> > >  	POWER_DOMAIN_PORT_DDI_E_IO,
> > > +	POWER_DOMAIN_PORT_DDI_F_IO,
> > >  	POWER_DOMAIN_PORT_DSI,
> > >  	POWER_DOMAIN_PORT_CRT,
> > >  	POWER_DOMAIN_PORT_OTHER,
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 3526b563b8ec..30e50ea16960 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -94,6 +94,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > >  		return "PORT_DDI_D_LANES";
> > >  	case POWER_DOMAIN_PORT_DDI_E_LANES:
> > >  		return "PORT_DDI_E_LANES";
> > > +	case POWER_DOMAIN_PORT_DDI_F_LANES:
> > > +		return "PORT_DDI_F_LANES";
> > >  	case POWER_DOMAIN_PORT_DDI_A_IO:
> > >  		return "PORT_DDI_A_IO";
> > >  	case POWER_DOMAIN_PORT_DDI_B_IO:
> > > @@ -104,6 +106,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > >  		return "PORT_DDI_D_IO";
> > >  	case POWER_DOMAIN_PORT_DDI_E_IO:
> > >  		return "PORT_DDI_E_IO";
> > > +	case POWER_DOMAIN_PORT_DDI_F_IO:
> > > +		return "PORT_DDI_F_IO";
> > >  	case POWER_DOMAIN_PORT_DSI:
> > >  		return "PORT_DSI";
> > >  	case POWER_DOMAIN_PORT_CRT:
> > > @@ -1860,6 +1864,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > >  #define CNL_DISPLAY_AUX_F_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_F) |			\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > > +#define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (		\
> > > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |		\
> > > +	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
> > >  	CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> > >  	BIT_ULL(POWER_DOMAIN_GT_IRQ) |			\
> > > @@ -2411,6 +2418,12 @@ static struct i915_power_well cnl_power_wells[] = {
> > >  		.id = SKL_DISP_PW_DDI_D,
> > >  	},
> > >  	{
> > > +		.name = "DDI F IO power well",
> > > +		.domains = CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS,
> > > +		.ops = &hsw_power_well_ops,
> > > +		.id = CNL_DISP_PW_DDI_F,
> > > +	},
> > 
> > Again same question about the enabling order, can this be enabled after
> > power well2? 
> 
> Ok, now I see your question from another angle.
> 
> I hope it works or the proposed solution for power well won't work at all.
> 
> I can't recall anywhere on the spec where this order should matter. Imre?

The order shouldn't matter. The BSpec AUX programming lists the
dependencies in the AUX->PW#2 order. OTOH PW#2 could be already enabled
due to any external port being active at the time we want to enable AUX.
So what matters is that both PWs are enabled before starting the AUX
transfer.

> 
> If the order matters somehow we will need to bring the old patches back to
> add the full definition of wells. So I hope we don't need that.



> 
> > 
> >