diff mbox series

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

Message ID 20190626205010.18597-3-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Imre Deak June 26, 2019, 8:50 p.m. UTC 
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.

v2:
- Use DRM_DEBUG_KMS instead of DRM_DEBUG_DRIVER. (José)
v3:
- Detect TCCOLD any time PORT_TX_DFLEXDPCSSS is read. (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: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com> (v2)
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 16 +---
 drivers/gpu/drm/i915/display/intel_display.c | 10 +++
 drivers/gpu/drm/i915/display/intel_dp_mst.h  |  8 +-
 drivers/gpu/drm/i915/display/intel_tc.c      | 84 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_tc.h      |  2 +
 5 files changed, 104 insertions(+), 16 deletions(-)

Comments

Souza, Jose June 27, 2019, 8:38 p.m. UTC | #1 
On Wed, 2019-06-26 at 23:50 +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.
> 
> v2:
> - Use DRM_DEBUG_KMS instead of DRM_DEBUG_DRIVER. (José)
> v3:
> - Detect TCCOLD any time PORT_TX_DFLEXDPCSSS is read. (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: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com> (v2)

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

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 16 +---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.h  |  8 +-
>  drivers/gpu/drm/i915/display/intel_tc.c      | 84
> ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_tc.h      |  2 +
>  5 files changed, 104 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 2be7cdc319ba..0c5bfbd66b19 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3931,17 +3931,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);
> @@ -3957,7 +3946,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,
>  };
>  
> @@ -4328,9 +4317,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/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e47df8a8401e..dee9b89eb3ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -78,6 +78,7 @@
>  #include "intel_quirks.h"
>  #include "intel_sideband.h"
>  #include "intel_sprite.h"
> +#include "intel_tc.h"
>  
>  /* Primary plane formats for gen <= 3 */
>  static const u32 i8xx_primary_formats[] = {
> @@ -16844,6 +16845,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/display/intel_dp_mst.h
> b/drivers/gpu/drm/i915/display/intel_dp_mst.h
> index 1470c6e0514b..6754c211205a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
> +++ b/drivers/gpu/drm/i915/display/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/display/intel_tc.c
> b/drivers/gpu/drm/i915/display/intel_tc.c
> index f63ddf39b369..78340115b994 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include "intel_display.h"
> +#include "intel_dp_mst.h"
>  #include "i915_drv.h"
>  #include "intel_tc.h"
>  
> @@ -160,6 +161,22 @@ 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);
> +	u32 val;
> +
> +	val = I915_READ(PORT_TX_DFLEXDPCSSS);
> +	if (val == 0xffffffff) {
> +		DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, assume safe
> mode\n",
> +			      dig_port->tc_port_name);
> +		return true;
> +	}
> +
> +	return !(val & 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
> @@ -240,6 +257,46 @@ void icl_tc_phy_disconnect(struct
> intel_digital_port *dig_port)
>  	}
>  }
>  
> +static bool icl_tc_phy_is_connected(struct intel_digital_port
> *dig_port)
> +{
> +	if (!icl_tc_phy_status_complete(dig_port)) {
> +		DRM_DEBUG_KMS("Port %s: PHY status not complete\n",
> +			      dig_port->tc_port_name);
> +		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",
> +			      dig_port->tc_port_name);
> +
> +		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)
>  {
> @@ -266,6 +323,33 @@ 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 intel_encoder *encoder = &dig_port->base;
> +	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_KMS("Port %s: PHY disconnected with
> %d active link(s)\n",
> +				      dig_port->tc_port_name,
> active_links);
> +		goto out;
> +	}
> +
> +	if (dig_port->tc_legacy_port)
> +		icl_tc_phy_connect(dig_port);
> +
> +out:
> +	DRM_DEBUG_KMS("Port %s: sanitize mode (%s)\n",
> +		      dig_port->tc_port_name,
> +		      tc_port_mode_name(dig_port->tc_mode));
> +}
> +
>  static bool intel_tc_port_needs_reset(struct intel_digital_port
> *dig_port)
>  {
>  	return intel_tc_port_get_target_mode(dig_port) != dig_port-
> >tc_mode;
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> b/drivers/gpu/drm/i915/display/intel_tc.h
> index 8c338c45796d..5a7876a74522 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -16,6 +16,8 @@ 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_sanitize(struct intel_digital_port *dig_port);
> +
>  void intel_tc_port_init(struct intel_digital_port *dig_port, bool
> is_legacy);
>  
>  #endif /* __INTEL_TC_H__ */
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 2be7cdc319ba..0c5bfbd66b19 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3931,17 +3931,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);
@@ -3957,7 +3946,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,
 };
 
@@ -4328,9 +4317,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/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e47df8a8401e..dee9b89eb3ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -78,6 +78,7 @@ 
 #include "intel_quirks.h"
 #include "intel_sideband.h"
 #include "intel_sprite.h"
+#include "intel_tc.h"
 
 /* Primary plane formats for gen <= 3 */
 static const u32 i8xx_primary_formats[] = {
@@ -16844,6 +16845,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/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
index 1470c6e0514b..6754c211205a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/display/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/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index f63ddf39b369..78340115b994 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -4,6 +4,7 @@ 
  */
 
 #include "intel_display.h"
+#include "intel_dp_mst.h"
 #include "i915_drv.h"
 #include "intel_tc.h"
 
@@ -160,6 +161,22 @@  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);
+	u32 val;
+
+	val = I915_READ(PORT_TX_DFLEXDPCSSS);
+	if (val == 0xffffffff) {
+		DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, assume safe mode\n",
+			      dig_port->tc_port_name);
+		return true;
+	}
+
+	return !(val & 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
@@ -240,6 +257,46 @@  void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
 	}
 }
 
+static bool icl_tc_phy_is_connected(struct intel_digital_port *dig_port)
+{
+	if (!icl_tc_phy_status_complete(dig_port)) {
+		DRM_DEBUG_KMS("Port %s: PHY status not complete\n",
+			      dig_port->tc_port_name);
+		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",
+			      dig_port->tc_port_name);
+
+		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)
 {
@@ -266,6 +323,33 @@  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 intel_encoder *encoder = &dig_port->base;
+	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_KMS("Port %s: PHY disconnected with %d active link(s)\n",
+				      dig_port->tc_port_name, active_links);
+		goto out;
+	}
+
+	if (dig_port->tc_legacy_port)
+		icl_tc_phy_connect(dig_port);
+
+out:
+	DRM_DEBUG_KMS("Port %s: sanitize mode (%s)\n",
+		      dig_port->tc_port_name,
+		      tc_port_mode_name(dig_port->tc_mode));
+}
+
 static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
 {
 	return intel_tc_port_get_target_mode(dig_port) != dig_port->tc_mode;
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 8c338c45796d..5a7876a74522 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -16,6 +16,8 @@  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_sanitize(struct intel_digital_port *dig_port);
+
 void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
 
 #endif /* __INTEL_TC_H__ */