All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Remaining ICL display patches, v2
@ 2018-07-25  0:28 Paulo Zanoni
  2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

Hi

This new version of the series excludes the controversial patch about
the HPD connect flow. Let's reach an agreement on this part first,
then we discuss what to do without having to rebase too many times the
patches we already agree on. Only patches 1 and 2 need review.

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

Thanks,
Paulo

Animesh Manna (1):
  drm/i915/icl: Update FIA supported lane count for hpd.

Paulo Zanoni (4):
  drm/i915/icl: implement icl_digital_port_connected()
  drm/i915/icl: store the port type for TC ports
  drm/i915/icl: program MG_DP_MODE
  drm/i915/icl: toggle PHY clock gating around link training

 drivers/gpu/drm/i915/i915_reg.h      |  46 +++++
 drivers/gpu/drm/i915/intel_ddi.c     |   5 +
 drivers/gpu/drm/i915/intel_display.h |   7 +
 drivers/gpu/drm/i915/intel_dp.c      | 256 ++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 5 files changed, 316 insertions(+), 2 deletions(-)

-- 
2.17.1

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
@ 2018-07-25  0:28 ` Paulo Zanoni
  2018-07-25  1:19   ` Lucas De Marchi
  2018-07-25 19:59   ` [PATCH v4 1/5] " Paulo Zanoni
  2018-07-25  0:28 ` [PATCH 2/5] drm/i915/icl: store the port type for TC ports Paulo Zanoni
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

Do like the other functions and check for the status bits. The "Hot
Plug Detection" page from our documentation says we can't just use the
ISR bits on the CPU side, so use the correct registers.

v2: Rebase.
v3:
  - Simplify true/false assignment (Rodrigo).
  - Reorganize is_gen if ladder (Rodrigo).
  - Don't use the ISR for TC/TBT CPU bits.

Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  8 +++++
 drivers/gpu/drm/i915/intel_dp.c | 55 ++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

My understanding is that there's agreement on this patch since it just
mimicks what the other gens do. Let's have this one agreed on (merged)
first before we continue the discussions, especially since this one
significantly affects the CI system.

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 73946055aa15..3eaff32dd74e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7210,6 +7210,7 @@ enum {
 #define  GEN11_TC3_HOTPLUG			(1 << 18)
 #define  GEN11_TC2_HOTPLUG			(1 << 17)
 #define  GEN11_TC1_HOTPLUG			(1 << 16)
+#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port) + 16))
 #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLUG | \
 						 GEN11_TC3_HOTPLUG | \
 						 GEN11_TC2_HOTPLUG | \
@@ -7218,6 +7219,7 @@ enum {
 #define  GEN11_TBT3_HOTPLUG			(1 << 2)
 #define  GEN11_TBT2_HOTPLUG			(1 << 1)
 #define  GEN11_TBT1_HOTPLUG			(1 << 0)
+#define  GEN11_TBT_HOTPLUG(tc_port)		(1 << (tc_port))
 #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTPLUG | \
 						 GEN11_TBT3_HOTPLUG | \
 						 GEN11_TBT2_HOTPLUG | \
@@ -7590,6 +7592,8 @@ enum {
 #define SDE_GMBUS_ICP			(1 << 23)
 #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
 #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
+#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) + 24))
+#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
 #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
 					 SDE_DDIA_HOTPLUG_ICP)
 #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP |	\
@@ -10654,4 +10658,8 @@ enum skl_power_gate {
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
 
+#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
+#define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
+#define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cd0f649b57a5..998d698788f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4586,6 +4586,57 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
 	return I915_READ(GEN8_DE_PORT_ISR) & bit;
 }
 
+static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
+				     struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+
+	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
+}
+
+static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
+				  struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	bool is_legacy, is_typec, is_tbt;
+	u32 dpsp;
+
+	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
+
+	/*
+	 * The spec says we shouldn't be using the ISR bits for detecting
+	 * between TC and TBT. We should use DFLEXDPSP.
+	 */
+	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
+	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
+	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
+
+	WARN_ON(is_legacy + is_typec + is_tbt > 1);
+
+	return is_legacy || is_typec || is_tbt;
+}
+
+static bool icl_digital_port_connected(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_A:
+	case HPD_PORT_B:
+		return icl_combo_port_connected(dev_priv, dig_port);
+	case HPD_PORT_C:
+	case HPD_PORT_D:
+	case HPD_PORT_E:
+	case HPD_PORT_F:
+		return icl_tc_port_connected(dev_priv, dig_port);
+	default:
+		MISSING_CASE(encoder->hpd_pin);
+		return false;
+	}
+}
+
 /*
  * intel_digital_port_connected - is the specified port connected?
  * @encoder: intel_encoder
@@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
 		return bdw_digital_port_connected(encoder);
 	else if (IS_GEN9_LP(dev_priv))
 		return bxt_digital_port_connected(encoder);
-	else
+	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
 		return spt_digital_port_connected(encoder);
+	else
+		return icl_digital_port_connected(encoder);
 }
 
 static struct edid *
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/5] drm/i915/icl: store the port type for TC ports
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
  2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
@ 2018-07-25  0:28 ` Paulo Zanoni
  2018-07-25 16:41   ` Rodrigo Vivi
  2018-07-25  0:28 ` [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd Paulo Zanoni
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

The type is detected based on the live status bits. Once detected,
it's not supposed to be changed, so we have some sanity checks for
that.

v2: Rebase.

Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.h |  7 +++++
 drivers/gpu/drm/i915/intel_dp.c      | 40 ++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 9292001cdd14..0a79a46d5805 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -137,6 +137,13 @@ enum tc_port {
 	I915_MAX_TC_PORTS
 };
 
+enum tc_port_type {
+	TC_PORT_UNKNOWN = 0,
+	TC_PORT_TYPEC,
+	TC_PORT_TBT,
+	TC_PORT_LEGACY,
+};
+
 enum dpio_channel {
 	DPIO_CH0,
 	DPIO_CH1
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 998d698788f9..90c5ba6b222b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4594,6 +4594,38 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
 	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
 }
 
+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) {
+		intel_dig_port->tc_type = TC_PORT_LEGACY;
+		type_str = "legacy";
+	} else if (is_typec) {
+		intel_dig_port->tc_type = TC_PORT_TYPEC;
+		type_str = "typec";
+	} else if (is_tbt) {
+		intel_dig_port->tc_type = TC_PORT_TBT;
+		type_str = "tbt";
+	} else {
+		return;
+	}
+
+	/* Types are not supposed to be changed at runtime. */
+	WARN_ON(old_type != TC_PORT_UNKNOWN &&
+		old_type != intel_dig_port->tc_type);
+
+	if (old_type != intel_dig_port->tc_type)
+		DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
+			      type_str);
+}
+
 static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 				  struct intel_digital_port *intel_dig_port)
 {
@@ -4612,9 +4644,13 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
 	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
 
-	WARN_ON(is_legacy + is_typec + is_tbt > 1);
+	if (!is_legacy && !is_typec && !is_tbt)
+		return false;
+
+	icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec,
+				is_tbt);
 
-	return is_legacy || is_typec || is_tbt;
+	return true;
 }
 
 static bool icl_digital_port_connected(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c275f91244a6..5e225d8ba09a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1164,6 +1164,7 @@ struct intel_digital_port {
 	bool release_cl2_override;
 	uint8_t max_lanes;
 	enum intel_display_power_domain ddi_io_power_domain;
+	enum tc_port_type tc_type;
 
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd.
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
  2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
  2018-07-25  0:28 ` [PATCH 2/5] drm/i915/icl: store the port type for TC ports Paulo Zanoni
@ 2018-07-25  0:28 ` Paulo Zanoni
  2018-07-25 16:52   ` Rodrigo Vivi
  2018-07-25  0:28 ` [PATCH 4/5] drm/i915/icl: program MG_DP_MODE Paulo Zanoni
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: Animesh Manna <animesh.manna@intel.com>

In ICL, Flexible IO Adapter (FIA) muxes data and clocks of USB 3.1,
tbt and display controller. In DP alt mode FIA configure the
number of lanes and will be used apart from DPCD read to calculate max
available lanes for DP enablement.

v2 (from Paulo): Simple rebase.

Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com> (v1).
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
[Paulo: significant rewrite of the patch.]
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 93de6f724e77..72acecaad5c1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10705,5 +10705,8 @@ enum skl_power_gate {
 #define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
 #define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
 #define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
+#define   DP_LANE_ASSIGNMENT_SHIFT(tc_port)	((tc_port) * 8)
+#define   DP_LANE_ASSIGNMENT_MASK(tc_port)	(0xf << ((tc_port) * 8))
+#define   DP_LANE_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 8))
 
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 90c5ba6b222b..bb59e71d6f9c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -176,14 +176,45 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp)
 	return intel_dp->common_rates[intel_dp->num_common_rates - 1];
 }
 
+static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
+	u32 lane_info;
+
+	if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
+		return 4;
+
+	lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
+		     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
+		    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+
+	switch (lane_info) {
+	default:
+		MISSING_CASE(lane_info);
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+		return 1;
+	case 3:
+	case 12:
+		return 2;
+	case 15:
+		return 4;
+	}
+}
+
 /* Theoretical max between source and sink */
 static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	int source_max = intel_dig_port->max_lanes;
 	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
+	int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp);
 
-	return min(source_max, sink_max);
+	return min3(source_max, sink_max, fia_max);
 }
 
 int intel_dp_max_lane_count(struct intel_dp *intel_dp)
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/5] drm/i915/icl: program MG_DP_MODE
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (2 preceding siblings ...)
  2018-07-25  0:28 ` [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd Paulo Zanoni
@ 2018-07-25  0:28 ` Paulo Zanoni
  2018-07-25  0:28 ` [PATCH 5/5] drm/i915/icl: toggle PHY clock gating around link training Paulo Zanoni
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

Programming this register is part of the Enable Sequence for
DisplayPort on ICL. Do as the spec says.

v2: Simple rebase.

Cc: Animesh Manna <animesh.manna@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 15 ++++++++
 drivers/gpu/drm/i915/intel_ddi.c |  2 +
 drivers/gpu/drm/i915/intel_dp.c  | 66 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 4 files changed, 84 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72acecaad5c1..cf1d2bbb0613 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2092,6 +2092,21 @@ enum i915_power_well_id {
 #define   CFG_AMI_CK_DIV_OVERRIDE_VAL_MASK	(0x3 << 25)
 #define   CFG_AMI_CK_DIV_OVERRIDE_EN		(1 << 24)
 
+#define MG_DP_MODE_LN0_ACU_PORT1			0x1683A0
+#define MG_DP_MODE_LN1_ACU_PORT1			0x1687A0
+#define MG_DP_MODE_LN0_ACU_PORT2			0x1693A0
+#define MG_DP_MODE_LN1_ACU_PORT2			0x1697A0
+#define MG_DP_MODE_LN0_ACU_PORT3			0x16A3A0
+#define MG_DP_MODE_LN1_ACU_PORT3			0x16A7A0
+#define MG_DP_MODE_LN0_ACU_PORT4			0x16B3A0
+#define MG_DP_MODE_LN1_ACU_PORT4			0x16B7A0
+#define MG_DP_MODE(port, ln)	\
+	MG_PHY_PORT_LN(port, ln, MG_DP_MODE_LN0_ACU_PORT1, \
+				 MG_DP_MODE_LN0_ACU_PORT2, \
+				 MG_DP_MODE_LN1_ACU_PORT1)
+#define   MG_DP_MODE_CFG_DP_X2_MODE			(1 << 7)
+#define   MG_DP_MODE_CFG_DP_X1_MODE			(1 << 6)
+
 /* The spec defines this only for BXT PHY0, but lets assume that this
  * would exist for PHY1 too if it had a second channel.
  */
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 01c07a000464..399c438bd210 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2809,6 +2809,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
+	icl_program_mg_dp_mode(intel_dp);
+
 	if (IS_ICELAKE(dev_priv))
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, encoder->type);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bb59e71d6f9c..28de73be4507 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -229,6 +229,72 @@ intel_dp_link_required(int pixel_clock, int bpp)
 	return DIV_ROUND_UP(pixel_clock * bpp, 8);
 }
 
+void icl_program_mg_dp_mode(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	enum port port = intel_dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	u32 ln0, ln1, lane_info;
+
+	if (tc_port == PORT_TC_NONE || intel_dig_port->tc_type == TC_PORT_TBT)
+		return;
+
+	ln0 = I915_READ(MG_DP_MODE(port, 0));
+	ln1 = I915_READ(MG_DP_MODE(port, 1));
+
+	switch (intel_dig_port->tc_type) {
+	case TC_PORT_TYPEC:
+		ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);
+		ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);
+
+		lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
+			     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
+			    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+
+		switch (lane_info) {
+		case 0x1:
+		case 0x4:
+			break;
+		case 0x2:
+			ln0 |= MG_DP_MODE_CFG_DP_X1_MODE;
+			break;
+		case 0x3:
+			ln0 |= MG_DP_MODE_CFG_DP_X1_MODE |
+			       MG_DP_MODE_CFG_DP_X2_MODE;
+			break;
+		case 0x8:
+			ln1 |= MG_DP_MODE_CFG_DP_X1_MODE;
+			break;
+		case 0xC:
+			ln1 |= MG_DP_MODE_CFG_DP_X1_MODE |
+			       MG_DP_MODE_CFG_DP_X2_MODE;
+			break;
+		case 0xF:
+			ln0 |= MG_DP_MODE_CFG_DP_X1_MODE |
+			       MG_DP_MODE_CFG_DP_X2_MODE;
+			ln1 |= MG_DP_MODE_CFG_DP_X1_MODE |
+			       MG_DP_MODE_CFG_DP_X2_MODE;
+			break;
+		default:
+			MISSING_CASE(lane_info);
+		}
+		break;
+
+	case TC_PORT_LEGACY:
+		ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE;
+		ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE;
+		break;
+
+	default:
+		MISSING_CASE(intel_dig_port->tc_type);
+		return;
+	}
+
+	I915_WRITE(MG_DP_MODE(port, 0), ln0);
+	I915_WRITE(MG_DP_MODE(port, 1), ln1);
+}
+
 int
 intel_dp_max_data_rate(int max_link_clock, int max_lanes)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5e225d8ba09a..4e5b00052b5b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1714,6 +1714,7 @@ void intel_edp_drrs_invalidate(struct drm_i915_private *dev_priv,
 			       unsigned int frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_i915_private *dev_priv,
 			  unsigned int frontbuffer_bits);
+void icl_program_mg_dp_mode(struct intel_dp *intel_dp);
 
 void
 intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/5] drm/i915/icl: toggle PHY clock gating around link training
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (3 preceding siblings ...)
  2018-07-25  0:28 ` [PATCH 4/5] drm/i915/icl: program MG_DP_MODE Paulo Zanoni
@ 2018-07-25  0:28 ` Paulo Zanoni
  2018-07-25  0:52 ` ✗ Fi.CI.SPARSE: warning for drm/i915/icl: implement icl_digital_port_connected() (rev2) Patchwork
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

The Gen11 TypeC PHY DDI Buffer chapter, PHY Clock Gating Programming
section says that PHY clock gating should be disabled before starting
voltage swing programming, then enabled after any link training is
complete.

v2: Simple rebase.

Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 20 ++++++++++
 drivers/gpu/drm/i915/intel_ddi.c |  3 ++
 drivers/gpu/drm/i915/intel_dp.c  | 66 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 +
 4 files changed, 91 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cf1d2bbb0613..5530c470f30d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2106,6 +2106,26 @@ enum i915_power_well_id {
 				 MG_DP_MODE_LN1_ACU_PORT1)
 #define   MG_DP_MODE_CFG_DP_X2_MODE			(1 << 7)
 #define   MG_DP_MODE_CFG_DP_X1_MODE			(1 << 6)
+#define   MG_DP_MODE_CFG_TR2PWR_GATING			(1 << 5)
+#define   MG_DP_MODE_CFG_TRPWR_GATING			(1 << 4)
+#define   MG_DP_MODE_CFG_CLNPWR_GATING			(1 << 3)
+#define   MG_DP_MODE_CFG_DIGPWR_GATING			(1 << 2)
+#define   MG_DP_MODE_CFG_GAONPWR_GATING			(1 << 1)
+
+#define MG_MISC_SUS0_PORT1				0x168814
+#define MG_MISC_SUS0_PORT2				0x169814
+#define MG_MISC_SUS0_PORT3				0x16A814
+#define MG_MISC_SUS0_PORT4				0x16B814
+#define MG_MISC_SUS0(tc_port) \
+	_MMIO(_PORT(tc_port, MG_MISC_SUS0_PORT1, MG_MISC_SUS0_PORT2))
+#define   MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK	(3 << 14)
+#define   MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(x)	((x) << 14)
+#define   MG_MISC_SUS0_CFG_TR2PWR_GATING		(1 << 12)
+#define   MG_MISC_SUS0_CFG_CL2PWR_GATING		(1 << 11)
+#define   MG_MISC_SUS0_CFG_GAONPWR_GATING		(1 << 10)
+#define   MG_MISC_SUS0_CFG_TRPWR_GATING			(1 << 7)
+#define   MG_MISC_SUS0_CFG_CL1PWR_GATING		(1 << 6)
+#define   MG_MISC_SUS0_CFG_DGPWR_GATING			(1 << 5)
 
 /* The spec defines this only for BXT PHY0, but lets assume that this
  * would exist for PHY1 too if it had a second channel.
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 399c438bd210..0adc043529f2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2810,6 +2810,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
 	icl_program_mg_dp_mode(intel_dp);
+	icl_disable_phy_clock_gating(dig_port);
 
 	if (IS_ICELAKE(dev_priv))
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
@@ -2828,6 +2829,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
 		intel_dp_stop_link_train(intel_dp);
 
+	icl_enable_phy_clock_gating(dig_port);
+
 	intel_ddi_enable_pipe_clock(crtc_state);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 28de73be4507..cc33d7c6ba19 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -295,6 +295,72 @@ void icl_program_mg_dp_mode(struct intel_dp *intel_dp)
 	I915_WRITE(MG_DP_MODE(port, 1), ln1);
 }
 
+void icl_enable_phy_clock_gating(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	enum port port = dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	i915_reg_t mg_regs[2] = { MG_DP_MODE(port, 0), MG_DP_MODE(port, 1) };
+	u32 val;
+	int i;
+
+	if (tc_port == PORT_TC_NONE)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(mg_regs); i++) {
+		val = I915_READ(mg_regs[i]);
+		val |= MG_DP_MODE_CFG_TR2PWR_GATING |
+		       MG_DP_MODE_CFG_TRPWR_GATING |
+		       MG_DP_MODE_CFG_CLNPWR_GATING |
+		       MG_DP_MODE_CFG_DIGPWR_GATING |
+		       MG_DP_MODE_CFG_GAONPWR_GATING;
+		I915_WRITE(mg_regs[i], val);
+	}
+
+	val = I915_READ(MG_MISC_SUS0(tc_port));
+	val |= MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3) |
+	       MG_MISC_SUS0_CFG_TR2PWR_GATING |
+	       MG_MISC_SUS0_CFG_CL2PWR_GATING |
+	       MG_MISC_SUS0_CFG_GAONPWR_GATING |
+	       MG_MISC_SUS0_CFG_TRPWR_GATING |
+	       MG_MISC_SUS0_CFG_CL1PWR_GATING |
+	       MG_MISC_SUS0_CFG_DGPWR_GATING;
+	I915_WRITE(MG_MISC_SUS0(tc_port), val);
+}
+
+void icl_disable_phy_clock_gating(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+	enum port port = dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	i915_reg_t mg_regs[2] = { MG_DP_MODE(port, 0), MG_DP_MODE(port, 1) };
+	u32 val;
+	int i;
+
+	if (tc_port == PORT_TC_NONE)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(mg_regs); i++) {
+		val = I915_READ(mg_regs[i]);
+		val &= ~(MG_DP_MODE_CFG_TR2PWR_GATING |
+			 MG_DP_MODE_CFG_TRPWR_GATING |
+			 MG_DP_MODE_CFG_CLNPWR_GATING |
+			 MG_DP_MODE_CFG_DIGPWR_GATING |
+			 MG_DP_MODE_CFG_GAONPWR_GATING);
+		I915_WRITE(mg_regs[i], val);
+	}
+
+	val = I915_READ(MG_MISC_SUS0(tc_port));
+	val &= ~(MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK |
+		 MG_MISC_SUS0_CFG_TR2PWR_GATING |
+		 MG_MISC_SUS0_CFG_CL2PWR_GATING |
+		 MG_MISC_SUS0_CFG_GAONPWR_GATING |
+		 MG_MISC_SUS0_CFG_TRPWR_GATING |
+		 MG_MISC_SUS0_CFG_CL1PWR_GATING |
+		 MG_MISC_SUS0_CFG_DGPWR_GATING);
+	I915_WRITE(MG_MISC_SUS0(tc_port), val);
+}
+
 int
 intel_dp_max_data_rate(int max_link_clock, int max_lanes)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4e5b00052b5b..99a5f5be5b82 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1715,6 +1715,8 @@ void intel_edp_drrs_invalidate(struct drm_i915_private *dev_priv,
 void intel_edp_drrs_flush(struct drm_i915_private *dev_priv,
 			  unsigned int frontbuffer_bits);
 void icl_program_mg_dp_mode(struct intel_dp *intel_dp);
+void icl_enable_phy_clock_gating(struct intel_digital_port *dig_port);
+void icl_disable_phy_clock_gating(struct intel_digital_port *dig_port);
 
 void
 intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/icl: implement icl_digital_port_connected() (rev2)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (4 preceding siblings ...)
  2018-07-25  0:28 ` [PATCH 5/5] drm/i915/icl: toggle PHY clock gating around link training Paulo Zanoni
@ 2018-07-25  0:52 ` Patchwork
  2018-07-25  1:09 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25  0:52 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev2)
URL   : https://patchwork.freedesktop.org/series/47151/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/icl: implement icl_digital_port_connected()
Okay!

Commit: drm/i915/icl: store the port type for TC ports
Okay!

Commit: drm/i915/icl: Update FIA supported lane count for hpd.
-O:drivers/gpu/drm/i915/intel_dp.c:186:16: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_dp.c:186:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)

Commit: drm/i915/icl: program MG_DP_MODE
Okay!

Commit: drm/i915/icl: toggle PHY clock gating around link training
Okay!

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/icl: implement icl_digital_port_connected() (rev2)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (5 preceding siblings ...)
  2018-07-25  0:52 ` ✗ Fi.CI.SPARSE: warning for drm/i915/icl: implement icl_digital_port_connected() (rev2) Patchwork
@ 2018-07-25  1:09 ` Patchwork
  2018-07-25  2:08 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25  1:09 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev2)
URL   : https://patchwork.freedesktop.org/series/47151/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4537 -> Patchwork_9760 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> DMESG-FAIL (fdo#106947, fdo#106560)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-bdw-5557u:       DMESG-FAIL (fdo#106560) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947


== Participating hosts (49 -> 41) ==

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-caroline fi-byt-clapper fi-bdw-samus 


== Build changes ==

    * Linux: CI_DRM_4537 -> Patchwork_9760

  CI_DRM_4537: d14ba41a37d886280f69ba42c82c34654de09efc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4573: 2884f91dd6d7682ea738ef6f0943cc591643dda2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9760: a800e1f994246df689b693d76b41ea631d5df2fd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a800e1f99424 drm/i915/icl: toggle PHY clock gating around link training
52d60f864c4d drm/i915/icl: program MG_DP_MODE
055dce408288 drm/i915/icl: Update FIA supported lane count for hpd.
198919f4ca28 drm/i915/icl: store the port type for TC ports
33275d39deb6 drm/i915/icl: implement icl_digital_port_connected()

== Logs ==

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
@ 2018-07-25  1:19   ` Lucas De Marchi
  2018-07-25 16:59     ` Paulo Zanoni
  2018-07-25 17:27     ` Paulo Zanoni
  2018-07-25 19:59   ` [PATCH v4 1/5] " Paulo Zanoni
  1 sibling, 2 replies; 25+ messages in thread
From: Lucas De Marchi @ 2018-07-25  1:19 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Rodrigo Vivi

On Tue, Jul 24, 2018 at 05:28:09PM -0700, Paulo Zanoni wrote:
> Do like the other functions and check for the status bits. The "Hot
> Plug Detection" page from our documentation says we can't just use the
> ISR bits on the CPU side, so use the correct registers.

nit: here you are saying it's for all of them rather than just a subset.
My suggestion is:

Do like the other functions and check for the status bits. For TypeC/TBT
on North Display Engine the "Hot Plug Detection" page from our
documentation says we can't just use the ISR bits to find the live
state, so use the correct registers: DFLEXDPSP TC Live State field.


> 
> v2: Rebase.
> v3:
>   - Simplify true/false assignment (Rodrigo).
>   - Reorganize is_gen if ladder (Rodrigo).
>   - Don't use the ISR for TC/TBT CPU bits.
> 
> Cc: Animesh Manna <animesh.manna@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
>  drivers/gpu/drm/i915/intel_dp.c | 55 ++++++++++++++++++++++++++++++++-
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> My understanding is that there's agreement on this patch since it just
> mimicks what the other gens do. Let's have this one agreed on (merged)
> first before we continue the discussions, especially since this one
> significantly affects the CI system.
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 73946055aa15..3eaff32dd74e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7210,6 +7210,7 @@ enum {
>  #define  GEN11_TC3_HOTPLUG			(1 << 18)
>  #define  GEN11_TC2_HOTPLUG			(1 << 17)
>  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> +#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port) + 16))
>  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLUG | \
>  						 GEN11_TC3_HOTPLUG | \
>  						 GEN11_TC2_HOTPLUG | \
> @@ -7218,6 +7219,7 @@ enum {
>  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
>  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
>  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 << (tc_port))
>  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTPLUG | \
>  						 GEN11_TBT3_HOTPLUG | \
>  						 GEN11_TBT2_HOTPLUG | \
> @@ -7590,6 +7592,8 @@ enum {
>  #define SDE_GMBUS_ICP			(1 << 23)
>  #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
>  #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> +#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) + 24))
> +#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
>  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
>  					 SDE_DDIA_HOTPLUG_ICP)
>  #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP |	\
> @@ -10654,4 +10658,8 @@ enum skl_power_gate {
>  						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
>  						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
>  
> +#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> +#define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
> +#define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cd0f649b57a5..998d698788f9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4586,6 +4586,57 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
>  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
>  }
>  
> +static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
> +				     struct intel_digital_port *intel_dig_port)
> +{
> +	enum port port = intel_dig_port->base.port;
> +
> +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> +}
> +
> +static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> +				  struct intel_digital_port *intel_dig_port)
> +{
> +	enum port port = intel_dig_port->base.port;
> +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> +	bool is_legacy, is_typec, is_tbt;
> +	u32 dpsp;
> +
> +	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
> +
> +	/*
> +	 * The spec says we shouldn't be using the ISR bits for detecting
> +	 * between TC and TBT. We should use DFLEXDPSP.
> +	 */
> +	dpsp = I915_READ(PORT_TX_DFLEXDPSP);

I only wonder if we should be reading the North DE even if is_legacy is
true above rather than using an early return. Anyway:

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

> +	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> +	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> +
> +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> +
> +	return is_legacy || is_typec || is_tbt;
> +}
> +
> +static bool icl_digital_port_connected(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +
> +	switch (encoder->hpd_pin) {
> +	case HPD_PORT_A:
> +	case HPD_PORT_B:
> +		return icl_combo_port_connected(dev_priv, dig_port);
> +	case HPD_PORT_C:
> +	case HPD_PORT_D:
> +	case HPD_PORT_E:
> +	case HPD_PORT_F:
> +		return icl_tc_port_connected(dev_priv, dig_port);
> +	default:
> +		MISSING_CASE(encoder->hpd_pin);
> +		return false;
> +	}
> +}
> +
>  /*
>   * intel_digital_port_connected - is the specified port connected?
>   * @encoder: intel_encoder
> @@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
>  		return bdw_digital_port_connected(encoder);
>  	else if (IS_GEN9_LP(dev_priv))
>  		return bxt_digital_port_connected(encoder);
> -	else
> +	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
>  		return spt_digital_port_connected(encoder);
> +	else
> +		return icl_digital_port_connected(encoder);
>  }
>  
>  static struct edid *
> -- 
> 2.17.1
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/icl: implement icl_digital_port_connected() (rev2)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (6 preceding siblings ...)
  2018-07-25  1:09 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-25  2:08 ` Patchwork
  2018-07-25 11:07 ` ✓ Fi.CI.BAT: " Patchwork
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25  2:08 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev2)
URL   : https://patchwork.freedesktop.org/series/47151/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4537_full -> Patchwork_9760_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9760_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9760_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_9760_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_plane_lowres@pipe-b-tiling-none:
      shard-snb:          PASS -> SKIP +2

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@forcewake:
      shard-snb:          PASS -> DMESG-WARN (fdo#102365)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-hsw:          PASS -> FAIL (fdo#105363)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          FAIL (fdo#105900) -> PASS

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-hsw:          INCOMPLETE (fdo#103540) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105189) -> PASS

    igt@kms_flip@dpms-vs-vblank-race:
      shard-snb:          FAIL (fdo#103060) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4537 -> Patchwork_9760

  CI_DRM_4537: d14ba41a37d886280f69ba42c82c34654de09efc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4573: 2884f91dd6d7682ea738ef6f0943cc591643dda2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9760: a800e1f994246df689b693d76b41ea631d5df2fd @ 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_9760/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/icl: implement icl_digital_port_connected() (rev2)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (7 preceding siblings ...)
  2018-07-25  2:08 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-25 11:07 ` Patchwork
  2018-07-25 12:29 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25 11:07 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev2)
URL   : https://patchwork.freedesktop.org/series/47151/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4539 -> Patchwork_9762 =

== Summary - SUCCESS ==

  No regressions found.

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@debugfs_test@read_all_entries:
      {fi-icl-u}:         NOTRUN -> DMESG-WARN +5

    {igt@kms_psr@primary_page_flip}:
      {fi-icl-u}:         NOTRUN -> FAIL +3

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)
      fi-bxt-dsi:         PASS -> DMESG-FAIL (fdo#106560)

    igt@drv_selftest@live_objects:
      {fi-icl-u}:         NOTRUN -> DMESG-FAIL (fdo#107339)

    igt@gem_ctx_param@basic-default:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@gem_workarounds@basic-read:
      {fi-icl-u}:         NOTRUN -> FAIL (fdo#107338)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7567u:       DMESG-FAIL (fdo#106560, fdo#106947) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107338 https://bugs.freedesktop.org/show_bug.cgi?id=107338
  fdo#107339 https://bugs.freedesktop.org/show_bug.cgi?id=107339
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (51 -> 46) ==

  Additional (1): fi-icl-u 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4539 -> Patchwork_9762

  CI_DRM_4539: 764eb9fdd5683545c98da3e1c144824519306876 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4573: 2884f91dd6d7682ea738ef6f0943cc591643dda2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9762: 640806fc157865b6078add10a6ee3dc59c39ed26 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

640806fc1578 drm/i915/icl: toggle PHY clock gating around link training
a477e462b9ab drm/i915/icl: program MG_DP_MODE
857c57ed0253 drm/i915/icl: Update FIA supported lane count for hpd.
d19a5f3fb312 drm/i915/icl: store the port type for TC ports
779d73b3ab5d drm/i915/icl: implement icl_digital_port_connected()

== Logs ==

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/icl: implement icl_digital_port_connected() (rev2)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (8 preceding siblings ...)
  2018-07-25 11:07 ` ✓ Fi.CI.BAT: " Patchwork
@ 2018-07-25 12:29 ` Patchwork
  2018-07-25 20:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: implement icl_digital_port_connected() (rev3) Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25 12:29 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev2)
URL   : https://patchwork.freedesktop.org/series/47151/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4539_full -> Patchwork_9762_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9762_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9762_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_9762_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-render:
      shard-kbl:          SKIP -> PASS

    igt@kms_busy@extended-modeset-hang-oldfb-render-b:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    {igt@gem_userptr_blits@readonly-unsync}:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          DMESG-FAIL (fdo#106947, fdo#106560) -> PASS

    igt@gem_workarounds@suspend-resume:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105189) -> PASS

    igt@kms_flip@plain-flip-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4539 -> Patchwork_9762

  CI_DRM_4539: 764eb9fdd5683545c98da3e1c144824519306876 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4573: 2884f91dd6d7682ea738ef6f0943cc591643dda2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9762: 640806fc157865b6078add10a6ee3dc59c39ed26 @ 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_9762/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/i915/icl: store the port type for TC ports
  2018-07-25  0:28 ` [PATCH 2/5] drm/i915/icl: store the port type for TC ports Paulo Zanoni
@ 2018-07-25 16:41   ` Rodrigo Vivi
  2018-07-25 17:03     ` Rodrigo Vivi
  0 siblings, 1 reply; 25+ messages in thread
From: Rodrigo Vivi @ 2018-07-25 16:41 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

On Tue, Jul 24, 2018 at 05:28:10PM -0700, Paulo Zanoni wrote:
> The type is detected based on the live status bits. Once detected,
> it's not supposed to be changed, so we have some sanity checks for
> that.
> 
> v2: Rebase.
> 
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.h |  7 +++++
>  drivers/gpu/drm/i915/intel_dp.c      | 40 ++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  3 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 9292001cdd14..0a79a46d5805 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -137,6 +137,13 @@ enum tc_port {
>  	I915_MAX_TC_PORTS
>  };
>  
> +enum tc_port_type {
> +	TC_PORT_UNKNOWN = 0,
> +	TC_PORT_TYPEC,
> +	TC_PORT_TBT,
> +	TC_PORT_LEGACY,
> +};
> +
>  enum dpio_channel {
>  	DPIO_CH0,
>  	DPIO_CH1
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 998d698788f9..90c5ba6b222b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4594,6 +4594,38 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
>  	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
>  }
>  
> +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) {
> +		intel_dig_port->tc_type = TC_PORT_LEGACY;
> +		type_str = "legacy";
> +	} else if (is_typec) {
> +		intel_dig_port->tc_type = TC_PORT_TYPEC;
> +		type_str = "typec";
> +	} else if (is_tbt) {
> +		intel_dig_port->tc_type = TC_PORT_TBT;
> +		type_str = "tbt";
> +	} else {
> +		return;
> +	}

nip: switch seems more appropriate for this block.

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

> +
> +	/* Types are not supposed to be changed at runtime. */
> +	WARN_ON(old_type != TC_PORT_UNKNOWN &&
> +		old_type != intel_dig_port->tc_type);
> +
> +	if (old_type != intel_dig_port->tc_type)
> +		DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
> +			      type_str);
> +}
> +
>  static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
>  				  struct intel_digital_port *intel_dig_port)
>  {
> @@ -4612,9 +4644,13 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
>  	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
>  	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
>  
> -	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> +	if (!is_legacy && !is_typec && !is_tbt)
> +		return false;
> +
> +	icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec,
> +				is_tbt);
>  
> -	return is_legacy || is_typec || is_tbt;
> +	return true;
>  }
>  
>  static bool icl_digital_port_connected(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c275f91244a6..5e225d8ba09a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1164,6 +1164,7 @@ struct intel_digital_port {
>  	bool release_cl2_override;
>  	uint8_t max_lanes;
>  	enum intel_display_power_domain ddi_io_power_domain;
> +	enum tc_port_type tc_type;
>  
>  	void (*write_infoframe)(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -- 
> 2.17.1
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd.
  2018-07-25  0:28 ` [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd Paulo Zanoni
@ 2018-07-25 16:52   ` Rodrigo Vivi
  2018-07-25 17:12     ` Paulo Zanoni
  2018-07-25 17:13     ` Rodrigo Vivi
  0 siblings, 2 replies; 25+ messages in thread
From: Rodrigo Vivi @ 2018-07-25 16:52 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

On Tue, Jul 24, 2018 at 05:28:11PM -0700, Paulo Zanoni wrote:
> From: Animesh Manna <animesh.manna@intel.com>
> 
> In ICL, Flexible IO Adapter (FIA) muxes data and clocks of USB 3.1,
> tbt and display controller. In DP alt mode FIA configure the
> number of lanes and will be used apart from DPCD read to calculate max
> available lanes for DP enablement.
> 
> v2 (from Paulo): Simple rebase.
> 
> Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com> (v1).
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> [Paulo: significant rewrite of the patch.]
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++++++++++++++++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 93de6f724e77..72acecaad5c1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10705,5 +10705,8 @@ enum skl_power_gate {
>  #define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
>  #define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
>  #define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
> +#define   DP_LANE_ASSIGNMENT_SHIFT(tc_port)	((tc_port) * 8)
> +#define   DP_LANE_ASSIGNMENT_MASK(tc_port)	(0xf << ((tc_port) * 8))
> +#define   DP_LANE_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 8))
>  
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 90c5ba6b222b..bb59e71d6f9c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -176,14 +176,45 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp)
>  	return intel_dp->common_rates[intel_dp->num_common_rates - 1];
>  }
>  
> +static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> +	u32 lane_info;
> +
> +	if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
> +		return 4;
> +
> +	lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> +		     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> +		    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> +
> +	switch (lane_info) {
> +	default:
> +		MISSING_CASE(lane_info);
> +	case 1:
> +	case 2:
> +	case 4:
> +	case 8:
> +		return 1;
> +	case 3:
> +	case 12:
> +		return 2;
> +	case 15:
> +		return 4;

what about replacing this entire switch per a simple

return hweight8(lane_info);

?

> +	}
> +}
> +
>  /* Theoretical max between source and sink */
>  static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	int source_max = intel_dig_port->max_lanes;
>  	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
> +	int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp);
>  
> -	return min(source_max, sink_max);
> +	return min3(source_max, sink_max, fia_max);
>  }
>  
>  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> -- 
> 2.17.1
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25  1:19   ` Lucas De Marchi
@ 2018-07-25 16:59     ` Paulo Zanoni
  2018-07-25 17:27     ` Paulo Zanoni
  1 sibling, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25 16:59 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Rodrigo Vivi

Em Ter, 2018-07-24 às 18:19 -0700, Lucas De Marchi escreveu:
> On Tue, Jul 24, 2018 at 05:28:09PM -0700, Paulo Zanoni wrote:
> > Do like the other functions and check for the status bits. The "Hot
> > Plug Detection" page from our documentation says we can't just use
> > the
> > ISR bits on the CPU side, so use the correct registers.
> 
> nit: here you are saying it's for all of them rather than just a
> subset.

No, I'm saying it's for the CPU side, the CPU side is a subset which
excludes the PCH. There are 3 types: the legacy ones (on the PCH, not
CPU), the TC ones (on the CPU) and the TBT ones (on the CPU too). On
our driver, "CPU" in these contexts has been used as a synonym for
North Display since the PCH was introduced.

But I can be more explicit if you want, no problem.

> My suggestion is:
> 
> Do like the other functions and check for the status bits. For
> TypeC/TBT
> on North Display Engine the "Hot Plug Detection" page from our
> documentation says we can't just use the ISR bits to find the live
> state, so use the correct registers: DFLEXDPSP TC Live State field.
> 
> 
> > 
> > v2: Rebase.
> > v3:
> >   - Simplify true/false assignment (Rodrigo).
> >   - Reorganize is_gen if ladder (Rodrigo).
> >   - Don't use the ISR for TC/TBT CPU bits.
> > 
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
> >  drivers/gpu/drm/i915/intel_dp.c | 55
> > ++++++++++++++++++++++++++++++++-
> >  2 files changed, 62 insertions(+), 1 deletion(-)
> > 
> > My understanding is that there's agreement on this patch since it
> > just
> > mimicks what the other gens do. Let's have this one agreed on
> > (merged)
> > first before we continue the discussions, especially since this one
> > significantly affects the CI system.
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 73946055aa15..3eaff32dd74e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7210,6 +7210,7 @@ enum {
> >  #define  GEN11_TC3_HOTPLUG			(1 << 18)
> >  #define  GEN11_TC2_HOTPLUG			(1 << 17)
> >  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> > +#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port)
> > + 16))
> >  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLU
> > G | \
> >  						 GEN11_TC3_HOTPLUG
> > | \
> >  						 GEN11_TC2_HOTPLUG
> > | \
> > @@ -7218,6 +7219,7 @@ enum {
> >  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
> >  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
> >  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> > +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 <<
> > (tc_port))
> >  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTP
> > LUG | \
> >  						 GEN11_TBT3_HOTPLU
> > G | \
> >  						 GEN11_TBT2_HOTPLU
> > G | \
> > @@ -7590,6 +7592,8 @@ enum {
> >  #define SDE_GMBUS_ICP			(1 << 23)
> >  #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
> >  #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> > +#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) + 24))
> > +#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
> >  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	
> > \
> >  					 SDE_DDIA_HOTPLUG_ICP)
> >  #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_IC
> > P |	\
> > @@ -10654,4 +10658,8 @@ enum skl_power_gate {
> >  						_ICL_DSC1_RC_BUF_T
> > HRESH_1_UDW_PB, \
> >  						_ICL_DSC1_RC_BUF_T
> > HRESH_1_UDW_PC)
> >  
> > +#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> > +#define   TC_LIVE_STATE_TBT(tc_port)		(1 <<
> > ((tc_port) * 8 + 6))
> > +#define   TC_LIVE_STATE_TC(tc_port)		(1 <<
> > ((tc_port) * 8 + 5))
> > +
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index cd0f649b57a5..998d698788f9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4586,6 +4586,57 @@ static bool
> > bxt_digital_port_connected(struct intel_encoder *encoder)
> >  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
> >  }
> >  
> > +static bool icl_combo_port_connected(struct drm_i915_private
> > *dev_priv,
> > +				     struct intel_digital_port
> > *intel_dig_port)
> > +{
> > +	enum port port = intel_dig_port->base.port;
> > +
> > +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> > +}
> > +
> > +static bool icl_tc_port_connected(struct drm_i915_private
> > *dev_priv,
> > +				  struct intel_digital_port
> > *intel_dig_port)
> > +{
> > +	enum port port = intel_dig_port->base.port;
> > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > +	bool is_legacy, is_typec, is_tbt;
> > +	u32 dpsp;
> > +
> > +	is_legacy = I915_READ(SDEISR) &
> > SDE_TC_HOTPLUG_ICP(tc_port);
> > +
> > +	/*
> > +	 * The spec says we shouldn't be using the ISR bits for
> > detecting
> > +	 * between TC and TBT. We should use DFLEXDPSP.
> > +	 */
> > +	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
> 
> I only wonder if we should be reading the North DE even if is_legacy
> is
> true above rather than using an early return. Anyway:
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> Lucas De Marchi
> 
> > +	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> > +	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> > +
> > +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> > +
> > +	return is_legacy || is_typec || is_tbt;
> > +}
> > +
> > +static bool icl_digital_port_connected(struct intel_encoder
> > *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > >base.dev);
> > +	struct intel_digital_port *dig_port =
> > enc_to_dig_port(&encoder->base);
> > +
> > +	switch (encoder->hpd_pin) {
> > +	case HPD_PORT_A:
> > +	case HPD_PORT_B:
> > +		return icl_combo_port_connected(dev_priv,
> > dig_port);
> > +	case HPD_PORT_C:
> > +	case HPD_PORT_D:
> > +	case HPD_PORT_E:
> > +	case HPD_PORT_F:
> > +		return icl_tc_port_connected(dev_priv, dig_port);
> > +	default:
> > +		MISSING_CASE(encoder->hpd_pin);
> > +		return false;
> > +	}
> > +}
> > +
> >  /*
> >   * intel_digital_port_connected - is the specified port connected?
> >   * @encoder: intel_encoder
> > @@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct
> > intel_encoder *encoder)
> >  		return bdw_digital_port_connected(encoder);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		return bxt_digital_port_connected(encoder);
> > -	else
> > +	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> >  		return spt_digital_port_connected(encoder);
> > +	else
> > +		return icl_digital_port_connected(encoder);
> >  }
> >  
> >  static struct edid *
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/i915/icl: store the port type for TC ports
  2018-07-25 16:41   ` Rodrigo Vivi
@ 2018-07-25 17:03     ` Rodrigo Vivi
  0 siblings, 0 replies; 25+ messages in thread
From: Rodrigo Vivi @ 2018-07-25 17:03 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

On Wed, Jul 25, 2018 at 09:41:18AM -0700, Rodrigo Vivi wrote:
> On Tue, Jul 24, 2018 at 05:28:10PM -0700, Paulo Zanoni wrote:
> > The type is detected based on the live status bits. Once detected,
> > it's not supposed to be changed, so we have some sanity checks for
> > that.
> > 
> > v2: Rebase.
> > 
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.h |  7 +++++
> >  drivers/gpu/drm/i915/intel_dp.c      | 40 ++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  3 files changed, 46 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index 9292001cdd14..0a79a46d5805 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -137,6 +137,13 @@ enum tc_port {
> >  	I915_MAX_TC_PORTS
> >  };
> >  
> > +enum tc_port_type {
> > +	TC_PORT_UNKNOWN = 0,
> > +	TC_PORT_TYPEC,
> > +	TC_PORT_TBT,
> > +	TC_PORT_LEGACY,
> > +};
> > +
> >  enum dpio_channel {
> >  	DPIO_CH0,
> >  	DPIO_CH1
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 998d698788f9..90c5ba6b222b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4594,6 +4594,38 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
> >  	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> >  }
> >  
> > +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) {
> > +		intel_dig_port->tc_type = TC_PORT_LEGACY;
> > +		type_str = "legacy";
> > +	} else if (is_typec) {
> > +		intel_dig_port->tc_type = TC_PORT_TYPEC;
> > +		type_str = "typec";
> > +	} else if (is_tbt) {
> > +		intel_dig_port->tc_type = TC_PORT_TBT;
> > +		type_str = "tbt";
> > +	} else {
> > +		return;
> > +	}
> 
> nip: switch seems more appropriate for this block.
> 
> Anyway:

ops... ignore this part above... arguments come separated...
coffee didn't kick off yet today :P

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > +
> > +	/* Types are not supposed to be changed at runtime. */
> > +	WARN_ON(old_type != TC_PORT_UNKNOWN &&
> > +		old_type != intel_dig_port->tc_type);
> > +
> > +	if (old_type != intel_dig_port->tc_type)
> > +		DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
> > +			      type_str);
> > +}
> > +
> >  static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  				  struct intel_digital_port *intel_dig_port)
> >  {
> > @@ -4612,9 +4644,13 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> >  	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> >  	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> >  
> > -	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> > +	if (!is_legacy && !is_typec && !is_tbt)
> > +		return false;
> > +
> > +	icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec,
> > +				is_tbt);
> >  
> > -	return is_legacy || is_typec || is_tbt;
> > +	return true;
> >  }
> >  
> >  static bool icl_digital_port_connected(struct intel_encoder *encoder)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index c275f91244a6..5e225d8ba09a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1164,6 +1164,7 @@ struct intel_digital_port {
> >  	bool release_cl2_override;
> >  	uint8_t max_lanes;
> >  	enum intel_display_power_domain ddi_io_power_domain;
> > +	enum tc_port_type tc_type;
> >  
> >  	void (*write_infoframe)(struct drm_encoder *encoder,
> >  				const struct intel_crtc_state *crtc_state,
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd.
  2018-07-25 16:52   ` Rodrigo Vivi
@ 2018-07-25 17:12     ` Paulo Zanoni
  2018-07-25 17:13     ` Rodrigo Vivi
  1 sibling, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25 17:12 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

Em Qua, 2018-07-25 às 09:52 -0700, Rodrigo Vivi escreveu:
> On Tue, Jul 24, 2018 at 05:28:11PM -0700, Paulo Zanoni wrote:
> > From: Animesh Manna <animesh.manna@intel.com>
> > 
> > In ICL, Flexible IO Adapter (FIA) muxes data and clocks of USB 3.1,
> > tbt and display controller. In DP alt mode FIA configure the
> > number of lanes and will be used apart from DPCD read to calculate
> > max
> > available lanes for DP enablement.
> > 
> > v2 (from Paulo): Simple rebase.
> > 
> > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com> (v1).
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > [Paulo: significant rewrite of the patch.]
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  3 +++
> >  drivers/gpu/drm/i915/intel_dp.c | 33
> > ++++++++++++++++++++++++++++++++-
> >  2 files changed, 35 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 93de6f724e77..72acecaad5c1 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10705,5 +10705,8 @@ enum skl_power_gate {
> >  #define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> >  #define   TC_LIVE_STATE_TBT(tc_port)		(1 <<
> > ((tc_port) * 8 + 6))
> >  #define   TC_LIVE_STATE_TC(tc_port)		(1 <<
> > ((tc_port) * 8 + 5))
> > +#define   DP_LANE_ASSIGNMENT_SHIFT(tc_port)	((tc_port) * 8)
> > +#define   DP_LANE_ASSIGNMENT_MASK(tc_port)	(0xf <<
> > ((tc_port) * 8))
> > +#define   DP_LANE_ASSIGNMENT(tc_port, x)	((x) << ((tc_port)
> > * 8))
> >  
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 90c5ba6b222b..bb59e71d6f9c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -176,14 +176,45 @@ static int intel_dp_max_common_rate(struct
> > intel_dp *intel_dp)
> >  	return intel_dp->common_rates[intel_dp->num_common_rates -
> > 1];
> >  }
> >  
> > +static int intel_dp_get_fia_supported_lane_count(struct intel_dp
> > *intel_dp)
> > +{
> > +	struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> > +	enum tc_port tc_port = intel_port_to_tc(dev_priv,
> > dig_port->base.port);
> > +	u32 lane_info;
> > +
> > +	if (tc_port == PORT_TC_NONE || dig_port->tc_type !=
> > TC_PORT_TYPEC)
> > +		return 4;
> > +
> > +	lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> > +		     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > +		    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +
> > +	switch (lane_info) {
> > +	default:
> > +		MISSING_CASE(lane_info);
> > +	case 1:
> > +	case 2:
> > +	case 4:
> > +	case 8:
> > +		return 1;
> > +	case 3:
> > +	case 12:
> > +		return 2;
> > +	case 15:
> > +		return 4;
> 
> what about replacing this entire switch per a simple
> 
> return hweight8(lane_info);
> 
> ?

It kills our ability to hit MISSING_CASE for 5, 6, 7, 9, 10, 11, 13 and
14. I think it's always good to revalidate our hardware/spec
assumptions for the newer platforms (although we don't seem to be
hitting this as far as I have tested).

> 
> > +	}
> > +}
> > +
> >  /* Theoretical max between source and sink */
> >  static int intel_dp_max_common_lane_count(struct intel_dp
> > *intel_dp)
> >  {
> >  	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> >  	int source_max = intel_dig_port->max_lanes;
> >  	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
> > +	int fia_max =
> > intel_dp_get_fia_supported_lane_count(intel_dp);
> >  
> > -	return min(source_max, sink_max);
> > +	return min3(source_max, sink_max, fia_max);
> >  }
> >  
> >  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd.
  2018-07-25 16:52   ` Rodrigo Vivi
  2018-07-25 17:12     ` Paulo Zanoni
@ 2018-07-25 17:13     ` Rodrigo Vivi
  1 sibling, 0 replies; 25+ messages in thread
From: Rodrigo Vivi @ 2018-07-25 17:13 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

On Wed, Jul 25, 2018 at 09:52:25AM -0700, Rodrigo Vivi wrote:
> On Tue, Jul 24, 2018 at 05:28:11PM -0700, Paulo Zanoni wrote:
> > From: Animesh Manna <animesh.manna@intel.com>
> > 
> > In ICL, Flexible IO Adapter (FIA) muxes data and clocks of USB 3.1,
> > tbt and display controller. In DP alt mode FIA configure the
> > number of lanes and will be used apart from DPCD read to calculate max
> > available lanes for DP enablement.
> > 
> > v2 (from Paulo): Simple rebase.
> > 
> > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com> (v1).
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > [Paulo: significant rewrite of the patch.]
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  3 +++
> >  drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++++++++++++++++-
> >  2 files changed, 35 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 93de6f724e77..72acecaad5c1 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10705,5 +10705,8 @@ enum skl_power_gate {
> >  #define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> >  #define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
> >  #define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
> > +#define   DP_LANE_ASSIGNMENT_SHIFT(tc_port)	((tc_port) * 8)
> > +#define   DP_LANE_ASSIGNMENT_MASK(tc_port)	(0xf << ((tc_port) * 8))
> > +#define   DP_LANE_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 8))
> >  
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 90c5ba6b222b..bb59e71d6f9c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -176,14 +176,45 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp)
> >  	return intel_dp->common_rates[intel_dp->num_common_rates - 1];
> >  }
> >  
> > +static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> > +	u32 lane_info;
> > +
> > +	if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
> > +		return 4;
> > +
> > +	lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
> > +		     DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > +		    DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +
> > +	switch (lane_info) {
> > +	default:
> > +		MISSING_CASE(lane_info);
> > +	case 1:
> > +	case 2:
> > +	case 4:
> > +	case 8:
> > +		return 1;
> > +	case 3:
> > +	case 12:
> > +		return 2;
> > +	case 15:
> > +		return 4;
> 
> what about replacing this entire switch per a simple
> 
> return hweight8(lane_info);

ignore this.
specially considering that SOC FW sets these bits, not HW,
I agree that MISSING_CASE is very important.


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



> 
> ?
> 
> > +	}
> > +}
> > +
> >  /* Theoretical max between source and sink */
> >  static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >  	int source_max = intel_dig_port->max_lanes;
> >  	int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
> > +	int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp);
> >  
> > -	return min(source_max, sink_max);
> > +	return min3(source_max, sink_max, fia_max);
> >  }
> >  
> >  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25  1:19   ` Lucas De Marchi
  2018-07-25 16:59     ` Paulo Zanoni
@ 2018-07-25 17:27     ` Paulo Zanoni
  2018-07-25 17:59       ` Paulo Zanoni
  1 sibling, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25 17:27 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Rodrigo Vivi

Em Ter, 2018-07-24 às 18:19 -0700, Lucas De Marchi escreveu:
> On Tue, Jul 24, 2018 at 05:28:09PM -0700, Paulo Zanoni wrote:
> > Do like the other functions and check for the status bits. The "Hot
> > Plug Detection" page from our documentation says we can't just use
> > the
> > ISR bits on the CPU side, so use the correct registers.
> 
> nit: here you are saying it's for all of them rather than just a
> subset.
> My suggestion is:
> 
> Do like the other functions and check for the status bits. For
> TypeC/TBT
> on North Display Engine the "Hot Plug Detection" page from our
> documentation says we can't just use the ISR bits to find the live
> state, so use the correct registers: DFLEXDPSP TC Live State field.
> 
> 
> > 
> > v2: Rebase.
> > v3:
> >   - Simplify true/false assignment (Rodrigo).
> >   - Reorganize is_gen if ladder (Rodrigo).
> >   - Don't use the ISR for TC/TBT CPU bits.
> > 
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
> >  drivers/gpu/drm/i915/intel_dp.c | 55
> > ++++++++++++++++++++++++++++++++-
> >  2 files changed, 62 insertions(+), 1 deletion(-)
> > 
> > My understanding is that there's agreement on this patch since it
> > just
> > mimicks what the other gens do. Let's have this one agreed on
> > (merged)
> > first before we continue the discussions, especially since this one
> > significantly affects the CI system.
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 73946055aa15..3eaff32dd74e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7210,6 +7210,7 @@ enum {
> >  #define  GEN11_TC3_HOTPLUG			(1 << 18)
> >  #define  GEN11_TC2_HOTPLUG			(1 << 17)
> >  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> > +#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port)
> > + 16))
> >  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLU
> > G | \
> >  						 GEN11_TC3_HOTPLUG
> > | \
> >  						 GEN11_TC2_HOTPLUG
> > | \
> > @@ -7218,6 +7219,7 @@ enum {
> >  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
> >  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
> >  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> > +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 <<
> > (tc_port))
> >  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTP
> > LUG | \
> >  						 GEN11_TBT3_HOTPLU
> > G | \
> >  						 GEN11_TBT2_HOTPLU
> > G | \
> > @@ -7590,6 +7592,8 @@ enum {
> >  #define SDE_GMBUS_ICP			(1 << 23)
> >  #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
> >  #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> > +#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) + 24))
> > +#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
> >  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	
> > \
> >  					 SDE_DDIA_HOTPLUG_ICP)
> >  #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_IC
> > P |	\
> > @@ -10654,4 +10658,8 @@ enum skl_power_gate {
> >  						_ICL_DSC1_RC_BUF_T
> > HRESH_1_UDW_PB, \
> >  						_ICL_DSC1_RC_BUF_T
> > HRESH_1_UDW_PC)
> >  
> > +#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> > +#define   TC_LIVE_STATE_TBT(tc_port)		(1 <<
> > ((tc_port) * 8 + 6))
> > +#define   TC_LIVE_STATE_TC(tc_port)		(1 <<
> > ((tc_port) * 8 + 5))
> > +
> >  #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index cd0f649b57a5..998d698788f9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4586,6 +4586,57 @@ static bool
> > bxt_digital_port_connected(struct intel_encoder *encoder)
> >  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
> >  }
> >  
> > +static bool icl_combo_port_connected(struct drm_i915_private
> > *dev_priv,
> > +				     struct intel_digital_port
> > *intel_dig_port)
> > +{
> > +	enum port port = intel_dig_port->base.port;
> > +
> > +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> > +}
> > +
> > +static bool icl_tc_port_connected(struct drm_i915_private
> > *dev_priv,
> > +				  struct intel_digital_port
> > *intel_dig_port)
> > +{
> > +	enum port port = intel_dig_port->base.port;
> > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > +	bool is_legacy, is_typec, is_tbt;
> > +	u32 dpsp;
> > +
> > +	is_legacy = I915_READ(SDEISR) &
> > SDE_TC_HOTPLUG_ICP(tc_port);
> > +
> > +	/*
> > +	 * The spec says we shouldn't be using the ISR bits for
> > detecting
> > +	 * between TC and TBT. We should use DFLEXDPSP.
> > +	 */
> > +	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
> 
> I only wonder if we should be reading the North DE even if is_legacy
> is
> true above rather than using an early return. Anyway:

(missed this part in the other email)

That makes sense in the current state of the code. It would remove our
ability to hit the WARN_ON below, but that doesn't seem to be a major
issue.

Now when we go back to discussing our implementation for the connect
flows we may end up having to revert to the current form, but we can
always do that if we need it.

> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Thanks! I'll submit v4.

> 
> Lucas De Marchi
> 
> > +	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> > +	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> > +
> > +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> > +
> > +	return is_legacy || is_typec || is_tbt;
> > +}
> > +
> > +static bool icl_digital_port_connected(struct intel_encoder
> > *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > >base.dev);
> > +	struct intel_digital_port *dig_port =
> > enc_to_dig_port(&encoder->base);
> > +
> > +	switch (encoder->hpd_pin) {
> > +	case HPD_PORT_A:
> > +	case HPD_PORT_B:
> > +		return icl_combo_port_connected(dev_priv,
> > dig_port);
> > +	case HPD_PORT_C:
> > +	case HPD_PORT_D:
> > +	case HPD_PORT_E:
> > +	case HPD_PORT_F:
> > +		return icl_tc_port_connected(dev_priv, dig_port);
> > +	default:
> > +		MISSING_CASE(encoder->hpd_pin);
> > +		return false;
> > +	}
> > +}
> > +
> >  /*
> >   * intel_digital_port_connected - is the specified port connected?
> >   * @encoder: intel_encoder
> > @@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct
> > intel_encoder *encoder)
> >  		return bdw_digital_port_connected(encoder);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		return bxt_digital_port_connected(encoder);
> > -	else
> > +	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> >  		return spt_digital_port_connected(encoder);
> > +	else
> > +		return icl_digital_port_connected(encoder);
> >  }
> >  
> >  static struct edid *
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25 17:27     ` Paulo Zanoni
@ 2018-07-25 17:59       ` Paulo Zanoni
  2018-07-25 20:04         ` Lucas De Marchi
  0 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25 17:59 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Rodrigo Vivi

Em Qua, 2018-07-25 às 10:27 -0700, Paulo Zanoni escreveu:
> Em Ter, 2018-07-24 às 18:19 -0700, Lucas De Marchi escreveu:
> > On Tue, Jul 24, 2018 at 05:28:09PM -0700, Paulo Zanoni wrote:
> > > Do like the other functions and check for the status bits. The
> > > "Hot
> > > Plug Detection" page from our documentation says we can't just
> > > use
> > > the
> > > ISR bits on the CPU side, so use the correct registers.
> > 
> > nit: here you are saying it's for all of them rather than just a
> > subset.
> > My suggestion is:
> > 
> > Do like the other functions and check for the status bits. For
> > TypeC/TBT
> > on North Display Engine the "Hot Plug Detection" page from our
> > documentation says we can't just use the ISR bits to find the live
> > state, so use the correct registers: DFLEXDPSP TC Live State field.
> > 
> > 
> > > 
> > > v2: Rebase.
> > > v3:
> > >   - Simplify true/false assignment (Rodrigo).
> > >   - Reorganize is_gen if ladder (Rodrigo).
> > >   - Don't use the ISR for TC/TBT CPU bits.
> > > 
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
> > >  drivers/gpu/drm/i915/intel_dp.c | 55
> > > ++++++++++++++++++++++++++++++++-
> > >  2 files changed, 62 insertions(+), 1 deletion(-)
> > > 
> > > My understanding is that there's agreement on this patch since it
> > > just
> > > mimicks what the other gens do. Let's have this one agreed on
> > > (merged)
> > > first before we continue the discussions, especially since this
> > > one
> > > significantly affects the CI system.
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 73946055aa15..3eaff32dd74e 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7210,6 +7210,7 @@ enum {
> > >  #define  GEN11_TC3_HOTPLUG			(1 << 18)
> > >  #define  GEN11_TC2_HOTPLUG			(1 << 17)
> > >  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> > > +#define  GEN11_TC_HOTPLUG(tc_port)		(1 <<
> > > ((tc_port)
> > > + 16))
> > >  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTP
> > > LU
> > > G | \
> > >  						 GEN11_TC3_HOTPL
> > > UG
> > > > \
> > > 
> > >  						 GEN11_TC2_HOTPL
> > > UG
> > > > \
> > > 
> > > @@ -7218,6 +7219,7 @@ enum {
> > >  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
> > >  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
> > >  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> > > +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 <<
> > > (tc_port))
> > >  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HO
> > > TP
> > > LUG | \
> > >  						 GEN11_TBT3_HOTP
> > > LU
> > > G | \
> > >  						 GEN11_TBT2_HOTP
> > > LU
> > > G | \
> > > @@ -7590,6 +7592,8 @@ enum {
> > >  #define SDE_GMBUS_ICP			(1 << 23)
> > >  #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
> > >  #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> > > +#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) +
> > > 24))
> > > +#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
> > >  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	
> > > \
> > >  					 SDE_DDIA_HOTPLUG_ICP)
> > >  #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_
> > > IC
> > > P |	\
> > > @@ -10654,4 +10658,8 @@ enum skl_power_gate {
> > >  						_ICL_DSC1_RC_BUF
> > > _T
> > > HRESH_1_UDW_PB, \
> > >  						_ICL_DSC1_RC_BUF
> > > _T
> > > HRESH_1_UDW_PC)
> > >  
> > > +#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> > > +#define   TC_LIVE_STATE_TBT(tc_port)		(1 <<
> > > ((tc_port) * 8 + 6))
> > > +#define   TC_LIVE_STATE_TC(tc_port)		(1 <<
> > > ((tc_port) * 8 + 5))
> > > +
> > >  #endif /* _I915_REG_H_ */
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index cd0f649b57a5..998d698788f9 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4586,6 +4586,57 @@ static bool
> > > bxt_digital_port_connected(struct intel_encoder *encoder)
> > >  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
> > >  }
> > >  
> > > +static bool icl_combo_port_connected(struct drm_i915_private
> > > *dev_priv,
> > > +				     struct intel_digital_port
> > > *intel_dig_port)
> > > +{
> > > +	enum port port = intel_dig_port->base.port;
> > > +
> > > +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> > > +}
> > > +
> > > +static bool icl_tc_port_connected(struct drm_i915_private
> > > *dev_priv,
> > > +				  struct intel_digital_port
> > > *intel_dig_port)
> > > +{
> > > +	enum port port = intel_dig_port->base.port;
> > > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > > +	bool is_legacy, is_typec, is_tbt;
> > > +	u32 dpsp;
> > > +
> > > +	is_legacy = I915_READ(SDEISR) &
> > > SDE_TC_HOTPLUG_ICP(tc_port);
> > > +
> > > +	/*
> > > +	 * The spec says we shouldn't be using the ISR bits for
> > > detecting
> > > +	 * between TC and TBT. We should use DFLEXDPSP.
> > > +	 */
> > > +	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
> > 
> > I only wonder if we should be reading the North DE even if
> > is_legacy
> > is
> > true above rather than using an early return. Anyway:
> 
> (missed this part in the other email)
> 
> That makes sense in the current state of the code. It would remove
> our
> ability to hit the WARN_ON below, but that doesn't seem to be a major
> issue.
> 
> Now when we go back to discussing our implementation for the connect
> flows we may end up having to revert to the current form, but we can
> always do that if we need it.

Forget what I just said: patch 2 will already have to revert the early
return.

> 
> > 
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> Thanks! I'll submit v4.
> 
> > 
> > Lucas De Marchi
> > 
> > > +	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> > > +	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> > > +
> > > +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> > > +
> > > +	return is_legacy || is_typec || is_tbt;
> > > +}
> > > +
> > > +static bool icl_digital_port_connected(struct intel_encoder
> > > *encoder)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > > > base.dev);
> > > 
> > > +	struct intel_digital_port *dig_port =
> > > enc_to_dig_port(&encoder->base);
> > > +
> > > +	switch (encoder->hpd_pin) {
> > > +	case HPD_PORT_A:
> > > +	case HPD_PORT_B:
> > > +		return icl_combo_port_connected(dev_priv,
> > > dig_port);
> > > +	case HPD_PORT_C:
> > > +	case HPD_PORT_D:
> > > +	case HPD_PORT_E:
> > > +	case HPD_PORT_F:
> > > +		return icl_tc_port_connected(dev_priv,
> > > dig_port);
> > > +	default:
> > > +		MISSING_CASE(encoder->hpd_pin);
> > > +		return false;
> > > +	}
> > > +}
> > > +
> > >  /*
> > >   * intel_digital_port_connected - is the specified port
> > > connected?
> > >   * @encoder: intel_encoder
> > > @@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct
> > > intel_encoder *encoder)
> > >  		return bdw_digital_port_connected(encoder);
> > >  	else if (IS_GEN9_LP(dev_priv))
> > >  		return bxt_digital_port_connected(encoder);
> > > -	else
> > > +	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> > >  		return spt_digital_port_connected(encoder);
> > > +	else
> > > +		return icl_digital_port_connected(encoder);
> > >  }
> > >  
> > >  static struct edid *
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 1/5] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
  2018-07-25  1:19   ` Lucas De Marchi
@ 2018-07-25 19:59   ` Paulo Zanoni
  1 sibling, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2018-07-25 19:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi, Paulo Zanoni, Rodrigo Vivi

Do like the other functions and check for the status bits. The "Hot Plug
Detection" page from our documentation says we can't just use the ISR bits on
the CPU side (North Display, which has the TC and TBT modes), so use the correct
register: DFLEXDPSP, TC Live State field.

v2: Rebase.
v3:
  - Simplify true/false assignment (Rodrigo).
  - Reorganize is_gen if ladder (Rodrigo).
  - Don't use the ISR for TC/TBT CPU bits.
v4:
  - Improve commit message wording (Lucas).

Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> (v3).
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  8 ++++++
 drivers/gpu/drm/i915/intel_dp.c | 55 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 477e694b8cc4..93de6f724e77 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7254,6 +7254,7 @@ enum {
 #define  GEN11_TC3_HOTPLUG			(1 << 18)
 #define  GEN11_TC2_HOTPLUG			(1 << 17)
 #define  GEN11_TC1_HOTPLUG			(1 << 16)
+#define  GEN11_TC_HOTPLUG(tc_port)		(1 << ((tc_port) + 16))
 #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTPLUG | \
 						 GEN11_TC3_HOTPLUG | \
 						 GEN11_TC2_HOTPLUG | \
@@ -7262,6 +7263,7 @@ enum {
 #define  GEN11_TBT3_HOTPLUG			(1 << 2)
 #define  GEN11_TBT2_HOTPLUG			(1 << 1)
 #define  GEN11_TBT1_HOTPLUG			(1 << 0)
+#define  GEN11_TBT_HOTPLUG(tc_port)		(1 << (tc_port))
 #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HOTPLUG | \
 						 GEN11_TBT3_HOTPLUG | \
 						 GEN11_TBT2_HOTPLUG | \
@@ -7634,6 +7636,8 @@ enum {
 #define SDE_GMBUS_ICP			(1 << 23)
 #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
 #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
+#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) + 24))
+#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
 #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	\
 					 SDE_DDIA_HOTPLUG_ICP)
 #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_ICP |	\
@@ -10698,4 +10702,8 @@ enum skl_power_gate {
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
 
+#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
+#define   TC_LIVE_STATE_TBT(tc_port)		(1 << ((tc_port) * 8 + 6))
+#define   TC_LIVE_STATE_TC(tc_port)		(1 << ((tc_port) * 8 + 5))
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cd0f649b57a5..998d698788f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4586,6 +4586,57 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
 	return I915_READ(GEN8_DE_PORT_ISR) & bit;
 }
 
+static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
+				     struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+
+	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
+}
+
+static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
+				  struct intel_digital_port *intel_dig_port)
+{
+	enum port port = intel_dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	bool is_legacy, is_typec, is_tbt;
+	u32 dpsp;
+
+	is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
+
+	/*
+	 * The spec says we shouldn't be using the ISR bits for detecting
+	 * between TC and TBT. We should use DFLEXDPSP.
+	 */
+	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
+	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
+	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
+
+	WARN_ON(is_legacy + is_typec + is_tbt > 1);
+
+	return is_legacy || is_typec || is_tbt;
+}
+
+static bool icl_digital_port_connected(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	switch (encoder->hpd_pin) {
+	case HPD_PORT_A:
+	case HPD_PORT_B:
+		return icl_combo_port_connected(dev_priv, dig_port);
+	case HPD_PORT_C:
+	case HPD_PORT_D:
+	case HPD_PORT_E:
+	case HPD_PORT_F:
+		return icl_tc_port_connected(dev_priv, dig_port);
+	default:
+		MISSING_CASE(encoder->hpd_pin);
+		return false;
+	}
+}
+
 /*
  * intel_digital_port_connected - is the specified port connected?
  * @encoder: intel_encoder
@@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
 		return bdw_digital_port_connected(encoder);
 	else if (IS_GEN9_LP(dev_priv))
 		return bxt_digital_port_connected(encoder);
-	else
+	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
 		return spt_digital_port_connected(encoder);
+	else
+		return icl_digital_port_connected(encoder);
 }
 
 static struct edid *
-- 
2.14.4

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] drm/i915/icl: implement icl_digital_port_connected()
  2018-07-25 17:59       ` Paulo Zanoni
@ 2018-07-25 20:04         ` Lucas De Marchi
  0 siblings, 0 replies; 25+ messages in thread
From: Lucas De Marchi @ 2018-07-25 20:04 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Rodrigo Vivi

On Wed, Jul 25, 2018 at 10:59:32AM -0700, Paulo Zanoni wrote:
> Em Qua, 2018-07-25 às 10:27 -0700, Paulo Zanoni escreveu:
> > Em Ter, 2018-07-24 às 18:19 -0700, Lucas De Marchi escreveu:
> > > On Tue, Jul 24, 2018 at 05:28:09PM -0700, Paulo Zanoni wrote:
> > > > Do like the other functions and check for the status bits. The
> > > > "Hot
> > > > Plug Detection" page from our documentation says we can't just
> > > > use
> > > > the
> > > > ISR bits on the CPU side, so use the correct registers.
> > > 
> > > nit: here you are saying it's for all of them rather than just a
> > > subset.
> > > My suggestion is:
> > > 
> > > Do like the other functions and check for the status bits. For
> > > TypeC/TBT
> > > on North Display Engine the "Hot Plug Detection" page from our
> > > documentation says we can't just use the ISR bits to find the live
> > > state, so use the correct registers: DFLEXDPSP TC Live State field.
> > > 
> > > 
> > > > 
> > > > v2: Rebase.
> > > > v3:
> > > >   - Simplify true/false assignment (Rodrigo).
> > > >   - Reorganize is_gen if ladder (Rodrigo).
> > > >   - Don't use the ISR for TC/TBT CPU bits.
> > > > 
> > > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h |  8 +++++
> > > >  drivers/gpu/drm/i915/intel_dp.c | 55
> > > > ++++++++++++++++++++++++++++++++-
> > > >  2 files changed, 62 insertions(+), 1 deletion(-)
> > > > 
> > > > My understanding is that there's agreement on this patch since it
> > > > just
> > > > mimicks what the other gens do. Let's have this one agreed on
> > > > (merged)
> > > > first before we continue the discussions, especially since this
> > > > one
> > > > significantly affects the CI system.
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 73946055aa15..3eaff32dd74e 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -7210,6 +7210,7 @@ enum {
> > > >  #define  GEN11_TC3_HOTPLUG			(1 << 18)
> > > >  #define  GEN11_TC2_HOTPLUG			(1 << 17)
> > > >  #define  GEN11_TC1_HOTPLUG			(1 << 16)
> > > > +#define  GEN11_TC_HOTPLUG(tc_port)		(1 <<
> > > > ((tc_port)
> > > > + 16))
> > > >  #define  GEN11_DE_TC_HOTPLUG_MASK		(GEN11_TC4_HOTP
> > > > LU
> > > > G | \
> > > >  						 GEN11_TC3_HOTPL
> > > > UG
> > > > > \
> > > > 
> > > >  						 GEN11_TC2_HOTPL
> > > > UG
> > > > > \
> > > > 
> > > > @@ -7218,6 +7219,7 @@ enum {
> > > >  #define  GEN11_TBT3_HOTPLUG			(1 << 2)
> > > >  #define  GEN11_TBT2_HOTPLUG			(1 << 1)
> > > >  #define  GEN11_TBT1_HOTPLUG			(1 << 0)
> > > > +#define  GEN11_TBT_HOTPLUG(tc_port)		(1 <<
> > > > (tc_port))
> > > >  #define  GEN11_DE_TBT_HOTPLUG_MASK		(GEN11_TBT4_HO
> > > > TP
> > > > LUG | \
> > > >  						 GEN11_TBT3_HOTP
> > > > LU
> > > > G | \
> > > >  						 GEN11_TBT2_HOTP
> > > > LU
> > > > G | \
> > > > @@ -7590,6 +7592,8 @@ enum {
> > > >  #define SDE_GMBUS_ICP			(1 << 23)
> > > >  #define SDE_DDIB_HOTPLUG_ICP		(1 << 17)
> > > >  #define SDE_DDIA_HOTPLUG_ICP		(1 << 16)
> > > > +#define SDE_TC_HOTPLUG_ICP(tc_port)	(1 << ((tc_port) +
> > > > 24))
> > > > +#define SDE_DDI_HOTPLUG_ICP(port)	(1 << ((port) + 16))
> > > >  #define SDE_DDI_MASK_ICP		(SDE_DDIB_HOTPLUG_ICP |	
> > > > \
> > > >  					 SDE_DDIA_HOTPLUG_ICP)
> > > >  #define SDE_TC_MASK_ICP			(SDE_TC4_HOTPLUG_
> > > > IC
> > > > P |	\
> > > > @@ -10654,4 +10658,8 @@ enum skl_power_gate {
> > > >  						_ICL_DSC1_RC_BUF
> > > > _T
> > > > HRESH_1_UDW_PB, \
> > > >  						_ICL_DSC1_RC_BUF
> > > > _T
> > > > HRESH_1_UDW_PC)
> > > >  
> > > > +#define PORT_TX_DFLEXDPSP			_MMIO(0x1638A0)
> > > > +#define   TC_LIVE_STATE_TBT(tc_port)		(1 <<
> > > > ((tc_port) * 8 + 6))
> > > > +#define   TC_LIVE_STATE_TC(tc_port)		(1 <<
> > > > ((tc_port) * 8 + 5))
> > > > +
> > > >  #endif /* _I915_REG_H_ */
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index cd0f649b57a5..998d698788f9 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4586,6 +4586,57 @@ static bool
> > > > bxt_digital_port_connected(struct intel_encoder *encoder)
> > > >  	return I915_READ(GEN8_DE_PORT_ISR) & bit;
> > > >  }
> > > >  
> > > > +static bool icl_combo_port_connected(struct drm_i915_private
> > > > *dev_priv,
> > > > +				     struct intel_digital_port
> > > > *intel_dig_port)
> > > > +{
> > > > +	enum port port = intel_dig_port->base.port;
> > > > +
> > > > +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> > > > +}
> > > > +
> > > > +static bool icl_tc_port_connected(struct drm_i915_private
> > > > *dev_priv,
> > > > +				  struct intel_digital_port
> > > > *intel_dig_port)
> > > > +{
> > > > +	enum port port = intel_dig_port->base.port;
> > > > +	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > > > +	bool is_legacy, is_typec, is_tbt;
> > > > +	u32 dpsp;
> > > > +
> > > > +	is_legacy = I915_READ(SDEISR) &
> > > > SDE_TC_HOTPLUG_ICP(tc_port);
> > > > +
> > > > +	/*
> > > > +	 * The spec says we shouldn't be using the ISR bits for
> > > > detecting
> > > > +	 * between TC and TBT. We should use DFLEXDPSP.
> > > > +	 */
> > > > +	dpsp = I915_READ(PORT_TX_DFLEXDPSP);
> > > 
> > > I only wonder if we should be reading the North DE even if
> > > is_legacy
> > > is
> > > true above rather than using an early return. Anyway:
> > 
> > (missed this part in the other email)
> > 
> > That makes sense in the current state of the code. It would remove
> > our
> > ability to hit the WARN_ON below, but that doesn't seem to be a major
> > issue.
> > 
> > Now when we go back to discussing our implementation for the connect
> > flows we may end up having to revert to the current form, but we can
> > always do that if we need it.
> 
> Forget what I just said: patch 2 will already have to revert the early
> return.

yeah, I should have read patch 2 before commenting here. I thought we
were moving away from updating the type while we check if it's
connected.

LGTM

Lucas De Marchi

> 
> > 
> > > 
> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > 
> > Thanks! I'll submit v4.
> > 
> > > 
> > > Lucas De Marchi
> > > 
> > > > +	is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
> > > > +	is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
> > > > +
> > > > +	WARN_ON(is_legacy + is_typec + is_tbt > 1);
> > > > +
> > > > +	return is_legacy || is_typec || is_tbt;
> > > > +}
> > > > +
> > > > +static bool icl_digital_port_connected(struct intel_encoder
> > > > *encoder)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > > > > base.dev);
> > > > 
> > > > +	struct intel_digital_port *dig_port =
> > > > enc_to_dig_port(&encoder->base);
> > > > +
> > > > +	switch (encoder->hpd_pin) {
> > > > +	case HPD_PORT_A:
> > > > +	case HPD_PORT_B:
> > > > +		return icl_combo_port_connected(dev_priv,
> > > > dig_port);
> > > > +	case HPD_PORT_C:
> > > > +	case HPD_PORT_D:
> > > > +	case HPD_PORT_E:
> > > > +	case HPD_PORT_F:
> > > > +		return icl_tc_port_connected(dev_priv,
> > > > dig_port);
> > > > +	default:
> > > > +		MISSING_CASE(encoder->hpd_pin);
> > > > +		return false;
> > > > +	}
> > > > +}
> > > > +
> > > >  /*
> > > >   * intel_digital_port_connected - is the specified port
> > > > connected?
> > > >   * @encoder: intel_encoder
> > > > @@ -4613,8 +4664,10 @@ bool intel_digital_port_connected(struct
> > > > intel_encoder *encoder)
> > > >  		return bdw_digital_port_connected(encoder);
> > > >  	else if (IS_GEN9_LP(dev_priv))
> > > >  		return bxt_digital_port_connected(encoder);
> > > > -	else
> > > > +	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> > > >  		return spt_digital_port_connected(encoder);
> > > > +	else
> > > > +		return icl_digital_port_connected(encoder);
> > > >  }
> > > >  
> > > >  static struct edid *
> > > > -- 
> > > > 2.17.1
> > > > 
> > > > _______________________________________________
> > > > 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: implement icl_digital_port_connected() (rev3)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (9 preceding siblings ...)
  2018-07-25 12:29 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-25 20:23 ` Patchwork
  2018-07-25 20:26 ` ✗ Fi.CI.SPARSE: " Patchwork
  2018-07-25 20:46 ` ✗ Fi.CI.BAT: failure " Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25 20:23 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev3)
URL   : https://patchwork.freedesktop.org/series/47151/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
82dbbab3500e drm/i915/icl: implement icl_digital_port_connected()
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7: 
Detection" page from our documentation says we can't just use the ISR bits on

total: 0 errors, 1 warnings, 0 checks, 98 lines checked
e2119a7b93d3 drm/i915/icl: store the port type for TC ports
3d2f8c9f4cbd drm/i915/icl: Update FIA supported lane count for hpd.
c797f962af7e drm/i915/icl: program MG_DP_MODE
7d1f77f586ea drm/i915/icl: toggle PHY clock gating around link training

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/icl: implement icl_digital_port_connected() (rev3)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (10 preceding siblings ...)
  2018-07-25 20:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: implement icl_digital_port_connected() (rev3) Patchwork
@ 2018-07-25 20:26 ` Patchwork
  2018-07-25 20:46 ` ✗ Fi.CI.BAT: failure " Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25 20:26 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev3)
URL   : https://patchwork.freedesktop.org/series/47151/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/icl: implement icl_digital_port_connected()
Okay!

Commit: drm/i915/icl: store the port type for TC ports
Okay!

Commit: drm/i915/icl: Update FIA supported lane count for hpd.
-O:drivers/gpu/drm/i915/intel_dp.c:186:16: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_dp.c:186:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dp.c:217:16: warning: expression using sizeof(void)

Commit: drm/i915/icl: program MG_DP_MODE
Okay!

Commit: drm/i915/icl: toggle PHY clock gating around link training
Okay!

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/icl: implement icl_digital_port_connected() (rev3)
  2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
                   ` (11 preceding siblings ...)
  2018-07-25 20:26 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-07-25 20:46 ` Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2018-07-25 20:46 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/icl: implement icl_digital_port_connected() (rev3)
URL   : https://patchwork.freedesktop.org/series/47151/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4542 -> Patchwork_9769 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9769 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9769, 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/47151/revisions/3/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_objects:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@drv_module_reload@basic-no-display:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#105395) +5

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> DMESG-FAIL (fdo#106560, fdo#106947)
      fi-bdw-5557u:       PASS -> DMESG-FAIL (fdo#106560)

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107292)

    igt@gem_exec_nop@basic-series:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097, fdo#106000)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-glk-j4005:       PASS -> FAIL (fdo#106211)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-cfl-8109u}:     PASS -> INCOMPLETE (fdo#106070)

    {igt@kms_psr@cursor_plane_move}:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107372)

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#107372) +1

    
    ==== Possible fixes ====

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       FAIL (fdo#103841) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       FAIL (fdo#100368) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (50 -> 45) ==

  Additional (1): fi-cnl-psr 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4542 -> Patchwork_9769

  CI_DRM_4542: b804f30998ca9919cf37c73fc37fbd6a4c6c5e38 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4575: fe908a01012c9daafafb3410b9407725ca9d4f21 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9769: 7d1f77f586ea723387bb3c4866f8218e0975a6b5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7d1f77f586ea drm/i915/icl: toggle PHY clock gating around link training
c797f962af7e drm/i915/icl: program MG_DP_MODE
3d2f8c9f4cbd drm/i915/icl: Update FIA supported lane count for hpd.
e2119a7b93d3 drm/i915/icl: store the port type for TC ports
82dbbab3500e drm/i915/icl: implement icl_digital_port_connected()

== Logs ==

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-07-25 20:46 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-25  0:28 [PATCH 0/5] Remaining ICL display patches, v2 Paulo Zanoni
2018-07-25  0:28 ` [PATCH] drm/i915/icl: implement icl_digital_port_connected() Paulo Zanoni
2018-07-25  1:19   ` Lucas De Marchi
2018-07-25 16:59     ` Paulo Zanoni
2018-07-25 17:27     ` Paulo Zanoni
2018-07-25 17:59       ` Paulo Zanoni
2018-07-25 20:04         ` Lucas De Marchi
2018-07-25 19:59   ` [PATCH v4 1/5] " Paulo Zanoni
2018-07-25  0:28 ` [PATCH 2/5] drm/i915/icl: store the port type for TC ports Paulo Zanoni
2018-07-25 16:41   ` Rodrigo Vivi
2018-07-25 17:03     ` Rodrigo Vivi
2018-07-25  0:28 ` [PATCH 3/5] drm/i915/icl: Update FIA supported lane count for hpd Paulo Zanoni
2018-07-25 16:52   ` Rodrigo Vivi
2018-07-25 17:12     ` Paulo Zanoni
2018-07-25 17:13     ` Rodrigo Vivi
2018-07-25  0:28 ` [PATCH 4/5] drm/i915/icl: program MG_DP_MODE Paulo Zanoni
2018-07-25  0:28 ` [PATCH 5/5] drm/i915/icl: toggle PHY clock gating around link training Paulo Zanoni
2018-07-25  0:52 ` ✗ Fi.CI.SPARSE: warning for drm/i915/icl: implement icl_digital_port_connected() (rev2) Patchwork
2018-07-25  1:09 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-25  2:08 ` ✓ Fi.CI.IGT: " Patchwork
2018-07-25 11:07 ` ✓ Fi.CI.BAT: " Patchwork
2018-07-25 12:29 ` ✓ Fi.CI.IGT: " Patchwork
2018-07-25 20:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: implement icl_digital_port_connected() (rev3) Patchwork
2018-07-25 20:26 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-07-25 20:46 ` ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.