[13/23] drm/i915: Fix the TypeC port mode sanitization during loading/resume

Submitted by Imre Deak on June 4, 2019, 2:58 p.m.

Details

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

Not browsing as part of any series.

Commit Message

Imre Deak June 4, 2019, 2:58 p.m.
For using the correct AUX power domains we have to sanitize the TypeC
port mode early, so move that before encoder sanitization. To do this
properly read out the actual port mode instead of just relying on the
VBT legacy port flag (which can be incorrect).

We also verify that the PHY is connected as expected if the port is
active. In case the port is inactive we connect the PHY in case of a
legacy port - as we did so far. The PHY will be connected during
detection for DP-alt mode - as it was done so far. For TBT-alt mode
nothing needs to be done to connect the PHY.

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: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 16 +-----
 drivers/gpu/drm/i915/intel_display.c | 10 ++++
 drivers/gpu/drm/i915/intel_dp_mst.h  |  8 ++-
 drivers/gpu/drm/i915/intel_tc.c      | 83 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_tc.h      |  2 +
 5 files changed, 103 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d236839bee19..2bc3b4f2c9a5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3918,17 +3918,6 @@  static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
 		icl_tc_phy_disconnect(dig_port);
 }
 
-static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder)
-{
-	struct intel_digital_port *dig_port = enc_to_dig_port(drm_encoder);
-	struct drm_i915_private *i915 = to_i915(drm_encoder->dev);
-
-	if (intel_port_is_tc(i915, dig_port->base.port))
-		intel_digital_port_connected(&dig_port->base);
-
-	intel_dp_encoder_reset(drm_encoder);
-}
-
 static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
@@ -3944,7 +3933,7 @@  static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
 }
 
 static const struct drm_encoder_funcs intel_ddi_funcs = {
-	.reset = intel_ddi_encoder_reset,
+	.reset = intel_dp_encoder_reset,
 	.destroy = intel_ddi_encoder_destroy,
 };
 
@@ -4309,9 +4298,6 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 
 	intel_infoframe_init(intel_dig_port);
 
-	if (intel_port_is_tc(dev_priv, port))
-		intel_digital_port_connected(intel_encoder);
-
 	return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0e425a6eebf3..91d18cd0371c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -75,6 +75,7 @@ 
 #include "intel_sdvo.h"
 #include "intel_sideband.h"
 #include "intel_sprite.h"
+#include "intel_tc.h"
 #include "intel_tv.h"
 #include "intel_vdsc.h"
 
@@ -16683,6 +16684,15 @@  intel_modeset_setup_hw_state(struct drm_device *dev,
 	intel_modeset_readout_hw_state(dev);
 
 	/* HW state is read out, now we need to sanitize this mess. */
+
+	/* Sanitize the TypeC port mode upfront, encoders depend on this */
+	for_each_intel_encoder(dev, encoder) {
+		/* We need to sanitize only the MST primary port. */
+		if (encoder->type != INTEL_OUTPUT_DP_MST &&
+		    intel_port_is_tc(dev_priv, encoder->port))
+			intel_tc_port_sanitize(enc_to_dig_port(&encoder->base));
+	}
+
 	get_encoder_power_domains(dev_priv);
 
 	if (HAS_PCH_IBX(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.h b/drivers/gpu/drm/i915/intel_dp_mst.h
index 1470c6e0514b..6754c211205a 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/intel_dp_mst.h
@@ -6,9 +6,15 @@ 
 #ifndef __INTEL_DP_MST_H__
 #define __INTEL_DP_MST_H__
 
-struct intel_digital_port;
+#include "intel_drv.h"
 
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
 void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
+static inline int
+intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port)
+{
+	return intel_dig_port->dp.active_mst_links;
+}
+
 
 #endif /* __INTEL_DP_MST_H__ */
diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c
index 9832e2ddb92e..9ebf25d7931c 100644
--- a/drivers/gpu/drm/i915/intel_tc.c
+++ b/drivers/gpu/drm/i915/intel_tc.c
@@ -3,6 +3,7 @@ 
  * Copyright © 2019 Intel Corporation
  */
 #include "intel_display.h"
+#include "intel_dp_mst.h"
 #include "i915_drv.h"
 #include "intel_tc.h"
 
@@ -167,6 +168,15 @@  static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
 	return true;
 }
 
+static bool icl_tc_phy_is_in_safe_mode(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_DFLEXDPCSSS) &
+		 DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
+}
+
 /*
  * This function implements the first part of the Connect Flow described by our
  * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading
@@ -250,6 +260,49 @@  void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
 	}
 }
 
+static bool icl_tc_phy_is_connected(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 (!icl_tc_phy_status_complete(dig_port)) {
+		DRM_DEBUG_KMS("Port %s: PHY status not complete\n",
+			      tc_port_name(dev_priv, tc_port));
+		return dig_port->tc_mode == TC_PORT_TBT_ALT;
+	}
+
+	if (icl_tc_phy_is_in_safe_mode(dig_port)) {
+		DRM_DEBUG_KMS("Port %s: PHY still in safe mode\n",
+			      tc_port_name(dev_priv, tc_port));
+
+		return false;
+	}
+
+	return dig_port->tc_mode == TC_PORT_DP_ALT ||
+	       dig_port->tc_mode == TC_PORT_LEGACY;
+}
+
+static enum tc_port_mode
+intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
+{
+	u32 live_status_mask = tc_port_live_status_mask(dig_port);
+	bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
+	enum tc_port_mode mode;
+
+	if (in_safe_mode || WARN_ON(!icl_tc_phy_status_complete(dig_port)))
+		return TC_PORT_TBT_ALT;
+
+	mode = dig_port->tc_legacy_port ? TC_PORT_LEGACY : TC_PORT_DP_ALT;
+	if (live_status_mask) {
+		enum tc_port_mode live_mode = fls(live_status_mask) - 1;
+
+		if (!WARN_ON(live_mode == TC_PORT_TBT_ALT))
+			mode = live_mode;
+	}
+
+	return mode;
+}
+
 static enum tc_port_mode
 intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)
 {
@@ -278,6 +331,36 @@  static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
 		      tc_port_mode_name(dig_port->tc_mode));
 }
 
+void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	struct intel_encoder *encoder = &dig_port->base;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
+	int active_links = 0;
+
+	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
+	if (dig_port->dp.is_mst)
+		active_links = intel_dp_mst_encoder_active_links(dig_port);
+	else if (encoder->base.crtc)
+		active_links = to_intel_crtc(encoder->base.crtc)->active;
+
+	if (active_links) {
+		if (!icl_tc_phy_is_connected(dig_port))
+			DRM_DEBUG_DRIVER("Port %s: PHY disconnected with %d active link(s)\n",
+					 tc_port_name(dev_priv, tc_port),
+					 active_links);
+		goto out;
+	}
+
+	if (dig_port->tc_legacy_port)
+		icl_tc_phy_connect(dig_port);
+
+out:
+	DRM_DEBUG_DRIVER("Port %s: sanitize mode (%s)\n",
+			 tc_port_name(dev_priv, tc_port),
+			 tc_port_mode_name(dig_port->tc_mode));
+}
+
 static bool
 intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
 {
diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h
index e937f5326959..10f0f0d81ee4 100644
--- a/drivers/gpu/drm/i915/intel_tc.h
+++ b/drivers/gpu/drm/i915/intel_tc.h
@@ -11,4 +11,6 @@  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
 u32 intel_tc_port_get_lane_info(struct intel_digital_port *dig_port);
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
 
+void intel_tc_port_sanitize(struct intel_digital_port *dig_port);
+
 #endif /* __INTEL_TC_H__ */

Comments

On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> For using the correct AUX power domains we have to sanitize the TypeC

> port mode early, so move that before encoder sanitization. To do this

> properly read out the actual port mode instead of just relying on the

> VBT legacy port flag (which can be incorrect).

> 

> We also verify that the PHY is connected as expected if the port is

> active. In case the port is inactive we connect the PHY in case of a

> legacy port - as we did so far. The PHY will be connected during

> detection for DP-alt mode - as it was done so far. For TBT-alt mode

> nothing needs to be done to connect the PHY.

> 

> 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: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---

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

>  drivers/gpu/drm/i915/intel_display.c | 10 ++++

>  drivers/gpu/drm/i915/intel_dp_mst.h  |  8 ++-

>  drivers/gpu/drm/i915/intel_tc.c      | 83

> ++++++++++++++++++++++++++++

>  drivers/gpu/drm/i915/intel_tc.h      |  2 +

>  5 files changed, 103 insertions(+), 16 deletions(-)

> 

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

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

> index d236839bee19..2bc3b4f2c9a5 100644

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

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

> @@ -3918,17 +3918,6 @@ static void intel_ddi_encoder_suspend(struct

> intel_encoder *encoder)

>  		icl_tc_phy_disconnect(dig_port);

>  }

>  

> -static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder)

> -{

> -	struct intel_digital_port *dig_port =

> enc_to_dig_port(drm_encoder);

> -	struct drm_i915_private *i915 = to_i915(drm_encoder->dev);

> -

> -	if (intel_port_is_tc(i915, dig_port->base.port))

> -		intel_digital_port_connected(&dig_port->base);

> -

> -	intel_dp_encoder_reset(drm_encoder);

> -}

> -

>  static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)

>  {

>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);

> @@ -3944,7 +3933,7 @@ static void intel_ddi_encoder_destroy(struct

> drm_encoder *encoder)

>  }

>  

>  static const struct drm_encoder_funcs intel_ddi_funcs = {

> -	.reset = intel_ddi_encoder_reset,

> +	.reset = intel_dp_encoder_reset,

>  	.destroy = intel_ddi_encoder_destroy,

>  };

>  

> @@ -4309,9 +4298,6 @@ void intel_ddi_init(struct drm_i915_private

> *dev_priv, enum port port)

>  

>  	intel_infoframe_init(intel_dig_port);

>  

> -	if (intel_port_is_tc(dev_priv, port))

> -		intel_digital_port_connected(intel_encoder);

> -

>  	return;

>  

>  err:

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

> b/drivers/gpu/drm/i915/intel_display.c

> index 0e425a6eebf3..91d18cd0371c 100644

> --- a/drivers/gpu/drm/i915/intel_display.c

> +++ b/drivers/gpu/drm/i915/intel_display.c

> @@ -75,6 +75,7 @@

>  #include "intel_sdvo.h"

>  #include "intel_sideband.h"

>  #include "intel_sprite.h"

> +#include "intel_tc.h"

>  #include "intel_tv.h"

>  #include "intel_vdsc.h"

>  

> @@ -16683,6 +16684,15 @@ intel_modeset_setup_hw_state(struct

> drm_device *dev,

>  	intel_modeset_readout_hw_state(dev);

>  

>  	/* HW state is read out, now we need to sanitize this mess. */

> +

> +	/* Sanitize the TypeC port mode upfront, encoders depend on

> this */

> +	for_each_intel_encoder(dev, encoder) {

> +		/* We need to sanitize only the MST primary port. */

> +		if (encoder->type != INTEL_OUTPUT_DP_MST &&

> +		    intel_port_is_tc(dev_priv, encoder->port))

> +			intel_tc_port_sanitize(enc_to_dig_port(&encoder

> ->base));

> +	}

> +

>  	get_encoder_power_domains(dev_priv);

>  

>  	if (HAS_PCH_IBX(dev_priv))

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

> b/drivers/gpu/drm/i915/intel_dp_mst.h

> index 1470c6e0514b..6754c211205a 100644

> --- a/drivers/gpu/drm/i915/intel_dp_mst.h

> +++ b/drivers/gpu/drm/i915/intel_dp_mst.h

> @@ -6,9 +6,15 @@

>  #ifndef __INTEL_DP_MST_H__

>  #define __INTEL_DP_MST_H__

>  

> -struct intel_digital_port;

> +#include "intel_drv.h"

>  

>  int intel_dp_mst_encoder_init(struct intel_digital_port

> *intel_dig_port, int conn_id);

>  void intel_dp_mst_encoder_cleanup(struct intel_digital_port

> *intel_dig_port);

> +static inline int

> +intel_dp_mst_encoder_active_links(struct intel_digital_port

> *intel_dig_port)

> +{

> +	return intel_dig_port->dp.active_mst_links;

> +}

> +

>  

>  #endif /* __INTEL_DP_MST_H__ */

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

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

> index 9832e2ddb92e..9ebf25d7931c 100644

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

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

> @@ -3,6 +3,7 @@

>   * Copyright © 2019 Intel Corporation

>   */

>  #include "intel_display.h"

> +#include "intel_dp_mst.h"

>  #include "i915_drv.h"

>  #include "intel_tc.h"

>  

> @@ -167,6 +168,15 @@ static bool icl_tc_phy_set_safe_mode(struct

> intel_digital_port *dig_port,

>  	return true;

>  }

>  

> +static bool icl_tc_phy_is_in_safe_mode(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_DFLEXDPCSSS) &

> +		 DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));

> +}

> +

>  /*

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

> described by our

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

> flow (reading

> @@ -250,6 +260,49 @@ void icl_tc_phy_disconnect(struct

> intel_digital_port *dig_port)

>  	}

>  }

>  

> +static bool icl_tc_phy_is_connected(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 (!icl_tc_phy_status_complete(dig_port)) {

> +		DRM_DEBUG_KMS("Port %s: PHY status not complete\n",

> +			      tc_port_name(dev_priv, tc_port));

> +		return dig_port->tc_mode == TC_PORT_TBT_ALT;

> +	}

> +

> +	if (icl_tc_phy_is_in_safe_mode(dig_port)) {

> +		DRM_DEBUG_KMS("Port %s: PHY still in safe mode\n",

> +			      tc_port_name(dev_priv, tc_port));

> +

> +		return false;

> +	}

> +

> +	return dig_port->tc_mode == TC_PORT_DP_ALT ||

> +	       dig_port->tc_mode == TC_PORT_LEGACY;

> +}

> +

> +static enum tc_port_mode

> +intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)

> +{

> +	u32 live_status_mask = tc_port_live_status_mask(dig_port);

> +	bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);

> +	enum tc_port_mode mode;

> +

> +	if (in_safe_mode ||

> WARN_ON(!icl_tc_phy_status_complete(dig_port)))

> +		return TC_PORT_TBT_ALT;

> +

> +	mode = dig_port->tc_legacy_port ? TC_PORT_LEGACY :

> TC_PORT_DP_ALT;

> +	if (live_status_mask) {

> +		enum tc_port_mode live_mode = fls(live_status_mask) -

> 1;

> +

> +		if (!WARN_ON(live_mode == TC_PORT_TBT_ALT))

> +			mode = live_mode;

> +	}

> +

> +	return mode;

> +}

> +

>  static enum tc_port_mode

>  intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)

>  {

> @@ -278,6 +331,36 @@ static void intel_tc_port_reset_mode(struct

> intel_digital_port *dig_port)

>  		      tc_port_mode_name(dig_port->tc_mode));

>  }

>  

> +void intel_tc_port_sanitize(struct intel_digital_port *dig_port)

> +{

> +	struct drm_i915_private *dev_priv = to_i915(dig_port-

> >base.base.dev);

> +	struct intel_encoder *encoder = &dig_port->base;

> +	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder-

> >port);

> +	int active_links = 0;

> +

> +	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);

> +	if (dig_port->dp.is_mst)

> +		active_links =

> intel_dp_mst_encoder_active_links(dig_port);

> +	else if (encoder->base.crtc)

> +		active_links = to_intel_crtc(encoder->base.crtc)-

> >active;

> +

> +	if (active_links) {

> +		if (!icl_tc_phy_is_connected(dig_port))

> +			DRM_DEBUG_DRIVER("Port %s: PHY disconnected

> with %d active link(s)\n",

> +					 tc_port_name(dev_priv,

> tc_port),

> +					 active_links);

> +		goto out;


And then probe sequences will take care of call
intel_digital_port_connected() to reset TC state. 

> +	}

> +

> +	if (dig_port->tc_legacy_port)

> +		icl_tc_phy_connect(dig_port);

> +

> +out:

> +	DRM_DEBUG_DRIVER("Port %s: sanitize mode (%s)\n",

> +			 tc_port_name(dev_priv, tc_port),

> +			 tc_port_mode_name(dig_port->tc_mode));


Should be:

DRM_DEBUG_DRIVER()

I guess I missed this on previous patches too.


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



> +}

> +

>  static bool

>  intel_tc_port_needs_reset(struct intel_digital_port *dig_port)

>  {

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

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

> index e937f5326959..10f0f0d81ee4 100644

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

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

> @@ -11,4 +11,6 @@ bool intel_tc_port_connected(struct

> intel_digital_port *dig_port);

>  u32 intel_tc_port_get_lane_info(struct intel_digital_port

> *dig_port);

>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port

> *dig_port);

>  

> +void intel_tc_port_sanitize(struct intel_digital_port *dig_port);

> +

>  #endif /* __INTEL_TC_H__ */
On Sat, Jun 08, 2019 at 01:39:36AM +0300, Souza, Jose wrote:
> On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > For using the correct AUX power domains we have to sanitize the TypeC
> > port mode early, so move that before encoder sanitization. To do this
> > properly read out the actual port mode instead of just relying on the
> > VBT legacy port flag (which can be incorrect).
> > 
> > We also verify that the PHY is connected as expected if the port is
> > active. In case the port is inactive we connect the PHY in case of a
> > legacy port - as we did so far. The PHY will be connected during
> > detection for DP-alt mode - as it was done so far. For TBT-alt mode
> > nothing needs to be done to connect the PHY.
> > 
> > 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: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c     | 16 +-----
> >  drivers/gpu/drm/i915/intel_display.c | 10 ++++
> >  drivers/gpu/drm/i915/intel_dp_mst.h  |  8 ++-
> >  drivers/gpu/drm/i915/intel_tc.c      | 83
> > ++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_tc.h      |  2 +
> >  5 files changed, 103 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index d236839bee19..2bc3b4f2c9a5 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3918,17 +3918,6 @@ static void intel_ddi_encoder_suspend(struct
> > intel_encoder *encoder)
> >  		icl_tc_phy_disconnect(dig_port);
> >  }
> >  
> > -static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder)
> > -{
> > -	struct intel_digital_port *dig_port =
> > enc_to_dig_port(drm_encoder);
> > -	struct drm_i915_private *i915 = to_i915(drm_encoder->dev);
> > -
> > -	if (intel_port_is_tc(i915, dig_port->base.port))
> > -		intel_digital_port_connected(&dig_port->base);
> > -
> > -	intel_dp_encoder_reset(drm_encoder);
> > -}
> > -
> >  static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
> >  {
> >  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > @@ -3944,7 +3933,7 @@ static void intel_ddi_encoder_destroy(struct
> > drm_encoder *encoder)
> >  }
> >  
> >  static const struct drm_encoder_funcs intel_ddi_funcs = {
> > -	.reset = intel_ddi_encoder_reset,
> > +	.reset = intel_dp_encoder_reset,
> >  	.destroy = intel_ddi_encoder_destroy,
> >  };
> >  
> > @@ -4309,9 +4298,6 @@ void intel_ddi_init(struct drm_i915_private
> > *dev_priv, enum port port)
> >  
> >  	intel_infoframe_init(intel_dig_port);
> >  
> > -	if (intel_port_is_tc(dev_priv, port))
> > -		intel_digital_port_connected(intel_encoder);
> > -
> >  	return;
> >  
> >  err:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 0e425a6eebf3..91d18cd0371c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -75,6 +75,7 @@
> >  #include "intel_sdvo.h"
> >  #include "intel_sideband.h"
> >  #include "intel_sprite.h"
> > +#include "intel_tc.h"
> >  #include "intel_tv.h"
> >  #include "intel_vdsc.h"
> >  
> > @@ -16683,6 +16684,15 @@ intel_modeset_setup_hw_state(struct
> > drm_device *dev,
> >  	intel_modeset_readout_hw_state(dev);
> >  
> >  	/* HW state is read out, now we need to sanitize this mess. */
> > +
> > +	/* Sanitize the TypeC port mode upfront, encoders depend on
> > this */
> > +	for_each_intel_encoder(dev, encoder) {
> > +		/* We need to sanitize only the MST primary port. */
> > +		if (encoder->type != INTEL_OUTPUT_DP_MST &&
> > +		    intel_port_is_tc(dev_priv, encoder->port))
> > +			intel_tc_port_sanitize(enc_to_dig_port(&encoder
> > ->base));
> > +	}
> > +
> >  	get_encoder_power_domains(dev_priv);
> >  
> >  	if (HAS_PCH_IBX(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.h
> > b/drivers/gpu/drm/i915/intel_dp_mst.h
> > index 1470c6e0514b..6754c211205a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.h
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.h
> > @@ -6,9 +6,15 @@
> >  #ifndef __INTEL_DP_MST_H__
> >  #define __INTEL_DP_MST_H__
> >  
> > -struct intel_digital_port;
> > +#include "intel_drv.h"
> >  
> >  int intel_dp_mst_encoder_init(struct intel_digital_port
> > *intel_dig_port, int conn_id);
> >  void intel_dp_mst_encoder_cleanup(struct intel_digital_port
> > *intel_dig_port);
> > +static inline int
> > +intel_dp_mst_encoder_active_links(struct intel_digital_port
> > *intel_dig_port)
> > +{
> > +	return intel_dig_port->dp.active_mst_links;
> > +}
> > +
> >  
> >  #endif /* __INTEL_DP_MST_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_tc.c
> > b/drivers/gpu/drm/i915/intel_tc.c
> > index 9832e2ddb92e..9ebf25d7931c 100644
> > --- a/drivers/gpu/drm/i915/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/intel_tc.c
> > @@ -3,6 +3,7 @@
> >   * Copyright © 2019 Intel Corporation
> >   */
> >  #include "intel_display.h"
> > +#include "intel_dp_mst.h"
> >  #include "i915_drv.h"
> >  #include "intel_tc.h"
> >  
> > @@ -167,6 +168,15 @@ static bool icl_tc_phy_set_safe_mode(struct
> > intel_digital_port *dig_port,
> >  	return true;
> >  }
> >  
> > +static bool icl_tc_phy_is_in_safe_mode(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_DFLEXDPCSSS) &
> > +		 DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
> > +}
> > +
> >  /*
> >   * This function implements the first part of the Connect Flow
> > described by our
> >   * specification, Gen11 TypeC Programming chapter. The rest of the
> > flow (reading
> > @@ -250,6 +260,49 @@ void icl_tc_phy_disconnect(struct
> > intel_digital_port *dig_port)
> >  	}
> >  }
> >  
> > +static bool icl_tc_phy_is_connected(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 (!icl_tc_phy_status_complete(dig_port)) {
> > +		DRM_DEBUG_KMS("Port %s: PHY status not complete\n",
> > +			      tc_port_name(dev_priv, tc_port));
> > +		return dig_port->tc_mode == TC_PORT_TBT_ALT;
> > +	}
> > +
> > +	if (icl_tc_phy_is_in_safe_mode(dig_port)) {
> > +		DRM_DEBUG_KMS("Port %s: PHY still in safe mode\n",
> > +			      tc_port_name(dev_priv, tc_port));
> > +
> > +		return false;
> > +	}
> > +
> > +	return dig_port->tc_mode == TC_PORT_DP_ALT ||
> > +	       dig_port->tc_mode == TC_PORT_LEGACY;
> > +}
> > +
> > +static enum tc_port_mode
> > +intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
> > +{
> > +	u32 live_status_mask = tc_port_live_status_mask(dig_port);
> > +	bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
> > +	enum tc_port_mode mode;
> > +
> > +	if (in_safe_mode ||
> > WARN_ON(!icl_tc_phy_status_complete(dig_port)))
> > +		return TC_PORT_TBT_ALT;
> > +
> > +	mode = dig_port->tc_legacy_port ? TC_PORT_LEGACY :
> > TC_PORT_DP_ALT;
> > +	if (live_status_mask) {
> > +		enum tc_port_mode live_mode = fls(live_status_mask) -
> > 1;
> > +
> > +		if (!WARN_ON(live_mode == TC_PORT_TBT_ALT))
> > +			mode = live_mode;
> > +	}
> > +
> > +	return mode;
> > +}
> > +
> >  static enum tc_port_mode
> >  intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)
> >  {
> > @@ -278,6 +331,36 @@ static void intel_tc_port_reset_mode(struct
> > intel_digital_port *dig_port)
> >  		      tc_port_mode_name(dig_port->tc_mode));
> >  }
> >  
> > +void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> > +	struct intel_encoder *encoder = &dig_port->base;
> > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder-
> > >port);
> > +	int active_links = 0;
> > +
> > +	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
> > +	if (dig_port->dp.is_mst)
> > +		active_links =
> > intel_dp_mst_encoder_active_links(dig_port);
> > +	else if (encoder->base.crtc)
> > +		active_links = to_intel_crtc(encoder->base.crtc)-
> > >active;
> > +
> > +	if (active_links) {
> > +		if (!icl_tc_phy_is_connected(dig_port))
> > +			DRM_DEBUG_DRIVER("Port %s: PHY disconnected
> > with %d active link(s)\n",
> > +					 tc_port_name(dev_priv,
> > tc_port),
> > +					 active_links);
> > +		goto out;
> 
> And then probe sequences will take care of call
> intel_digital_port_connected() to reset TC state. 

Right, once the active mode (we detected here) gets disabled, since
before that you can't reset the mode. Btw, we'll also check if a mode
reset is needed before AUX transfers (if the port is not active).

> 
> > +	}
> > +
> > +	if (dig_port->tc_legacy_port)
> > +		icl_tc_phy_connect(dig_port);
> > +
> > +out:
> > +	DRM_DEBUG_DRIVER("Port %s: sanitize mode (%s)\n",
> > +			 tc_port_name(dev_priv, tc_port),
> > +			 tc_port_mode_name(dig_port->tc_mode));
> 
> Should be:
> 
> DRM_DEBUG_DRIVER()
> 
> I guess I missed this on previous patches too.

Ah right, DRM_DEBUG_KMS().

> 
> 
> With that:
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> 
> > +}
> > +
> >  static bool
> >  intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_tc.h
> > b/drivers/gpu/drm/i915/intel_tc.h
> > index e937f5326959..10f0f0d81ee4 100644
> > --- a/drivers/gpu/drm/i915/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/intel_tc.h
> > @@ -11,4 +11,6 @@ bool intel_tc_port_connected(struct
> > intel_digital_port *dig_port);
> >  u32 intel_tc_port_get_lane_info(struct intel_digital_port
> > *dig_port);
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port);
> >  
> > +void intel_tc_port_sanitize(struct intel_digital_port *dig_port);
> > +
> >  #endif /* __INTEL_TC_H__ */