Re: [PATCH 01/28] drm/i915: Add modular FIA

[Lucas De Marchi <lucas.demarchi@xxxxxxxxx>]

On Wed, Jun 26, 2019 at 06:50:14PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 25, 2019 at 10:54:10AM -0700, Lucas De Marchi wrote:
> > From: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> >
> > Some platforms may have Modular FIA. If Modular FIA is used in the SOC,
> > then Display Driver will access the additional instances of
> > FIA based on pre-assigned offset in GTTMADDR space.
> >
> > Each Modular FIA instance has its own IOSF Sideband Port ID
> > and it houses only 2 Type-C Port. In SOC that has more than
> > two Type-C Ports, there are multiple instances of Modular FIA.
> > Gunit will need to use different destination ID when it access
> > different pair of Type-C Port.
> >
> > The DFLEXDPSP register has Modular FIA bit. If Modular FIA is
> > used in the SOC, this register bit exists in all the instances of
> > Modular FIA. IOM FW is required to program only the MF bit in
> > first FIA instance that houses the Type-C Port 0 and Port 1, for
> > Display Driver to read from.
> >
> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     |  9 ++++---
> >  drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_display.h |  6 +++++
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 25 ++++++++++++------
> >  drivers/gpu/drm/i915/i915_reg.h              | 13 +++++++---
> >  drivers/gpu/drm/i915/intel_device_info.h     |  1 +
> >  drivers/gpu/drm/i915/intel_drv.h             |  2 ++
> >  7 files changed, 68 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 7925a176f900..b717562fcce5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2997,6 +2997,7 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
> >  	enum port port = intel_dig_port->base.port;
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> >  	u32 ln0, ln1, lane_info;
> > +	enum display_fia fia;
>
> Can we stick this into the new enum phy namespace we're going to need
> anyway for ehl?

each modular FIA maps up to 2 phys (as opposed to ICL's FIA that has 4
phys). I don't think it should be in the same enum namespace.

Yes, with patch
"drm/i915/gen11: Start distinguishing 'phy' from 'port'" merged we
should rather do the phy -> fia mapping here and it could be as simple
as `phy / 2`.

I would rather merge this as is and convert it when that patch is
applied. Also, that series is still missing the conversion for TC since
EHL has only combo phys. This one is about TC.

Once that series is merged and also the pending one from Imre to
separate the TC handling, I can rework this.


Lucas De Marchi

> >  	if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type == TC_PORT_TBT)
> >  		return;
> > @@ -3009,7 +3010,8 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
> >  		ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);
> >  		ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);
> >
> > -		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> > +		fia = intel_tc_port_to_fia(dev_priv, tc_port);
> > +		lane_info = (I915_READ(PORT_TX_DFLEXDPSP(fia)) &
> >  			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> >  			    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> >
> > @@ -3598,7 +3600,8 @@ static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > -	u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
> > +	enum display_fia fia = intel_tc_port_to_fia(dev_priv, tc_port);
> > +	u32 val = I915_READ(PORT_TX_DFLEXDPMLE1(fia));
> >  	bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> >
> >  	val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> > @@ -3617,7 +3620,7 @@ static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
> >  	default:
> >  		MISSING_CASE(pipe_config->lane_count);
> >  	}
> > -	I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
> > +	I915_WRITE(PORT_TX_DFLEXDPMLE1(fia), val);
> >  }
> >
> >  static void
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8592a7d422de..6217b5bcea2a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6590,6 +6590,33 @@ enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv, enum port port)
> >  	return port - PORT_C;
> >  }
> >
> > +static bool intel_has_modular_fia(struct drm_i915_private *dev_priv)
> > +{
> > +	if (!INTEL_INFO(dev_priv)->display.has_modular_fia)
> > +		return false;
> > +
> > +	return I915_READ(PORT_TX_DFLEXDPSP(FIA_1)) & MODULAR_FIA_MASK;
> > +}
> > +
> > +enum display_fia intel_tc_port_to_fia(struct drm_i915_private *dev_priv,
> > +				      enum tc_port tc_port)
> > +{
> > +	if (!intel_has_modular_fia(dev_priv))
> > +		return FIA_1;
> > +
> > +	switch (tc_port) {
> > +	case PORT_TC1:
> > +	case PORT_TC2:
> > +		return FIA_1;
> > +	case PORT_TC3:
> > +	case PORT_TC4:
> > +		return FIA_2;
> > +	default:
> > +		WARN_ON(tc_port);
> > +		return FIA_1;
> > +	}
> > +}
> > +
> >  enum intel_display_power_domain intel_port_to_power_domain(enum port port)
> >  {
> >  	switch (port) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index ee6b8194a459..12ded01ed5d3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -196,6 +196,12 @@ enum tc_port_type {
> >  	TC_PORT_LEGACY,
> >  };
> >
> > +enum display_fia {
> > +	FIA_1 = 0,
> > +	FIA_2,
> > +	FIA_3,
> > +};
> > +
> >  enum dpio_channel {
> >  	DPIO_CH0,
> >  	DPIO_CH1
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 4336df46fe78..5ed6e49fef33 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -218,13 +218,15 @@ static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp)
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> >  	intel_wakeref_t wakeref;
> >  	u32 lane_info;
> > +	enum display_fia fia;
> >
> >  	if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
> >  		return 4;
> >
> > +	fia  = intel_tc_port_to_fia(dev_priv, tc_port);
> >  	lane_info = 0;
> >  	with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref)
> > -		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> > +		lane_info = (I915_READ(PORT_TX_DFLEXDPSP(fia)) &
> >  			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> >  				DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> >
> > @@ -5300,12 +5302,14 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> >  {
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> >  	u32 val;
> > +	enum display_fia fia;
> >
> >  	if (dig_port->tc_type != TC_PORT_LEGACY &&
> >  	    dig_port->tc_type != TC_PORT_TYPEC)
> >  		return true;
> >
> > -	val = I915_READ(PORT_TX_DFLEXDPPMS);
> > +	fia = intel_tc_port_to_fia(dev_priv, tc_port);
> > +	val = I915_READ(PORT_TX_DFLEXDPPMS(fia));
> >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> >  		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
> >  		WARN_ON(dig_port->tc_legacy_port);
> > @@ -5316,10 +5320,10 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> >  	 * This function may be called many times in a row without an HPD event
> >  	 * in between, so try to avoid the write when we can.
> >  	 */
> > -	val = I915_READ(PORT_TX_DFLEXDPCSSS);
> > +	val = I915_READ(PORT_TX_DFLEXDPCSSS(fia));
> >  	if (!(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port))) {
> >  		val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > -		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> > +		I915_WRITE(PORT_TX_DFLEXDPCSSS(fia), val);
> >  	}
> >
> >  	/*
> > @@ -5327,7 +5331,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> >  	 * became disconnected. Not necessary for legacy mode.
> >  	 */
> >  	if (dig_port->tc_type == TC_PORT_TYPEC &&
> > -	    !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
> > +	    !(I915_READ(PORT_TX_DFLEXDPSP(fia)) & TC_LIVE_STATE_TC(tc_port))) {
> >  		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port);
> >  		icl_tc_phy_disconnect(dev_priv, dig_port);
> >  		return false;
> > @@ -5344,10 +5348,13 @@ void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> >  			   struct intel_digital_port *dig_port)
> >  {
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> > +	enum display_fia fia;
> >
> >  	if (dig_port->tc_type == TC_PORT_UNKNOWN)
> >  		return;
> >
> > +	fia = intel_tc_port_to_fia(dev_priv, tc_port);
> > +
> >  	/*
> >  	 * TBT disconnection flow is read the live status, what was done in
> >  	 * caller.
> > @@ -5356,9 +5363,9 @@ void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> >  	    dig_port->tc_type == TC_PORT_LEGACY) {
> >  		u32 val;
> >
> > -		val = I915_READ(PORT_TX_DFLEXDPCSSS);
> > +		val = I915_READ(PORT_TX_DFLEXDPCSSS(fia));
> >  		val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > -		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> > +		I915_WRITE(PORT_TX_DFLEXDPCSSS(fia), val);
> >  	}
> >
> >  	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
> > @@ -5383,6 +5390,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  {
> >  	enum port port = intel_dig_port->base.port;
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > +	enum display_fia fia = intel_tc_port_to_fia(dev_priv, tc_port);
> >  	bool is_legacy, is_typec, is_tbt;
> >  	u32 dpsp;
> >
> > @@ -5402,7 +5410,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  	 * The spec says we shouldn't be using the ISR bits for detecting
> >  	 * between TC and TBT. We should use DFLEXDPSP.
> >  	 */
> > -	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
> > +
> > +	dpsp = I915_READ(PORT_TX_DFLEXDPSP(fia));
> >  	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> >  	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 7e6009cefb18..8047f1bed314 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2201,9 +2201,13 @@ enum i915_power_well_id {
> >  #define   DW6_OLDO_DYN_PWR_DOWN_EN	(1 << 28)
> >
> >  #define FIA1_BASE			0x163000
> > +#define FIA2_BASE			0x16E000
> > +#define FIA3_BASE			0x16F000
> > +#define _FIA(fia)			_PICK((fia), FIA1_BASE, FIA2_BASE, FIA3_BASE)
> > +#define _MMIO_FIA(fia, off)		_MMIO(_FIA(fia) + (off))
> >
> >  /* ICL PHY DFLEX registers */
> > -#define PORT_TX_DFLEXDPMLE1		_MMIO(FIA1_BASE + 0x008C0)
> > +#define PORT_TX_DFLEXDPMLE1(fia)	_MMIO_FIA((fia),  0x008C0)
> >  #define   DFLEXDPMLE1_DPMLETC_MASK(tc_port)	(0xf << (4 * (tc_port)))
> >  #define   DFLEXDPMLE1_DPMLETC_ML0(tc_port)	(1 << (4 * (tc_port)))
> >  #define   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port)	(3 << (4 * (tc_port)))
> > @@ -11461,17 +11465,18 @@ enum skl_power_gate {
> >  						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
> >  						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
> >
> > -#define PORT_TX_DFLEXDPSP			_MMIO(FIA1_BASE + 0x008A0)
> > +#define PORT_TX_DFLEXDPSP(fia)			_MMIO_FIA((fia), 0x008A0)
> > +#define   MODULAR_FIA_MASK			(1 << 4)
> >  #define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
> >  #define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
> >  #define   DP_LANE_ASSIGNMENT_SHIFT(tc_port)	((tc_port) * 8)
> >  #define   DP_LANE_ASSIGNMENT_MASK(tc_port)	(0xf << ((tc_port) * 8))
> >  #define   DP_LANE_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 8))
> >
> > -#define PORT_TX_DFLEXDPPMS				_MMIO(FIA1_BASE + 0x00890)
> > +#define PORT_TX_DFLEXDPPMS(fia)			_MMIO_FIA((fia), 0x00890)
> >  #define   DP_PHY_MODE_STATUS_COMPLETED(tc_port)		(1 << (tc_port))
> >
> > -#define PORT_TX_DFLEXDPCSSS			_MMIO(FIA1_BASE + 0x00894)
> > +#define PORT_TX_DFLEXDPCSSS(fia)		_MMIO_FIA((fia), 0x00894)
> >  #define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)		(1 << (tc_port))
> >
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> > index ddafc819bf30..e9dc86ed517b 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -136,6 +136,7 @@ enum intel_ppgtt_type {
> >  	func(has_gmch); \
> >  	func(has_hotplug); \
> >  	func(has_ipc); \
> > +	func(has_modular_fia); \
> >  	func(has_overlay); \
> >  	func(has_psr); \
> >  	func(overlay_needs_physical); \
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 1d58f7ec5d84..e30cb4be4997 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1477,6 +1477,8 @@ bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port);
> >  bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port port);
> >  enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv,
> >  			      enum port port);
> > +enum display_fia intel_tc_port_to_fia(struct drm_i915_private *dev_priv,
> > +				      enum tc_port tc_port);
> >  int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void *data,
> >  				      struct drm_file *file_priv);
> >  enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
> > --
> > 2.21.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx