All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] TGL TC enabling
@ 2019-09-13 22:32 José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence José Roberto de Souza
                   ` (16 more replies)
  0 siblings, 17 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

This is all the patches required to have TC alt-mode working on TGL, no TBT or legacy support intented here but much of the work here will help those.

The dkl pll calculation is not 100% ready, so it is using the hardcoded table provided but even with this table it results in a port_clock state mismatch when running at 5.4Ghz.
Also I'm still debugging why enable clock gating after link training complete breaks all the following trainings.
All of above is noted in the respective commit message.

Other than the above the series is pretty much ready for reviews and testing.
Make sure you have firmware of TC retimers updated.

José Roberto de Souza (5):
  drm/i915/tgl: Finish modular FIA support on registers
  drm/i915/icl: Unify disable and enable phy clock gating functions
  drm/i915/tgl: Fix dkl phy register space addressing
  drm/i915/tgl: Check the UC health of tc controllers after power on
  drm/i915: Add dkl phy pll calculations

Lucas De Marchi (2):
  drm/i915/tgl: Add initial dkl pll support
  drm/i915/tgl: initialize TC and TBT ports

Taylor, Clinton A (5):
  drm/i915/tgl: Add missing ddi clock select during DP init sequence
  drm/i915/tgl: TC helper function to return pin mapping
  drm/i915/tgl: Fix driver crash when update_active_dpll is called
  drm/i915/tgl: Add dkl phy programming sequences
  drm/i915/tgl: Use dkl pll hardcoded values

Vandita Kulkarni (2):
  drm/i915/tgl: Add dkl phy registers
  drm/i915/tgl: Add support for dkl pll write

 drivers/gpu/drm/i915/display/intel_ddi.c      | 343 +++++++++++--
 drivers/gpu/drm/i915/display/intel_display.c  |   9 +-
 .../drm/i915/display/intel_display_power.c    |  16 +
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 456 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_tc.c       |  64 ++-
 drivers/gpu/drm/i915/display/intel_tc.h       |   3 +
 drivers/gpu/drm/i915/i915_drv.h               |   3 +
 drivers/gpu/drm/i915/i915_reg.h               | 206 +++++++-
 8 files changed, 1006 insertions(+), 94 deletions(-)

-- 
2.23.0

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

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

* [PATCH 01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor

From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>

Step 4.b was complete missed because it is only required to TC and TBT.

Bspec: 49190
Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3e6394139964..81792a04e0aa 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3224,11 +3224,14 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	intel_edp_panel_on(intel_dp);
 
 	/*
-	 * 1.b, 3. and 4. is done before tgl_ddi_pre_enable_dp() by:
+	 * 1.b, 3. and 4.a is done before tgl_ddi_pre_enable_dp() by:
 	 * haswell_crtc_enable()->intel_encoders_pre_pll_enable() and
 	 * haswell_crtc_enable()->intel_enable_shared_dpll()
 	 */
 
+	/* 4.b */
+	intel_ddi_clk_select(encoder, crtc_state);
+
 	/* 5. */
 	if (!intel_phy_is_tc(dev_priv, phy) ||
 	    dig_port->tc_mode != TC_PORT_TBT_ALT)
-- 
2.23.0

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

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

* [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  5:54   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers José Roberto de Souza
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor

From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>

Add a helper function to return pin map for use during dkl phy
DP_MODE settings, PORT_TX_DFLEXPA1 exist on ICL but we don't need it.

The user of this function will come in future TC patches.

Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/display/intel_tc.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 85743a43bee2..3a7302e360cc 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -61,6 +61,22 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 	       DP_LANE_ASSIGNMENT_SHIFT(tc_port);
 }
 
+u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
+	struct intel_uncore *uncore = &i915->uncore;
+	u32 pin_mask;
+
+	pin_mask = intel_uncore_read(uncore,
+				     PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
+
+	WARN_ON(pin_mask == 0xffffffff);
+
+	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
+	       DP_PIN_ASSIGNMENT_SHIFT(tc_port);
+}
+
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 783d75531435..463f1b3c836f 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -13,6 +13,7 @@ struct intel_digital_port;
 
 bool intel_tc_port_connected(struct intel_digital_port *dig_port);
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
+u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port);
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
 void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
 				      int required_lanes);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bf37ecebc82f..16d5548adb84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11683,4 +11683,9 @@ enum skl_power_gate {
 #define PORT_TX_DFLEXDPCSSS(fia)		_MMIO_FIA((fia), 0x00894)
 #define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)		(1 << (tc_port))
 
+#define PORT_TX_DFLEXPA1(fia)			_MMIO_FIA((fia), 0x00880)
+#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)		((tc_port) * 4)
+#define   DP_PIN_ASSIGNMENT_MASK(tc_port)		(0xf << ((tc_port) * 4))
+#define   DP_PIN_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 4))
+
 #endif /* _I915_REG_H_ */
-- 
2.23.0

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

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

* [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  6:24   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called José Roberto de Souza
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

If platform supports and has modular FIA is enabled, the registers
bits also change, example: reading TC3 registers with modular FIA
enabled, driver should read from FIA2 but with TC1 bits offsets.

It is described in BSpec 50231 for DFLEXDPSP, other registers don't
have the BSpec description but testing in real hardware have proven
that it had moved for all other registers too.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  2 +
 drivers/gpu/drm/i915/display/intel_tc.c      | 52 ++++++++++++--------
 drivers/gpu/drm/i915/display/intel_tc.h      |  2 +
 drivers/gpu/drm/i915/i915_drv.h              |  3 ++
 drivers/gpu/drm/i915/i915_reg.h              | 45 ++++++++---------
 5 files changed, 61 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4e001113e828..cd0a248bfe49 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15384,6 +15384,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
+	intel_tc_init(dev_priv);
+
 	if (INTEL_GEN(dev_priv) >= 12) {
 		/* TODO: initialize TC ports as well */
 		intel_ddi_init(dev_priv, PORT_A);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 3a7302e360cc..fc5d0e73cf21 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -23,19 +23,21 @@ static const char *tc_port_mode_name(enum tc_port_mode mode)
 	return names[mode];
 }
 
-static bool has_modular_fia(struct drm_i915_private *i915)
+void intel_tc_init(struct drm_i915_private *i915)
 {
+	u32 val;
+
 	if (!INTEL_INFO(i915)->display.has_modular_fia)
-		return false;
+		return;
 
-	return intel_uncore_read(&i915->uncore,
-				 PORT_TX_DFLEXDPSP(FIA1)) & MODULAR_FIA_MASK;
+	val = intel_uncore_read(&i915->uncore, PORT_TX_DFLEXDPSP(FIA1));
+	i915->has_modular_fia = val & MODULAR_FIA_MASK;
 }
 
 static enum phy_fia tc_port_to_fia(struct drm_i915_private *i915,
 				   enum tc_port tc_port)
 {
-	if (!has_modular_fia(i915))
+	if (!i915->has_modular_fia)
 		return FIA1;
 
 	/*
@@ -51,14 +53,15 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	struct intel_uncore *uncore = &i915->uncore;
 	u32 lane_mask;
+	bool mod_fia = i915->has_modular_fia;
 
 	lane_mask = intel_uncore_read(uncore,
 				      PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
 
 	WARN_ON(lane_mask == 0xffffffff);
 
-	return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
-	       DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+	return (lane_mask & DP_LANE_ASSIGNMENT_MASK(mod_fia, tc_port)) >>
+	       DP_LANE_ASSIGNMENT_SHIFT(mod_fia, tc_port);
 }
 
 u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
@@ -66,6 +69,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	struct intel_uncore *uncore = &i915->uncore;
+	bool mod_fia = i915->has_modular_fia;
 	u32 pin_mask;
 
 	pin_mask = intel_uncore_read(uncore,
@@ -73,8 +77,8 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
 
 	WARN_ON(pin_mask == 0xffffffff);
 
-	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
-	       DP_PIN_ASSIGNMENT_SHIFT(tc_port);
+	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(mod_fia, tc_port)) >>
+	       DP_PIN_ASSIGNMENT_SHIFT(mod_fia, tc_port);
 }
 
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
@@ -114,25 +118,28 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
 	struct intel_uncore *uncore = &i915->uncore;
+	bool mod_fia = i915->has_modular_fia;
 	u32 val;
 
 	WARN_ON(lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
 
 	val = intel_uncore_read(uncore,
 				PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
-	val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
+	val &= ~DFLEXDPMLE1_DPMLETC_MASK(mod_fia, tc_port);
 
 	switch (required_lanes) {
 	case 1:
-		val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
-			DFLEXDPMLE1_DPMLETC_ML0(tc_port);
+		val |= lane_reversal ?
+			DFLEXDPMLE1_DPMLETC_ML3(mod_fia, tc_port) :
+			DFLEXDPMLE1_DPMLETC_ML0(mod_fia, tc_port);
 		break;
 	case 2:
-		val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
-			DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
+		val |= lane_reversal ?
+			DFLEXDPMLE1_DPMLETC_ML3_2(mod_fia, tc_port) :
+			DFLEXDPMLE1_DPMLETC_ML1_0(mod_fia, tc_port);
 		break;
 	case 4:
-		val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
+		val |= DFLEXDPMLE1_DPMLETC_ML3_0(mod_fia, tc_port);
 		break;
 	default:
 		MISSING_CASE(required_lanes);
@@ -180,9 +187,9 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
 		return mask;
 	}
 
-	if (val & TC_LIVE_STATE_TBT(tc_port))
+	if (val & TC_LIVE_STATE_TBT(i915->has_modular_fia, tc_port))
 		mask |= BIT(TC_PORT_TBT_ALT);
-	if (val & TC_LIVE_STATE_TC(tc_port))
+	if (val & TC_LIVE_STATE_TC(i915->has_modular_fia, tc_port))
 		mask |= BIT(TC_PORT_DP_ALT);
 
 	if (intel_uncore_read(uncore, SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
@@ -200,6 +207,7 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	struct intel_uncore *uncore = &i915->uncore;
+	bool mod_fia = i915->has_modular_fia;
 	u32 val;
 
 	val = intel_uncore_read(uncore,
@@ -210,7 +218,7 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
 		return false;
 	}
 
-	return val & DP_PHY_MODE_STATUS_COMPLETED(tc_port);
+	return val & DP_PHY_MODE_STATUS_COMPLETED(mod_fia, tc_port);
 }
 
 static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
@@ -219,6 +227,7 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	struct intel_uncore *uncore = &i915->uncore;
+	bool mod_fia = i915->has_modular_fia;
 	u32 val;
 
 	val = intel_uncore_read(uncore,
@@ -231,9 +240,9 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
 		return false;
 	}
 
-	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port);
 	if (!enable)
-		val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+		val |= DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port);
 
 	intel_uncore_write(uncore,
 			   PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia), val);
@@ -250,6 +259,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
 	struct intel_uncore *uncore = &i915->uncore;
+	bool mod_fia = i915->has_modular_fia;
 	u32 val;
 
 	val = intel_uncore_read(uncore,
@@ -260,7 +270,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
 		return true;
 	}
 
-	return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
+	return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port));
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 463f1b3c836f..2374352d7c31 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -10,6 +10,7 @@
 #include <linux/types.h>
 
 struct intel_digital_port;
+struct drm_i915_private;
 
 bool intel_tc_port_connected(struct intel_digital_port *dig_port);
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
@@ -27,5 +28,6 @@ void intel_tc_port_put_link(struct intel_digital_port *dig_port);
 bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
 
 void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
+void intel_tc_init(struct drm_i915_private *dev_priv);
 
 #endif /* __INTEL_TC_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf600888b3f1..a36d1929fde1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1765,6 +1765,9 @@ struct drm_i915_private {
 	/* Mutex to protect the above hdcp component related values. */
 	struct mutex hdcp_comp_mutex;
 
+	/* Monolithic or modular FIA */
+	bool has_modular_fia;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 16d5548adb84..8aaf53283200 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2165,14 +2165,15 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define _FIA(fia)			_PICK((fia), FIA1_BASE, FIA2_BASE, FIA3_BASE)
 #define _MMIO_FIA(fia, off)		_MMIO(_FIA(fia) + (off))
 
+#define _TC_MOD_PORT_ID(has_modular_fia, tc_port)	((has_modular_fia) ? (tc_port) % 2 : (tc_port))
 /* ICL PHY DFLEX registers */
-#define PORT_TX_DFLEXDPMLE1(fia)	_MMIO_FIA((fia),  0x008C0)
-#define   DFLEXDPMLE1_DPMLETC_MASK(tc_port)	(0xf << (4 * (tc_port)))
-#define   DFLEXDPMLE1_DPMLETC_ML0(tc_port)	(1 << (4 * (tc_port)))
-#define   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port)	(3 << (4 * (tc_port)))
-#define   DFLEXDPMLE1_DPMLETC_ML3(tc_port)	(8 << (4 * (tc_port)))
-#define   DFLEXDPMLE1_DPMLETC_ML3_2(tc_port)	(12 << (4 * (tc_port)))
-#define   DFLEXDPMLE1_DPMLETC_ML3_0(tc_port)	(15 << (4 * (tc_port)))
+#define PORT_TX_DFLEXDPMLE1(fia)			_MMIO_FIA((fia),  0x008C0)
+#define   DFLEXDPMLE1_DPMLETC_MASK(mod, tc_port)	(0xf << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
+#define   DFLEXDPMLE1_DPMLETC_ML0(mod, tc_port)		(1 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
+#define   DFLEXDPMLE1_DPMLETC_ML1_0(mod, tc_port)	(3 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
+#define   DFLEXDPMLE1_DPMLETC_ML3(mod, tc_port)		(8 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
+#define   DFLEXDPMLE1_DPMLETC_ML3_2(mod, tc_port)	(12 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
+#define   DFLEXDPMLE1_DPMLETC_ML3_0(mod, tc_port)	(15 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
 
 /* BXT PHY Ref registers */
 #define _PORT_REF_DW3_A			0x16218C
@@ -11669,23 +11670,23 @@ enum skl_power_gate {
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
 						_ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
 
-#define PORT_TX_DFLEXDPSP(fia)			_MMIO_FIA((fia), 0x008A0)
-#define   MODULAR_FIA_MASK			(1 << 4)
-#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))
+#define PORT_TX_DFLEXDPSP(fia)				_MMIO_FIA((fia), 0x008A0)
+#define   MODULAR_FIA_MASK				(1 << 4)
+#define   TC_LIVE_STATE_TBT(mod, tc_port)		(1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 6))
+#define   TC_LIVE_STATE_TC(mod, tc_port)		(1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 5))
+#define   DP_LANE_ASSIGNMENT_SHIFT(mod, tc_port)	((_TC_MOD_PORT_ID(mod, tc_port)) * 8)
+#define   DP_LANE_ASSIGNMENT_MASK(mod, tc_port)		(0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))
+#define   DP_LANE_ASSIGNMENT(mod, tc_port, x)		((x) << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))
 
-#define PORT_TX_DFLEXDPPMS(fia)			_MMIO_FIA((fia), 0x00890)
-#define   DP_PHY_MODE_STATUS_COMPLETED(tc_port)		(1 << (tc_port))
+#define PORT_TX_DFLEXDPPMS(fia)				_MMIO_FIA((fia), 0x00890)
+#define   DP_PHY_MODE_STATUS_COMPLETED(mod, tc_port)	(1 << (_TC_MOD_PORT_ID(mod, tc_port)))
 
-#define PORT_TX_DFLEXDPCSSS(fia)		_MMIO_FIA((fia), 0x00894)
-#define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)		(1 << (tc_port))
+#define PORT_TX_DFLEXDPCSSS(fia)			_MMIO_FIA((fia), 0x00894)
+#define   DP_PHY_MODE_STATUS_NOT_SAFE(mod, tc_port)	(1 << (_TC_MOD_PORT_ID(mod, tc_port)))
 
-#define PORT_TX_DFLEXPA1(fia)			_MMIO_FIA((fia), 0x00880)
-#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)		((tc_port) * 4)
-#define   DP_PIN_ASSIGNMENT_MASK(tc_port)		(0xf << ((tc_port) * 4))
-#define   DP_PIN_ASSIGNMENT(tc_port, x)	((x) << ((tc_port) * 4))
+#define PORT_TX_DFLEXPA1(fia)				_MMIO_FIA((fia), 0x00880)
+#define   DP_PIN_ASSIGNMENT_SHIFT(mod, tc_port)		((_TC_MOD_PORT_ID(mod, tc_port)) * 4)
+#define   DP_PIN_ASSIGNMENT_MASK(mod, tc_port)	(0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
+#define   DP_PIN_ASSIGNMENT(mod, tc_port, x)	((x) << ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
 
 #endif /* _I915_REG_H_ */
-- 
2.23.0

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

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

* [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (2 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  6:32   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 05/14] drm/i915/tgl: Add dkl phy registers José Roberto de Souza
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor

From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>

TGL PLL function table doesn't include and update_active_pll function.
The driver attempts to make a call to this function and crashes during
PLL changes.

Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 98288edf88f0..84e734d44828 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3479,6 +3479,7 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
 	.dpll_info = tgl_plls,
 	.get_dplls = icl_get_dplls,
 	.put_dplls = icl_put_dplls,
+	.update_active_dpll = icl_update_active_dpll,
 	.dump_hw_state = icl_dump_hw_state,
 };
 
-- 
2.23.0

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

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

* [PATCH 05/14] drm/i915/tgl: Add dkl phy registers
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (3 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 06/14] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor, Lucas De Marchi

From: Vandita Kulkarni <vandita.kulkarni@intel.com>

These are the registers needed to program Dekel phy. Some register
definitions will be reused from MG PHY definitions, so adding a
comment on those.

Bspec: 49295

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 161 ++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8aaf53283200..95a4232c8e0a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10105,6 +10105,167 @@ enum skl_power_gate {
 						   _TGL_DPLL1_CFGCR1, \
 						   _TGL_TBTPLL_CFGCR1)
 
+#define _DKL_PHY1_BASE			0x168000
+#define _DKL_PHY2_BASE			0x169000
+#define _DKL_PHY3_BASE			0x16A000
+#define _DKL_PHY4_BASE			0x16B000
+#define _DKL_PHY5_BASE			0x16C000
+#define _DKL_PHY6_BASE			0x16D000
+
+/* DEKEL PHY MMIO Address = Phy base + (internal address & ~index_mask) */
+#define _DKL_PLL_DIV0			0x200
+#define   DKL_PLL_DIV0_INTEG_COEFF(x)	((x) << 16)
+#define   DKL_PLL_DIV0_INTEG_COEFF_MASK	(0x1F << 16)
+#define   DKL_PLL_DIV0_PROP_COEFF(x)	((x) << 12)
+#define   DKL_PLL_DIV0_PROP_COEFF_MASK	(0xF << 12)
+#define   DKL_PLL_DIV0_FBPREDIV_SHIFT   (8)
+#define   DKL_PLL_DIV0_FBPREDIV(x)	((x) << DKL_PLL_DIV0_FBPREDIV_SHIFT)
+#define   DKL_PLL_DIV0_FBPREDIV_MASK	(0xF << DKL_PLL_DIV0_FBPREDIV_SHIFT)
+#define   DKL_PLL_DIV0_FBDIV_INT(x)	((x) << 0)
+#define   DKL_PLL_DIV0_FBDIV_INT_MASK	(0xFF << 0)
+#define DKL_PLL_DIV0(tc_port)		_MMIO(_PORT(tc_port, _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+						    _DKL_PLL_DIV0)
+
+#define _DKL_PLL_DIV1				0x204
+#define   DKL_PLL_DIV1_IREF_TRIM(x)		((x) << 16)
+#define   DKL_PLL_DIV1_IREF_TRIM_MASK		(0x1F << 16)
+#define   DKL_PLL_DIV1_TDC_TARGET_CNT(x)	((x) << 0)
+#define   DKL_PLL_DIV1_TDC_TARGET_CNT_MASK	(0xFF << 0)
+#define DKL_PLL_DIV1(tc_port)		_MMIO(_PORT(tc_port, _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+						    _DKL_PLL_DIV1)
+
+#define _DKL_PLL_SSC				0x210
+#define   DKL_PLL_SSC_IREF_NDIV_RATIO(x)	((x) << 29)
+#define   DKL_PLL_SSC_IREF_NDIV_RATIO_MASK	(0x7 << 29)
+#define   DKL_PLL_SSC_STEP_LEN(x)		((x) << 16)
+#define   DKL_PLL_SSC_STEP_LEN_MASK		(0xFF << 16)
+#define   DKL_PLL_SSC_STEP_NUM(x)		((x) << 11)
+#define   DKL_PLL_SSC_STEP_NUM_MASK		(0x7 << 11)
+#define   DKL_PLL_SSC_EN			(1 << 9)
+#define DKL_PLL_SSC(tc_port)		_MMIO(_PORT(tc_port, _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+						    _DKL_PLL_SSC)
+
+#define _DKL_PLL_BIAS			0x214
+#define   DKL_PLL_BIAS_FRAC_EN_H	(1 << 30)
+#define   DKL_PLL_BIAS_FBDIV_SHIFT	(8)
+#define   DKL_PLL_BIAS_FBDIV_FRAC(x)	((x) << DKL_PLL_BIAS_FBDIV_SHIFT)
+#define   DKL_PLL_BIAS_FBDIV_FRAC_MASK	(0x3FFFFF << DKL_PLL_BIAS_FBDIV_SHIFT)
+#define DKL_PLL_BIAS(tc_port)		_MMIO(_PORT(tc_port, _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+						    _DKL_PLL_BIAS)
+
+#define _DKL_PLL_TDC_COLDST_BIAS		0x218
+#define   DKL_PLL_TDC_SSC_STEP_SIZE(x)		((x) << 8)
+#define   DKL_PLL_TDC_SSC_STEP_SIZE_MASK	(0xFF << 8)
+#define   DKL_PLL_TDC_FEED_FWD_GAIN(x)		((x) << 0)
+#define   DKL_PLL_TDC_FEED_FWD_GAIN_MASK	(0xFF << 0)
+#define DKL_PLL_TDC_COLDST_BIAS(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_PLL_TDC_COLDST_BIAS)
+
+#define _DKL_REFCLKIN_CTL		0x12C
+/* Bits are the same as MG_REFCLKIN_CTL */
+#define DKL_REFCLKIN_CTL(tc_port)	_MMIO(_PORT(tc_port, \
+						    _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+					      _DKL_REFCLKIN_CTL)
+
+#define _DKL_CLKTOP2_HSCLKCTL		0xD4
+/* Bits are the same as MG_CLKTOP2_HSCLKCTL */
+#define DKL_CLKTOP2_HSCLKCTL(tc_port)	_MMIO(_PORT(tc_port, \
+						    _DKL_PHY1_BASE, \
+						    _DKL_PHY2_BASE) + \
+					      _DKL_CLKTOP2_HSCLKCTL)
+
+#define _DKL_CLKTOP2_CORECLKCTL1		0xD8
+/* Bits are the same as MG_CLKTOP2_CORECLKCTL1 */
+#define DKL_CLKTOP2_CORECLKCTL1(tc_port)	_MMIO(_PORT(tc_port, \
+							    _DKL_PHY1_BASE, \
+							    _DKL_PHY2_BASE) + \
+						      _DKL_CLKTOP2_CORECLKCTL1)
+
+#define _DKL_TX_DPCNTL0				0x2C0
+#define  DKL_TX_PRESHOOT_COEFF(x)			((x) << 13)
+#define  DKL_TX_PRESHOOT_COEFF_MASK			(0x1f << 13)
+#define  DKL_TX_DE_EMPHASIS_COEFF(x)		((x) << 8)
+#define  DKL_TX_DE_EMPAHSIS_COEFF_MASK		(0x1f << 8)
+#define  DKL_TX_VSWING_CONTROL(x)			((x) << 0)
+#define  DKL_TX_VSWING_CONTROL_MASK			(0x7 << 0)
+#define DKL_TX_DPCNTL0(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_DPCNTL0)
+
+#define _DKL_TX_DPCNTL1				0x2C4
+/* Bits are the same as DKL_TX_DPCNTRL0 */
+#define DKL_TX_DPCNTL1(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_DPCNTL1)
+
+#define _DKL_TX_DPCNTL2				0x2C8
+#define  DKL_TX_DP20BITMODE				(1 << 2)
+#define DKL_TX_DPCNTL2(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_DPCNTL2)
+
+#define _DKL_TX_FW_CALIB				0x2F8
+#define  DKL_TX_CFG_DISABLE_WAIT_INIT			(1 << 7)
+#define DKL_TX_FW_CALIB(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_FW_CALIB)
+
+#define _DKL_TX_DW17					0xDC4
+#define DKL_TX_DW17(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_DW17)
+
+#define _DKL_TX_DW18					0xDC8
+#define DKL_TX_DW18(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_TX_DW18)
+
+#define _DKL_DP_MODE					0xA0
+#define  DKL_DP_MODE_CFG_GAONPWR_GATING		(1 << 1)
+#define  DKL_DP_MODE_CFG_DIGPWR_GATING		(1 << 2)
+#define  DKL_DP_MODE_CFG_CLNPWR_GATING		(1 << 3)
+#define  DKL_DP_MODE_CFG_TRPWR_GATING		(1 << 4)
+#define  DKL_DP_MODE_CFG_TR2PWR_GATING		(1 << 5)
+#define  DKL_DP_MODE_CFG_GATING_CTRL_MASK	(0x1f << 1)
+#define  DKL_DP_MODE_CFG_DP_X1_MODE			(1 << 6)
+#define  DKL_DP_MODE_CFG_DP_X2_MODE			(1 << 7)
+#define DKL_DP_MODE(tc_port) _MMIO(_PORT(tc_port, \
+						     _DKL_PHY1_BASE, \
+						     _DKL_PHY2_BASE) + \
+						     _DKL_DP_MODE)
+
+#define _DKL_CMN_UC_DW27			0x36C
+#define  DKL_CMN_UC_DW27_UC_HEALTH		(0x1 << 15)
+#define DKL_CMN_UC_DW_27(tc_port)		_MMIO(_PORT(tc_port, \
+							    _DKL_PHY1_BASE, \
+							    _DKL_PHY2_BASE) + \
+							    _DKL_CMN_UC_DW27)
+
+/*
+ * Each Dekel PHY is addressed through a 4KB aperture. Each PHY has more than
+ * 4KB of register space, so a separate index is programmed in HIP_INDEX_REG0
+ * or HIP_INDEX_REG1, based on the port number, to set the upper 2 address
+ * bits that point the 4KB window into the full PHY register space.
+ */
+#define _HIP_INDEX_REG0		0x1010A0
+#define _HIP_INDEX_REG1		0x1010A4
+#define HIP_INDEX_REG(tc_port)	_MMIO((tc_port) < 4 \
+				      ? _HIP_INDEX_REG0 \
+				      : _HIP_INDEX_REG1)
+
 /* BXT display engine PLL */
 #define BXT_DE_PLL_CTL			_MMIO(0x6d000)
 #define   BXT_DE_PLL_RATIO(x)		(x)	/* {60,65,100} * 19.2MHz */
-- 
2.23.0

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

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

* [PATCH 06/14] drm/i915/tgl: Add initial dkl pll support
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (4 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 05/14] drm/i915/tgl: Add dkl phy registers José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.demarchi@intel.com>

The disable function can be the same as for MG phy since the same
registers are used. The others are different as registers changed,
also adding a empty dkl_pll_write() to be implemented later.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 115 +++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 84e734d44828..424f9213c80d 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3086,6 +3086,76 @@ static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static bool dkl_pll_get_hw_state(struct drm_i915_private *dev_priv,
+				 struct intel_shared_dpll *pll,
+				 struct intel_dpll_hw_state *hw_state)
+{
+	const enum intel_dpll_id id = pll->info->id;
+	enum tc_port tc_port = icl_pll_id_to_tc_port(id);
+	intel_wakeref_t wakeref;
+	bool ret = false;
+	u32 val;
+
+	wakeref = intel_display_power_get_if_enabled(dev_priv,
+						     POWER_DOMAIN_DISPLAY_CORE);
+	if (!wakeref)
+		return false;
+
+	val = I915_READ(MG_PLL_ENABLE(tc_port));
+	if (!(val & PLL_ENABLE))
+		goto out;
+
+	/*
+	 * All registers read here have the same HIP_INDEX_REG even though
+	 * they are on different building blocks
+	 */
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+
+	hw_state->mg_refclkin_ctl = I915_READ(DKL_REFCLKIN_CTL(tc_port));
+	hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
+
+	hw_state->mg_clktop2_hsclkctl =
+		I915_READ(DKL_CLKTOP2_HSCLKCTL(tc_port));
+	hw_state->mg_clktop2_hsclkctl &=
+		MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
+		MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
+		MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
+		MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK;
+
+	hw_state->mg_clktop2_coreclkctl1 =
+		I915_READ(DKL_CLKTOP2_CORECLKCTL1(tc_port));
+	hw_state->mg_clktop2_coreclkctl1 &=
+		MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
+
+	hw_state->mg_pll_div0 = I915_READ(DKL_PLL_DIV0(tc_port));
+	hw_state->mg_pll_div0 &= (DKL_PLL_DIV0_INTEG_COEFF_MASK |
+				  DKL_PLL_DIV0_PROP_COEFF_MASK |
+				  DKL_PLL_DIV0_FBPREDIV_MASK |
+				  DKL_PLL_DIV0_FBDIV_INT_MASK);
+
+	hw_state->mg_pll_div1 = I915_READ(DKL_PLL_DIV1(tc_port));
+	hw_state->mg_pll_div1 &= (DKL_PLL_DIV1_IREF_TRIM_MASK |
+				  DKL_PLL_DIV1_TDC_TARGET_CNT_MASK);
+
+	hw_state->mg_pll_ssc = I915_READ(DKL_PLL_SSC(tc_port));
+	hw_state->mg_pll_ssc &= (DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
+				 DKL_PLL_SSC_STEP_LEN_MASK |
+				 DKL_PLL_SSC_STEP_NUM_MASK |
+				 DKL_PLL_SSC_EN);
+
+	hw_state->mg_pll_bias = I915_READ(DKL_PLL_BIAS(tc_port));
+	hw_state->mg_pll_bias &= (DKL_PLL_BIAS_FRAC_EN_H |
+				  DKL_PLL_BIAS_FBDIV_FRAC_MASK);
+
+	hw_state->mg_pll_tdc_coldst_bias =
+		I915_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
+
+	ret = true;
+out:
+	intel_display_power_put(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref);
+	return ret;
+}
+
 static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll,
 				 struct intel_dpll_hw_state *hw_state,
@@ -3220,6 +3290,12 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
 	POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port));
 }
 
+static void dkl_pll_write(struct drm_i915_private *dev_priv,
+			  struct intel_shared_dpll *pll)
+{
+	/* TODO */
+}
+
 static void icl_pll_power_enable(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll,
 				 i915_reg_t enable_reg)
@@ -3325,6 +3401,32 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
 	/* DVFS post sequence would be here. See the comment above. */
 }
 
+static void dkl_pll_enable(struct drm_i915_private *dev_priv,
+			   struct intel_shared_dpll *pll)
+{
+	/*
+	 * From spec: MG register instances are being used for TypeC in general.
+	 * The same MG register instances should be programmed for Dekel PLLs
+	 * as well.
+	 */
+	i915_reg_t enable_reg =
+		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
+
+	icl_pll_power_enable(dev_priv, pll, enable_reg);
+
+	dkl_pll_write(dev_priv, pll);
+
+	/*
+	 * DVFS pre sequence would be here, but in our driver the cdclk code
+	 * paths should already be setting the appropriate voltage, hence we do
+	 * nothing here.
+	 */
+
+	icl_pll_enable(dev_priv, pll, enable_reg);
+
+	/* DVFS post sequence would be here. See the comment above. */
+}
+
 static void icl_pll_disable(struct drm_i915_private *dev_priv,
 			    struct intel_shared_dpll *pll,
 			    i915_reg_t enable_reg)
@@ -3467,11 +3569,22 @@ static const struct intel_dpll_mgr ehl_pll_mgr = {
 	.dump_hw_state = icl_dump_hw_state,
 };
 
+static const struct intel_shared_dpll_funcs dkl_pll_funcs = {
+	.enable = dkl_pll_enable,
+	.disable = mg_pll_disable,
+	.get_hw_state = dkl_pll_get_hw_state,
+};
+
 static const struct dpll_info tgl_plls[] = {
 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0,  0 },
 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1,  0 },
 	{ "TBT PLL",  &tbt_pll_funcs, DPLL_ID_ICL_TBTPLL, 0 },
-	/* TODO: Add typeC plls */
+	{ "TC PLL 1", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL1, 0 },
+	{ "TC PLL 2", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL2, 0 },
+	{ "TC PLL 3", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL3, 0 },
+	{ "TC PLL 4", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL4, 0 },
+	{ "TC PLL 5", &dkl_pll_funcs, DPLL_ID_TGL_MGPLL5, 0 },
+	{ "TC PLL 6", &dkl_pll_funcs, DPLL_ID_TGL_MGPLL6, 0 },
 	{ },
 };
 
-- 
2.23.0

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

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

* [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (5 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 06/14] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  6:41   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 08/14] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

From: Vandita Kulkarni <vandita.kulkarni@intel.com>

Add a new function to write to dkl phy pll registers. As per the
bspec all the registers are read modify write.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 65 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 424f9213c80d..afc9b609b63d 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3293,7 +3293,70 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
 static void dkl_pll_write(struct drm_i915_private *dev_priv,
 			  struct intel_shared_dpll *pll)
 {
-	/* TODO */
+	struct intel_dpll_hw_state *hw_state = &pll->state.hw_state;
+	enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id);
+	u32 val;
+
+	/*
+	 * All registers programmed here have the same HIP_INDEX_REG even
+	 * though on different building block
+	 */
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+
+	/* All the registers are RMW */
+	val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
+	val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK;
+	val |= hw_state->mg_refclkin_ctl;
+	I915_WRITE(DKL_REFCLKIN_CTL(tc_port), val);
+
+	val = I915_READ(DKL_CLKTOP2_CORECLKCTL1(tc_port));
+	val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
+	val |= hw_state->mg_clktop2_coreclkctl1;
+	I915_WRITE(DKL_CLKTOP2_CORECLKCTL1(tc_port), val);
+
+	val = I915_READ(DKL_CLKTOP2_HSCLKCTL(tc_port));
+	val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
+	       MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
+	       MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
+	       MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK);
+	val |= hw_state->mg_clktop2_hsclkctl;
+	I915_WRITE(DKL_CLKTOP2_HSCLKCTL(tc_port), val);
+
+	val = I915_READ(DKL_PLL_DIV0(tc_port));
+	val &= ~(DKL_PLL_DIV0_INTEG_COEFF_MASK |
+		DKL_PLL_DIV0_PROP_COEFF_MASK |
+		DKL_PLL_DIV0_FBPREDIV_MASK |
+		DKL_PLL_DIV0_FBDIV_INT_MASK);
+	val |= hw_state->mg_pll_div0;
+	I915_WRITE(DKL_PLL_DIV0(tc_port), val);
+
+	val = I915_READ(DKL_PLL_DIV1(tc_port));
+	val &= ~(DKL_PLL_DIV1_IREF_TRIM_MASK |
+		 DKL_PLL_DIV1_TDC_TARGET_CNT_MASK);
+	val |= hw_state->mg_pll_div1;
+	I915_WRITE(DKL_PLL_DIV1(tc_port), val);
+
+	val = I915_READ(DKL_PLL_SSC(tc_port));
+	val &= ~(DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
+		DKL_PLL_SSC_STEP_LEN_MASK |
+		DKL_PLL_SSC_STEP_NUM_MASK |
+		DKL_PLL_SSC_EN);
+	val |= hw_state->mg_pll_ssc;
+	I915_WRITE(DKL_PLL_SSC(tc_port), val);
+
+	val = I915_READ(DKL_PLL_BIAS(tc_port));
+	val &= ~(DKL_PLL_BIAS_FRAC_EN_H |
+		DKL_PLL_BIAS_FBDIV_FRAC_MASK);
+	val |= hw_state->mg_pll_bias;
+	I915_WRITE(DKL_PLL_BIAS(tc_port), val);
+
+	val = I915_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
+	val &= ~(DKL_PLL_TDC_SSC_STEP_SIZE_MASK |
+		DKL_PLL_TDC_FEED_FWD_GAIN_MASK);
+	val |= hw_state->mg_pll_tdc_coldst_bias;
+	I915_WRITE(DKL_PLL_TDC_COLDST_BIAS(tc_port), val);
+
+	POSTING_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
 }
 
 static void icl_pll_power_enable(struct drm_i915_private *dev_priv,
-- 
2.23.0

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

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

* [PATCH 08/14] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (6 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 09/14] drm/i915/icl: Unify disable and enable phy clock gating functions José Roberto de Souza
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor

From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>

Added DKL Phy sequences and helpers functions to program voltage
swing, clock gating and dp mode.

It is not written in DP enabling sequence but "PHY Clockgating
programming" states that clock gating should be enabled after the
link training but doing so causes all the following trainings to fail
so not enabling it for.

BSpec: 49292
BSpec: 49190

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 246 +++++++++++++++++++++--
 1 file changed, 234 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 81792a04e0aa..dd3db5844bcb 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -586,6 +586,25 @@ static const struct icl_mg_phy_ddi_buf_trans icl_mg_phy_ddi_translations[] = {
 	{ 0x0, 0x00, 0x00 },	/* 3              0   */
 };
 
+struct tgl_dkl_phy_ddi_buf_trans {
+	u32 dkl_vswing_control;
+	u32 dkl_preshoot_control;
+	u32 dkl_de_emphasis_control;
+};
+
+static const struct tgl_dkl_phy_ddi_buf_trans tgl_dkl_phy_ddi_translations[] = {
+	{ 0x7, 0x0, 0x00 },	/* 0     0   400mV  0 dB   */
+	{ 0x5, 0x0, 0x03 },	/* 0     1   400mV  3.5 dB */
+	{ 0x2, 0x0, 0x0b },	/* 0     2   400mV  6 dB   */
+	{ 0x0, 0x0, 0x19 },	/* 0     3   400mV  9.5 dB */
+	{ 0x5, 0x0, 0x00 },	/* 1     0   600mV  0 dB   */
+	{ 0x2, 0x0, 0x03 },	/* 1     1   600mV  3.5 dB */
+	{ 0x0, 0x0, 0x14 },	/* 1     2   600mV  6 dB   */
+	{ 0x2, 0x0, 0x00 },	/* 2     0   800mV  0 dB   */
+	{ 0x0, 0x0, 0x0B },	/* 2     1   800mV  3.5 dB */
+	{ 0x0, 0x0, 0x00 },	/* 3     0  1200mV  0 dB    HDMI Default */
+};
+
 static const struct ddi_buf_trans *
 bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -873,11 +892,15 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
 	if (INTEL_GEN(dev_priv) >= 11) {
-		if (intel_phy_is_combo(dev_priv, phy))
+		if (intel_phy_is_combo(dev_priv, phy)) {
 			icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI,
 						0, &n_entries);
-		else
-			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
+		} else {
+			if (INTEL_GEN(dev_priv) >= 12)
+				n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations);
+			else
+				n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
+		}
 		default_entry = n_entries - 1;
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
@@ -2308,11 +2331,15 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 	int n_entries;
 
 	if (INTEL_GEN(dev_priv) >= 11) {
-		if (intel_phy_is_combo(dev_priv, phy))
+		if (intel_phy_is_combo(dev_priv, phy)) {
 			icl_get_combo_buf_trans(dev_priv, encoder->type,
 						intel_dp->link_rate, &n_entries);
-		else
-			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
+		} else {
+			if (INTEL_GEN(dev_priv) >= 12)
+				n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations);
+			else
+				n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
+		}
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		if (encoder->type == INTEL_OUTPUT_EDP)
 			cnl_get_buf_trans_edp(dev_priv, &n_entries);
@@ -2749,6 +2776,66 @@ static void icl_ddi_vswing_sequence(struct intel_encoder *encoder,
 		icl_mg_phy_ddi_vswing_sequence(encoder, link_clock, level);
 }
 
+static void
+tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int link_clock,
+				u32 level)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
+	const struct tgl_dkl_phy_ddi_buf_trans *ddi_translations;
+	u32 n_entries, val, ln;
+
+	n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations);
+	ddi_translations = tgl_dkl_phy_ddi_translations;
+
+	if (level > n_entries)
+		level = n_entries - 1;
+
+	/*
+	 * All registers programmed here use HIP_INDEX_REG 0 or 1
+	 */
+	for (ln = 0; ln < 2; ln++) {
+		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+
+		/* All the registers are RMW */
+		val = I915_READ(DKL_TX_DPCNTL0(tc_port));
+		val &= ~(DKL_TX_PRESHOOT_COEFF_MASK |
+			 DKL_TX_DE_EMPAHSIS_COEFF_MASK |
+			 DKL_TX_VSWING_CONTROL_MASK);
+		val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control);
+		val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control);
+		val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control);
+		I915_WRITE(DKL_TX_DPCNTL0(tc_port), val);
+
+		val = I915_READ(DKL_TX_DPCNTL1(tc_port));
+		val &= ~(DKL_TX_PRESHOOT_COEFF_MASK |
+			 DKL_TX_DE_EMPAHSIS_COEFF_MASK |
+			 DKL_TX_VSWING_CONTROL_MASK);
+		val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control);
+		val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control);
+		val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control);
+		I915_WRITE(DKL_TX_DPCNTL1(tc_port), val);
+
+		val = I915_READ(DKL_TX_DPCNTL2(tc_port));
+		val &= ~DKL_TX_DP20BITMODE;
+		I915_WRITE(DKL_TX_DPCNTL2(tc_port), val);
+	}
+}
+
+static void tgl_ddi_vswing_sequence(struct intel_encoder *encoder,
+				    int link_clock,
+				    u32 level,
+				    enum intel_output_type type)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
+
+	if (intel_phy_is_combo(dev_priv, phy))
+		icl_combo_phy_ddi_vswing_sequence(encoder, level, type);
+	else
+		tgl_dkl_phy_ddi_vswing_sequence(encoder, link_clock, level);
+}
+
 static u32 translate_signal_level(int signal_levels)
 {
 	int i;
@@ -2780,7 +2867,10 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_encoder *encoder = &dport->base;
 	int level = intel_ddi_dp_level(intel_dp);
 
-	if (INTEL_GEN(dev_priv) >= 11)
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
+					level, encoder->type);
+	else if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
 					level, encoder->type);
 	else if (IS_CANNONLAKE(dev_priv))
@@ -3027,6 +3117,34 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	}
 }
 
+static void
+tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
+{
+	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);
+	u32 val, regs;
+	int ln;
+
+	if (tc_port == PORT_TC_NONE)
+		return;
+
+	regs = DKL_DP_MODE_CFG_TR2PWR_GATING | DKL_DP_MODE_CFG_TRPWR_GATING |
+	       DKL_DP_MODE_CFG_CLNPWR_GATING | DKL_DP_MODE_CFG_DIGPWR_GATING |
+	       DKL_DP_MODE_CFG_GAONPWR_GATING;
+
+	for (ln = 0; ln < 2; ln++) {
+		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+
+		val = I915_READ(DKL_DP_MODE(tc_port));
+		if (enable)
+			val |= regs;
+		else
+			val &= ~regs;
+		I915_WRITE(DKL_DP_MODE(tc_port), val);
+	}
+}
+
 static 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);
@@ -3153,6 +3271,95 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
 	I915_WRITE(MG_DP_MODE(1, port), ln1);
 }
 
+static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+	enum port port = intel_dig_port->base.port;
+	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+	u32 ln0, ln1, lane_mask, pin_mask;
+	int num_lanes;
+
+	if (tc_port == PORT_TC_NONE ||
+	    intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
+		return;
+
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	ln0 = I915_READ(DKL_DP_MODE(tc_port));
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+	ln1 = I915_READ(DKL_DP_MODE(tc_port));
+
+	num_lanes = intel_dig_port->dp.lane_count;
+
+	switch (intel_dig_port->tc_mode) {
+	case TC_PORT_DP_ALT:
+		ln0 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X1_MODE);
+		ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE);
+
+		lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */
+		pin_mask = intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC */
+
+		switch (pin_mask) {
+		case 0x0:
+			if (num_lanes == 1) {
+				ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE;
+			} else {
+				ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+			}
+			break;
+		case 0x1:
+			if (num_lanes == 4) {
+				ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+			}
+			break;
+		case 0x2:
+			if (num_lanes == 2) {
+				ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+			}
+			break;
+		case 0x3:
+		case 0x5:
+			if (num_lanes == 1) {
+				ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE;
+			} else {
+				ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+			}
+			break;
+		case 0x4:
+		case 0x6:
+			if (num_lanes == 1) {
+				ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE;
+			} else {
+				ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+				ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE;
+			}
+			break;
+		default:
+			MISSING_CASE(lane_mask);
+		}
+		break;
+
+	case TC_PORT_LEGACY:
+		ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE;
+		ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE;
+		break;
+
+	default:
+		MISSING_CASE(intel_dig_port->tc_mode);
+		return;
+	}
+
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	I915_WRITE(DKL_DP_MODE(tc_port), ln0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+	I915_WRITE(DKL_DP_MODE(tc_port), ln1);
+}
+
 static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
 					const struct intel_crtc_state *crtc_state)
 {
@@ -3239,7 +3446,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
 					dig_port->ddi_io_power_domain);
 
 	/* 6. */
-	icl_program_mg_dp_mode(dig_port);
+	tgl_program_dkl_dp_mode(dig_port);
 
 	/*
 	 * 7.a - Steps in this function should only be executed over MST
@@ -3252,10 +3459,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	intel_ddi_config_transcoder_func(crtc_state);
 
 	/* 7.d */
-	icl_disable_phy_clock_gating(dig_port);
+	tgl_phy_clock_gating(dig_port, false);
 
 	/* 7.e */
-	icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level,
+	tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level,
 				encoder->type);
 
 	/* 7.f */
@@ -3287,6 +3494,15 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	/* 7.k */
 	intel_dp_stop_link_train(intel_dp);
 
+	/*
+	 * TODO: enable clock gating
+	 *
+	 * It is not written in DP enabling sequence but "PHY Clockgating
+	 * programming" states that clock gating should be enabled after the
+	 * link training but doing so causes all the following trainings to fail
+	 * so not enabling it for.
+	 */
+
 	/* 7.l */
 	intel_ddi_enable_fec(encoder, crtc_state);
 	intel_dsc_enable(encoder, crtc_state);
@@ -3392,7 +3608,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
 	icl_program_mg_dp_mode(dig_port);
-	icl_disable_phy_clock_gating(dig_port);
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_phy_clock_gating(dig_port, false);
+	else
+		icl_disable_phy_clock_gating(dig_port);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
@@ -3404,7 +3623,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	else
 		intel_prepare_hdmi_ddi_buffers(encoder, level);
 
-	icl_enable_phy_clock_gating(dig_port);
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_phy_clock_gating(dig_port, true);
+	else
+		icl_enable_phy_clock_gating(dig_port);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
-- 
2.23.0

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

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

* [PATCH 09/14] drm/i915/icl: Unify disable and enable phy clock gating functions
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (7 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 08/14] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing José Roberto de Souza
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

Adding a enable parameters allow us to share most of the code between
enable and disable functions.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 71 ++++++++----------------
 1 file changed, 22 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index dd3db5844bcb..a6a2e00cc075 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3145,67 +3145,40 @@ tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
 	}
 }
 
-static void icl_enable_phy_clock_gating(struct intel_digital_port *dig_port)
+static void
+icl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
 {
 	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);
-	u32 val;
+	u32 val, regs;
 	int ln;
 
 	if (tc_port == PORT_TC_NONE)
 		return;
 
-	for (ln = 0; ln < 2; ln++) {
-		val = I915_READ(MG_DP_MODE(ln, port));
-		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_DP_MODE(ln, port), 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);
-}
-
-static 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);
-	u32 val;
-	int ln;
-
-	if (tc_port == PORT_TC_NONE)
-		return;
+	regs = 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;
 
 	for (ln = 0; ln < 2; ln++) {
 		val = I915_READ(MG_DP_MODE(ln, port));
-		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);
+		if (enable)
+			val |= regs;
+		else
+			val &= ~regs;
 		I915_WRITE(MG_DP_MODE(ln, port), val);
 	}
 
+	regs = 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;
+
 	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);
+	if (enable)
+		val |= (regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3));
+	else
+		val &= ~(regs | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK);
 	I915_WRITE(MG_MISC_SUS0(tc_port), val);
 }
 
@@ -3538,7 +3511,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder *encoder,
 					dig_port->ddi_io_power_domain);
 
 	icl_program_mg_dp_mode(dig_port);
-	icl_disable_phy_clock_gating(dig_port);
+	icl_phy_clock_gating(dig_port, false);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
@@ -3571,7 +3544,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_ddi_enable_fec(encoder, crtc_state);
 
-	icl_enable_phy_clock_gating(dig_port);
+	icl_phy_clock_gating(dig_port, true);
 
 	if (!is_mst)
 		intel_ddi_enable_pipe_clock(crtc_state);
@@ -3611,7 +3584,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	if (INTEL_GEN(dev_priv) >= 12)
 		tgl_phy_clock_gating(dig_port, false);
 	else
-		icl_disable_phy_clock_gating(dig_port);
+		icl_phy_clock_gating(dig_port, false);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
@@ -3626,7 +3599,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	if (INTEL_GEN(dev_priv) >= 12)
 		tgl_phy_clock_gating(dig_port, true);
 	else
-		icl_enable_phy_clock_gating(dig_port);
+		icl_phy_clock_gating(dig_port, true);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
-- 
2.23.0

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

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

* [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (8 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 09/14] drm/i915/icl: Unify disable and enable phy clock gating functions José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  7:26   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 11/14] drm/i915/tgl: Check the UC health of tc controllers after power on José Roberto de Souza
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

It was always modifing register space of the first phy in the
HIP_INDEX_REG for all ports while it should shift 8 bits for each
port inside of HIP_INDEX_REG.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 34 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
 drivers/gpu/drm/i915/i915_reg.h               |  3 ++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a6a2e00cc075..981e24120a87 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2795,7 +2795,10 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int link_clock,
 	 * All registers programmed here use HIP_INDEX_REG 0 or 1
 	 */
 	for (ln = 0; ln < 2; ln++) {
-		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+		val = I915_READ(HIP_INDEX_REG(tc_port));
+		val &= ~HIP_INDEX_MASK(tc_port);
+		val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
+		I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 		/* All the registers are RMW */
 		val = I915_READ(DKL_TX_DPCNTL0(tc_port));
@@ -3134,7 +3137,10 @@ tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
 	       DKL_DP_MODE_CFG_GAONPWR_GATING;
 
 	for (ln = 0; ln < 2; ln++) {
-		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+		val = I915_READ(HIP_INDEX_REG(tc_port));
+		val &= ~HIP_INDEX_MASK(tc_port);
+		val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
+		I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 		val = I915_READ(DKL_DP_MODE(tc_port));
 		if (enable)
@@ -3249,16 +3255,23 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 	enum port port = intel_dig_port->base.port;
 	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
-	u32 ln0, ln1, lane_mask, pin_mask;
+	u32 ln0, ln1, lane_mask, pin_mask, val;
 	int num_lanes;
 
 	if (tc_port == PORT_TC_NONE ||
 	    intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
 		return;
 
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	ln0 = I915_READ(DKL_DP_MODE(tc_port));
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	ln1 = I915_READ(DKL_DP_MODE(tc_port));
 
 	num_lanes = intel_dig_port->dp.lane_count;
@@ -3327,9 +3340,16 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
 		return;
 	}
 
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	I915_WRITE(DKL_DP_MODE(tc_port), ln0);
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	I915_WRITE(DKL_DP_MODE(tc_port), ln1);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index afc9b609b63d..9304606c1c0a 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3109,7 +3109,10 @@ static bool dkl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	 * All registers read here have the same HIP_INDEX_REG even though
 	 * they are on different building blocks
 	 */
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 	hw_state->mg_refclkin_ctl = I915_READ(DKL_REFCLKIN_CTL(tc_port));
 	hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
@@ -3301,7 +3304,10 @@ static void dkl_pll_write(struct drm_i915_private *dev_priv,
 	 * All registers programmed here have the same HIP_INDEX_REG even
 	 * though on different building block
 	 */
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 	/* All the registers are RMW */
 	val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95a4232c8e0a..625f458e9b1c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10265,6 +10265,9 @@ enum skl_power_gate {
 #define HIP_INDEX_REG(tc_port)	_MMIO((tc_port) < 4 \
 				      ? _HIP_INDEX_REG0 \
 				      : _HIP_INDEX_REG1)
+#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
+#define HIP_INDEX_MASK(tc_port) (0xff << _HIP_INDEX_SHIFT(tc_port))
+#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) << _HIP_INDEX_SHIFT(tc_port))
 
 /* BXT display engine PLL */
 #define BXT_DE_PLL_CTL			_MMIO(0x6d000)
-- 
2.23.0

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

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

* [PATCH 11/14] drm/i915/tgl: Check the UC health of tc controllers after power on
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (9 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 12/14] drm/i915: Add dkl phy pll calculations José Roberto de Souza
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

New step added for TGL, requiring for us to check the TC
microcontroller health after power on TC aux.

BSpec: 49294

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../gpu/drm/i915/display/intel_display_power.c   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index ce88a27229ef..14e4ac6ee54d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -562,6 +562,8 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
 
 #endif
 
+#define TGL_AUX_PW_TO_TC_PORT(pw_idx)	((pw_idx) - TGL_PW_CTL_IDX_AUX_TC1)
+
 static void
 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 				 struct i915_power_well *power_well)
@@ -578,6 +580,20 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 	I915_WRITE(DP_AUX_CH_CTL(aux_ch), val);
 
 	hsw_power_well_enable(dev_priv, power_well);
+
+	if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
+		enum tc_port tc_port;
+
+		tc_port = TGL_AUX_PW_TO_TC_PORT(power_well->desc->hsw.idx);
+		val = I915_READ(HIP_INDEX_REG(tc_port));
+		val &= ~HIP_INDEX_MASK(tc_port);
+		val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
+		I915_WRITE(HIP_INDEX_REG(tc_port), val);
+
+		if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port),
+					  DKL_CMN_UC_DW27_UC_HEALTH, 1))
+			DRM_WARN("Timeout waiting TC uC health\n");
+	}
 }
 
 static void
-- 
2.23.0

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

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

* [PATCH 12/14] drm/i915: Add dkl phy pll calculations
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (10 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 11/14] drm/i915/tgl: Check the UC health of tc controllers after power on José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-14  7:38   ` Lucas De Marchi
  2019-09-13 22:32 ` [PATCH 13/14] drm/i915/tgl: Use dkl pll hardcoded values José Roberto de Souza
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx

The _pll_find_divisors and _calc_dkl_pll_state TGL versions are
similar to ICL ones but the BSpec algorithm conversion from real to
integer number plus the differences makes it easier and safer to
implement it in new functions.

BSpec: 49204

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  29 ++-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 205 +++++++++++++++++-
 2 files changed, 227 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 981e24120a87..521e5b2e6f1c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1436,11 +1436,30 @@ static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
 
 	ref_clock = dev_priv->cdclk.hw.ref;
 
-	m1 = pll_state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
-	m2_int = pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
-	m2_frac = (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
-		(pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
-		MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
+	if (INTEL_GEN(dev_priv) >= 12) {
+		m1 = pll_state->mg_pll_div0 & DKL_PLL_DIV0_FBPREDIV_MASK;
+		m1 = m1 >> DKL_PLL_DIV0_FBPREDIV_SHIFT;
+		m2_int = pll_state->mg_pll_div0 & DKL_PLL_DIV0_FBDIV_INT_MASK;
+
+		if (pll_state->mg_pll_bias & DKL_PLL_BIAS_FRAC_EN_H) {
+			m2_frac = pll_state->mg_pll_bias &
+				  DKL_PLL_BIAS_FBDIV_FRAC_MASK;
+			m2_frac = m2_frac >> DKL_PLL_BIAS_FBDIV_SHIFT;
+		} else {
+			m2_frac = 0;
+		}
+	} else {
+		m1 = pll_state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
+		m2_int = pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
+
+		if (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) {
+			m2_frac = pll_state->mg_pll_div0 &
+				  MG_PLL_DIV0_FBDIV_FRAC_MASK;
+			m2_frac = m2_frac >> MG_PLL_DIV0_FBDIV_FRAC_SHIFT;
+		} else {
+			m2_frac = 0;
+		}
+	}
 
 	switch (pll_state->mg_clktop2_hsclkctl &
 		MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 9304606c1c0a..25be6229b122 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2605,6 +2605,202 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
 	return tc_port + DPLL_ID_ICL_MGPLL1;
 }
 
+static bool tgl_dkl_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
+				      u32 *target_dco_khz,
+				      struct intel_dpll_hw_state *state)
+{
+	u32 dco_min_freq, dco_max_freq;
+	int div1_vals[] = {7, 5, 3, 2};
+	int i, div2;
+
+	dco_min_freq = is_dp ? 8100000 : use_ssc ? 8000000 : 7992000;
+	dco_max_freq = is_dp ? 8100000 : 10000000;
+
+	for (i = 0; i < ARRAY_SIZE(div1_vals); i++) {
+		int div1 = div1_vals[i];
+
+		for (div2 = 10; div2 > 0; div2--) {
+			int dco = div1 * div2 * clock_khz * 5;
+			int inputsel, tlinedrv;
+			u32 hsdiv;
+
+			if (dco < dco_min_freq || dco > dco_max_freq)
+				continue;
+
+			state->mg_refclkin_ctl = MG_REFCLKIN_CTL_OD_2_MUX(1);
+			state->mg_clktop2_coreclkctl1 =
+					MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(5);
+
+			tlinedrv = div2 >= 2 ? 1 : 2;
+			inputsel = is_dp ? 0 : 1;
+
+			switch (div1) {
+			default:
+				MISSING_CASE(div1);
+				/* fall through */
+			case 2:
+				hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2;
+				break;
+			case 3:
+				hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3;
+				break;
+			case 5:
+				hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5;
+				break;
+			case 7:
+				hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7;
+				break;
+			}
+
+			*target_dco_khz = dco;
+
+			state->mg_clktop2_hsclkctl =
+				MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL(tlinedrv) |
+				MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL(inputsel) |
+				hsdiv |
+				MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(div2);
+
+			return true;
+		}
+	}
+
+	return false;
+}
+
+/*
+ * The specification for this function uses real numbers, so the math had to be
+ * adapted to integer-only calculation, that's why it looks so different.
+ */
+static bool tgl_calc_dkl_pll_state(struct intel_crtc_state *crtc_state,
+				   struct intel_dpll_hw_state *pll_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	int refclk_khz = dev_priv->cdclk.hw.ref;
+	int symbol_frequency = crtc_state->port_clock;
+	u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
+	u32 iref_ndiv, iref_trim;
+	u32 prop_coeff, int_coeff;
+	u32 tdc_targetcnt, feedfwgain;
+	u64 ssc_stepsize, ssc_steplen, ssc_steplog;
+	u64 tmp;
+	bool use_ssc = true;
+	bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
+
+	memset(pll_state, 0, sizeof(*pll_state));
+
+	if (!tgl_dkl_pll_find_divisors(symbol_frequency, is_dp, use_ssc,
+				       &dco_khz, pll_state)) {
+		DRM_DEBUG_KMS("Failed to find divisors for clock %d\n",
+			      symbol_frequency);
+		return false;
+	}
+
+	m1div = 2;
+	m2div_int = dco_khz / (refclk_khz * m1div);
+	if (m2div_int > 255) {
+		DRM_DEBUG_KMS("Failed to find mdiv for clock %d\n",
+			      symbol_frequency);
+		return false;
+	}
+	m2div_rem = dco_khz % (refclk_khz * m1div);
+
+	tmp = (u64)m2div_rem * (1 << 22);
+	do_div(tmp, refclk_khz * m1div);
+	m2div_frac = tmp;
+
+	switch (refclk_khz) {
+	case 19200:
+		iref_ndiv = 1;
+		break;
+	case 24000:
+		iref_ndiv = 1;
+		break;
+	case 38400:
+		iref_ndiv = 2;
+		break;
+	default:
+		MISSING_CASE(refclk_khz);
+		return false;
+	}
+
+	/*
+	 * tdc_res = 0.000003
+	 * tdc_targetcnt = int(2 / (tdc_res * 8 * 50 * 1.1) / refclk_mhz + 0.5)
+	 *
+	 * The multiplication by 1000 is due to refclk MHz to KHz conversion. It
+	 * was supposed to be a division, but we rearranged the operations of
+	 * the formula to avoid early divisions so we don't multiply the
+	 * rounding errors.
+	 *
+	 * 0.000003 * 8 * 50 * 1.1 = 0.00132, also known as 132 / 100000, which
+	 * we also rearrange to work with integers.
+	 *
+	 * The 0.5 transformed to 5 results in a multiplication by 10 and the
+	 * last division by 10.
+	 */
+	tdc_targetcnt = (2 * 1000 * 100000 * 10 / (132 * refclk_khz) + 5) / 10;
+
+	/*
+	 * Here we divide dco_khz by 10 in order to allow the dividend to fit in
+	 * 32 bits. That's not a problem since we round the division down
+	 * anyway.
+	 */
+	feedfwgain = (use_ssc || m2div_rem > 0) ?
+		m1div * 1000000 * 100 / (dco_khz * 3 / 10) : 0;
+
+	tmp = refclk_khz / iref_ndiv;
+	if (tmp <= 19200)
+		iref_trim = 28;
+	else if (tmp <= 25000)
+		iref_trim = 25;
+	else
+		iref_trim = 24;
+
+	if (dco_khz >= 9000000) {
+		prop_coeff = 5;
+		int_coeff = 10;
+	} else {
+		prop_coeff = 4;
+		int_coeff = 8;
+	}
+
+	if (use_ssc) {
+		tmp = mul_u32_u32(dco_khz, 47 * 32);
+		do_div(tmp, refclk_khz * m1div * 10000);
+		ssc_stepsize = tmp;
+
+		tmp = mul_u32_u32(dco_khz, 1000);
+		ssc_steplen = DIV_ROUND_UP_ULL(tmp, 32 * 2 * 32);
+	} else {
+		ssc_stepsize = 0;
+		ssc_steplen = 0;
+	}
+	ssc_steplog = 4;
+
+	/* write pll_state calculations */
+	pll_state->mg_pll_div0 = DKL_PLL_DIV0_INTEG_COEFF(int_coeff) |
+				 DKL_PLL_DIV0_PROP_COEFF(prop_coeff) |
+				 DKL_PLL_DIV0_FBPREDIV(m1div) |
+				 DKL_PLL_DIV0_FBDIV_INT(m2div_int);
+
+	pll_state->mg_pll_div1 = DKL_PLL_DIV1_IREF_TRIM(iref_trim) |
+				 DKL_PLL_DIV1_TDC_TARGET_CNT(tdc_targetcnt);
+
+	pll_state->mg_pll_ssc = DKL_PLL_SSC_IREF_NDIV_RATIO(iref_ndiv) |
+				DKL_PLL_SSC_STEP_LEN(ssc_steplen) |
+				DKL_PLL_SSC_STEP_NUM(ssc_steplog) |
+				(use_ssc ? DKL_PLL_SSC_EN : 0);
+
+	pll_state->mg_pll_bias = (m2div_frac > 0 ? DKL_PLL_BIAS_FRAC_EN_H : 0) |
+				 DKL_PLL_BIAS_FBDIV_FRAC(m2div_frac);
+
+	pll_state->mg_pll_tdc_coldst_bias =
+			DKL_PLL_TDC_SSC_STEP_SIZE(ssc_stepsize) |
+			DKL_PLL_TDC_FEED_FWD_GAIN(feedfwgain);
+
+	return true;
+}
+
 static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
 				     u32 *target_dco_khz,
 				     struct intel_dpll_hw_state *state)
@@ -2932,6 +3128,7 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct icl_port_dpll *port_dpll;
 	enum intel_dpll_id dpll_id;
+	bool r;
 
 	port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
 	if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) {
@@ -2950,9 +3147,13 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
 	intel_reference_shared_dpll(state, crtc,
 				    port_dpll->pll, &port_dpll->hw_state);
 
-
 	port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_MG_PHY];
-	if (!icl_calc_mg_pll_state(crtc_state, &port_dpll->hw_state)) {
+	if (INTEL_GEN(dev_priv) >= 12)
+		r = tgl_calc_dkl_pll_state(crtc_state, &port_dpll->hw_state);
+	else
+		r = icl_calc_mg_pll_state(crtc_state, &port_dpll->hw_state);
+
+	if (!r) {
 		DRM_DEBUG_KMS("Could not calculate MG PHY PLL state.\n");
 		goto err_unreference_tbt_pll;
 	}
-- 
2.23.0

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

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

* [PATCH 13/14] drm/i915/tgl: Use dkl pll hardcoded values
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (11 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 12/14] drm/i915: Add dkl phy pll calculations José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:32 ` [PATCH 14/14] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Taylor

From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>

BSpec PLL calculation are not validated/ready yet, so for now it is
providing a table with hardcoded values to all DP link rates.
So for now lets override the calculated values with the hardcoded
ones.

With this hardcoded values the port clock calculation for 5.4Ghz
don't match but this is a minor error that we can live for now.

Bspec: 49204

Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 25be6229b122..5b568dd57a5a 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2667,6 +2667,65 @@ static bool tgl_dkl_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
 	return false;
 }
 
+struct tgl_dp_frequencies {
+	u32 hsclkctl;
+	u32 coreclkctl1;
+	u32 ssc_reg;
+};
+
+static void
+tgl_dkl_pll_overwrite_with_hardcoded_values(int clock_khz,
+					    struct intel_dpll_hw_state *state,
+					    bool is_dp)
+{
+	const struct tgl_dp_frequencies tgl_dkl_pll_dp_frequencies[] = {
+		{ 0x011D, 0x10080510, 0x401320ff },	/* 8p1 */
+		{ 0x121D, 0x10080510, 0x401320ff },	/* 5p4 */
+		{ 0x521D, 0x10080A12, 0x401320ff },	/* 2p7 */
+		{ 0x621D, 0x10080A12, 0x401320ff },	/* 1p62 */
+	};
+	int i;
+
+	if (!is_dp) {
+		/* No hardcoded values for HDMI */
+		MISSING_CASE(!is_dp);
+		return;
+	}
+
+	switch (clock_khz) {
+	case 810000:
+		i = 0;
+		break;
+	case 540000:
+		i = 1;
+		break;
+	case 270000:
+		i = 2;
+		break;
+	case 162000:
+		i = 3;
+		break;
+	default:
+		MISSING_CASE(clock_khz);
+		return;
+	}
+
+	state->mg_clktop2_coreclkctl1 = tgl_dkl_pll_dp_frequencies[i].coreclkctl1;
+	state->mg_clktop2_coreclkctl1 &= MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
+
+	state->mg_clktop2_hsclkctl = tgl_dkl_pll_dp_frequencies[i].hsclkctl;
+	state->mg_clktop2_hsclkctl &= (MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
+				       MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
+				       MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
+				       MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK);
+
+	state->mg_pll_ssc = tgl_dkl_pll_dp_frequencies[i].ssc_reg;
+	state->mg_pll_ssc &= (DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
+			      DKL_PLL_SSC_STEP_LEN_MASK |
+			      DKL_PLL_SSC_STEP_NUM_MASK |
+			      DKL_PLL_SSC_EN);
+}
+
 /*
  * The specification for this function uses real numbers, so the math had to be
  * adapted to integer-only calculation, that's why it looks so different.
@@ -2798,6 +2857,13 @@ static bool tgl_calc_dkl_pll_state(struct intel_crtc_state *crtc_state,
 			DKL_PLL_TDC_SSC_STEP_SIZE(ssc_stepsize) |
 			DKL_PLL_TDC_FEED_FWD_GAIN(feedfwgain);
 
+	/*
+	 * BSpec PLL calculations are not validated/ready yet, so for now lets
+	 * fallback to the hardcoded table.
+	 */
+	tgl_dkl_pll_overwrite_with_hardcoded_values(symbol_frequency,
+						    pll_state, is_dp);
+
 	return true;
 }
 
-- 
2.23.0

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

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

* [PATCH 14/14] drm/i915/tgl: initialize TC and TBT ports
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (12 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 13/14] drm/i915/tgl: Use dkl pll hardcoded values José Roberto de Souza
@ 2019-09-13 22:32 ` José Roberto de Souza
  2019-09-13 22:53 ` ✗ Fi.CI.CHECKPATCH: warning for TGL TC enabling Patchwork
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: José Roberto de Souza @ 2019-09-13 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.demarchi@intel.com>

Now that TC support was added, initialize DDIs.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cd0a248bfe49..5c52c04fef7b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15387,9 +15387,14 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	intel_tc_init(dev_priv);
 
 	if (INTEL_GEN(dev_priv) >= 12) {
-		/* TODO: initialize TC ports as well */
 		intel_ddi_init(dev_priv, PORT_A);
 		intel_ddi_init(dev_priv, PORT_B);
+		intel_ddi_init(dev_priv, PORT_D);
+		intel_ddi_init(dev_priv, PORT_E);
+		intel_ddi_init(dev_priv, PORT_F);
+		intel_ddi_init(dev_priv, PORT_G);
+		intel_ddi_init(dev_priv, PORT_H);
+		intel_ddi_init(dev_priv, PORT_I);
 		icl_dsi_init(dev_priv);
 	} else if (IS_ELKHARTLAKE(dev_priv)) {
 		intel_ddi_init(dev_priv, PORT_A);
-- 
2.23.0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for TGL TC enabling
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (13 preceding siblings ...)
  2019-09-13 22:32 ` [PATCH 14/14] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
@ 2019-09-13 22:53 ` Patchwork
  2019-09-13 23:12 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-09-15  7:07 ` ✓ Fi.CI.IGT: " Patchwork
  16 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2019-09-13 22:53 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: TGL TC enabling
URL   : https://patchwork.freedesktop.org/series/66695/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
aa9a2d4bd5f2 drm/i915/tgl: Add missing ddi clock select during DP init sequence
6a06996e8b87 drm/i915/tgl: TC helper function to return pin mapping
9c066f860365 drm/i915/tgl: Finish modular FIA support on registers
-:242: WARNING:LONG_LINE: line over 100 characters
#242: FILE: drivers/gpu/drm/i915/i915_reg.h:2168:
+#define _TC_MOD_PORT_ID(has_modular_fia, tc_port)	((has_modular_fia) ? (tc_port) % 2 : (tc_port))

-:242: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'tc_port' - possible side-effects?
#242: FILE: drivers/gpu/drm/i915/i915_reg.h:2168:
+#define _TC_MOD_PORT_ID(has_modular_fia, tc_port)	((has_modular_fia) ? (tc_port) % 2 : (tc_port))

-:252: WARNING:LONG_LINE: line over 100 characters
#252: FILE: drivers/gpu/drm/i915/i915_reg.h:2171:
+#define   DFLEXDPMLE1_DPMLETC_MASK(mod, tc_port)	(0xf << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))

-:256: WARNING:LONG_LINE: line over 100 characters
#256: FILE: drivers/gpu/drm/i915/i915_reg.h:2175:
+#define   DFLEXDPMLE1_DPMLETC_ML3_2(mod, tc_port)	(12 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))

-:257: WARNING:LONG_LINE: line over 100 characters
#257: FILE: drivers/gpu/drm/i915/i915_reg.h:2176:
+#define   DFLEXDPMLE1_DPMLETC_ML3_0(mod, tc_port)	(15 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))

-:274: WARNING:LONG_LINE: line over 100 characters
#274: FILE: drivers/gpu/drm/i915/i915_reg.h:11675:
+#define   TC_LIVE_STATE_TBT(mod, tc_port)		(1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 6))

-:275: WARNING:LONG_LINE: line over 100 characters
#275: FILE: drivers/gpu/drm/i915/i915_reg.h:11676:
+#define   TC_LIVE_STATE_TC(mod, tc_port)		(1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 5))

-:277: WARNING:LONG_LINE: line over 100 characters
#277: FILE: drivers/gpu/drm/i915/i915_reg.h:11678:
+#define   DP_LANE_ASSIGNMENT_MASK(mod, tc_port)		(0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))

-:278: WARNING:LONG_LINE: line over 100 characters
#278: FILE: drivers/gpu/drm/i915/i915_reg.h:11679:
+#define   DP_LANE_ASSIGNMENT(mod, tc_port, x)		((x) << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))

total: 0 errors, 8 warnings, 1 checks, 243 lines checked
675571336c4f drm/i915/tgl: Fix driver crash when update_active_dpll is called
77a194507dda drm/i915/tgl: Add dkl phy registers
fb5057c3fd44 drm/i915/tgl: Add initial dkl pll support
5d7cf4a2d9b6 drm/i915/tgl: Add support for dkl pll write
777ae3564243 drm/i915/tgl: Add dkl phy programming sequences
9a8c52b002c1 drm/i915/icl: Unify disable and enable phy clock gating functions
a1c3093ee5cd drm/i915/tgl: Fix dkl phy register space addressing
e00f55852bed drm/i915/tgl: Check the UC health of tc controllers after power on
c375eb564833 drm/i915: Add dkl phy pll calculations
e15b18d1f053 drm/i915/tgl: Use dkl pll hardcoded values
2a0737c7916f drm/i915/tgl: initialize TC and TBT ports

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

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

* ✓ Fi.CI.BAT: success for TGL TC enabling
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (14 preceding siblings ...)
  2019-09-13 22:53 ` ✗ Fi.CI.CHECKPATCH: warning for TGL TC enabling Patchwork
@ 2019-09-13 23:12 ` Patchwork
  2019-09-15  7:07 ` ✓ Fi.CI.IGT: " Patchwork
  16 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2019-09-13 23:12 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: TGL TC enabling
URL   : https://patchwork.freedesktop.org/series/66695/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6894 -> Patchwork_14410
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-icl-u3/igt@gem_cpu_reloc@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-icl-u3/igt@gem_cpu_reloc@basic.html

  * igt@i915_module_load@reload:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724] / [fdo#111214])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-icl-u3/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-icl-u3/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][5] -> [DMESG-FAIL][6] ([fdo#111108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@legacy-render:
    - {fi-icl-guc}:       [INCOMPLETE][9] ([fdo#107713] / [fdo#111381]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-icl-guc/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_mmap_gtt@basic-write-cpu-read-gtt:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-icl-u3/igt@gem_mmap_gtt@basic-write-cpu-read-gtt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-icl-u3/igt@gem_mmap_gtt@basic-write-cpu-read-gtt.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-icl-u2:          [FAIL][13] ([fdo#109635 ]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][15] ([fdo#111096]) -> [FAIL][16] ([fdo#111407])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
  [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111562]: https://bugs.freedesktop.org/show_bug.cgi?id=111562
  [fdo#111597]: https://bugs.freedesktop.org/show_bug.cgi?id=111597


Participating hosts (54 -> 47)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6894 -> Patchwork_14410

  CI-20190529: 20190529
  CI_DRM_6894: a323fd657c577491b1660662624bac36bb964222 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5182: f7104497049e3761ac297b66fd5586849b3cfcc8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14410: 2a0737c7916fe702fca52471d012f9af5032ccb6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2a0737c7916f drm/i915/tgl: initialize TC and TBT ports
e15b18d1f053 drm/i915/tgl: Use dkl pll hardcoded values
c375eb564833 drm/i915: Add dkl phy pll calculations
e00f55852bed drm/i915/tgl: Check the UC health of tc controllers after power on
a1c3093ee5cd drm/i915/tgl: Fix dkl phy register space addressing
9a8c52b002c1 drm/i915/icl: Unify disable and enable phy clock gating functions
777ae3564243 drm/i915/tgl: Add dkl phy programming sequences
5d7cf4a2d9b6 drm/i915/tgl: Add support for dkl pll write
fb5057c3fd44 drm/i915/tgl: Add initial dkl pll support
77a194507dda drm/i915/tgl: Add dkl phy registers
675571336c4f drm/i915/tgl: Fix driver crash when update_active_dpll is called
9c066f860365 drm/i915/tgl: Finish modular FIA support on registers
6a06996e8b87 drm/i915/tgl: TC helper function to return pin mapping
aa9a2d4bd5f2 drm/i915/tgl: Add missing ddi clock select during DP init sequence

== Logs ==

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

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

* Re: [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping
  2019-09-13 22:32 ` [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
@ 2019-09-14  5:54   ` Lucas De Marchi
  2019-09-17 21:15     ` Souza, Jose
  0 siblings, 1 reply; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  5:54 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Taylor

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>
>
> Add a helper function to return pin map for use during dkl phy
> DP_MODE settings, PORT_TX_DFLEXPA1 exist on ICL but we don't need it.
>
> The user of this function will come in future TC patches.

It's actually harder to review if the function is out of the context
of those patches. Will it really be needed outside of intel_tc.c ?

>
> Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>

you need your s-o-b too if you are sending this patch.

> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_tc.h |  1 +
>  drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 85743a43bee2..3a7302e360cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -61,6 +61,22 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>                DP_LANE_ASSIGNMENT_SHIFT(tc_port);
>  }
>
> +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
> +{
> +       struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +       enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
> +       struct intel_uncore *uncore = &i915->uncore;
> +       u32 pin_mask;
> +
> +       pin_mask = intel_uncore_read(uncore,
> +                                    PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
> +
> +       WARN_ON(pin_mask == 0xffffffff);
> +
> +       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
> +              DP_PIN_ASSIGNMENT_SHIFT(tc_port);
> +}
> +
>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
>  {
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> index 783d75531435..463f1b3c836f 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -13,6 +13,7 @@ struct intel_digital_port;
>
>  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
>  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
> +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port);
>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
>  void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
>                                       int required_lanes);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bf37ecebc82f..16d5548adb84 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11683,4 +11683,9 @@ enum skl_power_gate {
>  #define PORT_TX_DFLEXDPCSSS(fia)               _MMIO_FIA((fia), 0x00894)
>  #define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)         (1 << (tc_port))
>
> +#define PORT_TX_DFLEXPA1(fia)                  _MMIO_FIA((fia), 0x00880)
> +#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)             ((tc_port) * 4)
> +#define   DP_PIN_ASSIGNMENT_MASK(tc_port)              (0xf << ((tc_port) * 4))

Use DP_PIN_ASSIGNMENT_SHIFT() here

> +#define   DP_PIN_ASSIGNMENT(tc_port, x)        ((x) << ((tc_port) * 4))

ditto.

Lucas De Marchi

> +
>  #endif /* _I915_REG_H_ */
> --
> 2.23.0
>
> _______________________________________________
> 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] 29+ messages in thread

* Re: [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers
  2019-09-13 22:32 ` [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers José Roberto de Souza
@ 2019-09-14  6:24   ` Lucas De Marchi
  2019-09-18  1:08     ` Souza, Jose
  0 siblings, 1 reply; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  6:24 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> If platform supports and has modular FIA is enabled, the registers
> bits also change, example: reading TC3 registers with modular FIA
> enabled, driver should read from FIA2 but with TC1 bits offsets.
>
> It is described in BSpec 50231 for DFLEXDPSP, other registers don't
> have the BSpec description but testing in real hardware have proven
> that it had moved for all other registers too.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  2 +
>  drivers/gpu/drm/i915/display/intel_tc.c      | 52 ++++++++++++--------
>  drivers/gpu/drm/i915/display/intel_tc.h      |  2 +
>  drivers/gpu/drm/i915/i915_drv.h              |  3 ++
>  drivers/gpu/drm/i915/i915_reg.h              | 45 ++++++++---------
>  5 files changed, 61 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 4e001113e828..cd0a248bfe49 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15384,6 +15384,8 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>         if (!HAS_DISPLAY(dev_priv))
>                 return;
>
> +       intel_tc_init(dev_priv);
> +
>         if (INTEL_GEN(dev_priv) >= 12) {
>                 /* TODO: initialize TC ports as well */
>                 intel_ddi_init(dev_priv, PORT_A);
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 3a7302e360cc..fc5d0e73cf21 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -23,19 +23,21 @@ static const char *tc_port_mode_name(enum tc_port_mode mode)
>         return names[mode];
>  }
>
> -static bool has_modular_fia(struct drm_i915_private *i915)
> +void intel_tc_init(struct drm_i915_private *i915)
>  {
> +       u32 val;
> +
>         if (!INTEL_INFO(i915)->display.has_modular_fia)
> -               return false;
> +               return;
>
> -       return intel_uncore_read(&i915->uncore,
> -                                PORT_TX_DFLEXDPSP(FIA1)) & MODULAR_FIA_MASK;
> +       val = intel_uncore_read(&i915->uncore, PORT_TX_DFLEXDPSP(FIA1));
> +       i915->has_modular_fia = val & MODULAR_FIA_MASK;

Not a fan of stuffing tc-private details in i195 struct. Since this is
only executed on initialization maybe it's
not worth the few register reads we spare. If we go with this
approach, then I think we should not use
"has_" prefix so we don't get confused by the device_info fields.

>  }
>
>  static enum phy_fia tc_port_to_fia(struct drm_i915_private *i915,
>                                    enum tc_port tc_port)
>  {
> -       if (!has_modular_fia(i915))
> +       if (!i915->has_modular_fia)
>                 return FIA1;
>
>         /*
> @@ -51,14 +53,15 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         struct intel_uncore *uncore = &i915->uncore;
>         u32 lane_mask;
> +       bool mod_fia = i915->has_modular_fia;

s/mod_fia/modular_fia/ or just don't add the additional var

>
>         lane_mask = intel_uncore_read(uncore,
>                                       PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
>
>         WARN_ON(lane_mask == 0xffffffff);
>
> -       return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> -              DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> +       return (lane_mask & DP_LANE_ASSIGNMENT_MASK(mod_fia, tc_port)) >>
> +              DP_LANE_ASSIGNMENT_SHIFT(mod_fia, tc_port);
>  }
>
>  u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
> @@ -66,6 +69,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         struct intel_uncore *uncore = &i915->uncore;
> +       bool mod_fia = i915->has_modular_fia;
>         u32 pin_mask;
>
>         pin_mask = intel_uncore_read(uncore,
> @@ -73,8 +77,8 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
>
>         WARN_ON(pin_mask == 0xffffffff);
>
> -       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
> -              DP_PIN_ASSIGNMENT_SHIFT(tc_port);
> +       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(mod_fia, tc_port)) >>
> +              DP_PIN_ASSIGNMENT_SHIFT(mod_fia, tc_port);
>  }
>
>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
> @@ -114,25 +118,28 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
>         struct intel_uncore *uncore = &i915->uncore;
> +       bool mod_fia = i915->has_modular_fia;
>         u32 val;
>
>         WARN_ON(lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
>
>         val = intel_uncore_read(uncore,
>                                 PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
> -       val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> +       val &= ~DFLEXDPMLE1_DPMLETC_MASK(mod_fia, tc_port);
>
>         switch (required_lanes) {
>         case 1:
> -               val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> -                       DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> +               val |= lane_reversal ?
> +                       DFLEXDPMLE1_DPMLETC_ML3(mod_fia, tc_port) :
> +                       DFLEXDPMLE1_DPMLETC_ML0(mod_fia, tc_port);
>                 break;
>         case 2:
> -               val |= lane_reversal ? DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> -                       DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> +               val |= lane_reversal ?
> +                       DFLEXDPMLE1_DPMLETC_ML3_2(mod_fia, tc_port) :
> +                       DFLEXDPMLE1_DPMLETC_ML1_0(mod_fia, tc_port);
>                 break;
>         case 4:
> -               val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
> +               val |= DFLEXDPMLE1_DPMLETC_ML3_0(mod_fia, tc_port);
>                 break;
>         default:
>                 MISSING_CASE(required_lanes);
> @@ -180,9 +187,9 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
>                 return mask;
>         }
>
> -       if (val & TC_LIVE_STATE_TBT(tc_port))
> +       if (val & TC_LIVE_STATE_TBT(i915->has_modular_fia, tc_port))
>                 mask |= BIT(TC_PORT_TBT_ALT);
> -       if (val & TC_LIVE_STATE_TC(tc_port))
> +       if (val & TC_LIVE_STATE_TC(i915->has_modular_fia, tc_port))
>                 mask |= BIT(TC_PORT_DP_ALT);
>
>         if (intel_uncore_read(uncore, SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
> @@ -200,6 +207,7 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         struct intel_uncore *uncore = &i915->uncore;
> +       bool mod_fia = i915->has_modular_fia;
>         u32 val;
>
>         val = intel_uncore_read(uncore,
> @@ -210,7 +218,7 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
>                 return false;
>         }
>
> -       return val & DP_PHY_MODE_STATUS_COMPLETED(tc_port);
> +       return val & DP_PHY_MODE_STATUS_COMPLETED(mod_fia, tc_port);
>  }
>
>  static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
> @@ -219,6 +227,7 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         struct intel_uncore *uncore = &i915->uncore;
> +       bool mod_fia = i915->has_modular_fia;
>         u32 val;
>
>         val = intel_uncore_read(uncore,
> @@ -231,9 +240,9 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
>                 return false;
>         }
>
> -       val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> +       val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port);
>         if (!enable)
> -               val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> +               val |= DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port);
>
>         intel_uncore_write(uncore,
>                            PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia), val);
> @@ -250,6 +259,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
>         struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>         enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port);
>         struct intel_uncore *uncore = &i915->uncore;
> +       bool mod_fia = i915->has_modular_fia;
>         u32 val;
>
>         val = intel_uncore_read(uncore,
> @@ -260,7 +270,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
>                 return true;
>         }
>
> -       return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
> +       return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port));
>  }
>
>  /*
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> index 463f1b3c836f..2374352d7c31 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.h
> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> @@ -10,6 +10,7 @@
>  #include <linux/types.h>
>
>  struct intel_digital_port;
> +struct drm_i915_private;
>
>  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
>  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
> @@ -27,5 +28,6 @@ void intel_tc_port_put_link(struct intel_digital_port *dig_port);
>  bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
>
>  void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
> +void intel_tc_init(struct drm_i915_private *dev_priv);
>
>  #endif /* __INTEL_TC_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bf600888b3f1..a36d1929fde1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1765,6 +1765,9 @@ struct drm_i915_private {
>         /* Mutex to protect the above hdcp component related values. */
>         struct mutex hdcp_comp_mutex;
>
> +       /* Monolithic or modular FIA */
> +       bool has_modular_fia;
> +
>         /*
>          * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>          * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 16d5548adb84..8aaf53283200 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2165,14 +2165,15 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _FIA(fia)                      _PICK((fia), FIA1_BASE, FIA2_BASE, FIA3_BASE)
>  #define _MMIO_FIA(fia, off)            _MMIO(_FIA(fia) + (off))
>
> +#define _TC_MOD_PORT_ID(has_modular_fia, tc_port)      ((has_modular_fia) ? (tc_port) % 2 : (tc_port))

instead of doing all of this, what about storing a
dig_port->tc_phy_fia_idx that is set to
tc_port on monolithic FIA and to tc_port % 2 on modular...? Then it
would be initialized together with
dig_port->tc_phy_fia and we could even remove tc_port_to_fia() that
would not be used anymore since we would
check that in the caller.

>  /* ICL PHY DFLEX registers */
> -#define PORT_TX_DFLEXDPMLE1(fia)       _MMIO_FIA((fia),  0x008C0)
> -#define   DFLEXDPMLE1_DPMLETC_MASK(tc_port)    (0xf << (4 * (tc_port)))

in all these macros it would be s/tc_port/idx/

Lucas De Marchi

> -#define   DFLEXDPMLE1_DPMLETC_ML0(tc_port)     (1 << (4 * (tc_port)))
> -#define   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port)   (3 << (4 * (tc_port)))
> -#define   DFLEXDPMLE1_DPMLETC_ML3(tc_port)     (8 << (4 * (tc_port)))
> -#define   DFLEXDPMLE1_DPMLETC_ML3_2(tc_port)   (12 << (4 * (tc_port)))
> -#define   DFLEXDPMLE1_DPMLETC_ML3_0(tc_port)   (15 << (4 * (tc_port)))
> +#define PORT_TX_DFLEXDPMLE1(fia)                       _MMIO_FIA((fia),  0x008C0)
> +#define   DFLEXDPMLE1_DPMLETC_MASK(mod, tc_port)       (0xf << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> +#define   DFLEXDPMLE1_DPMLETC_ML0(mod, tc_port)                (1 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> +#define   DFLEXDPMLE1_DPMLETC_ML1_0(mod, tc_port)      (3 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> +#define   DFLEXDPMLE1_DPMLETC_ML3(mod, tc_port)                (8 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> +#define   DFLEXDPMLE1_DPMLETC_ML3_2(mod, tc_port)      (12 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> +#define   DFLEXDPMLE1_DPMLETC_ML3_0(mod, tc_port)      (15 << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
>
>  /* BXT PHY Ref registers */
>  #define _PORT_REF_DW3_A                        0x16218C
> @@ -11669,23 +11670,23 @@ enum skl_power_gate {
>                                                 _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \
>                                                 _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC)
>
> -#define PORT_TX_DFLEXDPSP(fia)                 _MMIO_FIA((fia), 0x008A0)
> -#define   MODULAR_FIA_MASK                     (1 << 4)
> -#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))
> +#define PORT_TX_DFLEXDPSP(fia)                         _MMIO_FIA((fia), 0x008A0)
> +#define   MODULAR_FIA_MASK                             (1 << 4)
> +#define   TC_LIVE_STATE_TBT(mod, tc_port)              (1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 6))
> +#define   TC_LIVE_STATE_TC(mod, tc_port)               (1 << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 5))
> +#define   DP_LANE_ASSIGNMENT_SHIFT(mod, tc_port)       ((_TC_MOD_PORT_ID(mod, tc_port)) * 8)
> +#define   DP_LANE_ASSIGNMENT_MASK(mod, tc_port)                (0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))
> +#define   DP_LANE_ASSIGNMENT(mod, tc_port, x)          ((x) << ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))
>
> -#define PORT_TX_DFLEXDPPMS(fia)                        _MMIO_FIA((fia), 0x00890)
> -#define   DP_PHY_MODE_STATUS_COMPLETED(tc_port)                (1 << (tc_port))
> +#define PORT_TX_DFLEXDPPMS(fia)                                _MMIO_FIA((fia), 0x00890)
> +#define   DP_PHY_MODE_STATUS_COMPLETED(mod, tc_port)   (1 << (_TC_MOD_PORT_ID(mod, tc_port)))
>
> -#define PORT_TX_DFLEXDPCSSS(fia)               _MMIO_FIA((fia), 0x00894)
> -#define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)         (1 << (tc_port))
> +#define PORT_TX_DFLEXDPCSSS(fia)                       _MMIO_FIA((fia), 0x00894)
> +#define   DP_PHY_MODE_STATUS_NOT_SAFE(mod, tc_port)    (1 << (_TC_MOD_PORT_ID(mod, tc_port)))
>
> -#define PORT_TX_DFLEXPA1(fia)                  _MMIO_FIA((fia), 0x00880)
> -#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)             ((tc_port) * 4)
> -#define   DP_PIN_ASSIGNMENT_MASK(tc_port)              (0xf << ((tc_port) * 4))
> -#define   DP_PIN_ASSIGNMENT(tc_port, x)        ((x) << ((tc_port) * 4))
> +#define PORT_TX_DFLEXPA1(fia)                          _MMIO_FIA((fia), 0x00880)
> +#define   DP_PIN_ASSIGNMENT_SHIFT(mod, tc_port)                ((_TC_MOD_PORT_ID(mod, tc_port)) * 4)
> +#define   DP_PIN_ASSIGNMENT_MASK(mod, tc_port) (0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
> +#define   DP_PIN_ASSIGNMENT(mod, tc_port, x)   ((x) << ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
>
>  #endif /* _I915_REG_H_ */
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

* Re: [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called
  2019-09-13 22:32 ` [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called José Roberto de Souza
@ 2019-09-14  6:32   ` Lucas De Marchi
  2019-09-17 22:59     ` Souza, Jose
  0 siblings, 1 reply; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  6:32 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Taylor

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>
>
> TGL PLL function table doesn't include and update_active_pll function.
> The driver attempts to make a call to this function and crashes during
> PLL changes.

the crash would only occur if the port was initialized. The ordering
in this series means this is
not a "fix" but rather finishing the implementation for TC ports
before initializing them. So we may
want to adjust the commit message accordingly. The reason we missed
that for TGL is that its need came
in parallel to the TGL support hitting upstream.

My nit with this hook is that `update_active_dpll` is exclusively used
by *TC* ports on gen 11+ and
a) it's  not clear about that from the name and/or b) if it's generic
it should not crash when it's missing.

I think Imre had a patch to address it, at least renaming the hook, I
don't remember. +Imre.

Lucas De Marchi

>
> Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 98288edf88f0..84e734d44828 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3479,6 +3479,7 @@ static const struct intel_dpll_mgr tgl_pll_mgr = {
>         .dpll_info = tgl_plls,
>         .get_dplls = icl_get_dplls,
>         .put_dplls = icl_put_dplls,
> +       .update_active_dpll = icl_update_active_dpll,
>         .dump_hw_state = icl_dump_hw_state,
>  };
>
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

* Re: [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write
  2019-09-13 22:32 ` [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
@ 2019-09-14  6:41   ` Lucas De Marchi
  2019-09-16  8:48     ` Jani Nikula
  0 siblings, 1 reply; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  6:41 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Lucas De Marchi

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> From: Vandita Kulkarni <vandita.kulkarni@intel.com>
>
> Add a new function to write to dkl phy pll registers. As per the
> bspec all the registers are read modify write.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 65 ++++++++++++++++++-
>  1 file changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 424f9213c80d..afc9b609b63d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3293,7 +3293,70 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
>  static void dkl_pll_write(struct drm_i915_private *dev_priv,
>                           struct intel_shared_dpll *pll)
>  {
> -       /* TODO */
> +       struct intel_dpll_hw_state *hw_state = &pll->state.hw_state;
> +       enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id);
> +       u32 val;
> +
> +       /*
> +        * All registers programmed here have the same HIP_INDEX_REG even
> +        * though on different building block
> +        */
> +       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> +
> +       /* All the registers are RMW */

I wish we could use intel_uncore_rmw() here to make this comment go
away and make it simpler
in general. But I understand the conversion to use uncore should still
not be done in display. Should we add
I195_RMW() meanwhile or it's not worth it? +Jani.

Lucas De Marchi

> +       val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
> +       val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> +       val |= hw_state->mg_refclkin_ctl;
> +       I915_WRITE(DKL_REFCLKIN_CTL(tc_port), val);
> +
> +       val = I915_READ(DKL_CLKTOP2_CORECLKCTL1(tc_port));
> +       val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
> +       val |= hw_state->mg_clktop2_coreclkctl1;
> +       I915_WRITE(DKL_CLKTOP2_CORECLKCTL1(tc_port), val);
> +
> +       val = I915_READ(DKL_CLKTOP2_HSCLKCTL(tc_port));
> +       val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
> +              MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
> +              MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
> +              MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK);
> +       val |= hw_state->mg_clktop2_hsclkctl;
> +       I915_WRITE(DKL_CLKTOP2_HSCLKCTL(tc_port), val);
> +
> +       val = I915_READ(DKL_PLL_DIV0(tc_port));
> +       val &= ~(DKL_PLL_DIV0_INTEG_COEFF_MASK |
> +               DKL_PLL_DIV0_PROP_COEFF_MASK |
> +               DKL_PLL_DIV0_FBPREDIV_MASK |
> +               DKL_PLL_DIV0_FBDIV_INT_MASK);
> +       val |= hw_state->mg_pll_div0;
> +       I915_WRITE(DKL_PLL_DIV0(tc_port), val);
> +
> +       val = I915_READ(DKL_PLL_DIV1(tc_port));
> +       val &= ~(DKL_PLL_DIV1_IREF_TRIM_MASK |
> +                DKL_PLL_DIV1_TDC_TARGET_CNT_MASK);
> +       val |= hw_state->mg_pll_div1;
> +       I915_WRITE(DKL_PLL_DIV1(tc_port), val);
> +
> +       val = I915_READ(DKL_PLL_SSC(tc_port));
> +       val &= ~(DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
> +               DKL_PLL_SSC_STEP_LEN_MASK |
> +               DKL_PLL_SSC_STEP_NUM_MASK |
> +               DKL_PLL_SSC_EN);
> +       val |= hw_state->mg_pll_ssc;
> +       I915_WRITE(DKL_PLL_SSC(tc_port), val);
> +
> +       val = I915_READ(DKL_PLL_BIAS(tc_port));
> +       val &= ~(DKL_PLL_BIAS_FRAC_EN_H |
> +               DKL_PLL_BIAS_FBDIV_FRAC_MASK);
> +       val |= hw_state->mg_pll_bias;
> +       I915_WRITE(DKL_PLL_BIAS(tc_port), val);
> +
> +       val = I915_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
> +       val &= ~(DKL_PLL_TDC_SSC_STEP_SIZE_MASK |
> +               DKL_PLL_TDC_FEED_FWD_GAIN_MASK);
> +       val |= hw_state->mg_pll_tdc_coldst_bias;
> +       I915_WRITE(DKL_PLL_TDC_COLDST_BIAS(tc_port), val);
> +
> +       POSTING_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
>  }
>
>  static void icl_pll_power_enable(struct drm_i915_private *dev_priv,
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

* Re: [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing
  2019-09-13 22:32 ` [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing José Roberto de Souza
@ 2019-09-14  7:26   ` Lucas De Marchi
  2019-09-18 19:55     ` Souza, Jose
  0 siblings, 1 reply; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  7:26 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> It was always modifing register space of the first phy in the
> HIP_INDEX_REG for all ports while it should shift 8 bits for each
> port inside of HIP_INDEX_REG.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 34 +++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
>  drivers/gpu/drm/i915/i915_reg.h               |  3 ++
>  3 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a6a2e00cc075..981e24120a87 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2795,7 +2795,10 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int link_clock,
>          * All registers programmed here use HIP_INDEX_REG 0 or 1
>          */
>         for (ln = 0; ln < 2; ln++) {
> -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> +               val = I915_READ(HIP_INDEX_REG(tc_port));
> +               val &= ~HIP_INDEX_MASK(tc_port);
> +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);

INDEX_INDEX?

> +               I915_WRITE(HIP_INDEX_REG(tc_port), val);

we don't really care for a RMW here, do we? We don't access these
registers in parallel for other ports
(It would be inherently racy if we did), so

        I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln));

should be sufficient.

Also I think all these fixes should be squashed in their correspondent
patches in this series with changelogs
updated.

>
>                 /* All the registers are RMW */
>                 val = I915_READ(DKL_TX_DPCNTL0(tc_port));
> @@ -3134,7 +3137,10 @@ tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
>                DKL_DP_MODE_CFG_GAONPWR_GATING;
>
>         for (ln = 0; ln < 2; ln++) {
> -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> +               val = I915_READ(HIP_INDEX_REG(tc_port));
> +               val &= ~HIP_INDEX_MASK(tc_port);
> +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>                 val = I915_READ(DKL_DP_MODE(tc_port));
>                 if (enable)
> @@ -3249,16 +3255,23 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
>         struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>         enum port port = intel_dig_port->base.port;
>         enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> -       u32 ln0, ln1, lane_mask, pin_mask;
> +       u32 ln0, ln1, lane_mask, pin_mask, val;
>         int num_lanes;
>
>         if (tc_port == PORT_TC_NONE ||
>             intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
>                 return;
>
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         ln0 = I915_READ(DKL_DP_MODE(tc_port));
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> +
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         ln1 = I915_READ(DKL_DP_MODE(tc_port));
>
>         num_lanes = intel_dig_port->dp.lane_count;
> @@ -3327,9 +3340,16 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
>                 return;
>         }
>
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> +
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         I915_WRITE(DKL_DP_MODE(tc_port), ln1);
>  }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index afc9b609b63d..9304606c1c0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3109,7 +3109,10 @@ static bool dkl_pll_get_hw_state(struct drm_i915_private *dev_priv,
>          * All registers read here have the same HIP_INDEX_REG even though
>          * they are on different building blocks
>          */
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>         hw_state->mg_refclkin_ctl = I915_READ(DKL_REFCLKIN_CTL(tc_port));
>         hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> @@ -3301,7 +3304,10 @@ static void dkl_pll_write(struct drm_i915_private *dev_priv,
>          * All registers programmed here have the same HIP_INDEX_REG even
>          * though on different building block
>          */
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>         /* All the registers are RMW */
>         val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95a4232c8e0a..625f458e9b1c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10265,6 +10265,9 @@ enum skl_power_gate {
>  #define HIP_INDEX_REG(tc_port) _MMIO((tc_port) < 4 \
>                                       ? _HIP_INDEX_REG0 \
>                                       : _HIP_INDEX_REG1)
> +#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
> +#define HIP_INDEX_MASK(tc_port) (0xff << _HIP_INDEX_SHIFT(tc_port))
> +#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) << _HIP_INDEX_SHIFT(tc_port))

#define HIP_INDEX_VAL(index) ((index) | (index) << 8 | ( index) << 16
| (index) << 24)
?

Lucas De Marchi

>
>  /* BXT display engine PLL */
>  #define BXT_DE_PLL_CTL                 _MMIO(0x6d000)
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

* Re: [PATCH 12/14] drm/i915: Add dkl phy pll calculations
  2019-09-13 22:32 ` [PATCH 12/14] drm/i915: Add dkl phy pll calculations José Roberto de Souza
@ 2019-09-14  7:38   ` Lucas De Marchi
  0 siblings, 0 replies; 29+ messages in thread
From: Lucas De Marchi @ 2019-09-14  7:38 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> The _pll_find_divisors and _calc_dkl_pll_state TGL versions are
> similar to ICL ones but the BSpec algorithm conversion from real to
> integer number plus the differences makes it easier and safer to
> implement it in new functions.
>
> BSpec: 49204


I remember doing a diff in the algorithm from icl -> tgl and
concluding otherwise:
besides the register name noise it was easier to do the other way
around and just
add the small differences to the existing code. Has the spec changed recently?

Lucas De Marchi

>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  29 ++-
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 205 +++++++++++++++++-
>  2 files changed, 227 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 981e24120a87..521e5b2e6f1c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1436,11 +1436,30 @@ static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
>
>         ref_clock = dev_priv->cdclk.hw.ref;
>
> -       m1 = pll_state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
> -       m2_int = pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> -       m2_frac = (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
> -               (pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> -               MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
> +       if (INTEL_GEN(dev_priv) >= 12) {
> +               m1 = pll_state->mg_pll_div0 & DKL_PLL_DIV0_FBPREDIV_MASK;
> +               m1 = m1 >> DKL_PLL_DIV0_FBPREDIV_SHIFT;
> +               m2_int = pll_state->mg_pll_div0 & DKL_PLL_DIV0_FBDIV_INT_MASK;
> +
> +               if (pll_state->mg_pll_bias & DKL_PLL_BIAS_FRAC_EN_H) {
> +                       m2_frac = pll_state->mg_pll_bias &
> +                                 DKL_PLL_BIAS_FBDIV_FRAC_MASK;
> +                       m2_frac = m2_frac >> DKL_PLL_BIAS_FBDIV_SHIFT;
> +               } else {
> +                       m2_frac = 0;
> +               }
> +       } else {
> +               m1 = pll_state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
> +               m2_int = pll_state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> +
> +               if (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) {
> +                       m2_frac = pll_state->mg_pll_div0 &
> +                                 MG_PLL_DIV0_FBDIV_FRAC_MASK;
> +                       m2_frac = m2_frac >> MG_PLL_DIV0_FBDIV_FRAC_SHIFT;
> +               } else {
> +                       m2_frac = 0;
> +               }
> +       }
>
>         switch (pll_state->mg_clktop2_hsclkctl &
>                 MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 9304606c1c0a..25be6229b122 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2605,6 +2605,202 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
>         return tc_port + DPLL_ID_ICL_MGPLL1;
>  }
>
> +static bool tgl_dkl_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
> +                                     u32 *target_dco_khz,
> +                                     struct intel_dpll_hw_state *state)
> +{
> +       u32 dco_min_freq, dco_max_freq;
> +       int div1_vals[] = {7, 5, 3, 2};
> +       int i, div2;
> +
> +       dco_min_freq = is_dp ? 8100000 : use_ssc ? 8000000 : 7992000;
> +       dco_max_freq = is_dp ? 8100000 : 10000000;
> +
> +       for (i = 0; i < ARRAY_SIZE(div1_vals); i++) {
> +               int div1 = div1_vals[i];
> +
> +               for (div2 = 10; div2 > 0; div2--) {
> +                       int dco = div1 * div2 * clock_khz * 5;
> +                       int inputsel, tlinedrv;
> +                       u32 hsdiv;
> +
> +                       if (dco < dco_min_freq || dco > dco_max_freq)
> +                               continue;
> +
> +                       state->mg_refclkin_ctl = MG_REFCLKIN_CTL_OD_2_MUX(1);
> +                       state->mg_clktop2_coreclkctl1 =
> +                                       MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO(5);
> +
> +                       tlinedrv = div2 >= 2 ? 1 : 2;
> +                       inputsel = is_dp ? 0 : 1;
> +
> +                       switch (div1) {
> +                       default:
> +                               MISSING_CASE(div1);
> +                               /* fall through */
> +                       case 2:
> +                               hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2;
> +                               break;
> +                       case 3:
> +                               hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_3;
> +                               break;
> +                       case 5:
> +                               hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_5;
> +                               break;
> +                       case 7:
> +                               hsdiv = MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_7;
> +                               break;
> +                       }
> +
> +                       *target_dco_khz = dco;
> +
> +                       state->mg_clktop2_hsclkctl =
> +                               MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL(tlinedrv) |
> +                               MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL(inputsel) |
> +                               hsdiv |
> +                               MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO(div2);
> +
> +                       return true;
> +               }
> +       }
> +
> +       return false;
> +}
> +
> +/*
> + * The specification for this function uses real numbers, so the math had to be
> + * adapted to integer-only calculation, that's why it looks so different.
> + */
> +static bool tgl_calc_dkl_pll_state(struct intel_crtc_state *crtc_state,
> +                                  struct intel_dpll_hw_state *pll_state)
> +{
> +       struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> +       int refclk_khz = dev_priv->cdclk.hw.ref;
> +       int symbol_frequency = crtc_state->port_clock;
> +       u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
> +       u32 iref_ndiv, iref_trim;
> +       u32 prop_coeff, int_coeff;
> +       u32 tdc_targetcnt, feedfwgain;
> +       u64 ssc_stepsize, ssc_steplen, ssc_steplog;
> +       u64 tmp;
> +       bool use_ssc = true;
> +       bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
> +
> +       memset(pll_state, 0, sizeof(*pll_state));
> +
> +       if (!tgl_dkl_pll_find_divisors(symbol_frequency, is_dp, use_ssc,
> +                                      &dco_khz, pll_state)) {
> +               DRM_DEBUG_KMS("Failed to find divisors for clock %d\n",
> +                             symbol_frequency);
> +               return false;
> +       }
> +
> +       m1div = 2;
> +       m2div_int = dco_khz / (refclk_khz * m1div);
> +       if (m2div_int > 255) {
> +               DRM_DEBUG_KMS("Failed to find mdiv for clock %d\n",
> +                             symbol_frequency);
> +               return false;
> +       }
> +       m2div_rem = dco_khz % (refclk_khz * m1div);
> +
> +       tmp = (u64)m2div_rem * (1 << 22);
> +       do_div(tmp, refclk_khz * m1div);
> +       m2div_frac = tmp;
> +
> +       switch (refclk_khz) {
> +       case 19200:
> +               iref_ndiv = 1;
> +               break;
> +       case 24000:
> +               iref_ndiv = 1;
> +               break;
> +       case 38400:
> +               iref_ndiv = 2;
> +               break;
> +       default:
> +               MISSING_CASE(refclk_khz);
> +               return false;
> +       }
> +
> +       /*
> +        * tdc_res = 0.000003
> +        * tdc_targetcnt = int(2 / (tdc_res * 8 * 50 * 1.1) / refclk_mhz + 0.5)
> +        *
> +        * The multiplication by 1000 is due to refclk MHz to KHz conversion. It
> +        * was supposed to be a division, but we rearranged the operations of
> +        * the formula to avoid early divisions so we don't multiply the
> +        * rounding errors.
> +        *
> +        * 0.000003 * 8 * 50 * 1.1 = 0.00132, also known as 132 / 100000, which
> +        * we also rearrange to work with integers.
> +        *
> +        * The 0.5 transformed to 5 results in a multiplication by 10 and the
> +        * last division by 10.
> +        */
> +       tdc_targetcnt = (2 * 1000 * 100000 * 10 / (132 * refclk_khz) + 5) / 10;
> +
> +       /*
> +        * Here we divide dco_khz by 10 in order to allow the dividend to fit in
> +        * 32 bits. That's not a problem since we round the division down
> +        * anyway.
> +        */
> +       feedfwgain = (use_ssc || m2div_rem > 0) ?
> +               m1div * 1000000 * 100 / (dco_khz * 3 / 10) : 0;
> +
> +       tmp = refclk_khz / iref_ndiv;
> +       if (tmp <= 19200)
> +               iref_trim = 28;
> +       else if (tmp <= 25000)
> +               iref_trim = 25;
> +       else
> +               iref_trim = 24;
> +
> +       if (dco_khz >= 9000000) {
> +               prop_coeff = 5;
> +               int_coeff = 10;
> +       } else {
> +               prop_coeff = 4;
> +               int_coeff = 8;
> +       }
> +
> +       if (use_ssc) {
> +               tmp = mul_u32_u32(dco_khz, 47 * 32);
> +               do_div(tmp, refclk_khz * m1div * 10000);
> +               ssc_stepsize = tmp;
> +
> +               tmp = mul_u32_u32(dco_khz, 1000);
> +               ssc_steplen = DIV_ROUND_UP_ULL(tmp, 32 * 2 * 32);
> +       } else {
> +               ssc_stepsize = 0;
> +               ssc_steplen = 0;
> +       }
> +       ssc_steplog = 4;
> +
> +       /* write pll_state calculations */
> +       pll_state->mg_pll_div0 = DKL_PLL_DIV0_INTEG_COEFF(int_coeff) |
> +                                DKL_PLL_DIV0_PROP_COEFF(prop_coeff) |
> +                                DKL_PLL_DIV0_FBPREDIV(m1div) |
> +                                DKL_PLL_DIV0_FBDIV_INT(m2div_int);
> +
> +       pll_state->mg_pll_div1 = DKL_PLL_DIV1_IREF_TRIM(iref_trim) |
> +                                DKL_PLL_DIV1_TDC_TARGET_CNT(tdc_targetcnt);
> +
> +       pll_state->mg_pll_ssc = DKL_PLL_SSC_IREF_NDIV_RATIO(iref_ndiv) |
> +                               DKL_PLL_SSC_STEP_LEN(ssc_steplen) |
> +                               DKL_PLL_SSC_STEP_NUM(ssc_steplog) |
> +                               (use_ssc ? DKL_PLL_SSC_EN : 0);
> +
> +       pll_state->mg_pll_bias = (m2div_frac > 0 ? DKL_PLL_BIAS_FRAC_EN_H : 0) |
> +                                DKL_PLL_BIAS_FBDIV_FRAC(m2div_frac);
> +
> +       pll_state->mg_pll_tdc_coldst_bias =
> +                       DKL_PLL_TDC_SSC_STEP_SIZE(ssc_stepsize) |
> +                       DKL_PLL_TDC_FEED_FWD_GAIN(feedfwgain);
> +
> +       return true;
> +}
> +
>  static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
>                                      u32 *target_dco_khz,
>                                      struct intel_dpll_hw_state *state)
> @@ -2932,6 +3128,7 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
>                 intel_atomic_get_new_crtc_state(state, crtc);
>         struct icl_port_dpll *port_dpll;
>         enum intel_dpll_id dpll_id;
> +       bool r;
>
>         port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
>         if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) {
> @@ -2950,9 +3147,13 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
>         intel_reference_shared_dpll(state, crtc,
>                                     port_dpll->pll, &port_dpll->hw_state);
>
> -
>         port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_MG_PHY];
> -       if (!icl_calc_mg_pll_state(crtc_state, &port_dpll->hw_state)) {
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               r = tgl_calc_dkl_pll_state(crtc_state, &port_dpll->hw_state);
> +       else
> +               r = icl_calc_mg_pll_state(crtc_state, &port_dpll->hw_state);
> +
> +       if (!r) {
>                 DRM_DEBUG_KMS("Could not calculate MG PHY PLL state.\n");
>                 goto err_unreference_tbt_pll;
>         }
> --
> 2.23.0
>
> _______________________________________________
> 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] 29+ messages in thread

* ✓ Fi.CI.IGT: success for TGL TC enabling
  2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
                   ` (15 preceding siblings ...)
  2019-09-13 23:12 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-15  7:07 ` Patchwork
  16 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2019-09-15  7:07 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: TGL TC enabling
URL   : https://patchwork.freedesktop.org/series/66695/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6894_full -> Patchwork_14410_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb3/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_pwrite@big-cpu-forwards:
    - shard-apl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#103927])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-apl6/igt@gem_pwrite@big-cpu-forwards.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-apl3/igt@gem_pwrite@big-cpu-forwards.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +5 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-apl1/igt@gem_workarounds@suspend-resume-context.html

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-glk:          [PASS][9] -> [DMESG-WARN][10] ([fdo#105763] / [fdo#106538])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-glk7/igt@i915_pm_rpm@modeset-stress-extra-wait.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-glk8/igt@i915_pm_rpm@modeset-stress-extra-wait.html

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#105363])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [PASS][13] -> [INCOMPLETE][14] ([fdo#103540])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-hsw5/igt@kms_flip@flip-vs-suspend-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-hsw5/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-iclb:         [PASS][15] -> [INCOMPLETE][16] ([fdo#107713])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@kms_flip_tiling@flip-to-x-tiled.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@kms_flip_tiling@flip-to-x-tiled.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +6 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@kms_psr@psr2_cursor_render.html

  * igt@perf@polling:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#110728])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-skl2/igt@perf@polling.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-skl8/igt@perf@polling.html

  * igt@prime_busy@after-bsd2:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109276]) +17 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@prime_busy@after-bsd2.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@prime_busy@after-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][25] ([fdo#108566]) -> [PASS][26] +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_eio@reset-stress:
    - shard-iclb:         [FAIL][27] ([fdo#109661]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@gem_eio@reset-stress.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@gem_eio@reset-stress.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][29] ([fdo#109276]) -> [PASS][30] +10 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb7/igt@gem_exec_schedule@independent-bsd2.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb2/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_exec_schedule@preempt-self-bsd:
    - shard-iclb:         [SKIP][31] ([fdo#111325]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@gem_exec_schedule@preempt-self-bsd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@gem_exec_schedule@preempt-self-bsd.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen:
    - shard-hsw:          [INCOMPLETE][33] ([fdo#103540]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-hsw8/igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-hsw2/igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          [DMESG-WARN][35] ([fdo#108566]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +3 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][39] ([fdo#108145] / [fdo#110403]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb1/igt@kms_psr@psr2_primary_mmap_cpu.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_rotation_crc@cursor-rotation-180:
    - shard-iclb:         [INCOMPLETE][43] ([fdo#107713] / [fdo#110026]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb7/igt@kms_rotation_crc@cursor-rotation-180.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb4/igt@kms_rotation_crc@cursor-rotation-180.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][45] ([fdo#110728]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-skl1/igt@perf@blocking.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-skl2/igt@perf@blocking.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][47] ([fdo#111329]) -> [SKIP][48] ([fdo#109276])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb4/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-rc6-bsd2:
    - shard-iclb:         [SKIP][49] ([fdo#109276]) -> [FAIL][50] ([fdo#111330])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb7/igt@gem_mocs_settings@mocs-rc6-bsd2.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb2/igt@gem_mocs_settings@mocs-rc6-bsd2.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [FAIL][51] ([fdo#111330]) -> [SKIP][52] ([fdo#109276])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6894/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14410/shard-iclb5/igt@gem_mocs_settings@mocs-settings-bsd2.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110026]: https://bugs.freedesktop.org/show_bug.cgi?id=110026
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): pig-hsw-4770r 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6894 -> Patchwork_14410

  CI-20190529: 20190529
  CI_DRM_6894: a323fd657c577491b1660662624bac36bb964222 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5182: f7104497049e3761ac297b66fd5586849b3cfcc8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14410: 2a0737c7916fe702fca52471d012f9af5032ccb6 @ 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_14410/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write
  2019-09-14  6:41   ` Lucas De Marchi
@ 2019-09-16  8:48     ` Jani Nikula
  0 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2019-09-16  8:48 UTC (permalink / raw)
  To: Lucas De Marchi, José Roberto de Souza
  Cc: Intel Graphics, Lucas De Marchi

On Fri, 13 Sep 2019, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
>>
>> From: Vandita Kulkarni <vandita.kulkarni@intel.com>
>>
>> Add a new function to write to dkl phy pll registers. As per the
>> bspec all the registers are read modify write.
>>
>> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 65 ++++++++++++++++++-
>>  1 file changed, 64 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> index 424f9213c80d..afc9b609b63d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> @@ -3293,7 +3293,70 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
>>  static void dkl_pll_write(struct drm_i915_private *dev_priv,
>>                           struct intel_shared_dpll *pll)
>>  {
>> -       /* TODO */
>> +       struct intel_dpll_hw_state *hw_state = &pll->state.hw_state;
>> +       enum tc_port tc_port = icl_pll_id_to_tc_port(pll->info->id);
>> +       u32 val;
>> +
>> +       /*
>> +        * All registers programmed here have the same HIP_INDEX_REG even
>> +        * though on different building block
>> +        */
>> +       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
>> +
>> +       /* All the registers are RMW */
>
> I wish we could use intel_uncore_rmw() here to make this comment go
> away and make it simpler
> in general. But I understand the conversion to use uncore should still
> not be done in display. Should we add
> I195_RMW() meanwhile or it's not worth it? +Jani.

We have tons of code like this in display/ and in the big picture this
makes no difference. Please let's not add I915_RMW or anything such
while we do have a path forward with intel_de_{read,write,rwm}. It just
hasn't happened yet.

BR,
Jani.



>
> Lucas De Marchi
>
>> +       val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
>> +       val &= ~MG_REFCLKIN_CTL_OD_2_MUX_MASK;
>> +       val |= hw_state->mg_refclkin_ctl;
>> +       I915_WRITE(DKL_REFCLKIN_CTL(tc_port), val);
>> +
>> +       val = I915_READ(DKL_CLKTOP2_CORECLKCTL1(tc_port));
>> +       val &= ~MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
>> +       val |= hw_state->mg_clktop2_coreclkctl1;
>> +       I915_WRITE(DKL_CLKTOP2_CORECLKCTL1(tc_port), val);
>> +
>> +       val = I915_READ(DKL_CLKTOP2_HSCLKCTL(tc_port));
>> +       val &= ~(MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
>> +              MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
>> +              MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
>> +              MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK);
>> +       val |= hw_state->mg_clktop2_hsclkctl;
>> +       I915_WRITE(DKL_CLKTOP2_HSCLKCTL(tc_port), val);
>> +
>> +       val = I915_READ(DKL_PLL_DIV0(tc_port));
>> +       val &= ~(DKL_PLL_DIV0_INTEG_COEFF_MASK |
>> +               DKL_PLL_DIV0_PROP_COEFF_MASK |
>> +               DKL_PLL_DIV0_FBPREDIV_MASK |
>> +               DKL_PLL_DIV0_FBDIV_INT_MASK);
>> +       val |= hw_state->mg_pll_div0;
>> +       I915_WRITE(DKL_PLL_DIV0(tc_port), val);
>> +
>> +       val = I915_READ(DKL_PLL_DIV1(tc_port));
>> +       val &= ~(DKL_PLL_DIV1_IREF_TRIM_MASK |
>> +                DKL_PLL_DIV1_TDC_TARGET_CNT_MASK);
>> +       val |= hw_state->mg_pll_div1;
>> +       I915_WRITE(DKL_PLL_DIV1(tc_port), val);
>> +
>> +       val = I915_READ(DKL_PLL_SSC(tc_port));
>> +       val &= ~(DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
>> +               DKL_PLL_SSC_STEP_LEN_MASK |
>> +               DKL_PLL_SSC_STEP_NUM_MASK |
>> +               DKL_PLL_SSC_EN);
>> +       val |= hw_state->mg_pll_ssc;
>> +       I915_WRITE(DKL_PLL_SSC(tc_port), val);
>> +
>> +       val = I915_READ(DKL_PLL_BIAS(tc_port));
>> +       val &= ~(DKL_PLL_BIAS_FRAC_EN_H |
>> +               DKL_PLL_BIAS_FBDIV_FRAC_MASK);
>> +       val |= hw_state->mg_pll_bias;
>> +       I915_WRITE(DKL_PLL_BIAS(tc_port), val);
>> +
>> +       val = I915_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
>> +       val &= ~(DKL_PLL_TDC_SSC_STEP_SIZE_MASK |
>> +               DKL_PLL_TDC_FEED_FWD_GAIN_MASK);
>> +       val |= hw_state->mg_pll_tdc_coldst_bias;
>> +       I915_WRITE(DKL_PLL_TDC_COLDST_BIAS(tc_port), val);
>> +
>> +       POSTING_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
>>  }
>>
>>  static void icl_pll_power_enable(struct drm_i915_private *dev_priv,
>> --
>> 2.23.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping
  2019-09-14  5:54   ` Lucas De Marchi
@ 2019-09-17 21:15     ` Souza, Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Souza, Jose @ 2019-09-17 21:15 UTC (permalink / raw)
  To: lucas.de.marchi; +Cc: intel-gfx, Taylor

On Fri, 2019-09-13 at 22:54 -0700, Lucas De Marchi wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>
> > 
> > Add a helper function to return pin map for use during dkl phy
> > DP_MODE settings, PORT_TX_DFLEXPA1 exist on ICL but we don't need
> > it.
> > 
> > The user of this function will come in future TC patches.
> 
> It's actually harder to review if the function is out of the context
> of those patches. Will it really be needed outside of intel_tc.c ?

It is used in intel_ddi.c, the user is added in "drm/i915/tgl: Add dkl
phy programming sequences" I guess I can move this patch right before
that one.

> 
> > Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
> 
> you need your s-o-b too if you are sending this patch.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_tc.h |  1 +
> >  drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
> >  3 files changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 85743a43bee2..3a7302e360cc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -61,6 +61,22 @@ u32 intel_tc_port_get_lane_mask(struct
> > intel_digital_port *dig_port)
> >                DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> >  }
> > 
> > +u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port)
> > +{
> > +       struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > +       enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> > +       struct intel_uncore *uncore = &i915->uncore;
> > +       u32 pin_mask;
> > +
> > +       pin_mask = intel_uncore_read(uncore,
> > +                                    PORT_TX_DFLEXPA1(dig_port-
> > >tc_phy_fia));
> > +
> > +       WARN_ON(pin_mask == 0xffffffff);
> > +
> > +       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
> > +              DP_PIN_ASSIGNMENT_SHIFT(tc_port);
> > +}
> > +
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> >  {
> >         struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> > b/drivers/gpu/drm/i915/display/intel_tc.h
> > index 783d75531435..463f1b3c836f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -13,6 +13,7 @@ struct intel_digital_port;
> > 
> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> >  u32 intel_tc_port_get_lane_mask(struct intel_digital_port
> > *dig_port);
> > +u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port);
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port);
> >  void intel_tc_port_set_fia_lane_count(struct intel_digital_port
> > *dig_port,
> >                                       int required_lanes);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index bf37ecebc82f..16d5548adb84 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -11683,4 +11683,9 @@ enum skl_power_gate {
> >  #define PORT_TX_DFLEXDPCSSS(fia)               _MMIO_FIA((fia),
> > 0x00894)
> >  #define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)         (1 <<
> > (tc_port))
> > 
> > +#define PORT_TX_DFLEXPA1(fia)                  _MMIO_FIA((fia),
> > 0x00880)
> > +#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)             ((tc_port)
> > * 4)
> > +#define   DP_PIN_ASSIGNMENT_MASK(tc_port)              (0xf <<
> > ((tc_port) * 4))
> 
> Use DP_PIN_ASSIGNMENT_SHIFT() here
> 
> > +#define   DP_PIN_ASSIGNMENT(tc_port, x)        ((x) << ((tc_port)
> > * 4))
> 
> ditto.
> 
> Lucas De Marchi
> 
> > +
> >  #endif /* _I915_REG_H_ */
> > --
> > 2.23.0
> > 
> > _______________________________________________
> > 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] 29+ messages in thread

* Re: [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called
  2019-09-14  6:32   ` Lucas De Marchi
@ 2019-09-17 22:59     ` Souza, Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Souza, Jose @ 2019-09-17 22:59 UTC (permalink / raw)
  To: lucas.de.marchi; +Cc: intel-gfx

On Fri, 2019-09-13 at 23:32 -0700, Lucas De Marchi wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > From: "Taylor, Clinton A" <clinton.a.taylor@intel.com>
> > 
> > TGL PLL function table doesn't include and update_active_pll
> > function.
> > The driver attempts to make a call to this function and crashes
> > during
> > PLL changes.
> 
> the crash would only occur if the port was initialized. The ordering
> in this series means this is
> not a "fix" but rather finishing the implementation for TC ports
> before initializing them. So we may
> want to adjust the commit message accordingly. The reason we missed
> that for TGL is that its need came
> in parallel to the TGL support hitting upstream.
> 

Okay will change commit message and description to:

drm/i915/tgl/pll: Set update_active_dpll

Commit 24a7bfe0c2d7 ("drm/i915: Keep the TypeC port mode fixed when the
port is active") added this new hook while in parallel TGL upstream was
happening and this was missed.
Without this driver will crash when TC DDI is added and driver is
preparing to do a full modeset.

Sounds good?


> My nit with this hook is that `update_active_dpll` is exclusively
> used
> by *TC* ports on gen 11+ and
> a) it's  not clear about that from the name and/or b) if it's generic
> it should not crash when it's missing.
> 
> I think Imre had a patch to address it, at least renaming the hook, I
> don't remember. +Imre.

Well we can do that on top or in Imre series not planing to address
this in here.

> 
> Lucas De Marchi

@Clint

Looks like git send-email don't like "," in the name and replaces your
email address to Taylor@freedesktop.org so I'm going to replace your
signed-off-by to "Clinton A Taylor <clinton.a.taylor@intel.com>" or do
you prefer something else?

> 
> > Signed-off-by: Taylor, Clinton A <clinton.a.taylor@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index 98288edf88f0..84e734d44828 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3479,6 +3479,7 @@ static const struct intel_dpll_mgr
> > tgl_pll_mgr = {
> >         .dpll_info = tgl_plls,
> >         .get_dplls = icl_get_dplls,
> >         .put_dplls = icl_put_dplls,
> > +       .update_active_dpll = icl_update_active_dpll,
> >         .dump_hw_state = icl_dump_hw_state,
> >  };
> > 
> > --
> > 2.23.0
> > 
> > _______________________________________________
> > 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] 29+ messages in thread

* Re: [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers
  2019-09-14  6:24   ` Lucas De Marchi
@ 2019-09-18  1:08     ` Souza, Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Souza, Jose @ 2019-09-18  1:08 UTC (permalink / raw)
  To: lucas.de.marchi; +Cc: intel-gfx

On Fri, 2019-09-13 at 23:24 -0700, Lucas De Marchi wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > If platform supports and has modular FIA is enabled, the registers
> > bits also change, example: reading TC3 registers with modular FIA
> > enabled, driver should read from FIA2 but with TC1 bits offsets.
> > 
> > It is described in BSpec 50231 for DFLEXDPSP, other registers don't
> > have the BSpec description but testing in real hardware have proven
> > that it had moved for all other registers too.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c |  2 +
> >  drivers/gpu/drm/i915/display/intel_tc.c      | 52 ++++++++++++--
> > ------
> >  drivers/gpu/drm/i915/display/intel_tc.h      |  2 +
> >  drivers/gpu/drm/i915/i915_drv.h              |  3 ++
> >  drivers/gpu/drm/i915/i915_reg.h              | 45 ++++++++------
> > ---
> >  5 files changed, 61 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 4e001113e828..cd0a248bfe49 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -15384,6 +15384,8 @@ static void intel_setup_outputs(struct
> > drm_i915_private *dev_priv)
> >         if (!HAS_DISPLAY(dev_priv))
> >                 return;
> > 
> > +       intel_tc_init(dev_priv);
> > +
> >         if (INTEL_GEN(dev_priv) >= 12) {
> >                 /* TODO: initialize TC ports as well */
> >                 intel_ddi_init(dev_priv, PORT_A);
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 3a7302e360cc..fc5d0e73cf21 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -23,19 +23,21 @@ static const char *tc_port_mode_name(enum
> > tc_port_mode mode)
> >         return names[mode];
> >  }
> > 
> > -static bool has_modular_fia(struct drm_i915_private *i915)
> > +void intel_tc_init(struct drm_i915_private *i915)
> >  {
> > +       u32 val;
> > +
> >         if (!INTEL_INFO(i915)->display.has_modular_fia)
> > -               return false;
> > +               return;
> > 
> > -       return intel_uncore_read(&i915->uncore,
> > -                                PORT_TX_DFLEXDPSP(FIA1)) &
> > MODULAR_FIA_MASK;
> > +       val = intel_uncore_read(&i915->uncore,
> > PORT_TX_DFLEXDPSP(FIA1));
> > +       i915->has_modular_fia = val & MODULAR_FIA_MASK;
> 
> Not a fan of stuffing tc-private details in i195 struct. Since this
> is
> only executed on initialization maybe it's
> not worth the few register reads we spare. If we go with this
> approach, then I think we should not use
> "has_" prefix so we don't get confused by the device_info fields.
> 
> >  }
> > 
> >  static enum phy_fia tc_port_to_fia(struct drm_i915_private *i915,
> >                                    enum tc_port tc_port)
> >  {
> > -       if (!has_modular_fia(i915))
> > +       if (!i915->has_modular_fia)
> >                 return FIA1;
> > 
> >         /*
> > @@ -51,14 +53,15 @@ u32 intel_tc_port_get_lane_mask(struct
> > intel_digital_port *dig_port)
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         struct intel_uncore *uncore = &i915->uncore;
> >         u32 lane_mask;
> > +       bool mod_fia = i915->has_modular_fia;
> 
> s/mod_fia/modular_fia/ or just don't add the additional var
> 
> >         lane_mask = intel_uncore_read(uncore,
> >                                       PORT_TX_DFLEXDPSP(dig_port-
> > >tc_phy_fia));
> > 
> >         WARN_ON(lane_mask == 0xffffffff);
> > 
> > -       return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
> > -              DP_LANE_ASSIGNMENT_SHIFT(tc_port);
> > +       return (lane_mask & DP_LANE_ASSIGNMENT_MASK(mod_fia,
> > tc_port)) >>
> > +              DP_LANE_ASSIGNMENT_SHIFT(mod_fia, tc_port);
> >  }
> > 
> >  u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port)
> > @@ -66,6 +69,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port)
> >         struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         struct intel_uncore *uncore = &i915->uncore;
> > +       bool mod_fia = i915->has_modular_fia;
> >         u32 pin_mask;
> > 
> >         pin_mask = intel_uncore_read(uncore,
> > @@ -73,8 +77,8 @@ u32 intel_tc_port_get_pin_assignment_mask(struct
> > intel_digital_port *dig_port)
> > 
> >         WARN_ON(pin_mask == 0xffffffff);
> > 
> > -       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(tc_port)) >>
> > -              DP_PIN_ASSIGNMENT_SHIFT(tc_port);
> > +       return (pin_mask & DP_PIN_ASSIGNMENT_MASK(mod_fia,
> > tc_port)) >>
> > +              DP_PIN_ASSIGNMENT_SHIFT(mod_fia, tc_port);
> >  }
> > 
> >  int intel_tc_port_fia_max_lane_count(struct intel_digital_port
> > *dig_port)
> > @@ -114,25 +118,28 @@ void intel_tc_port_set_fia_lane_count(struct
> > intel_digital_port *dig_port,
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         bool lane_reversal = dig_port->saved_port_bits &
> > DDI_BUF_PORT_REVERSAL;
> >         struct intel_uncore *uncore = &i915->uncore;
> > +       bool mod_fia = i915->has_modular_fia;
> >         u32 val;
> > 
> >         WARN_ON(lane_reversal && dig_port->tc_mode !=
> > TC_PORT_LEGACY);
> > 
> >         val = intel_uncore_read(uncore,
> >                                 PORT_TX_DFLEXDPMLE1(dig_port-
> > >tc_phy_fia));
> > -       val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
> > +       val &= ~DFLEXDPMLE1_DPMLETC_MASK(mod_fia, tc_port);
> > 
> >         switch (required_lanes) {
> >         case 1:
> > -               val |= lane_reversal ?
> > DFLEXDPMLE1_DPMLETC_ML3(tc_port) :
> > -                       DFLEXDPMLE1_DPMLETC_ML0(tc_port);
> > +               val |= lane_reversal ?
> > +                       DFLEXDPMLE1_DPMLETC_ML3(mod_fia, tc_port) :
> > +                       DFLEXDPMLE1_DPMLETC_ML0(mod_fia, tc_port);
> >                 break;
> >         case 2:
> > -               val |= lane_reversal ?
> > DFLEXDPMLE1_DPMLETC_ML3_2(tc_port) :
> > -                       DFLEXDPMLE1_DPMLETC_ML1_0(tc_port);
> > +               val |= lane_reversal ?
> > +                       DFLEXDPMLE1_DPMLETC_ML3_2(mod_fia, tc_port)
> > :
> > +                       DFLEXDPMLE1_DPMLETC_ML1_0(mod_fia,
> > tc_port);
> >                 break;
> >         case 4:
> > -               val |= DFLEXDPMLE1_DPMLETC_ML3_0(tc_port);
> > +               val |= DFLEXDPMLE1_DPMLETC_ML3_0(mod_fia, tc_port);
> >                 break;
> >         default:
> >                 MISSING_CASE(required_lanes);
> > @@ -180,9 +187,9 @@ static u32 tc_port_live_status_mask(struct
> > intel_digital_port *dig_port)
> >                 return mask;
> >         }
> > 
> > -       if (val & TC_LIVE_STATE_TBT(tc_port))
> > +       if (val & TC_LIVE_STATE_TBT(i915->has_modular_fia,
> > tc_port))
> >                 mask |= BIT(TC_PORT_TBT_ALT);
> > -       if (val & TC_LIVE_STATE_TC(tc_port))
> > +       if (val & TC_LIVE_STATE_TC(i915->has_modular_fia, tc_port))
> >                 mask |= BIT(TC_PORT_DP_ALT);
> > 
> >         if (intel_uncore_read(uncore, SDEISR) &
> > SDE_TC_HOTPLUG_ICP(tc_port))
> > @@ -200,6 +207,7 @@ static bool icl_tc_phy_status_complete(struct
> > intel_digital_port *dig_port)
> >         struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         struct intel_uncore *uncore = &i915->uncore;
> > +       bool mod_fia = i915->has_modular_fia;
> >         u32 val;
> > 
> >         val = intel_uncore_read(uncore,
> > @@ -210,7 +218,7 @@ static bool icl_tc_phy_status_complete(struct
> > intel_digital_port *dig_port)
> >                 return false;
> >         }
> > 
> > -       return val & DP_PHY_MODE_STATUS_COMPLETED(tc_port);
> > +       return val & DP_PHY_MODE_STATUS_COMPLETED(mod_fia,
> > tc_port);
> >  }
> > 
> >  static bool icl_tc_phy_set_safe_mode(struct intel_digital_port
> > *dig_port,
> > @@ -219,6 +227,7 @@ static bool icl_tc_phy_set_safe_mode(struct
> > intel_digital_port *dig_port,
> >         struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         struct intel_uncore *uncore = &i915->uncore;
> > +       bool mod_fia = i915->has_modular_fia;
> >         u32 val;
> > 
> >         val = intel_uncore_read(uncore,
> > @@ -231,9 +240,9 @@ static bool icl_tc_phy_set_safe_mode(struct
> > intel_digital_port *dig_port,
> >                 return false;
> >         }
> > 
> > -       val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > +       val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia, tc_port);
> >         if (!enable)
> > -               val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
> > +               val |= DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia,
> > tc_port);
> > 
> >         intel_uncore_write(uncore,
> >                            PORT_TX_DFLEXDPCSSS(dig_port-
> > >tc_phy_fia), val);
> > @@ -250,6 +259,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct
> > intel_digital_port *dig_port)
> >         struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> >         enum tc_port tc_port = intel_port_to_tc(i915, dig_port-
> > >base.port);
> >         struct intel_uncore *uncore = &i915->uncore;
> > +       bool mod_fia = i915->has_modular_fia;
> >         u32 val;
> > 
> >         val = intel_uncore_read(uncore,
> > @@ -260,7 +270,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct
> > intel_digital_port *dig_port)
> >                 return true;
> >         }
> > 
> > -       return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port));
> > +       return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(mod_fia,
> > tc_port));
> >  }
> > 
> >  /*
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> > b/drivers/gpu/drm/i915/display/intel_tc.h
> > index 463f1b3c836f..2374352d7c31 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/types.h>
> > 
> >  struct intel_digital_port;
> > +struct drm_i915_private;
> > 
> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port);
> >  u32 intel_tc_port_get_lane_mask(struct intel_digital_port
> > *dig_port);
> > @@ -27,5 +28,6 @@ void intel_tc_port_put_link(struct
> > intel_digital_port *dig_port);
> >  bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
> > 
> >  void intel_tc_port_init(struct intel_digital_port *dig_port, bool
> > is_legacy);
> > +void intel_tc_init(struct drm_i915_private *dev_priv);
> > 
> >  #endif /* __INTEL_TC_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index bf600888b3f1..a36d1929fde1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1765,6 +1765,9 @@ struct drm_i915_private {
> >         /* Mutex to protect the above hdcp component related
> > values. */
> >         struct mutex hdcp_comp_mutex;
> > 
> > +       /* Monolithic or modular FIA */
> > +       bool has_modular_fia;
> > +
> >         /*
> >          * NOTE: This is the dri1/ums dungeon, don't add stuff
> > here. Your patch
> >          * will be rejected. Instead look for a better place.
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 16d5548adb84..8aaf53283200 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2165,14 +2165,15 @@ static inline bool
> > i915_mmio_reg_valid(i915_reg_t reg)
> >  #define _FIA(fia)                      _PICK((fia), FIA1_BASE,
> > FIA2_BASE, FIA3_BASE)
> >  #define _MMIO_FIA(fia, off)            _MMIO(_FIA(fia) + (off))
> > 
> > +#define _TC_MOD_PORT_ID(has_modular_fia,
> > tc_port)      ((has_modular_fia) ? (tc_port) % 2 : (tc_port))
> 
> instead of doing all of this, what about storing a
> dig_port->tc_phy_fia_idx that is set to
> tc_port on monolithic FIA and to tc_port % 2 on modular...? Then it
> would be initialized together with
> dig_port->tc_phy_fia and we could even remove tc_port_to_fia() that
> would not be used anymore since we would
> check that in the caller.

Looks, better going to move to tc_phy_fia_idx.

Thanks

> 
> >  /* ICL PHY DFLEX registers */
> > -#define PORT_TX_DFLEXDPMLE1(fia)       _MMIO_FIA((fia),  0x008C0)
> > -#define   DFLEXDPMLE1_DPMLETC_MASK(tc_port)    (0xf << (4 *
> > (tc_port)))
> 
> in all these macros it would be s/tc_port/idx/
> 
> Lucas De Marchi
> 
> > -#define   DFLEXDPMLE1_DPMLETC_ML0(tc_port)     (1 << (4 *
> > (tc_port)))
> > -#define   DFLEXDPMLE1_DPMLETC_ML1_0(tc_port)   (3 << (4 *
> > (tc_port)))
> > -#define   DFLEXDPMLE1_DPMLETC_ML3(tc_port)     (8 << (4 *
> > (tc_port)))
> > -#define   DFLEXDPMLE1_DPMLETC_ML3_2(tc_port)   (12 << (4 *
> > (tc_port)))
> > -#define   DFLEXDPMLE1_DPMLETC_ML3_0(tc_port)   (15 << (4 *
> > (tc_port)))
> > +#define
> > PORT_TX_DFLEXDPMLE1(fia)                       _MMIO_FIA((fia),  0x
> > 008C0)
> > +#define   DFLEXDPMLE1_DPMLETC_MASK(mod, tc_port)       (0xf << (4
> > * (_TC_MOD_PORT_ID(mod, tc_port))))
> > +#define   DFLEXDPMLE1_DPMLETC_ML0(mod, tc_port)                (1
> > << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> > +#define   DFLEXDPMLE1_DPMLETC_ML1_0(mod, tc_port)      (3 << (4 *
> > (_TC_MOD_PORT_ID(mod, tc_port))))
> > +#define   DFLEXDPMLE1_DPMLETC_ML3(mod, tc_port)                (8
> > << (4 * (_TC_MOD_PORT_ID(mod, tc_port))))
> > +#define   DFLEXDPMLE1_DPMLETC_ML3_2(mod, tc_port)      (12 << (4 *
> > (_TC_MOD_PORT_ID(mod, tc_port))))
> > +#define   DFLEXDPMLE1_DPMLETC_ML3_0(mod, tc_port)      (15 << (4 *
> > (_TC_MOD_PORT_ID(mod, tc_port))))
> > 
> >  /* BXT PHY Ref registers */
> >  #define _PORT_REF_DW3_A                        0x16218C
> > @@ -11669,23 +11670,23 @@ enum skl_power_gate {
> >                                                 _ICL_DSC1_RC_BUF_TH
> > RESH_1_UDW_PB, \
> >                                                 _ICL_DSC1_RC_BUF_TH
> > RESH_1_UDW_PC)
> > 
> > -#define PORT_TX_DFLEXDPSP(fia)                 _MMIO_FIA((fia),
> > 0x008A0)
> > -#define   MODULAR_FIA_MASK                     (1 << 4)
> > -#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))
> > +#define
> > PORT_TX_DFLEXDPSP(fia)                         _MMIO_FIA((fia),
> > 0x008A0)
> > +#define   MODULAR_FIA_MASK                             (1 << 4)
> > +#define   TC_LIVE_STATE_TBT(mod, tc_port)              (1 <<
> > ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 6))
> > +#define   TC_LIVE_STATE_TC(mod, tc_port)               (1 <<
> > ((_TC_MOD_PORT_ID(mod, tc_port)) * 8 + 5))
> > +#define   DP_LANE_ASSIGNMENT_SHIFT(mod,
> > tc_port)       ((_TC_MOD_PORT_ID(mod, tc_port)) * 8)
> > +#define   DP_LANE_ASSIGNMENT_MASK(mod,
> > tc_port)                (0xf << ((_TC_MOD_PORT_ID(mod, tc_port)) *
> > 8))
> > +#define   DP_LANE_ASSIGNMENT(mod, tc_port, x)          ((x) <<
> > ((_TC_MOD_PORT_ID(mod, tc_port)) * 8))
> > 
> > -#define
> > PORT_TX_DFLEXDPPMS(fia)                        _MMIO_FIA((fia),
> > 0x00890)
> > -#define   DP_PHY_MODE_STATUS_COMPLETED(tc_port)                (1
> > << (tc_port))
> > +#define
> > PORT_TX_DFLEXDPPMS(fia)                                _MMIO_FIA((f
> > ia), 0x00890)
> > +#define   DP_PHY_MODE_STATUS_COMPLETED(mod, tc_port)   (1 <<
> > (_TC_MOD_PORT_ID(mod, tc_port)))
> > 
> > -#define PORT_TX_DFLEXDPCSSS(fia)               _MMIO_FIA((fia),
> > 0x00894)
> > -#define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)         (1 <<
> > (tc_port))
> > +#define
> > PORT_TX_DFLEXDPCSSS(fia)                       _MMIO_FIA((fia),
> > 0x00894)
> > +#define   DP_PHY_MODE_STATUS_NOT_SAFE(mod, tc_port)    (1 <<
> > (_TC_MOD_PORT_ID(mod, tc_port)))
> > 
> > -#define PORT_TX_DFLEXPA1(fia)                  _MMIO_FIA((fia),
> > 0x00880)
> > -#define   DP_PIN_ASSIGNMENT_SHIFT(tc_port)             ((tc_port)
> > * 4)
> > -#define   DP_PIN_ASSIGNMENT_MASK(tc_port)              (0xf <<
> > ((tc_port) * 4))
> > -#define   DP_PIN_ASSIGNMENT(tc_port, x)        ((x) << ((tc_port)
> > * 4))
> > +#define
> > PORT_TX_DFLEXPA1(fia)                          _MMIO_FIA((fia),
> > 0x00880)
> > +#define   DP_PIN_ASSIGNMENT_SHIFT(mod,
> > tc_port)                ((_TC_MOD_PORT_ID(mod, tc_port)) * 4)
> > +#define   DP_PIN_ASSIGNMENT_MASK(mod, tc_port) (0xf <<
> > ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
> > +#define   DP_PIN_ASSIGNMENT(mod, tc_port, x)   ((x) <<
> > ((_TC_MOD_PORT_ID(mod, tc_port)) * 4))
> > 
> >  #endif /* _I915_REG_H_ */
> > --
> > 2.23.0
> > 
> > _______________________________________________
> > 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] 29+ messages in thread

* Re: [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing
  2019-09-14  7:26   ` Lucas De Marchi
@ 2019-09-18 19:55     ` Souza, Jose
  0 siblings, 0 replies; 29+ messages in thread
From: Souza, Jose @ 2019-09-18 19:55 UTC (permalink / raw)
  To: lucas.de.marchi; +Cc: intel-gfx

On Sat, 2019-09-14 at 00:26 -0700, Lucas De Marchi wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > It was always modifing register space of the first phy in the
> > HIP_INDEX_REG for all ports while it should shift 8 bits for each
> > port inside of HIP_INDEX_REG.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 34
> > +++++++++++++++----
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
> >  drivers/gpu/drm/i915/i915_reg.h               |  3 ++
> >  3 files changed, 38 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index a6a2e00cc075..981e24120a87 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2795,7 +2795,10 @@ tgl_dkl_phy_ddi_vswing_sequence(struct
> > intel_encoder *encoder, int link_clock,
> >          * All registers programmed here use HIP_INDEX_REG 0 or 1
> >          */
> >         for (ln = 0; ln < 2; ln++) {
> > -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> > +               val = I915_READ(HIP_INDEX_REG(tc_port));
> > +               val &= ~HIP_INDEX_MASK(tc_port);
> > +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> 
> INDEX_INDEX?
> 
> > +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
> 
> we don't really care for a RMW here, do we? We don't access these
> registers in parallel for other ports
> (It would be inherently racy if we did), so
> 
>         I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port,
> ln));
> 
> should be sufficient.
> 

Okay

> Also I think all these fixes should be squashed in their
> correspondent
> patches in this series with changelogs
> updated.
> 

Yeah, I was trying to avoid the fatigue :D
But I will squash in each patch

> >                 /* All the registers are RMW */
> >                 val = I915_READ(DKL_TX_DPCNTL0(tc_port));
> > @@ -3134,7 +3137,10 @@ tgl_phy_clock_gating(struct
> > intel_digital_port *dig_port, bool enable)
> >                DKL_DP_MODE_CFG_GAONPWR_GATING;
> > 
> >         for (ln = 0; ln < 2; ln++) {
> > -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> > +               val = I915_READ(HIP_INDEX_REG(tc_port));
> > +               val &= ~HIP_INDEX_MASK(tc_port);
> > +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> > +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >                 val = I915_READ(DKL_DP_MODE(tc_port));
> >                 if (enable)
> > @@ -3249,16 +3255,23 @@ static void tgl_program_dkl_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >         struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> >         enum port port = intel_dig_port->base.port;
> >         enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > -       u32 ln0, ln1, lane_mask, pin_mask;
> > +       u32 ln0, ln1, lane_mask, pin_mask, val;
> >         int num_lanes;
> > 
> >         if (tc_port == PORT_TC_NONE ||
> >             intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> >                 return;
> > 
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         ln0 = I915_READ(DKL_DP_MODE(tc_port));
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> > +
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         ln1 = I915_READ(DKL_DP_MODE(tc_port));
> > 
> >         num_lanes = intel_dig_port->dp.lane_count;
> > @@ -3327,9 +3340,16 @@ static void tgl_program_dkl_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >                 return;
> >         }
> > 
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> > +
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         I915_WRITE(DKL_DP_MODE(tc_port), ln1);
> >  }
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index afc9b609b63d..9304606c1c0a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3109,7 +3109,10 @@ static bool dkl_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >          * All registers read here have the same HIP_INDEX_REG even
> > though
> >          * they are on different building blocks
> >          */
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >         hw_state->mg_refclkin_ctl =
> > I915_READ(DKL_REFCLKIN_CTL(tc_port));
> >         hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> > @@ -3301,7 +3304,10 @@ static void dkl_pll_write(struct
> > drm_i915_private *dev_priv,
> >          * All registers programmed here have the same
> > HIP_INDEX_REG even
> >          * though on different building block
> >          */
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >         /* All the registers are RMW */
> >         val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 95a4232c8e0a..625f458e9b1c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10265,6 +10265,9 @@ enum skl_power_gate {
> >  #define HIP_INDEX_REG(tc_port) _MMIO((tc_port) < 4 \
> >                                       ? _HIP_INDEX_REG0 \
> >                                       : _HIP_INDEX_REG1)
> > +#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
> > +#define HIP_INDEX_MASK(tc_port) (0xff <<
> > _HIP_INDEX_SHIFT(tc_port))
> > +#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) <<
> > _HIP_INDEX_SHIFT(tc_port))
> 
> #define HIP_INDEX_VAL(index) ((index) | (index) << 8 | ( index) << 16
> > (index) << 24)
> ?

I guess the current one is better, more easy to understand.

> 
> Lucas De Marchi
> 
> >  /* BXT display engine PLL */
> >  #define BXT_DE_PLL_CTL                 _MMIO(0x6d000)
> > --
> > 2.23.0
> > 
> > _______________________________________________
> > 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] 29+ messages in thread

end of thread, other threads:[~2019-09-18 19:55 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 22:32 [PATCH 00/14] TGL TC enabling José Roberto de Souza
2019-09-13 22:32 ` [PATCH 01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence José Roberto de Souza
2019-09-13 22:32 ` [PATCH 02/14] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
2019-09-14  5:54   ` Lucas De Marchi
2019-09-17 21:15     ` Souza, Jose
2019-09-13 22:32 ` [PATCH 03/14] drm/i915/tgl: Finish modular FIA support on registers José Roberto de Souza
2019-09-14  6:24   ` Lucas De Marchi
2019-09-18  1:08     ` Souza, Jose
2019-09-13 22:32 ` [PATCH 04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called José Roberto de Souza
2019-09-14  6:32   ` Lucas De Marchi
2019-09-17 22:59     ` Souza, Jose
2019-09-13 22:32 ` [PATCH 05/14] drm/i915/tgl: Add dkl phy registers José Roberto de Souza
2019-09-13 22:32 ` [PATCH 06/14] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
2019-09-13 22:32 ` [PATCH 07/14] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
2019-09-14  6:41   ` Lucas De Marchi
2019-09-16  8:48     ` Jani Nikula
2019-09-13 22:32 ` [PATCH 08/14] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
2019-09-13 22:32 ` [PATCH 09/14] drm/i915/icl: Unify disable and enable phy clock gating functions José Roberto de Souza
2019-09-13 22:32 ` [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing José Roberto de Souza
2019-09-14  7:26   ` Lucas De Marchi
2019-09-18 19:55     ` Souza, Jose
2019-09-13 22:32 ` [PATCH 11/14] drm/i915/tgl: Check the UC health of tc controllers after power on José Roberto de Souza
2019-09-13 22:32 ` [PATCH 12/14] drm/i915: Add dkl phy pll calculations José Roberto de Souza
2019-09-14  7:38   ` Lucas De Marchi
2019-09-13 22:32 ` [PATCH 13/14] drm/i915/tgl: Use dkl pll hardcoded values José Roberto de Souza
2019-09-13 22:32 ` [PATCH 14/14] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
2019-09-13 22:53 ` ✗ Fi.CI.CHECKPATCH: warning for TGL TC enabling Patchwork
2019-09-13 23:12 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-15  7:07 ` ✓ Fi.CI.IGT: " 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.