[Intel-gfx] [PATCH 02/11] drm/i915/cnl: Add Port F definition.

Rodrigo Vivi rodrigo.vivi at intel.com
Thu Dec 28 16:45:28 UTC 2017


On Tue, Dec 26, 2017 at 08:50:57PM +0000, Pandiyan, Dhinakaran wrote:
> 
> On Fri, 2017-12-22 at 15:18 -0800, Rodrigo Vivi wrote:
> > Some Cannonlake SKUs will come with a full split between
> > port A and port E. This will be called port F although it
> > is not a 6th port, but only a split.
> 
> I am not sure if this is said in this spec.

Yeap... I fully understand you.
I could only read like this after Art pin point me the info.

Cannonlake Display Connections
SKU Differences
Display PHY
1 eDP + 4 DDI = (A, B, C, D, F)

And previous SKUs are maximum
1 eDP + 3 DDI = (A, B, C, D)

Plus the fact that port F interrupts re-use bits from portE
making DDI-E totally useless when DDI-F is present.

> 
> From to what I can read and understand -
> 1) port A and port E can share lanes.

There is no E in any CNL SKU.

> 2) port F and port E mutually exclusive usage.

Right.

> 
> So, you could have port A(x4) and port F(x4)
>     or port A(x2) and port E(x2)

This is exactly why Port F was created for. A Full split
so you don't have to reduce eDP lanes when the "4th DDI" is in use.
In HW engineer words: "Full port split".

My understanding is that nowadays eDPs are requesting more
data bandwidth so 2 lanes approach was useless. Hence the port split.

> but doesn't really say port F is not a different port from port E.

Exactly. but port E was never a full port... it was just stealing 2 lanes
from port A. So now port F comes to the picture to avoid that, but reusing
a lot from port E.

> Secondly, I also see that port F and port E have separate DDI_BUF_CTL
> and DDI_AUX_CTL registers.

Oh yeah... This is indeed confusing in the current design.
Really strange that the registers for port E weren't fully removed
from spec. But everything else makes clear that these registers for port E
are useless for all current SKUs. Maybe at some point they considered
the A(x2), E(x2), F(x2) and changed their minds later, maybe they just forgot
to remove from there, maybe it was just easier to add a new range that
really don't touch nor interact with port A in any ways....

Thanks,
Rodrigo.

> 
> > 
> > v2: Fix size of dvo_ports found by Ander.
> > v3: Adding missing cases from intel_bios.c for Port_F
> > v4: Adding other missing cases and fix the commit message.
> > v5: Rebase on top of display headers rework.
> > 
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Cc: Manasi Navare <manasi.d.navare at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c     | 9 +++++++++
> >  drivers/gpu/drm/i915/intel_display.h  | 1 +
> >  drivers/gpu/drm/i915/intel_dp.c       | 2 ++
> >  drivers/gpu/drm/i915/intel_vbt_defs.h | 2 ++
> >  include/drm/i915_component.h          | 3 +--
> >  5 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 51108ffc28d1..59a150e2adce 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1140,6 +1140,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> >  		{DVO_PORT_HDMIC, DVO_PORT_DPC, -1},
> >  		{DVO_PORT_HDMID, DVO_PORT_DPD, -1},
> >  		{DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
> > +		{DVO_PORT_HDMIF, DVO_PORT_DPF, -1},
> >  	};
> >  
> >  	/*
> > @@ -1688,6 +1689,7 @@ bool intel_bios_is_port_present(struct drm_i915_private *dev_priv, enum port por
> >  		[PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
> >  		[PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
> >  		[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> > +		[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> >  	};
> >  	int i;
> >  
> > @@ -1726,6 +1728,7 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> >  		[PORT_C] = DVO_PORT_DPC,
> >  		[PORT_D] = DVO_PORT_DPD,
> >  		[PORT_E] = DVO_PORT_DPE,
> > +		[PORT_F] = DVO_PORT_DPF,
> >  	};
> >  	int i;
> >  
> > @@ -1761,6 +1764,7 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child,
> >  		[PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
> >  		[PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
> >  		[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> > +		[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> >  	};
> >  
> >  	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> > @@ -1927,6 +1931,11 @@ intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
> >  			if (port == PORT_D)
> >  				return true;
> >  			break;
> > +		case DVO_PORT_DPF:
> > +		case DVO_PORT_HDMIF:
> > +			if (port == PORT_F)
> > +				return true;
> > +			break;
> >  		default:
> >  			break;
> >  		}
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index a0d2b6169361..e47638931b51 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -119,6 +119,7 @@ enum port {
> >  	PORT_C,
> >  	PORT_D,
> >  	PORT_E,
> > +	PORT_F,
> >  
> >  	I915_MAX_PORTS
> >  };
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 35c5299feab6..71721de39e6b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1378,6 +1378,7 @@ static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
> >  	case PORT_B:
> >  	case PORT_C:
> >  	case PORT_D:
> > +	case PORT_F:
> >  		return DP_AUX_CH_CTL(port);
> >  	default:
> >  		MISSING_CASE(port);
> > @@ -1393,6 +1394,7 @@ static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
> >  	case PORT_B:
> >  	case PORT_C:
> >  	case PORT_D:
> > +	case PORT_F:
> >  		return DP_AUX_CH_DATA(port, index);
> >  	default:
> >  		MISSING_CASE(port);
> > diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > index e3d7745a9151..8713e8295957 100644
> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > @@ -299,6 +299,8 @@ struct bdb_general_features {
> >  #define DVO_PORT_DPA		10
> >  #define DVO_PORT_DPE		11				/* 193 */
> >  #define DVO_PORT_HDMIE		12				/* 193 */
> > +#define DVO_PORT_DPF	13
> > +#define DVO_PORT_HDMIF	14
> >  #define DVO_PORT_MIPIA		21				/* 171 */
> >  #define DVO_PORT_MIPIB		22				/* 171 */
> >  #define DVO_PORT_MIPIC		23				/* 171 */
> > diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> > index 545c6e0fea7d..346b1f5cb180 100644
> > --- a/include/drm/i915_component.h
> > +++ b/include/drm/i915_component.h
> > @@ -26,9 +26,8 @@
> >  
> >  /* MAX_PORT is the number of port
> >   * It must be sync with I915_MAX_PORTS defined i915_drv.h
> > - * 5 should be enough as only HSW, BDW, SKL need such fix.
> >   */
> > -#define MAX_PORTS 5
> > +#define MAX_PORTS 6
> >  
> >  /**
> >   * struct i915_audio_component_ops - Ops implemented by i915 driver, called by hda driver


More information about the Intel-gfx mailing list