[PATCH v2 0/4] drm/i915/icl: Fix TypeC Legacy HPD handling

[PATCH v2 0/4] drm/i915/icl: Fix TypeC Legacy HPD handling

[Imre Deak]

This is v2 of [1]. The changes in v2:
- Use VBT for detecting legacy ports. There was already a flag for this
  in VBT, I just missed it. Thanks for Siva for pointing that out. With
  that we have a detection that works all the time for DP ports as well.
- Remove a redundant special casing of legacy ports during disconnection
  in patch 3.

[1] https://patchwork.freedesktop.org/series/54017/

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

Imre Deak (4):
  drm/i915/icl: Add a debug print for TypeC port disconnection
  drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags
  drm/i915/icl: Fix HPD handling for TypeC legacy ports
  drm/i915/icl: Add fallback detection method for TypeC legacy ports

 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 11 +++++--
 drivers/gpu/drm/i915/intel_ddi.c  | 63 ++++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp.c   | 61 ++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h  |  5 +++-
 5 files changed, 114 insertions(+), 28 deletions(-)

-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/4] drm/i915/icl: Add a debug print for TypeC port disconnection

[Imre Deak]

It's useful to see at which point a TypeC port gets disconnected, so add
a debug print for it.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e94faa0a42eb..082594bb65a7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5066,28 +5066,38 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
 	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
 }
 
+static const char *tc_type_name(enum tc_port_type type)
+{
+	static const char *names[] = {
+		[TC_PORT_UNKNOWN] = "unknown",
+		[TC_PORT_LEGACY] = "legacy",
+		[TC_PORT_TYPEC] = "typec",
+		[TC_PORT_TBT] = "tbt",
+	};
+
+	if (WARN_ON(type >= ARRAY_SIZE(names)))
+		type = TC_PORT_UNKNOWN;
+
+	return names[type];
+}
+
 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)
 {
 	enum port port = intel_dig_port->base.port;
 	enum tc_port_type old_type = intel_dig_port->tc_type;
-	const char *type_str;
 
 	WARN_ON(is_legacy + is_typec + is_tbt != 1);
 
-	if (is_legacy) {
+	if (is_legacy)
 		intel_dig_port->tc_type = TC_PORT_LEGACY;
-		type_str = "legacy";
-	} else if (is_typec) {
+	else if (is_typec)
 		intel_dig_port->tc_type = TC_PORT_TYPEC;
-		type_str = "typec";
-	} else if (is_tbt) {
+	else if (is_tbt)
 		intel_dig_port->tc_type = TC_PORT_TBT;
-		type_str = "tbt";
-	} else {
+	else
 		return;
-	}
 
 	/* Types are not supposed to be changed at runtime. */
 	WARN_ON(old_type != TC_PORT_UNKNOWN &&
@@ -5095,7 +5105,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 
 	if (old_type != intel_dig_port->tc_type)
 		DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
-			      type_str);
+			      tc_type_name(intel_dig_port->tc_type));
 }
 
 static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
@@ -5187,6 +5197,10 @@ static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
 		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
 	}
 
+	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
+		      port_name(dig_port->base.port),
+		      tc_type_name(dig_port->tc_type));
+
 	dig_port->tc_type = TC_PORT_UNKNOWN;
 }
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/4] drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags

[Imre Deak]

This is needed by the next patch to determine if a DDI TypeC port is
physically wired to a legacy DP or legacy HDMI connector or if the port
is wired to a USB-C/Thunderbolt connector.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 11 +++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e70707e79386..fd3cccaac89e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -935,6 +935,8 @@ struct ddi_vbt_port_info {
 	uint8_t supports_hdmi:1;
 	uint8_t supports_dp:1;
 	uint8_t supports_edp:1;
+	uint8_t supports_typec_usb:1;
+	uint8_t supports_tbt:1;
 
 	uint8_t alternate_aux_channel;
 	uint8_t alternate_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6d3e0260d49c..e59a5af45e94 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1386,8 +1386,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 	info->supports_dp = is_dp;
 	info->supports_edp = is_edp;
 
-	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
-		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
+	if (bdb_version >= 195)
+	       info->supports_typec_usb = child->dp_usb_type_c;
+
+	if (bdb_version >= 209)
+		info->supports_tbt = child->tbt;
+
+	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d TCUSB:%d TBT:%d\n",
+		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt,
+		      info->supports_typec_usb, info->supports_tbt);
 
 	if (is_edp && is_dvi)
 		DRM_DEBUG_KMS("Internal DP port %c is TMDS compatible\n",
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/4] drm/i915/icl: Fix HPD handling for TypeC legacy ports

[Imre Deak]

Atm HPD disconnect events on TypeC ports will break things, since we'll
switch the TypeC mode (between legacy and disconnected modes as well as
among USB DP alternate, Thunderbolt alternate and disconnected modes) on
the fly from the HPD disconnect interrupt work while the port may be
still active.

Even if the port happens to be not active during the disconnect we'd
still have a problem during a subsequent modeset or AUX transfer that
could happen regardless of the port's connected state. For instance the
system resume display mode restore code and userspace could perform a
modeset on the port or userspace could start an AUX transfer even if the
port is in disconnected state.

To fix this keep TypeC legacy ports in legacy mode whenever we're not
suspended. This mode is a static configuration as opposed to the
Thunderbolt and USB DP alternate modes between which we can switch
dynamically.

We determine if a TypeC port is legacy (wired to a legacy HDMI or a
legacy DP connector) via the VBT DDI port specific USB-TypeC and
Thunderbolt flags. If both these flags are cleared then the port is
configured for legacy mode.

On such legacy ports we'll run the TypeC PHY connect sequence explicitly
during driver loading and system resume (vs. running the sequence during
HPD processing). The connect will succeed even if the display is not
connected to begin with (or disappears during the suspended state) since
for legacy ports the PORT_TX_DFLEXDPPMS / DP_PHY_MODE_STATUS_COMPLETED
flag is always set (as opposed to the USB DP alternate mode where it
gets set only when a display is connected).

Correspondingly run the TypeC PHY disconnect sequence during system
suspend and driver unloading. For the unloading case I had to split
up intel_dp_encoder_destroy() to be able to have the 1. flush any
pending encoder work, 2. disconnect TC PHY, 3. call DRM core cleanup and
kfree on the encoder object.

For now run the PHY disconnect during suspend only for TypeC legacy
ports. We will need to disconnect even in USB DP alternate mode in the
future, but atm we don't have a way to reconnect the port in this mode
during resume if the display disappears while being suspended. So for
now punt on this case.

Note that we do not disconnect the port during runtime suspend; in
legacy mode there are no shared HW resources (PHY lanes) with other HW
blocks (USB), so no need to release / reacquire these resources as with
USB DP alternate mode. The only reason to disconnect legacy ports during
system suspend is that the PORT_TX_DFLEXDPPMS /
DP_PHY_MODE_STATUS_COMPLETED flag must be rechecked and the port must be
connected again during system resume. We'll also have to turn the check
for this flag into a poll, after figuring out what's the proper timeout
value for it.

v2:
- Remove the redundant special casing of legacy mode when doing a
  disconnect in icl_tc_port_connected(). It's guaranteed already that we
  won't disconnect legacy ports in that function.
- Add a note about the new intel_ddi_encoder_destroy() hook.
- Reword the commit message after switching to the VBT based detection.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108924
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dp.c  | 21 +++++++++-----
 drivers/gpu/drm/i915/intel_drv.h |  5 +++-
 3 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f3e1d6a0b7dd..295d75c50688 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3901,9 +3901,50 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 
 }
 
+static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
+{
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+
+	intel_dp_encoder_suspend(encoder);
+
+	/*
+	 * TODO: disconnect also from USB DP alternate mode once we have a
+	 * way to handle the modeset restore in that mode during resume
+	 * even if the sink has disappeared while being suspended.
+	 */
+	if (dig_port->tc_legacy_port)
+		icl_tc_phy_disconnect(i915, 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);
+	struct drm_i915_private *i915 = to_i915(encoder->dev);
+
+	intel_dp_encoder_flush_work(encoder);
+
+	if (intel_port_is_tc(i915, dig_port->base.port))
+		icl_tc_phy_disconnect(i915, dig_port);
+
+	drm_encoder_cleanup(encoder);
+	kfree(dig_port);
+}
+
 static const struct drm_encoder_funcs intel_ddi_funcs = {
-	.reset = intel_dp_encoder_reset,
-	.destroy = intel_dp_encoder_destroy,
+	.reset = intel_ddi_encoder_reset,
+	.destroy = intel_ddi_encoder_destroy,
 };
 
 static struct intel_connector *
@@ -4147,16 +4188,16 @@ intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
 
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
+	struct ddi_vbt_port_info *port_info =
+		&dev_priv->vbt.ddi_port_info[port];
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
 	struct drm_encoder *encoder;
 	bool init_hdmi, init_dp, init_lspcon = false;
 	enum pipe pipe;
 
-
-	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
-		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
-	init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
+	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
+	init_dp = port_info->supports_dp;
 
 	if (intel_bios_is_lspcon_present(dev_priv, port)) {
 		/*
@@ -4197,7 +4238,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->post_disable = intel_ddi_post_disable;
 	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
 	intel_encoder->get_config = intel_ddi_get_config;
-	intel_encoder->suspend = intel_dp_encoder_suspend;
+	intel_encoder->suspend = intel_ddi_encoder_suspend;
 	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
 	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->power_domain = intel_port_to_power_domain(port);
@@ -4216,6 +4257,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
 	intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
 
+	intel_dig_port->tc_legacy_port = intel_port_is_tc(dev_priv, port) &&
+					 !port_info->supports_typec_usb &&
+					 !port_info->supports_tbt;
+
 	switch (port) {
 	case PORT_A:
 		intel_dig_port->ddi_io_power_domain =
@@ -4274,6 +4319,10 @@ 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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 082594bb65a7..b2a3012478ca 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5108,9 +5108,6 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 			      tc_type_name(intel_dig_port->tc_type));
 }
 
-static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
-				  struct intel_digital_port *dig_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
@@ -5145,6 +5142,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
 	val = I915_READ(PORT_TX_DFLEXDPPMS);
 	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
 		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
+		WARN_ON(dig_port->tc_legacy_port);
 		return false;
 	}
 
@@ -5176,8 +5174,8 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
  * See the comment at the connect function. This implements the Disconnect
  * Flow.
  */
-static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
-				  struct intel_digital_port *dig_port)
+void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
+			   struct intel_digital_port *dig_port)
 {
 	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
 
@@ -5222,7 +5220,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 	bool is_legacy, is_typec, is_tbt;
 	u32 dpsp;
 
-	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
+	is_legacy = intel_dig_port->tc_legacy_port ||
+		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
 
 	/*
 	 * The spec says we shouldn't be using the ISR bits for detecting
@@ -5234,6 +5233,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 
 	if (!is_legacy && !is_typec && !is_tbt) {
 		icl_tc_phy_disconnect(dev_priv, intel_dig_port);
+
 		return false;
 	}
 
@@ -5542,7 +5542,7 @@ intel_dp_connector_unregister(struct drm_connector *connector)
 	intel_connector_unregister(connector);
 }
 
-void intel_dp_encoder_destroy(struct drm_encoder *encoder)
+void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
 {
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
@@ -5565,9 +5565,14 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	}
 
 	intel_dp_aux_fini(intel_dp);
+}
+
+static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
+{
+	intel_dp_encoder_flush_work(encoder);
 
 	drm_encoder_cleanup(encoder);
-	kfree(intel_dig_port);
+	kfree(enc_to_dig_port(encoder));
 }
 
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d08f08f607dd..97c422bea7fb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1234,6 +1234,7 @@ struct intel_digital_port {
 	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
 	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;
 
 	void (*write_infoframe)(struct intel_encoder *encoder,
@@ -1806,7 +1807,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 					   bool enable);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
-void intel_dp_encoder_destroy(struct drm_encoder *encoder);
+void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config,
 			     struct drm_connector_state *conn_state);
@@ -1874,6 +1875,8 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
+void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
+			   struct intel_digital_port *dig_port);
 
 /* intel_dp_aux_backlight.c */
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/4] drm/i915/icl: Add fallback detection method for TypeC legacy ports

[Imre Deak]

Add a fallback detection method for TypeC legacy ports in case the
VBT port information used to detect normally such ports is
incorrect.

For the fallback method we use the TypeC legacy mode specific HPD
interrupt flag which should only be raised for a legacy port.

WARN if the VBT port info is incorrect.

In a case where we'd detect the port in a contradicting way both as a
legacy and also as a USB DP and/or TBT alternate port treat the port
as legacy (by also emitting a WARN from icl_update_tc_port_type).

v2:
- Repurpose the detection as a fallback method instead of using
  it only for the DP legacy case. By now we should normally use VBT to
  detect DP legacy ports as well.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (v1)
---
 drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b2a3012478ca..d00af92cd089 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5220,8 +5220,14 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 	bool is_legacy, is_typec, is_tbt;
 	u32 dpsp;
 
-	is_legacy = intel_dig_port->tc_legacy_port ||
-		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
+	/*
+	 * WARN 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 (WARN_ON(!intel_dig_port->tc_legacy_port &&
+		    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)))
+		intel_dig_port->tc_legacy_port = true;
+	is_legacy = intel_dig_port->tc_legacy_port;
 
 	/*
 	 * The spec says we shouldn't be using the ISR bits for detecting
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: Fix TypeC legacy HPD handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix TypeC legacy HPD handling (rev2)
URL   : https://patchwork.freedesktop.org/series/54017/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
39a8e2bef8b8 drm/i915/icl: Add a debug print for TypeC port disconnection
-:29: WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
#29: FILE: drivers/gpu/drm/i915/intel_dp.c:5071:
+	static const char *names[] = {

total: 0 errors, 1 warnings, 0 checks, 65 lines checked
4038ff7e5289 drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags
-:43: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 15)
#43: FILE: drivers/gpu/drm/i915/intel_bios.c:1389:
+	if (bdb_version >= 195)
+	       info->supports_typec_usb = child->dp_usb_type_c;

total: 0 errors, 1 warnings, 0 checks, 25 lines checked
073e19935f67 drm/i915/icl: Fix HPD handling for TypeC legacy ports
-:270: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield.  Prefer bool bitfields as unsigned int or u<8|16|32>
#270: FILE: drivers/gpu/drm/i915/intel_drv.h:1237:
+	bool tc_legacy_port:1;

total: 0 errors, 1 warnings, 0 checks, 188 lines checked
4b9db3b474f7 drm/i915/icl: Add fallback detection method for TypeC legacy ports

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/i915/icl: Fix TypeC legacy HPD handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix TypeC legacy HPD handling (rev2)
URL   : https://patchwork.freedesktop.org/series/54017/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/icl: Add a debug print for TypeC port disconnection
Okay!

Commit: drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3548:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void)

Commit: drm/i915/icl: Fix HPD handling for TypeC legacy ports
Okay!

Commit: drm/i915/icl: Add fallback detection method for TypeC legacy ports
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/icl: Fix TypeC legacy HPD handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix TypeC legacy HPD handling (rev2)
URL   : https://patchwork.freedesktop.org/series/54017/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5318 -> Patchwork_11100
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11100 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11100, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54017/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11100:

### IGT changes ###

#### Warnings ####

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       SKIP -> PASS

  
Known issues
------------

  Here are the changes found in Patchwork_11100 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          NOTRUN -> DMESG-FAIL [fdo#108569]
    - fi-icl-u2:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-icl-u2:          DMESG-FAIL [fdo#103375] / [fdo#107732] / [fdo#108070] / [fdo#108924] -> PASS

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-icl-u3:          DMESG-WARN [fdo#108924] / [fdo#109044] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       FAIL -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108924]: https://bugs.freedesktop.org/show_bug.cgi?id=108924
  [fdo#109044]: https://bugs.freedesktop.org/show_bug.cgi?id=109044


Participating hosts (53 -> 46)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5318 -> Patchwork_11100

  CI_DRM_5318: 4ef95f23369b5408a57faa222c8c0f2bb8298de8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4747: ad821d1dc5d0eea4ac3a0e8e29c56c7f66191108 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11100: 4b9db3b474f751f0a624206dbf88ea911a54d6ce @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4b9db3b474f7 drm/i915/icl: Add fallback detection method for TypeC legacy ports
073e19935f67 drm/i915/icl: Fix HPD handling for TypeC legacy ports
4038ff7e5289 drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags
39a8e2bef8b8 drm/i915/icl: Add a debug print for TypeC port disconnection

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11100/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/icl: Fix TypeC legacy HPD handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix TypeC legacy HPD handling (rev2)
URL   : https://patchwork.freedesktop.org/series/54017/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5318_full -> Patchwork_11100_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11100_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11100_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11100_full:

### IGT changes ###

#### Warnings ####

  * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing:
    - shard-snb:          SKIP -> PASS +3

  
Known issues
------------

  Here are the changes found in Patchwork_11100_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#107139]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +51

  * igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
    - shard-glk:          PASS -> FAIL [fdo#103184]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-glk:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_rmfb@rmfb-ioctl:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +1

  * igt@pm_rpm@i2c:
    - shard-iclb:         PASS -> FAIL [fdo#104097]

  * igt@pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]

  
#### Possible fixes ####

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-snb:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
    - shard-apl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS
    - shard-skl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +11

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-untiled:
    - shard-iclb:         WARN [fdo#108336] -> PASS +2

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> PASS +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-rgb565-draw-pwrite:
    - shard-skl:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +5

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-WARN [fdo#105604] -> PASS

  * igt@pm_rpm@modeset-lpsp-stress-no-wait:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS

  * igt@pm_rpm@system-suspend-modeset:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#107725]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602]

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103167]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          FAIL [fdo#108145] / [fdo#108590] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145] +1

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5318 -> Patchwork_11100

  CI_DRM_5318: 4ef95f23369b5408a57faa222c8c0f2bb8298de8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4747: ad821d1dc5d0eea4ac3a0e8e29c56c7f66191108 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11100: 4b9db3b474f751f0a624206dbf88ea911a54d6ce @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11100/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/4] drm/i915/icl: Fix HPD handling for TypeC legacy ports

[Rodrigo Vivi]

On Fri, Dec 14, 2018 at 08:27:02PM +0200, Imre Deak wrote:
> Atm HPD disconnect events on TypeC ports will break things, since we'll
> switch the TypeC mode (between legacy and disconnected modes as well as
> among USB DP alternate, Thunderbolt alternate and disconnected modes) on
> the fly from the HPD disconnect interrupt work while the port may be
> still active.
> 
> Even if the port happens to be not active during the disconnect we'd
> still have a problem during a subsequent modeset or AUX transfer that
> could happen regardless of the port's connected state. For instance the
> system resume display mode restore code and userspace could perform a
> modeset on the port or userspace could start an AUX transfer even if the
> port is in disconnected state.
> 
> To fix this keep TypeC legacy ports in legacy mode whenever we're not
> suspended. This mode is a static configuration as opposed to the
> Thunderbolt and USB DP alternate modes between which we can switch
> dynamically.
> 
> We determine if a TypeC port is legacy (wired to a legacy HDMI or a
> legacy DP connector) via the VBT DDI port specific USB-TypeC and
> Thunderbolt flags. If both these flags are cleared then the port is
> configured for legacy mode.
> 
> On such legacy ports we'll run the TypeC PHY connect sequence explicitly
> during driver loading and system resume (vs. running the sequence during
> HPD processing). The connect will succeed even if the display is not
> connected to begin with (or disappears during the suspended state) since
> for legacy ports the PORT_TX_DFLEXDPPMS / DP_PHY_MODE_STATUS_COMPLETED
> flag is always set (as opposed to the USB DP alternate mode where it
> gets set only when a display is connected).
> 
> Correspondingly run the TypeC PHY disconnect sequence during system
> suspend and driver unloading. For the unloading case I had to split
> up intel_dp_encoder_destroy() to be able to have the 1. flush any
> pending encoder work, 2. disconnect TC PHY, 3. call DRM core cleanup and
> kfree on the encoder object.
> 
> For now run the PHY disconnect during suspend only for TypeC legacy
> ports. We will need to disconnect even in USB DP alternate mode in the
> future, but atm we don't have a way to reconnect the port in this mode
> during resume if the display disappears while being suspended. So for
> now punt on this case.
> 
> Note that we do not disconnect the port during runtime suspend; in
> legacy mode there are no shared HW resources (PHY lanes) with other HW
> blocks (USB), so no need to release / reacquire these resources as with
> USB DP alternate mode. The only reason to disconnect legacy ports during
> system suspend is that the PORT_TX_DFLEXDPPMS /
> DP_PHY_MODE_STATUS_COMPLETED flag must be rechecked and the port must be
> connected again during system resume. We'll also have to turn the check
> for this flag into a poll, after figuring out what's the proper timeout
> value for it.
> 
> v2:
> - Remove the redundant special casing of legacy mode when doing a
>   disconnect in icl_tc_port_connected(). It's guaranteed already that we
>   won't disconnect legacy ports in that function.
> - Add a note about the new intel_ddi_encoder_destroy() hook.
> - Reword the commit message after switching to the VBT based detection.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(just because you asked about what I feel non-symmetric I will write here,
but I do believe your versions is good and optimal and my "symmetric" one
would be just bad OCD.

the asymmetric part I see is:
ddi_reset -> dp_reset
ddi_destroy -> flush
dp_destroy -> flush

a more symmetric way would be
ddi_reset -> dp_reset
ddi_destroy -> dp_destroy

or

ddi_reset -> some_common_reset
dp_reset -> some_common_reset
ddi_destroy -> flush
dp_destroy -> flush
)

> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108924
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_dp.c  | 21 +++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h |  5 +++-
>  3 files changed, 73 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f3e1d6a0b7dd..295d75c50688 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3901,9 +3901,50 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  
>  }
>  
> +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> +	intel_dp_encoder_suspend(encoder);
> +
> +	/*
> +	 * TODO: disconnect also from USB DP alternate mode once we have a
> +	 * way to handle the modeset restore in that mode during resume
> +	 * even if the sink has disappeared while being suspended.
> +	 */
> +	if (dig_port->tc_legacy_port)
> +		icl_tc_phy_disconnect(i915, 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);
> +	struct drm_i915_private *i915 = to_i915(encoder->dev);
> +
> +	intel_dp_encoder_flush_work(encoder);
> +
> +	if (intel_port_is_tc(i915, dig_port->base.port))
> +		icl_tc_phy_disconnect(i915, dig_port);
> +
> +	drm_encoder_cleanup(encoder);
> +	kfree(dig_port);
> +}
> +
>  static const struct drm_encoder_funcs intel_ddi_funcs = {
> -	.reset = intel_dp_encoder_reset,
> -	.destroy = intel_dp_encoder_destroy,
> +	.reset = intel_ddi_encoder_reset,
> +	.destroy = intel_ddi_encoder_destroy,
>  };
>  
>  static struct intel_connector *
> @@ -4147,16 +4188,16 @@ intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
>  
>  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  {
> +	struct ddi_vbt_port_info *port_info =
> +		&dev_priv->vbt.ddi_port_info[port];
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *intel_encoder;
>  	struct drm_encoder *encoder;
>  	bool init_hdmi, init_dp, init_lspcon = false;
>  	enum pipe pipe;
>  
> -
> -	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> -		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> -	init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> +	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
> +	init_dp = port_info->supports_dp;
>  
>  	if (intel_bios_is_lspcon_present(dev_priv, port)) {
>  		/*
> @@ -4197,7 +4238,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->post_disable = intel_ddi_post_disable;
>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>  	intel_encoder->get_config = intel_ddi_get_config;
> -	intel_encoder->suspend = intel_dp_encoder_suspend;
> +	intel_encoder->suspend = intel_ddi_encoder_suspend;
>  	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
>  	intel_encoder->type = INTEL_OUTPUT_DDI;
>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> @@ -4216,6 +4257,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
>  	intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
>  
> +	intel_dig_port->tc_legacy_port = intel_port_is_tc(dev_priv, port) &&
> +					 !port_info->supports_typec_usb &&
> +					 !port_info->supports_tbt;
> +
>  	switch (port) {
>  	case PORT_A:
>  		intel_dig_port->ddi_io_power_domain =
> @@ -4274,6 +4319,10 @@ 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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 082594bb65a7..b2a3012478ca 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5108,9 +5108,6 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>  			      tc_type_name(intel_dig_port->tc_type));
>  }
>  
> -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> -				  struct intel_digital_port *dig_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
> @@ -5145,6 +5142,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
>  	val = I915_READ(PORT_TX_DFLEXDPPMS);
>  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
>  		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
> +		WARN_ON(dig_port->tc_legacy_port);
>  		return false;
>  	}
>  
> @@ -5176,8 +5174,8 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
>   * See the comment at the connect function. This implements the Disconnect
>   * Flow.
>   */
> -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> -				  struct intel_digital_port *dig_port)
> +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> +			   struct intel_digital_port *dig_port)
>  {
>  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>  
> @@ -5222,7 +5220,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
>  	bool is_legacy, is_typec, is_tbt;
>  	u32 dpsp;
>  
> -	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> +	is_legacy = intel_dig_port->tc_legacy_port ||
> +		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
>  
>  	/*
>  	 * The spec says we shouldn't be using the ISR bits for detecting
> @@ -5234,6 +5233,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
>  
>  	if (!is_legacy && !is_typec && !is_tbt) {
>  		icl_tc_phy_disconnect(dev_priv, intel_dig_port);
> +
>  		return false;
>  	}
>  
> @@ -5542,7 +5542,7 @@ intel_dp_connector_unregister(struct drm_connector *connector)
>  	intel_connector_unregister(connector);
>  }
>  
> -void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> +void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
>  {
>  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> @@ -5565,9 +5565,14 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  	}
>  
>  	intel_dp_aux_fini(intel_dp);
> +}
> +
> +static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> +{
> +	intel_dp_encoder_flush_work(encoder);
>  
>  	drm_encoder_cleanup(encoder);
> -	kfree(intel_dig_port);
> +	kfree(enc_to_dig_port(encoder));
>  }
>  
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d08f08f607dd..97c422bea7fb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1234,6 +1234,7 @@ struct intel_digital_port {
>  	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
>  	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;
>  
>  	void (*write_infoframe)(struct intel_encoder *encoder,
> @@ -1806,7 +1807,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
>  					   bool enable);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> -void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> +void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config,
>  			     struct drm_connector_state *conn_state);
> @@ -1874,6 +1875,8 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
>  int intel_dp_link_required(int pixel_clock, int bpp);
>  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
>  bool intel_digital_port_connected(struct intel_encoder *encoder);
> +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> +			   struct intel_digital_port *dig_port);
>  
>  /* intel_dp_aux_backlight.c */
>  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/4] drm/i915/icl: Fix HPD handling for TypeC legacy ports

[Imre Deak]

On Mon, Dec 17, 2018 at 10:05:40AM -0800, Rodrigo Vivi wrote:
> On Fri, Dec 14, 2018 at 08:27:02PM +0200, Imre Deak wrote:
> > Atm HPD disconnect events on TypeC ports will break things, since we'll
> > switch the TypeC mode (between legacy and disconnected modes as well as
> > among USB DP alternate, Thunderbolt alternate and disconnected modes) on
> > the fly from the HPD disconnect interrupt work while the port may be
> > still active.
> > 
> > Even if the port happens to be not active during the disconnect we'd
> > still have a problem during a subsequent modeset or AUX transfer that
> > could happen regardless of the port's connected state. For instance the
> > system resume display mode restore code and userspace could perform a
> > modeset on the port or userspace could start an AUX transfer even if the
> > port is in disconnected state.
> > 
> > To fix this keep TypeC legacy ports in legacy mode whenever we're not
> > suspended. This mode is a static configuration as opposed to the
> > Thunderbolt and USB DP alternate modes between which we can switch
> > dynamically.
> > 
> > We determine if a TypeC port is legacy (wired to a legacy HDMI or a
> > legacy DP connector) via the VBT DDI port specific USB-TypeC and
> > Thunderbolt flags. If both these flags are cleared then the port is
> > configured for legacy mode.
> > 
> > On such legacy ports we'll run the TypeC PHY connect sequence explicitly
> > during driver loading and system resume (vs. running the sequence during
> > HPD processing). The connect will succeed even if the display is not
> > connected to begin with (or disappears during the suspended state) since
> > for legacy ports the PORT_TX_DFLEXDPPMS / DP_PHY_MODE_STATUS_COMPLETED
> > flag is always set (as opposed to the USB DP alternate mode where it
> > gets set only when a display is connected).
> > 
> > Correspondingly run the TypeC PHY disconnect sequence during system
> > suspend and driver unloading. For the unloading case I had to split
> > up intel_dp_encoder_destroy() to be able to have the 1. flush any
> > pending encoder work, 2. disconnect TC PHY, 3. call DRM core cleanup and
> > kfree on the encoder object.
> > 
> > For now run the PHY disconnect during suspend only for TypeC legacy
> > ports. We will need to disconnect even in USB DP alternate mode in the
> > future, but atm we don't have a way to reconnect the port in this mode
> > during resume if the display disappears while being suspended. So for
> > now punt on this case.
> > 
> > Note that we do not disconnect the port during runtime suspend; in
> > legacy mode there are no shared HW resources (PHY lanes) with other HW
> > blocks (USB), so no need to release / reacquire these resources as with
> > USB DP alternate mode. The only reason to disconnect legacy ports during
> > system suspend is that the PORT_TX_DFLEXDPPMS /
> > DP_PHY_MODE_STATUS_COMPLETED flag must be rechecked and the port must be
> > connected again during system resume. We'll also have to turn the check
> > for this flag into a poll, after figuring out what's the proper timeout
> > value for it.
> > 
> > v2:
> > - Remove the redundant special casing of legacy mode when doing a
> >   disconnect in icl_tc_port_connected(). It's guaranteed already that we
> >   won't disconnect legacy ports in that function.
> > - Add a note about the new intel_ddi_encoder_destroy() hook.
> > - Reword the commit message after switching to the VBT based detection.
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> (just because you asked about what I feel non-symmetric I will write here,
> but I do believe your versions is good and optimal and my "symmetric" one
> would be just bad OCD.
> 
> the asymmetric part I see is:
> ddi_reset -> dp_reset
> ddi_destroy -> flush
> dp_destroy -> flush

Ok, I see what you mean now, but not sure if it's really an issue. As I
see the assymetry comes from the fact that intel_dp_encoder_reset() can be
reused by the ddi code as a whole while intel_dp_encoder_destroy() can't be
reused by the ddi code. IOW ..

> a more symmetric way would be
> ddi_reset -> dp_reset
> ddi_destroy -> dp_destroy
> 
> or
> 
> ddi_reset -> some_common_reset
> dp_reset -> some_common_reset

.. dp_reset() is just the same as some_common_reset().

> ddi_destroy -> flush
> dp_destroy -> flush
> )
> 
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108924
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++++++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_dp.c  | 21 +++++++++-----
> >  drivers/gpu/drm/i915/intel_drv.h |  5 +++-
> >  3 files changed, 73 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f3e1d6a0b7dd..295d75c50688 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3901,9 +3901,50 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  
> >  }
> >  
> > +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
> > +{
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +
> > +	intel_dp_encoder_suspend(encoder);
> > +
> > +	/*
> > +	 * TODO: disconnect also from USB DP alternate mode once we have a
> > +	 * way to handle the modeset restore in that mode during resume
> > +	 * even if the sink has disappeared while being suspended.
> > +	 */
> > +	if (dig_port->tc_legacy_port)
> > +		icl_tc_phy_disconnect(i915, 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);
> > +	struct drm_i915_private *i915 = to_i915(encoder->dev);
> > +
> > +	intel_dp_encoder_flush_work(encoder);
> > +
> > +	if (intel_port_is_tc(i915, dig_port->base.port))
> > +		icl_tc_phy_disconnect(i915, dig_port);
> > +
> > +	drm_encoder_cleanup(encoder);
> > +	kfree(dig_port);
> > +}
> > +
> >  static const struct drm_encoder_funcs intel_ddi_funcs = {
> > -	.reset = intel_dp_encoder_reset,
> > -	.destroy = intel_dp_encoder_destroy,
> > +	.reset = intel_ddi_encoder_reset,
> > +	.destroy = intel_ddi_encoder_destroy,
> >  };
> >  
> >  static struct intel_connector *
> > @@ -4147,16 +4188,16 @@ intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
> >  
> >  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  {
> > +	struct ddi_vbt_port_info *port_info =
> > +		&dev_priv->vbt.ddi_port_info[port];
> >  	struct intel_digital_port *intel_dig_port;
> >  	struct intel_encoder *intel_encoder;
> >  	struct drm_encoder *encoder;
> >  	bool init_hdmi, init_dp, init_lspcon = false;
> >  	enum pipe pipe;
> >  
> > -
> > -	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> > -		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> > -	init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> > +	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
> > +	init_dp = port_info->supports_dp;
> >  
> >  	if (intel_bios_is_lspcon_present(dev_priv, port)) {
> >  		/*
> > @@ -4197,7 +4238,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_encoder->post_disable = intel_ddi_post_disable;
> >  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> >  	intel_encoder->get_config = intel_ddi_get_config;
> > -	intel_encoder->suspend = intel_dp_encoder_suspend;
> > +	intel_encoder->suspend = intel_ddi_encoder_suspend;
> >  	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
> >  	intel_encoder->type = INTEL_OUTPUT_DDI;
> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> > @@ -4216,6 +4257,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
> >  	intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
> >  
> > +	intel_dig_port->tc_legacy_port = intel_port_is_tc(dev_priv, port) &&
> > +					 !port_info->supports_typec_usb &&
> > +					 !port_info->supports_tbt;
> > +
> >  	switch (port) {
> >  	case PORT_A:
> >  		intel_dig_port->ddi_io_power_domain =
> > @@ -4274,6 +4319,10 @@ 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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 082594bb65a7..b2a3012478ca 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5108,9 +5108,6 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
> >  			      tc_type_name(intel_dig_port->tc_type));
> >  }
> >  
> > -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > -				  struct intel_digital_port *dig_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
> > @@ -5145,6 +5142,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> >  	val = I915_READ(PORT_TX_DFLEXDPPMS);
> >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> >  		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
> > +		WARN_ON(dig_port->tc_legacy_port);
> >  		return false;
> >  	}
> >  
> > @@ -5176,8 +5174,8 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> >   * See the comment at the connect function. This implements the Disconnect
> >   * Flow.
> >   */
> > -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > -				  struct intel_digital_port *dig_port)
> > +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > +			   struct intel_digital_port *dig_port)
> >  {
> >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> >  
> > @@ -5222,7 +5220,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  	bool is_legacy, is_typec, is_tbt;
> >  	u32 dpsp;
> >  
> > -	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> > +	is_legacy = intel_dig_port->tc_legacy_port ||
> > +		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> >  
> >  	/*
> >  	 * The spec says we shouldn't be using the ISR bits for detecting
> > @@ -5234,6 +5233,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  
> >  	if (!is_legacy && !is_typec && !is_tbt) {
> >  		icl_tc_phy_disconnect(dev_priv, intel_dig_port);
> > +
> >  		return false;
> >  	}
> >  
> > @@ -5542,7 +5542,7 @@ intel_dp_connector_unregister(struct drm_connector *connector)
> >  	intel_connector_unregister(connector);
> >  }
> >  
> > -void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > +void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
> >  {
> >  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> > @@ -5565,9 +5565,14 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> >  	}
> >  
> >  	intel_dp_aux_fini(intel_dp);
> > +}
> > +
> > +static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > +{
> > +	intel_dp_encoder_flush_work(encoder);
> >  
> >  	drm_encoder_cleanup(encoder);
> > -	kfree(intel_dig_port);
> > +	kfree(enc_to_dig_port(encoder));
> >  }
> >  
> >  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index d08f08f607dd..97c422bea7fb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1234,6 +1234,7 @@ struct intel_digital_port {
> >  	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
> >  	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;
> >  
> >  	void (*write_infoframe)(struct intel_encoder *encoder,
> > @@ -1806,7 +1807,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
> >  					   bool enable);
> >  void intel_dp_encoder_reset(struct drm_encoder *encoder);
> >  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> > -void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> > +void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
> >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> >  			     struct intel_crtc_state *pipe_config,
> >  			     struct drm_connector_state *conn_state);
> > @@ -1874,6 +1875,8 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> >  int intel_dp_link_required(int pixel_clock, int bpp);
> >  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> >  bool intel_digital_port_connected(struct intel_encoder *encoder);
> > +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > +			   struct intel_digital_port *dig_port);
> >  
> >  /* intel_dp_aux_backlight.c */
> >  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > -- 
> > 2.13.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/4] drm/i915/icl: Fix HPD handling for TypeC legacy ports

[Rodrigo Vivi]

On Mon, Dec 17, 2018 at 09:35:05PM +0200, Imre Deak wrote:
> On Mon, Dec 17, 2018 at 10:05:40AM -0800, Rodrigo Vivi wrote:
> > On Fri, Dec 14, 2018 at 08:27:02PM +0200, Imre Deak wrote:
> > > Atm HPD disconnect events on TypeC ports will break things, since we'll
> > > switch the TypeC mode (between legacy and disconnected modes as well as
> > > among USB DP alternate, Thunderbolt alternate and disconnected modes) on
> > > the fly from the HPD disconnect interrupt work while the port may be
> > > still active.
> > > 
> > > Even if the port happens to be not active during the disconnect we'd
> > > still have a problem during a subsequent modeset or AUX transfer that
> > > could happen regardless of the port's connected state. For instance the
> > > system resume display mode restore code and userspace could perform a
> > > modeset on the port or userspace could start an AUX transfer even if the
> > > port is in disconnected state.
> > > 
> > > To fix this keep TypeC legacy ports in legacy mode whenever we're not
> > > suspended. This mode is a static configuration as opposed to the
> > > Thunderbolt and USB DP alternate modes between which we can switch
> > > dynamically.
> > > 
> > > We determine if a TypeC port is legacy (wired to a legacy HDMI or a
> > > legacy DP connector) via the VBT DDI port specific USB-TypeC and
> > > Thunderbolt flags. If both these flags are cleared then the port is
> > > configured for legacy mode.
> > > 
> > > On such legacy ports we'll run the TypeC PHY connect sequence explicitly
> > > during driver loading and system resume (vs. running the sequence during
> > > HPD processing). The connect will succeed even if the display is not
> > > connected to begin with (or disappears during the suspended state) since
> > > for legacy ports the PORT_TX_DFLEXDPPMS / DP_PHY_MODE_STATUS_COMPLETED
> > > flag is always set (as opposed to the USB DP alternate mode where it
> > > gets set only when a display is connected).
> > > 
> > > Correspondingly run the TypeC PHY disconnect sequence during system
> > > suspend and driver unloading. For the unloading case I had to split
> > > up intel_dp_encoder_destroy() to be able to have the 1. flush any
> > > pending encoder work, 2. disconnect TC PHY, 3. call DRM core cleanup and
> > > kfree on the encoder object.
> > > 
> > > For now run the PHY disconnect during suspend only for TypeC legacy
> > > ports. We will need to disconnect even in USB DP alternate mode in the
> > > future, but atm we don't have a way to reconnect the port in this mode
> > > during resume if the display disappears while being suspended. So for
> > > now punt on this case.
> > > 
> > > Note that we do not disconnect the port during runtime suspend; in
> > > legacy mode there are no shared HW resources (PHY lanes) with other HW
> > > blocks (USB), so no need to release / reacquire these resources as with
> > > USB DP alternate mode. The only reason to disconnect legacy ports during
> > > system suspend is that the PORT_TX_DFLEXDPPMS /
> > > DP_PHY_MODE_STATUS_COMPLETED flag must be rechecked and the port must be
> > > connected again during system resume. We'll also have to turn the check
> > > for this flag into a poll, after figuring out what's the proper timeout
> > > value for it.
> > > 
> > > v2:
> > > - Remove the redundant special casing of legacy mode when doing a
> > >   disconnect in icl_tc_port_connected(). It's guaranteed already that we
> > >   won't disconnect legacy ports in that function.
> > > - Add a note about the new intel_ddi_encoder_destroy() hook.
> > > - Reword the commit message after switching to the VBT based detection.
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > (just because you asked about what I feel non-symmetric I will write here,
> > but I do believe your versions is good and optimal and my "symmetric" one
> > would be just bad OCD.
> > 
> > the asymmetric part I see is:
> > ddi_reset -> dp_reset
> > ddi_destroy -> flush
> > dp_destroy -> flush
> 
> Ok, I see what you mean now, but not sure if it's really an issue.

I told it is not an issue but just a dummy OCD ;)
I just wrote to clarify, that's all! 

> As I
> see the assymetry comes from the fact that intel_dp_encoder_reset() can be
> reused by the ddi code as a whole while intel_dp_encoder_destroy() can't be
> reused by the ddi code. IOW ..
> 
> > a more symmetric way would be
> > ddi_reset -> dp_reset
> > ddi_destroy -> dp_destroy
> > 
> > or
> > 
> > ddi_reset -> some_common_reset
> > dp_reset -> some_common_reset
> 
> .. dp_reset() is just the same as some_common_reset().
> 
> > ddi_destroy -> flush
> > dp_destroy -> flush
> > )
> > 
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108924
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++++++++++++++++++-----
> > >  drivers/gpu/drm/i915/intel_dp.c  | 21 +++++++++-----
> > >  drivers/gpu/drm/i915/intel_drv.h |  5 +++-
> > >  3 files changed, 73 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index f3e1d6a0b7dd..295d75c50688 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -3901,9 +3901,50 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> > >  
> > >  }
> > >  
> > > +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
> > > +{
> > > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > +
> > > +	intel_dp_encoder_suspend(encoder);
> > > +
> > > +	/*
> > > +	 * TODO: disconnect also from USB DP alternate mode once we have a
> > > +	 * way to handle the modeset restore in that mode during resume
> > > +	 * even if the sink has disappeared while being suspended.
> > > +	 */
> > > +	if (dig_port->tc_legacy_port)
> > > +		icl_tc_phy_disconnect(i915, 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);
> > > +	struct drm_i915_private *i915 = to_i915(encoder->dev);
> > > +
> > > +	intel_dp_encoder_flush_work(encoder);
> > > +
> > > +	if (intel_port_is_tc(i915, dig_port->base.port))
> > > +		icl_tc_phy_disconnect(i915, dig_port);
> > > +
> > > +	drm_encoder_cleanup(encoder);
> > > +	kfree(dig_port);
> > > +}
> > > +
> > >  static const struct drm_encoder_funcs intel_ddi_funcs = {
> > > -	.reset = intel_dp_encoder_reset,
> > > -	.destroy = intel_dp_encoder_destroy,
> > > +	.reset = intel_ddi_encoder_reset,
> > > +	.destroy = intel_ddi_encoder_destroy,
> > >  };
> > >  
> > >  static struct intel_connector *
> > > @@ -4147,16 +4188,16 @@ intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
> > >  
> > >  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> > >  {
> > > +	struct ddi_vbt_port_info *port_info =
> > > +		&dev_priv->vbt.ddi_port_info[port];
> > >  	struct intel_digital_port *intel_dig_port;
> > >  	struct intel_encoder *intel_encoder;
> > >  	struct drm_encoder *encoder;
> > >  	bool init_hdmi, init_dp, init_lspcon = false;
> > >  	enum pipe pipe;
> > >  
> > > -
> > > -	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> > > -		     dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> > > -	init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> > > +	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
> > > +	init_dp = port_info->supports_dp;
> > >  
> > >  	if (intel_bios_is_lspcon_present(dev_priv, port)) {
> > >  		/*
> > > @@ -4197,7 +4238,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> > >  	intel_encoder->post_disable = intel_ddi_post_disable;
> > >  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> > >  	intel_encoder->get_config = intel_ddi_get_config;
> > > -	intel_encoder->suspend = intel_dp_encoder_suspend;
> > > +	intel_encoder->suspend = intel_ddi_encoder_suspend;
> > >  	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
> > >  	intel_encoder->type = INTEL_OUTPUT_DDI;
> > >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> > > @@ -4216,6 +4257,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> > >  	intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
> > >  	intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
> > >  
> > > +	intel_dig_port->tc_legacy_port = intel_port_is_tc(dev_priv, port) &&
> > > +					 !port_info->supports_typec_usb &&
> > > +					 !port_info->supports_tbt;
> > > +
> > >  	switch (port) {
> > >  	case PORT_A:
> > >  		intel_dig_port->ddi_io_power_domain =
> > > @@ -4274,6 +4319,10 @@ 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_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 082594bb65a7..b2a3012478ca 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5108,9 +5108,6 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
> > >  			      tc_type_name(intel_dig_port->tc_type));
> > >  }
> > >  
> > > -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > > -				  struct intel_digital_port *dig_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
> > > @@ -5145,6 +5142,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> > >  	val = I915_READ(PORT_TX_DFLEXDPPMS);
> > >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> > >  		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
> > > +		WARN_ON(dig_port->tc_legacy_port);
> > >  		return false;
> > >  	}
> > >  
> > > @@ -5176,8 +5174,8 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
> > >   * See the comment at the connect function. This implements the Disconnect
> > >   * Flow.
> > >   */
> > > -static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > > -				  struct intel_digital_port *dig_port)
> > > +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > > +			   struct intel_digital_port *dig_port)
> > >  {
> > >  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> > >  
> > > @@ -5222,7 +5220,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> > >  	bool is_legacy, is_typec, is_tbt;
> > >  	u32 dpsp;
> > >  
> > > -	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> > > +	is_legacy = intel_dig_port->tc_legacy_port ||
> > > +		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> > >  
> > >  	/*
> > >  	 * The spec says we shouldn't be using the ISR bits for detecting
> > > @@ -5234,6 +5233,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> > >  
> > >  	if (!is_legacy && !is_typec && !is_tbt) {
> > >  		icl_tc_phy_disconnect(dev_priv, intel_dig_port);
> > > +
> > >  		return false;
> > >  	}
> > >  
> > > @@ -5542,7 +5542,7 @@ intel_dp_connector_unregister(struct drm_connector *connector)
> > >  	intel_connector_unregister(connector);
> > >  }
> > >  
> > > -void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > > +void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
> > >  {
> > >  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> > >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> > > @@ -5565,9 +5565,14 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > >  	}
> > >  
> > >  	intel_dp_aux_fini(intel_dp);
> > > +}
> > > +
> > > +static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > > +{
> > > +	intel_dp_encoder_flush_work(encoder);
> > >  
> > >  	drm_encoder_cleanup(encoder);
> > > -	kfree(intel_dig_port);
> > > +	kfree(enc_to_dig_port(encoder));
> > >  }
> > >  
> > >  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index d08f08f607dd..97c422bea7fb 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1234,6 +1234,7 @@ struct intel_digital_port {
> > >  	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
> > >  	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;
> > >  
> > >  	void (*write_infoframe)(struct intel_encoder *encoder,
> > > @@ -1806,7 +1807,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
> > >  					   bool enable);
> > >  void intel_dp_encoder_reset(struct drm_encoder *encoder);
> > >  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> > > -void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> > > +void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
> > >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> > >  			     struct intel_crtc_state *pipe_config,
> > >  			     struct drm_connector_state *conn_state);
> > > @@ -1874,6 +1875,8 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> > >  int intel_dp_link_required(int pixel_clock, int bpp);
> > >  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> > >  bool intel_digital_port_connected(struct intel_encoder *encoder);
> > > +void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
> > > +			   struct intel_digital_port *dig_port);
> > >  
> > >  /* intel_dp_aux_backlight.c */
> > >  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> > > -- 
> > > 2.13.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/i915/bios: Parse the VBT TypeC and Thunderbolt port flags

[Rodrigo Vivi]

On Fri, Dec 14, 2018 at 08:27:01PM +0200, Imre Deak wrote:
> This is needed by the next patch to determine if a DDI TypeC port is
> physically wired to a legacy DP or legacy HDMI connector or if the port
> is wired to a USB-C/Thunderbolt connector.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 11 +++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e70707e79386..fd3cccaac89e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -935,6 +935,8 @@ struct ddi_vbt_port_info {
>  	uint8_t supports_hdmi:1;
>  	uint8_t supports_dp:1;
>  	uint8_t supports_edp:1;
> +	uint8_t supports_typec_usb:1;
> +	uint8_t supports_tbt:1;
>  
>  	uint8_t alternate_aux_channel;
>  	uint8_t alternate_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 6d3e0260d49c..e59a5af45e94 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1386,8 +1386,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  	info->supports_dp = is_dp;
>  	info->supports_edp = is_edp;
>  
> -	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
> -		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
> +	if (bdb_version >= 195)
> +	       info->supports_typec_usb = child->dp_usb_type_c;
> +
> +	if (bdb_version >= 209)
> +		info->supports_tbt = child->tbt;
> +
> +	DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d TCUSB:%d TBT:%d\n",
> +		      port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt,
> +		      info->supports_typec_usb, info->supports_tbt);
>  
>  	if (is_edp && is_dvi)
>  		DRM_DEBUG_KMS("Internal DP port %c is TMDS compatible\n",
> -- 
> 2.13.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/4] drm/i915/icl: Add fallback detection method for TypeC legacy ports

[Kahola, Mika]

The v2 looks ok to me.

On Fri, 2018-12-14 at 20:27 +0200, Imre Deak wrote:
> Add a fallback detection method for TypeC legacy ports in case the
> VBT port information used to detect normally such ports is
> incorrect.
> 
> For the fallback method we use the TypeC legacy mode specific HPD
> interrupt flag which should only be raised for a legacy port.
> 
> WARN if the VBT port info is incorrect.
> 
> In a case where we'd detect the port in a contradicting way both as a
> legacy and also as a USB DP and/or TBT alternate port treat the port
> as legacy (by also emitting a WARN from icl_update_tc_port_type).
> 
> v2:
> - Repurpose the detection as a fallback method instead of using
>   it only for the DP legacy case. By now we should normally use VBT
> to
>   detect DP legacy ports as well.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (v1)
Reviewed-by: Mika Kahola <mika.kahola@intel.com> (v2)

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index b2a3012478ca..d00af92cd089 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5220,8 +5220,14 @@ static bool icl_tc_port_connected(struct
> drm_i915_private *dev_priv,
>  	bool is_legacy, is_typec, is_tbt;
>  	u32 dpsp;
>  
> -	is_legacy = intel_dig_port->tc_legacy_port ||
> -		I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> +	/*
> +	 * WARN 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 (WARN_ON(!intel_dig_port->tc_legacy_port &&
> +		    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)))
> +		intel_dig_port->tc_legacy_port = true;
> +	is_legacy = intel_dig_port->tc_legacy_port;
>  
>  	/*
>  	 * The spec says we shouldn't be using the ISR bits for
> detecting
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915/icl: Fix TypeC legacy HPD handling (rev2)

[Imre Deak]

On Fri, Dec 14, 2018 at 09:28:33PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/icl: Fix TypeC legacy HPD handling (rev2)
> URL   : https://patchwork.freedesktop.org/series/54017/
> State : success

Pushed to -dinq, thanks for the reviews.

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5318_full -> Patchwork_11100_full
> ====================================================
> 
> Summary
> -------
> 
>   **WARNING**
> 
>   Minor unknown changes coming with Patchwork_11100_full need to be verified
>   manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_11100_full, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_11100_full:
> 
> ### IGT changes ###
> 
> #### Warnings ####
> 
>   * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing:
>     - shard-snb:          SKIP -> PASS +3
> 
>   
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_11100_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_exec_suspend@basic-s4-devices:
>     - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#107139]
> 
>   * igt@i915_selftest@live_workarounds:
>     - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-render-c:
>     - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
>     - shard-iclb:         PASS -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
>     - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_cursor_crc@cursor-256x256-onscreen:
>     - shard-glk:          PASS -> FAIL [fdo#103232] +1
> 
>   * igt@kms_cursor_crc@cursor-256x256-suspend:
>     - shard-skl:          NOTRUN -> FAIL [fdo#103191] / [fdo#103232]
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
>     - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +51
> 
>   * igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
>     - shard-glk:          PASS -> FAIL [fdo#103184]
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
>     - shard-apl:          PASS -> FAIL [fdo#103167]
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
>     - shard-iclb:         PASS -> FAIL [fdo#103167] +4
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
>     - shard-iclb:         NOTRUN -> FAIL [fdo#103167]
> 
>   * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
>     - shard-glk:          PASS -> FAIL [fdo#103167] +4
> 
>   * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
>     - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
>     - shard-skl:          NOTRUN -> FAIL [fdo#108145]
> 
>   * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
>     - shard-iclb:         PASS -> FAIL [fdo#103166]
> 
>   * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
>     - shard-glk:          PASS -> FAIL [fdo#103166]
> 
>   * igt@kms_rmfb@rmfb-ioctl:
>     - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +1
> 
>   * igt@pm_rpm@i2c:
>     - shard-iclb:         PASS -> FAIL [fdo#104097]
> 
>   * igt@pm_rpm@modeset-non-lpsp-stress:
>     - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]
> 
>   
> #### Possible fixes ####
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-render-a:
>     - shard-snb:          DMESG-WARN [fdo#107956] -> PASS
> 
>   * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
>     - shard-apl:          DMESG-WARN [fdo#107956] -> PASS
> 
>   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
>     - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS
>     - shard-skl:          DMESG-WARN [fdo#107956] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-128x128-suspend:
>     - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-256x256-onscreen:
>     - shard-skl:          FAIL [fdo#103232] -> PASS +1
> 
>   * igt@kms_cursor_crc@cursor-256x85-random:
>     - shard-glk:          FAIL [fdo#103232] -> PASS
> 
>   * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
>     - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +11
> 
>   * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-untiled:
>     - shard-iclb:         WARN [fdo#108336] -> PASS +2
> 
>   * igt@kms_flip@flip-vs-expired-vblank:
>     - shard-skl:          FAIL [fdo#105363] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
>     - shard-glk:          FAIL [fdo#103167] -> PASS +3
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
>     - shard-iclb:         DMESG-FAIL [fdo#107724] -> PASS +1
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
>     - shard-iclb:         FAIL [fdo#103167] -> PASS +2
> 
>   * igt@kms_frontbuffer_tracking@psr-rgb565-draw-pwrite:
>     - shard-skl:          FAIL [fdo#103167] -> PASS
> 
>   * igt@kms_plane@pixel-format-pipe-a-planes:
>     - shard-apl:          FAIL [fdo#103166] -> PASS
> 
>   * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
>     - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +5
> 
>   * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
>     - shard-skl:          FAIL [fdo#107815] -> PASS
> 
>   * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
>     - shard-glk:          FAIL [fdo#103166] -> PASS
>     - shard-iclb:         FAIL [fdo#103166] -> PASS
> 
>   * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
>     - shard-kbl:          DMESG-WARN [fdo#105604] -> PASS
> 
>   * igt@pm_rpm@modeset-lpsp-stress-no-wait:
>     - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS
> 
>   * igt@pm_rpm@system-suspend-modeset:
>     - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS
> 
>   
> #### Warnings ####
> 
>   * igt@i915_selftest@live_contexts:
>     - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]
> 
>   * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
>     - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#107725]
> 
>   * igt@kms_cursor_crc@cursor-128x42-sliding:
>     - shard-kbl:          FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602]
> 
>   * igt@kms_cursor_crc@cursor-256x256-random:
>     - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
>     - shard-iclb:         DMESG-FAIL [fdo#107724] -> FAIL [fdo#103167]
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
>     - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103167]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
>     - shard-kbl:          FAIL [fdo#108145] / [fdo#108590] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145] +1
> 
>   
>   [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
>   [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
>   [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
>   [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
>   [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
>   [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
>   [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
>   [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
>   [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
>   [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
>   [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
>   [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
>   [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
>   [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
>   [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
>   [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
>   [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
>   [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
>   [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
>   [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
>   [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
>   [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
> 
> 
> Participating hosts (7 -> 7)
> ------------------------------
> 
>   No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>     * Linux: CI_DRM_5318 -> Patchwork_11100
> 
>   CI_DRM_5318: 4ef95f23369b5408a57faa222c8c0f2bb8298de8 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4747: ad821d1dc5d0eea4ac3a0e8e29c56c7f66191108 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_11100: 4b9db3b474f751f0a624206dbf88ea911a54d6ce @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11100/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx