Re: [PATCH 04/23] drm/i915: Sanitize the terminology used for TypeC port modes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 07, 2019 at 10:15:12PM +0300, Souza, Jose wrote:
> On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > The TypeC port mode can switch dynamically, to reflect that better
> > call
> > the port's mode as 'mode' rather than 'type'.
> > 
> > While at it:
> > - s/TC_PORT_TBT/TC_PORT_TBT_ALT/ and s/TC_PORT_TYPEC/TC_PORT_DP_ALT/.
> >   'TYPEC' is ambiguous, TBT_ALT and DP_ALT better match the reality.
> > 
> > - Remove the 'unknown' TypeC port mode. The mode is always known,
> > it's
> >   the TBT-alt/safe mode after HW reset and after disconnecting the
> > PHY.
> >   Simplify the tc_port/tc_type checks accordingly.
> > 
> > - Don't WARN if the port mode changes, that can happen normally.
> > 
> > No functional changes.
> 
> There is, the default tc_mode value now is TC_PORT_TBT_ALT instead of
> TC_PORT_UNKNOWN.

That doesn't change the functionality. Before the change the two modes
were treated the same way; that fact motivated the change to remove
TC_PORT_UNKNOWN and just use TC_PORT_TBT_ALT instead, which I tried to
explain in the commit log.

> 
> With the change above:
> Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> 
> Also consider split this patch in two.

Makes sense to keep it as-is, based on keeping the
functionality as described above?

> > 
> > Cc: Animesh Manna <animesh.manna@xxxxxxxxx>
> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c      | 11 +++---
> >  drivers/gpu/drm/i915/intel_display.h  |  7 ++--
> >  drivers/gpu/drm/i915/intel_dp.c       |  2 +-
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c |  2 +-
> >  drivers/gpu/drm/i915/intel_drv.h      |  2 +-
> >  drivers/gpu/drm/i915/intel_tc.c       | 48 +++++++++++------------
> > ----
> >  6 files changed, 31 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 5a1c98438375..a3574f14a3d0 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2986,14 +2986,14 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> >  	u32 ln0, ln1, lane_info;
> >  
> > -	if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type ==
> > TC_PORT_TBT)
> > +	if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> >  		return;
> >  
> >  	ln0 = I915_READ(MG_DP_MODE(0, port));
> >  	ln1 = I915_READ(MG_DP_MODE(1, port));
> >  
> > -	switch (intel_dig_port->tc_type) {
> > -	case TC_PORT_TYPEC:
> > +	switch (intel_dig_port->tc_mode) {
> > +	case TC_PORT_DP_ALT:
> >  		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);
> >  
> > @@ -3036,7 +3036,7 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >  		break;
> >  
> >  	default:
> > -		MISSING_CASE(intel_dig_port->tc_type);
> > +		MISSING_CASE(intel_dig_port->tc_mode);
> >  		return;
> >  	}
> >  
> > @@ -3630,8 +3630,7 @@ intel_ddi_pre_pll_enable(struct intel_encoder
> > *encoder,
> >  	 * Program the lane count for static/dynamic connections on
> > Type-C ports.
> >  	 * Skip this step for TBT.
> >  	 */
> > -	if (dig_port->tc_type == TC_PORT_UNKNOWN ||
> > -	    dig_port->tc_type == TC_PORT_TBT)
> > +	if (dig_port->tc_mode == TC_PORT_TBT_ALT)
> >  		return;
> >  
> >  	intel_ddi_set_fia_lane_count(encoder, crtc_state, port);
> > diff --git a/drivers/gpu/drm/i915/intel_display.h
> > b/drivers/gpu/drm/i915/intel_display.h
> > index ee6b8194a459..d296556ed82e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -189,10 +189,9 @@ enum tc_port {
> >  	I915_MAX_TC_PORTS
> >  };
> >  
> > -enum tc_port_type {
> > -	TC_PORT_UNKNOWN = 0,
> > -	TC_PORT_TYPEC,
> > -	TC_PORT_TBT,
> > +enum tc_port_mode {
> > +	TC_PORT_TBT_ALT,
> > +	TC_PORT_DP_ALT,
> >  	TC_PORT_LEGACY,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b69310bd9914..e1e27662aa6d 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1175,7 +1175,7 @@ static u32 skl_get_aux_send_ctl(struct intel_dp
> > *intel_dp,
> >  	      DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
> >  	      DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> >  
> > -	if (intel_dig_port->tc_type == TC_PORT_TBT)
> > +	if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> >  		ret |= DP_AUX_CH_CTL_TBT_IO;
> >  
> >  	return ret;
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 69787f259677..f4787650a0d3 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -2817,7 +2817,7 @@ icl_get_dpll(struct intel_crtc_state
> > *crtc_state,
> >  			intel_dig_port = enc_to_dig_port(&encoder-
> > >base);
> >  		}
> >  
> > -		if (intel_dig_port->tc_type == TC_PORT_TBT) {
> > +		if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) {
> >  			min = DPLL_ID_ICL_TBTPLL;
> >  			max = min;
> >  			ret = icl_calc_dpll_state(crtc_state, encoder);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 0dcc03592d6e..30cd49dbd0d8 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1225,7 +1225,7 @@ struct intel_digital_port {
> >  	enum aux_ch aux_ch;
> >  	enum intel_display_power_domain ddi_io_power_domain;
> >  	bool tc_legacy_port:1;
> > -	enum tc_port_type tc_type;
> > +	enum tc_port_mode tc_mode;
> >  
> >  	void (*write_infoframe)(struct intel_encoder *encoder,
> >  				const struct intel_crtc_state
> > *crtc_state,
> > diff --git a/drivers/gpu/drm/i915/intel_tc.c
> > b/drivers/gpu/drm/i915/intel_tc.c
> > index 7a1b5870945f..a3057c44bec6 100644
> > --- a/drivers/gpu/drm/i915/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/intel_tc.c
> > @@ -6,19 +6,18 @@
> >  #include "i915_drv.h"
> >  #include "intel_tc.h"
> >  
> > -static const char *tc_type_name(enum tc_port_type type)
> > +static const char *tc_port_mode_name(enum tc_port_mode mode)
> >  {
> >  	static const char * const names[] = {
> > -		[TC_PORT_UNKNOWN] = "unknown",
> > +		[TC_PORT_TBT_ALT] = "tbt-alt",
> > +		[TC_PORT_DP_ALT] = "dp-alt",
> >  		[TC_PORT_LEGACY] = "legacy",
> > -		[TC_PORT_TYPEC] = "typec",
> > -		[TC_PORT_TBT] = "tbt",
> >  	};
> >  
> > -	if (WARN_ON(type >= ARRAY_SIZE(names)))
> > -		type = TC_PORT_UNKNOWN;
> > +	if (WARN_ON(mode >= ARRAY_SIZE(names)))
> > +		mode = TC_PORT_TBT_ALT;
> >  
> > -	return names[type];
> > +	return names[mode];
> >  }
> >  
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> > @@ -28,7 +27,7 @@ int intel_tc_port_fia_max_lane_count(struct
> > intel_digital_port *dig_port)
> >  	intel_wakeref_t wakeref;
> >  	u32 lane_info;
> >  
> > -	if (tc_port == PORT_TC_NONE || dig_port->tc_type !=
> > TC_PORT_TYPEC)
> > +	if (dig_port->tc_mode != TC_PORT_DP_ALT)
> >  		return 4;
> >  
> >  	lane_info = 0;
> > @@ -80,8 +79,8 @@ static bool icl_tc_phy_connect(struct
> > intel_digital_port *dig_port)
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port-
> > >base.port);
> >  	u32 val;
> >  
> > -	if (dig_port->tc_type != TC_PORT_LEGACY &&
> > -	    dig_port->tc_type != TC_PORT_TYPEC)
> > +	if (dig_port->tc_mode != TC_PORT_LEGACY &&
> > +	    dig_port->tc_mode != TC_PORT_DP_ALT)
> >  		return true;
> >  
> >  	val = I915_READ(PORT_TX_DFLEXDPPMS);
> > @@ -105,7 +104,7 @@ static bool icl_tc_phy_connect(struct
> > intel_digital_port *dig_port)
> >  	 * Now we have to re-check the live state, in case the port
> > recently
> >  	 * became disconnected. Not necessary for legacy mode.
> >  	 */
> > -	if (dig_port->tc_type == TC_PORT_TYPEC &&
> > +	if (dig_port->tc_mode == TC_PORT_DP_ALT &&
> >  	    !(I915_READ(PORT_TX_DFLEXDPSP) &
> > TC_LIVE_STATE_TC(tc_port))) {
> >  		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n",
> > tc_port);
> >  		icl_tc_phy_disconnect(dig_port);
> > @@ -124,15 +123,12 @@ void icl_tc_phy_disconnect(struct
> > intel_digital_port *dig_port)
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port-
> > >base.port);
> >  
> > -	if (dig_port->tc_type == TC_PORT_UNKNOWN)
> > -		return;
> > -
> >  	/*
> >  	 * TBT disconnection flow is read the live status, what was
> > done in
> >  	 * caller.
> >  	 */
> > -	if (dig_port->tc_type == TC_PORT_TYPEC ||
> > -	    dig_port->tc_type == TC_PORT_LEGACY) {
> > +	if (dig_port->tc_mode == TC_PORT_DP_ALT ||
> > +	    dig_port->tc_mode == TC_PORT_LEGACY) {
> >  		u32 val;
> >  
> >  		val = I915_READ(PORT_TX_DFLEXDPCSSS);
> > @@ -142,9 +138,9 @@ void icl_tc_phy_disconnect(struct
> > intel_digital_port *dig_port)
> >  
> >  	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
> >  		      port_name(dig_port->base.port),
> > -		      tc_type_name(dig_port->tc_type));
> > +		      tc_port_mode_name(dig_port->tc_mode));
> >  
> > -	dig_port->tc_type = TC_PORT_UNKNOWN;
> > +	dig_port->tc_mode = TC_PORT_TBT_ALT;
> >  }
> >  
> >  static void icl_update_tc_port_type(struct drm_i915_private
> > *dev_priv,
> > @@ -152,26 +148,22 @@ static void icl_update_tc_port_type(struct
> > drm_i915_private *dev_priv,
> >  				    bool is_legacy, bool is_typec, bool
> > is_tbt)
> >  {
> >  	enum port port = intel_dig_port->base.port;
> > -	enum tc_port_type old_type = intel_dig_port->tc_type;
> > +	enum tc_port_mode old_mode = intel_dig_port->tc_mode;
> >  
> >  	WARN_ON(is_legacy + is_typec + is_tbt != 1);
> >  
> >  	if (is_legacy)
> > -		intel_dig_port->tc_type = TC_PORT_LEGACY;
> > +		intel_dig_port->tc_mode = TC_PORT_LEGACY;
> >  	else if (is_typec)
> > -		intel_dig_port->tc_type = TC_PORT_TYPEC;
> > +		intel_dig_port->tc_mode = TC_PORT_DP_ALT;
> >  	else if (is_tbt)
> > -		intel_dig_port->tc_type = TC_PORT_TBT;
> > +		intel_dig_port->tc_mode = TC_PORT_TBT_ALT;
> >  	else
> >  		return;
> >  
> > -	/* Types are not supposed to be changed at runtime. */
> > -	WARN_ON(old_type != TC_PORT_UNKNOWN &&
> > -		old_type != intel_dig_port->tc_type);
> > -
> > -	if (old_type != intel_dig_port->tc_type)
> > +	if (old_mode != intel_dig_port->tc_mode)
> >  		DRM_DEBUG_KMS("Port %c has TC type %s\n",
> > port_name(port),
> > -			      tc_type_name(intel_dig_port->tc_type));
> > +			      tc_port_mode_name(intel_dig_port-
> > >tc_mode));
> >  }
> >  
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux