[CI,v3,09/23] drm/i915: Factor out common parts from TypeC port handling functions

Submitted by Imre Deak on June 26, 2019, 8:50 p.m.

Details

Message ID 20190626205010.18597-1-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Fix TypeC port mode switching" ( rev: 4 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak June 26, 2019, 8:50 p.m.
Factor out helpers reading/parsing the TypeC specific registers, making
current users of them clearer and letting us use them later.

While at it also:
- Simplify icl_tc_phy_connect() with an early return in legacy mode.
- Simplify the live status check using one bitmask for all HPD bits.
- Remove a micro-optimisation of the repeated safe-mode clearing.
- Make sure we fix the legacy port flag in all cases.

Except for the last two, no functional changes.

v2:
- Don't do reg reads at variable declarations. (Jani)
- Prevent constant truncated compiler warning when assigning the
  valid_hpd_mask. (Nick)
- s/intel_tc_port_get_lane_info/intel_tc_port_get_lane_mask/ (Ville)
v3:
- Make valid_hpd_mask init clear. (Ville)

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  11 +-
 drivers/gpu/drm/i915/display/intel_tc.c  | 188 ++++++++++++++---------
 drivers/gpu/drm/i915/display/intel_tc.h  |   1 +
 3 files changed, 119 insertions(+), 81 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index c9143e2a6994..2be7cdc319ba 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2996,8 +2996,7 @@  static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 	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;
+	u32 ln0, ln1, lane_mask;
 
 	if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
 		return;
@@ -3010,11 +3009,9 @@  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) &
-			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
-			    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+		lane_mask = intel_tc_port_get_lane_mask(intel_dig_port);
 
-		switch (lane_info) {
+		switch (lane_mask) {
 		case 0x1:
 		case 0x4:
 			break;
@@ -3039,7 +3036,7 @@  static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
 			       MG_DP_MODE_CFG_DP_X2_MODE;
 			break;
 		default:
-			MISSING_CASE(lane_info);
+			MISSING_CASE(lane_mask);
 		}
 		break;
 
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index ca3b11e26474..f0688c7450c7 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -21,25 +21,34 @@  static const char *tc_port_mode_name(enum tc_port_mode mode)
 	return names[mode];
 }
 
-int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
+u32 intel_tc_port_get_lane_mask(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);
+	u32 lane_mask;
+
+	lane_mask = I915_READ(PORT_TX_DFLEXDPSP);
+
+	return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
+	       DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+}
+
+int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
 	intel_wakeref_t wakeref;
-	u32 lane_info;
+	u32 lane_mask;
 
 	if (dig_port->tc_mode != TC_PORT_DP_ALT)
 		return 4;
 
-	lane_info = 0;
+	lane_mask = 0;
 	with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref)
-		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
-			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
-				DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+		lane_mask = intel_tc_port_get_lane_mask(dig_port);
 
-	switch (lane_info) {
+	switch (lane_mask) {
 	default:
-		MISSING_CASE(lane_info);
+		MISSING_CASE(lane_mask);
 	case 1:
 	case 2:
 	case 4:
@@ -53,6 +62,76 @@  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 	}
 }
 
+static void tc_port_fixup_legacy_flag(struct intel_digital_port *dig_port,
+				      u32 live_status_mask)
+{
+	u32 valid_hpd_mask;
+
+	if (dig_port->tc_legacy_port)
+		valid_hpd_mask = BIT(TC_PORT_LEGACY);
+	else
+		valid_hpd_mask = BIT(TC_PORT_DP_ALT) |
+				 BIT(TC_PORT_TBT_ALT);
+
+	if (!(live_status_mask & ~valid_hpd_mask))
+		return;
+
+	/* If live status mismatches the VBT flag, trust the live status. */
+	DRM_ERROR("Port %s: live status %08x mismatches the legacy port flag, fix flag\n",
+		  dig_port->tc_port_name, live_status_mask);
+
+	dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
+}
+
+static u32 tc_port_live_status_mask(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);
+	u32 mask = 0;
+	u32 val;
+
+	val = I915_READ(PORT_TX_DFLEXDPSP);
+
+	if (val & TC_LIVE_STATE_TBT(tc_port))
+		mask |= BIT(TC_PORT_TBT_ALT);
+	if (val & TC_LIVE_STATE_TC(tc_port))
+		mask |= BIT(TC_PORT_DP_ALT);
+
+	if (I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
+		mask |= BIT(TC_PORT_LEGACY);
+
+	/* The sink can be connected only in a single mode. */
+	if (!WARN_ON(hweight32(mask) > 1))
+		tc_port_fixup_legacy_flag(dig_port, mask);
+
+	return mask;
+}
+
+static bool icl_tc_phy_status_complete(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);
+
+	return I915_READ(PORT_TX_DFLEXDPPMS) &
+	       DP_PHY_MODE_STATUS_COMPLETED(tc_port);
+}
+
+static void icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
+				     bool enable)
+{
+	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);
+	u32 val;
+
+	val = I915_READ(PORT_TX_DFLEXDPCSSS);
+
+	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+	if (!enable)
+		val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+
+	I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
+}
+
 /*
  * This function implements the first part of the Connect Flow described by our
  * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading
@@ -76,38 +155,31 @@  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
  */
 static bool icl_tc_phy_connect(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);
-	u32 val;
+	u32 live_status_mask;
 
 	if (dig_port->tc_mode != TC_PORT_LEGACY &&
 	    dig_port->tc_mode != TC_PORT_DP_ALT)
 		return true;
 
-	val = I915_READ(PORT_TX_DFLEXDPPMS);
-	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
+	if (!icl_tc_phy_status_complete(dig_port)) {
 		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
 			      dig_port->tc_port_name);
 		WARN_ON(dig_port->tc_legacy_port);
 		return false;
 	}
 
-	/*
-	 * 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);
-	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);
-	}
+	icl_tc_phy_set_safe_mode(dig_port, false);
+
+	if (dig_port->tc_mode == TC_PORT_LEGACY)
+		return true;
+
+	live_status_mask = tc_port_live_status_mask(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_mode == TC_PORT_DP_ALT &&
-	    !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
+	if (!(live_status_mask & BIT(TC_PORT_DP_ALT))) {
 		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
 			      dig_port->tc_port_name);
 		icl_tc_phy_disconnect(dig_port);
@@ -123,46 +195,35 @@  static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
  */
 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);
-
-	/*
-	 * TBT disconnection flow is read the live status, what was done in
-	 * caller.
-	 */
-	if (dig_port->tc_mode == TC_PORT_DP_ALT ||
-	    dig_port->tc_mode == TC_PORT_LEGACY) {
-		u32 val;
-
-		val = I915_READ(PORT_TX_DFLEXDPCSSS);
-		val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
-		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
+	switch (dig_port->tc_mode) {
+	case TC_PORT_LEGACY:
+	case TC_PORT_DP_ALT:
+		icl_tc_phy_set_safe_mode(dig_port, true);
+		dig_port->tc_mode = TC_PORT_TBT_ALT;
+		break;
+	case TC_PORT_TBT_ALT:
+		/* Nothing to do, we stay in TBT-alt mode */
+		break;
+	default:
+		MISSING_CASE(dig_port->tc_mode);
 	}
 
 	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
 		      dig_port->tc_port_name,
 		      tc_port_mode_name(dig_port->tc_mode));
-
-	dig_port->tc_mode = TC_PORT_TBT_ALT;
 }
 
 static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 				    struct intel_digital_port *intel_dig_port,
-				    bool is_legacy, bool is_typec, bool is_tbt)
+				    u32 live_status_mask)
 {
 	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_mode = TC_PORT_LEGACY;
-	else if (is_typec)
-		intel_dig_port->tc_mode = TC_PORT_DP_ALT;
-	else if (is_tbt)
-		intel_dig_port->tc_mode = TC_PORT_TBT_ALT;
-	else
+	if (!live_status_mask)
 		return;
 
+	intel_dig_port->tc_mode = fls(live_status_mask) - 1;
+
 	if (old_mode != intel_dig_port->tc_mode)
 		DRM_DEBUG_KMS("Port %s: port has mode %s\n",
 			      intel_dig_port->tc_port_name,
@@ -182,40 +243,19 @@  static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 bool intel_tc_port_connected(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
-	enum port port = dig_port->base.port;
-	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
-	bool is_legacy, is_typec, is_tbt;
-	u32 dpsp;
-
-	/*
-	 * Complain if we got a legacy port HPD, but VBT didn't mark the port as
-	 * legacy. Treat the port as legacy from now on.
-	 */
-	if (!dig_port->tc_legacy_port &&
-	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
-		DRM_ERROR("Port %s: VBT incorrectly claims port is not TypeC legacy\n",
-			  dig_port->tc_port_name);
-		dig_port->tc_legacy_port = true;
-	}
-	is_legacy = dig_port->tc_legacy_port;
+	u32 live_status_mask = tc_port_live_status_mask(dig_port);
 
 	/*
 	 * 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);
-	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
-	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
-
-	if (!is_legacy && !is_typec && !is_tbt) {
+	if (!live_status_mask && !dig_port->tc_legacy_port) {
 		icl_tc_phy_disconnect(dig_port);
 
 		return false;
 	}
 
-	icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec,
-				is_tbt);
-
+	icl_update_tc_port_type(dev_priv, dig_port, live_status_mask);
 	if (!icl_tc_phy_connect(dig_port))
 		return false;
 
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index ca1735303252..8c338c45796d 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -13,6 +13,7 @@  struct intel_digital_port;
 void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
 
 bool intel_tc_port_connected(struct intel_digital_port *dig_port);
+u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
 
 void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);

Comments

On Wed, 2019-06-26 at 23:50 +0300, Imre Deak wrote:
> Factor out helpers reading/parsing the TypeC specific registers,

> making

> current users of them clearer and letting us use them later.

> 

> While at it also:

> - Simplify icl_tc_phy_connect() with an early return in legacy mode.

> - Simplify the live status check using one bitmask for all HPD bits.

> - Remove a micro-optimisation of the repeated safe-mode clearing.

> - Make sure we fix the legacy port flag in all cases.

> 

> Except for the last two, no functional changes.

> 

> v2:

> - Don't do reg reads at variable declarations. (Jani)

> - Prevent constant truncated compiler warning when assigning the

>   valid_hpd_mask. (Nick)

> - s/intel_tc_port_get_lane_info/intel_tc_port_get_lane_mask/ (Ville)

> v3:

> - Make valid_hpd_mask init clear. (Ville)

> 

> Cc: José Roberto de Souza <jose.souza@intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>

> Cc: Jani Nikula <jani.nikula@intel.com>

> Cc: Nick Desaulniers <ndesaulniers@google.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>

> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/display/intel_ddi.c |  11 +-

>  drivers/gpu/drm/i915/display/intel_tc.c  | 188 ++++++++++++++-------

> --

>  drivers/gpu/drm/i915/display/intel_tc.h  |   1 +

>  3 files changed, 119 insertions(+), 81 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c

> b/drivers/gpu/drm/i915/display/intel_ddi.c

> index c9143e2a6994..2be7cdc319ba 100644

> --- a/drivers/gpu/drm/i915/display/intel_ddi.c

> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c

> @@ -2996,8 +2996,7 @@ static void icl_program_mg_dp_mode(struct

> intel_digital_port *intel_dig_port)

>  {

>  	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-

> >base.base.dev);

>  	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;

> +	u32 ln0, ln1, lane_mask;

>  

>  	if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)

>  		return;

> @@ -3010,11 +3009,9 @@ 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) &

> -			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>

> -			    DP_LANE_ASSIGNMENT_SHIFT(tc_port);

> +		lane_mask =

> intel_tc_port_get_lane_mask(intel_dig_port);

>  

> -		switch (lane_info) {

> +		switch (lane_mask) {

>  		case 0x1:

>  		case 0x4:

>  			break;

> @@ -3039,7 +3036,7 @@ static void icl_program_mg_dp_mode(struct

> intel_digital_port *intel_dig_port)

>  			       MG_DP_MODE_CFG_DP_X2_MODE;

>  			break;

>  		default:

> -			MISSING_CASE(lane_info);

> +			MISSING_CASE(lane_mask);

>  		}

>  		break;

>  

> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c

> b/drivers/gpu/drm/i915/display/intel_tc.c

> index ca3b11e26474..f0688c7450c7 100644

> --- a/drivers/gpu/drm/i915/display/intel_tc.c

> +++ b/drivers/gpu/drm/i915/display/intel_tc.c

> @@ -21,25 +21,34 @@ static const char *tc_port_mode_name(enum

> tc_port_mode mode)

>  	return names[mode];

>  }

>  

> -int intel_tc_port_fia_max_lane_count(struct intel_digital_port

> *dig_port)

> +u32 intel_tc_port_get_lane_mask(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);

> +	u32 lane_mask;

> +

> +	lane_mask = I915_READ(PORT_TX_DFLEXDPSP);

> +

> +	return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>

> +	       DP_LANE_ASSIGNMENT_SHIFT(tc_port);

> +}

> +

> +int intel_tc_port_fia_max_lane_count(struct intel_digital_port

> *dig_port)

> +{

> +	struct drm_i915_private *dev_priv = to_i915(dig_port-

> >base.base.dev);

>  	intel_wakeref_t wakeref;

> -	u32 lane_info;

> +	u32 lane_mask;

>  

>  	if (dig_port->tc_mode != TC_PORT_DP_ALT)

>  		return 4;

>  

> -	lane_info = 0;

> +	lane_mask = 0;

>  	with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE,

> wakeref)

> -		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &

> -			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>

> -				DP_LANE_ASSIGNMENT_SHIFT(tc_port);

> +		lane_mask = intel_tc_port_get_lane_mask(dig_port);

>  

> -	switch (lane_info) {

> +	switch (lane_mask) {

>  	default:

> -		MISSING_CASE(lane_info);

> +		MISSING_CASE(lane_mask);

>  	case 1:

>  	case 2:

>  	case 4:

> @@ -53,6 +62,76 @@ int intel_tc_port_fia_max_lane_count(struct

> intel_digital_port *dig_port)

>  	}

>  }

>  

> +static void tc_port_fixup_legacy_flag(struct intel_digital_port

> *dig_port,

> +				      u32 live_status_mask)

> +{

> +	u32 valid_hpd_mask;

> +

> +	if (dig_port->tc_legacy_port)

> +		valid_hpd_mask = BIT(TC_PORT_LEGACY);

> +	else

> +		valid_hpd_mask = BIT(TC_PORT_DP_ALT) |

> +				 BIT(TC_PORT_TBT_ALT);

> +

> +	if (!(live_status_mask & ~valid_hpd_mask))

> +		return;

> +

> +	/* If live status mismatches the VBT flag, trust the live

> status. */

> +	DRM_ERROR("Port %s: live status %08x mismatches the legacy port

> flag, fix flag\n",

> +		  dig_port->tc_port_name, live_status_mask);

> +

> +	dig_port->tc_legacy_port = !dig_port->tc_legacy_port;

> +}

> +

> +static u32 tc_port_live_status_mask(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);

> +	u32 mask = 0;

> +	u32 val;

> +

> +	val = I915_READ(PORT_TX_DFLEXDPSP);

> +

> +	if (val & TC_LIVE_STATE_TBT(tc_port))

> +		mask |= BIT(TC_PORT_TBT_ALT);

> +	if (val & TC_LIVE_STATE_TC(tc_port))

> +		mask |= BIT(TC_PORT_DP_ALT);

> +

> +	if (I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))

> +		mask |= BIT(TC_PORT_LEGACY);

> +

> +	/* The sink can be connected only in a single mode. */

> +	if (!WARN_ON(hweight32(mask) > 1))

> +		tc_port_fixup_legacy_flag(dig_port, mask);

> +

> +	return mask;

> +}

> +

> +static bool icl_tc_phy_status_complete(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);

> +

> +	return I915_READ(PORT_TX_DFLEXDPPMS) &

> +	       DP_PHY_MODE_STATUS_COMPLETED(tc_port);

> +}

> +

> +static void icl_tc_phy_set_safe_mode(struct intel_digital_port

> *dig_port,

> +				     bool enable)

> +{

> +	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);

> +	u32 val;

> +

> +	val = I915_READ(PORT_TX_DFLEXDPCSSS);

> +

> +	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);

> +	if (!enable)

> +		val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);

> +

> +	I915_WRITE(PORT_TX_DFLEXDPCSSS, val);

> +}


Why those 2 have "icl_" while tc_port_fixup_legacy_flag() and
tc_port_live_status_mask() don't?

Other than that:

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>


> +

>  /*

>   * This function implements the first part of the Connect Flow

> described by our

>   * specification, Gen11 TypeC Programming chapter. The rest of the

> flow (reading

> @@ -76,38 +155,31 @@ int intel_tc_port_fia_max_lane_count(struct

> intel_digital_port *dig_port)

>   */

>  static bool icl_tc_phy_connect(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);

> -	u32 val;

> +	u32 live_status_mask;

>  

>  	if (dig_port->tc_mode != TC_PORT_LEGACY &&

>  	    dig_port->tc_mode != TC_PORT_DP_ALT)

>  		return true;

>  

> -	val = I915_READ(PORT_TX_DFLEXDPPMS);

> -	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {

> +	if (!icl_tc_phy_status_complete(dig_port)) {

>  		DRM_DEBUG_KMS("Port %s: PHY not ready\n",

>  			      dig_port->tc_port_name);

>  		WARN_ON(dig_port->tc_legacy_port);

>  		return false;

>  	}

>  

> -	/*

> -	 * 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);

> -	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);

> -	}

> +	icl_tc_phy_set_safe_mode(dig_port, false);

> +

> +	if (dig_port->tc_mode == TC_PORT_LEGACY)

> +		return true;

> +

> +	live_status_mask = tc_port_live_status_mask(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_mode == TC_PORT_DP_ALT &&

> -	    !(I915_READ(PORT_TX_DFLEXDPSP) &

> TC_LIVE_STATE_TC(tc_port))) {

> +	if (!(live_status_mask & BIT(TC_PORT_DP_ALT))) {

>  		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",

>  			      dig_port->tc_port_name);

>  		icl_tc_phy_disconnect(dig_port);

> @@ -123,46 +195,35 @@ static bool icl_tc_phy_connect(struct

> intel_digital_port *dig_port)

>   */

>  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);

> -

> -	/*

> -	 * TBT disconnection flow is read the live status, what was

> done in

> -	 * caller.

> -	 */

> -	if (dig_port->tc_mode == TC_PORT_DP_ALT ||

> -	    dig_port->tc_mode == TC_PORT_LEGACY) {

> -		u32 val;

> -

> -		val = I915_READ(PORT_TX_DFLEXDPCSSS);

> -		val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);

> -		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);

> +	switch (dig_port->tc_mode) {

> +	case TC_PORT_LEGACY:

> +	case TC_PORT_DP_ALT:

> +		icl_tc_phy_set_safe_mode(dig_port, true);

> +		dig_port->tc_mode = TC_PORT_TBT_ALT;

> +		break;

> +	case TC_PORT_TBT_ALT:

> +		/* Nothing to do, we stay in TBT-alt mode */

> +		break;

> +	default:

> +		MISSING_CASE(dig_port->tc_mode);

>  	}

>  

>  	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",

>  		      dig_port->tc_port_name,

>  		      tc_port_mode_name(dig_port->tc_mode));

> -

> -	dig_port->tc_mode = TC_PORT_TBT_ALT;

>  }

>  

>  static void icl_update_tc_port_type(struct drm_i915_private

> *dev_priv,

>  				    struct intel_digital_port

> *intel_dig_port,

> -				    bool is_legacy, bool is_typec, bool

> is_tbt)

> +				    u32 live_status_mask)

>  {

>  	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_mode = TC_PORT_LEGACY;

> -	else if (is_typec)

> -		intel_dig_port->tc_mode = TC_PORT_DP_ALT;

> -	else if (is_tbt)

> -		intel_dig_port->tc_mode = TC_PORT_TBT_ALT;

> -	else

> +	if (!live_status_mask)

>  		return;

>  

> +	intel_dig_port->tc_mode = fls(live_status_mask) - 1;

> +

>  	if (old_mode != intel_dig_port->tc_mode)

>  		DRM_DEBUG_KMS("Port %s: port has mode %s\n",

>  			      intel_dig_port->tc_port_name,

> @@ -182,40 +243,19 @@ static void icl_update_tc_port_type(struct

> drm_i915_private *dev_priv,

>  bool intel_tc_port_connected(struct intel_digital_port *dig_port)

>  {

>  	struct drm_i915_private *dev_priv = to_i915(dig_port-

> >base.base.dev);

> -	enum port port = dig_port->base.port;

> -	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);

> -	bool is_legacy, is_typec, is_tbt;

> -	u32 dpsp;

> -

> -	/*

> -	 * Complain if we got a legacy port HPD, but VBT didn't mark

> the port as

> -	 * legacy. Treat the port as legacy from now on.

> -	 */

> -	if (!dig_port->tc_legacy_port &&

> -	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {

> -		DRM_ERROR("Port %s: VBT incorrectly claims port is not

> TypeC legacy\n",

> -			  dig_port->tc_port_name);

> -		dig_port->tc_legacy_port = true;

> -	}

> -	is_legacy = dig_port->tc_legacy_port;

> +	u32 live_status_mask = tc_port_live_status_mask(dig_port);

>  

>  	/*

>  	 * 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);

> -	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);

> -	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);

> -

> -	if (!is_legacy && !is_typec && !is_tbt) {

> +	if (!live_status_mask && !dig_port->tc_legacy_port) {

>  		icl_tc_phy_disconnect(dig_port);

>  

>  		return false;

>  	}

>  

> -	icl_update_tc_port_type(dev_priv, dig_port, is_legacy,

> is_typec,

> -				is_tbt);

> -

> +	icl_update_tc_port_type(dev_priv, dig_port, live_status_mask);

>  	if (!icl_tc_phy_connect(dig_port))

>  		return false;

>  

> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h

> b/drivers/gpu/drm/i915/display/intel_tc.h

> index ca1735303252..8c338c45796d 100644

> --- a/drivers/gpu/drm/i915/display/intel_tc.h

> +++ b/drivers/gpu/drm/i915/display/intel_tc.h

> @@ -13,6 +13,7 @@ struct intel_digital_port;

>  void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);

>  

>  bool intel_tc_port_connected(struct intel_digital_port *dig_port);

> +u32 intel_tc_port_get_lane_mask(struct intel_digital_port

> *dig_port);

>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port

> *dig_port);

>  

>  void intel_tc_port_init(struct intel_digital_port *dig_port, bool

> is_legacy);
On Thu, Jun 27, 2019 at 01:55:57AM +0300, Souza, Jose wrote:
> On Wed, 2019-06-26 at 23:50 +0300, Imre Deak wrote:
> > Factor out helpers reading/parsing the TypeC specific registers,
> > making
> > current users of them clearer and letting us use them later.
> > 
> > While at it also:
> > - Simplify icl_tc_phy_connect() with an early return in legacy mode.
> > - Simplify the live status check using one bitmask for all HPD bits.
> > - Remove a micro-optimisation of the repeated safe-mode clearing.
> > - Make sure we fix the legacy port flag in all cases.
> > 
> > Except for the last two, no functional changes.
> > 
> > v2:
> > - Don't do reg reads at variable declarations. (Jani)
> > - Prevent constant truncated compiler warning when assigning the
> >   valid_hpd_mask. (Nick)
> > - s/intel_tc_port_get_lane_info/intel_tc_port_get_lane_mask/ (Ville)
> > v3:
> > - Make valid_hpd_mask init clear. (Ville)
> > 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c |  11 +-
> >  drivers/gpu/drm/i915/display/intel_tc.c  | 188 ++++++++++++++-------
> > --
> >  drivers/gpu/drm/i915/display/intel_tc.h  |   1 +
> >  3 files changed, 119 insertions(+), 81 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index c9143e2a6994..2be7cdc319ba 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2996,8 +2996,7 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> >  	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;
> > +	u32 ln0, ln1, lane_mask;
> >  
> >  	if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> >  		return;
> > @@ -3010,11 +3009,9 @@ 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) &
> > -			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > -			    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +		lane_mask =
> > intel_tc_port_get_lane_mask(intel_dig_port);
> >  
> > -		switch (lane_info) {
> > +		switch (lane_mask) {
> >  		case 0x1:
> >  		case 0x4:
> >  			break;
> > @@ -3039,7 +3036,7 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >  			       MG_DP_MODE_CFG_DP_X2_MODE;
> >  			break;
> >  		default:
> > -			MISSING_CASE(lane_info);
> > +			MISSING_CASE(lane_mask);
> >  		}
> >  		break;
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index ca3b11e26474..f0688c7450c7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -21,25 +21,34 @@ static const char *tc_port_mode_name(enum
> > tc_port_mode mode)
> >  	return names[mode];
> >  }
> >  
> > -int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> > +u32 intel_tc_port_get_lane_mask(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);
> > +	u32 lane_mask;
> > +
> > +	lane_mask = I915_READ(PORT_TX_DFLEXDPSP);
> > +
> > +	return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > +	       DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +}
> > +
> > +int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> >  	intel_wakeref_t wakeref;
> > -	u32 lane_info;
> > +	u32 lane_mask;
> >  
> >  	if (dig_port->tc_mode != TC_PORT_DP_ALT)
> >  		return 4;
> >  
> > -	lane_info = 0;
> > +	lane_mask = 0;
> >  	with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE,
> > wakeref)
> > -		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> > -			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > -				DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +		lane_mask = intel_tc_port_get_lane_mask(dig_port);
> >  
> > -	switch (lane_info) {
> > +	switch (lane_mask) {
> >  	default:
> > -		MISSING_CASE(lane_info);
> > +		MISSING_CASE(lane_mask);
> >  	case 1:
> >  	case 2:
> >  	case 4:
> > @@ -53,6 +62,76 @@ int intel_tc_port_fia_max_lane_count(struct
> > intel_digital_port *dig_port)
> >  	}
> >  }
> >  
> > +static void tc_port_fixup_legacy_flag(struct intel_digital_port
> > *dig_port,
> > +				      u32 live_status_mask)
> > +{
> > +	u32 valid_hpd_mask;
> > +
> > +	if (dig_port->tc_legacy_port)
> > +		valid_hpd_mask = BIT(TC_PORT_LEGACY);
> > +	else
> > +		valid_hpd_mask = BIT(TC_PORT_DP_ALT) |
> > +				 BIT(TC_PORT_TBT_ALT);
> > +
> > +	if (!(live_status_mask & ~valid_hpd_mask))
> > +		return;
> > +
> > +	/* If live status mismatches the VBT flag, trust the live
> > status. */
> > +	DRM_ERROR("Port %s: live status %08x mismatches the legacy port
> > flag, fix flag\n",
> > +		  dig_port->tc_port_name, live_status_mask);
> > +
> > +	dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
> > +}
> > +
> > +static u32 tc_port_live_status_mask(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);
> > +	u32 mask = 0;
> > +	u32 val;
> > +
> > +	val = I915_READ(PORT_TX_DFLEXDPSP);
> > +
> > +	if (val & TC_LIVE_STATE_TBT(tc_port))
> > +		mask |= BIT(TC_PORT_TBT_ALT);
> > +	if (val & TC_LIVE_STATE_TC(tc_port))
> > +		mask |= BIT(TC_PORT_DP_ALT);
> > +
> > +	if (I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
> > +		mask |= BIT(TC_PORT_LEGACY);
> > +
> > +	/* The sink can be connected only in a single mode. */
> > +	if (!WARN_ON(hweight32(mask) > 1))
> > +		tc_port_fixup_legacy_flag(dig_port, mask);
> > +
> > +	return mask;
> > +}
> > +
> > +static bool icl_tc_phy_status_complete(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);
> > +
> > +	return I915_READ(PORT_TX_DFLEXDPPMS) &
> > +	       DP_PHY_MODE_STATUS_COMPLETED(tc_port);
> > +}
> > +
> > +static void icl_tc_phy_set_safe_mode(struct intel_digital_port
> > *dig_port,
> > +				     bool enable)
> > +{
> > +	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);
> > +	u32 val;
> > +
> > +	val = I915_READ(PORT_TX_DFLEXDPCSSS);
> > +
> > +	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > +	if (!enable)
> > +		val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > +
> > +	I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> > +}
> 
> Why those 2 have "icl_" while tc_port_fixup_legacy_flag() and
> tc_port_live_status_mask() don't?

All the icl_tc_phy_* funcs are named as such as they are specific to PHY
programming, the rest are specific to the TypeC port.

> 
> Other than that:
> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > +
> >  /*
> >   * This function implements the first part of the Connect Flow
> > described by our
> >   * specification, Gen11 TypeC Programming chapter. The rest of the
> > flow (reading
> > @@ -76,38 +155,31 @@ int intel_tc_port_fia_max_lane_count(struct
> > intel_digital_port *dig_port)
> >   */
> >  static bool icl_tc_phy_connect(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);
> > -	u32 val;
> > +	u32 live_status_mask;
> >  
> >  	if (dig_port->tc_mode != TC_PORT_LEGACY &&
> >  	    dig_port->tc_mode != TC_PORT_DP_ALT)
> >  		return true;
> >  
> > -	val = I915_READ(PORT_TX_DFLEXDPPMS);
> > -	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> > +	if (!icl_tc_phy_status_complete(dig_port)) {
> >  		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
> >  			      dig_port->tc_port_name);
> >  		WARN_ON(dig_port->tc_legacy_port);
> >  		return false;
> >  	}
> >  
> > -	/*
> > -	 * 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);
> > -	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);
> > -	}
> > +	icl_tc_phy_set_safe_mode(dig_port, false);
> > +
> > +	if (dig_port->tc_mode == TC_PORT_LEGACY)
> > +		return true;
> > +
> > +	live_status_mask = tc_port_live_status_mask(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_mode == TC_PORT_DP_ALT &&
> > -	    !(I915_READ(PORT_TX_DFLEXDPSP) &
> > TC_LIVE_STATE_TC(tc_port))) {
> > +	if (!(live_status_mask & BIT(TC_PORT_DP_ALT))) {
> >  		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
> >  			      dig_port->tc_port_name);
> >  		icl_tc_phy_disconnect(dig_port);
> > @@ -123,46 +195,35 @@ static bool icl_tc_phy_connect(struct
> > intel_digital_port *dig_port)
> >   */
> >  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);
> > -
> > -	/*
> > -	 * TBT disconnection flow is read the live status, what was
> > done in
> > -	 * caller.
> > -	 */
> > -	if (dig_port->tc_mode == TC_PORT_DP_ALT ||
> > -	    dig_port->tc_mode == TC_PORT_LEGACY) {
> > -		u32 val;
> > -
> > -		val = I915_READ(PORT_TX_DFLEXDPCSSS);
> > -		val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > -		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> > +	switch (dig_port->tc_mode) {
> > +	case TC_PORT_LEGACY:
> > +	case TC_PORT_DP_ALT:
> > +		icl_tc_phy_set_safe_mode(dig_port, true);
> > +		dig_port->tc_mode = TC_PORT_TBT_ALT;
> > +		break;
> > +	case TC_PORT_TBT_ALT:
> > +		/* Nothing to do, we stay in TBT-alt mode */
> > +		break;
> > +	default:
> > +		MISSING_CASE(dig_port->tc_mode);
> >  	}
> >  
> >  	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
> >  		      dig_port->tc_port_name,
> >  		      tc_port_mode_name(dig_port->tc_mode));
> > -
> > -	dig_port->tc_mode = TC_PORT_TBT_ALT;
> >  }
> >  
> >  static void icl_update_tc_port_type(struct drm_i915_private
> > *dev_priv,
> >  				    struct intel_digital_port
> > *intel_dig_port,
> > -				    bool is_legacy, bool is_typec, bool
> > is_tbt)
> > +				    u32 live_status_mask)
> >  {
> >  	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_mode = TC_PORT_LEGACY;
> > -	else if (is_typec)
> > -		intel_dig_port->tc_mode = TC_PORT_DP_ALT;
> > -	else if (is_tbt)
> > -		intel_dig_port->tc_mode = TC_PORT_TBT_ALT;
> > -	else
> > +	if (!live_status_mask)
> >  		return;
> >  
> > +	intel_dig_port->tc_mode = fls(live_status_mask) - 1;
> > +
> >  	if (old_mode != intel_dig_port->tc_mode)
> >  		DRM_DEBUG_KMS("Port %s: port has mode %s\n",
> >  			      intel_dig_port->tc_port_name,
> > @@ -182,40 +243,19 @@ static void icl_update_tc_port_type(struct
> > drm_i915_private *dev_priv,
> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> > -	enum port port = dig_port->base.port;
> > -	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > -	bool is_legacy, is_typec, is_tbt;
> > -	u32 dpsp;
> > -
> > -	/*
> > -	 * Complain if we got a legacy port HPD, but VBT didn't mark
> > the port as
> > -	 * legacy. Treat the port as legacy from now on.
> > -	 */
> > -	if (!dig_port->tc_legacy_port &&
> > -	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
> > -		DRM_ERROR("Port %s: VBT incorrectly claims port is not
> > TypeC legacy\n",
> > -			  dig_port->tc_port_name);
> > -		dig_port->tc_legacy_port = true;
> > -	}
> > -	is_legacy = dig_port->tc_legacy_port;
> > +	u32 live_status_mask = tc_port_live_status_mask(dig_port);
> >  
> >  	/*
> >  	 * 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);
> > -	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> > -	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> > -
> > -	if (!is_legacy && !is_typec && !is_tbt) {
> > +	if (!live_status_mask && !dig_port->tc_legacy_port) {
> >  		icl_tc_phy_disconnect(dig_port);
> >  
> >  		return false;
> >  	}
> >  
> > -	icl_update_tc_port_type(dev_priv, dig_port, is_legacy,
> > is_typec,
> > -				is_tbt);
> > -
> > +	icl_update_tc_port_type(dev_priv, dig_port, live_status_mask);
> >  	if (!icl_tc_phy_connect(dig_port))
> >  		return false;
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> > b/drivers/gpu/drm/i915/display/intel_tc.h
> > index ca1735303252..8c338c45796d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -13,6 +13,7 @@ struct intel_digital_port;
> >  void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
> >  
> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> > +u32 intel_tc_port_get_lane_mask(struct intel_digital_port
> > *dig_port);
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port);
> >  
> >  void intel_tc_port_init(struct intel_digital_port *dig_port, bool
> > is_legacy);