All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] TGL TC enabling v3
@ 2019-09-23 19:55 José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx

v1: https://patchwork.freedesktop.org/series/66695/#rev1
v2: https://patchwork.freedesktop.org/series/66695/#rev2
v2 patches merged: https://patchwork.freedesktop.org/series/67022/

Major differences from v2 is that some patches were already merged and applied the fixes requested over v2 patches, all noted in each patch.

Clinton A Taylor (2):
  drm/i915/tgl: TC helper function to return pin mapping
  drm/i915/tgl: Add dkl phy programming sequences

José Roberto de Souza (3):
  drm/i915/tgl: Add dkl phy pll calculations
  drm/i915/tgl: Fix dkl link training
  drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports

Lucas De Marchi (3):
  drm/i915/tgl: Add initial dkl pll support
  drm/i915/tgl: re-indent code to prepare for DKL changes
  drm/i915/tgl: initialize TC and TBT ports

Vandita Kulkarni (1):
  drm/i915/tgl: Add support for dkl pll write

 drivers/gpu/drm/i915/display/intel_ddi.c      | 280 +++++++++++++-
 drivers/gpu/drm/i915/display/intel_display.c  |   7 +-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 343 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_tc.c       |  15 +
 drivers/gpu/drm/i915/display/intel_tc.h       |   1 +
 drivers/gpu/drm/i915/i915_reg.h               |   5 +
 6 files changed, 577 insertions(+), 74 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] 27+ messages in thread

* [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-24 14:20   ` Imre Deak
  2019-09-23 19:55 ` [PATCH v3 2/9] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 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.

v2:
Setting the right HIP_INDEX_REG bits (José)

Acked-by: Lucas De Marchi <lucas.demarchi@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 | 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..46dde614bfb5 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), HIP_INDEX_VAL(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] 27+ messages in thread

* [PATCH v3 2/9] drm/i915/tgl: Add support for dkl pll write
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 3/9] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 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.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
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 46dde614bfb5..21249997940d 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), HIP_INDEX_VAL(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] 27+ messages in thread

* [PATCH v3 3/9] drm/i915/tgl: TC helper function to return pin mapping
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 2/9] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 19:55 ` [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

From: Clinton A Taylor <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.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_tc.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/display/intel_tc.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index f923f9cbd33c..7773169b7331 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 	return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
 }
 
+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);
+	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(dig_port->tc_phy_fia_idx)) >>
+	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
+}
+
 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 a69c19aae5bb..d3c3a9ae8c32 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11862,6 +11862,11 @@ enum skl_power_gate {
 #define PORT_TX_DFLEXDPCSSS(fia)		_MMIO_FIA((fia), 0x00894)
 #define   DP_PHY_MODE_STATUS_NOT_SAFE(idx)	(1 << (idx))
 
+#define PORT_TX_DFLEXPA1(fia)			_MMIO_FIA((fia), 0x00880)
+#define   DP_PIN_ASSIGNMENT_SHIFT(idx)		((idx) * 4)
+#define   DP_PIN_ASSIGNMENT_MASK(idx)		(0xf << ((idx) * 4))
+#define   DP_PIN_ASSIGNMENT(idx, x)		((x) << ((idx) * 4))
+
 /* This register controls the Display State Buffer (DSB) engines. */
 #define _DSBSL_INSTANCE_BASE		0x70B00
 #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
-- 
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] 27+ messages in thread

* [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (2 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 3/9] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 22:02   ` Lucas De Marchi
  2019-09-23 19:55 ` [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes José Roberto de Souza
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx

From: Clinton A Taylor <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.

v2:
Setting the right HIP_INDEX_REG bits (José)

v3:
Adding the meaning of each column of tgl_dkl_phy_ddi_translations
Adding if gen >= 12 on intel_ddi_hdmi_level() and
intel_ddi_pre_enable_hdmi() instead of reuse part of gen >= 11 if

BSpec: 49292
BSpec: 49190

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

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 33cd766f9eea..1ab3e0c4c0a1 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -586,6 +586,26 @@ 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[] = {
+				/* VS	pre-emp	Non-trans mV	Pre-emph dB */
+	{ 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)
 {
@@ -872,7 +892,14 @@ 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_GEN(dev_priv) >= 12) {
+		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(tgl_dkl_phy_ddi_translations);
+		default_entry = n_entries - 1;
+	} else if (INTEL_GEN(dev_priv) == 11) {
 		if (intel_phy_is_combo(dev_priv, phy))
 			icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI,
 						0, &n_entries);
@@ -2313,7 +2340,13 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 	enum phy phy = intel_port_to_phy(dev_priv, port);
 	int n_entries;
 
-	if (INTEL_GEN(dev_priv) >= 11) {
+	if (INTEL_GEN(dev_priv) >= 12) {
+		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(tgl_dkl_phy_ddi_translations);
+	} else if (INTEL_GEN(dev_priv) == 11) {
 		if (intel_phy_is_combo(dev_priv, phy))
 			icl_get_combo_buf_trans(dev_priv, encoder->type,
 						intel_dp->link_rate, &n_entries);
@@ -2755,6 +2788,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), HIP_INDEX_VAL(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;
@@ -2786,7 +2879,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))
@@ -3033,6 +3129,34 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	}
 }
 
+static void
+tgl_phy_set_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, bits;
+	int ln;
+
+	if (tc_port == PORT_TC_NONE)
+		return;
+
+	bits = 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), HIP_INDEX_VAL(tc_port, ln));
+
+		val = I915_READ(DKL_DP_MODE(tc_port));
+		if (enable)
+			val |= bits;
+		else
+			val &= ~bits;
+		I915_WRITE(DKL_DP_MODE(tc_port), val);
+	}
+}
+
 static void
 icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable)
 {
@@ -3132,6 +3256,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), HIP_INDEX_VAL(tc_port, 0x0));
+	ln0 = I915_READ(DKL_DP_MODE(tc_port));
+	I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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), HIP_INDEX_VAL(tc_port, 0x0));
+	I915_WRITE(DKL_DP_MODE(tc_port), ln0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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)
 {
@@ -3218,7 +3431,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
@@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	intel_ddi_config_transcoder_func(crtc_state);
 
 	/* 7.d */
-	icl_phy_set_clock_gating(dig_port, false);
+	tgl_phy_set_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 */
@@ -3266,6 +3479,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 now.
+	 */
+
 	/* 7.l */
 	intel_ddi_enable_fec(encoder, crtc_state);
 	intel_dsc_enable(encoder, crtc_state);
@@ -3371,9 +3593,15 @@ 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_phy_set_clock_gating(dig_port, false);
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_phy_set_clock_gating(dig_port, false);
+	else
+		icl_phy_set_clock_gating(dig_port, false);
 
-	if (INTEL_GEN(dev_priv) >= 11)
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
+					level, INTEL_OUTPUT_HDMI);
+	else if (INTEL_GEN(dev_priv) == 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, INTEL_OUTPUT_HDMI);
 	else if (IS_CANNONLAKE(dev_priv))
@@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	else
 		intel_prepare_hdmi_ddi_buffers(encoder, level);
 
-	icl_phy_set_clock_gating(dig_port, true);
+	if (INTEL_GEN(dev_priv) >= 12)
+		tgl_phy_set_clock_gating(dig_port, true);
+	else
+		icl_phy_set_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] 27+ messages in thread

* [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (3 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 22:04   ` Lucas De Marchi
  2019-09-23 19:55 ` [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations José Roberto de Souza
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

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

The final save operation into pll_state of the calculations done will
be different for DKL PHY. Prepare for that by reindenting code so it's
easier to check for correctness. This one has no change in behavior.

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/display/intel_dpll_mgr.c | 119 ++++++++++--------
 1 file changed, 66 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 21249997940d..9bae6f2d0f36 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2786,60 +2786,73 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 	}
 	ssc_steplog = 4;
 
-	pll_state->mg_pll_div0 = (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
-				  MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
-				  MG_PLL_DIV0_FBDIV_INT(m2div_int);
-
-	pll_state->mg_pll_div1 = MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
-				 MG_PLL_DIV1_DITHER_DIV_2 |
-				 MG_PLL_DIV1_NDIVRATIO(1) |
-				 MG_PLL_DIV1_FBPREDIV(m1div);
-
-	pll_state->mg_pll_lf = MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
-			       MG_PLL_LF_AFCCNTSEL_512 |
-			       MG_PLL_LF_GAINCTRL(1) |
-			       MG_PLL_LF_INT_COEFF(int_coeff) |
-			       MG_PLL_LF_PROP_COEFF(prop_coeff);
-
-	pll_state->mg_pll_frac_lock = MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
-				      MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
-				      MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
-				      MG_PLL_FRAC_LOCK_DCODITHEREN |
-				      MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
-	if (use_ssc || m2div_rem > 0)
-		pll_state->mg_pll_frac_lock |= MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
-
-	pll_state->mg_pll_ssc = (use_ssc ? MG_PLL_SSC_EN : 0) |
-				MG_PLL_SSC_TYPE(2) |
-				MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
-				MG_PLL_SSC_STEPNUM(ssc_steplog) |
-				MG_PLL_SSC_FLLEN |
-				MG_PLL_SSC_STEPSIZE(ssc_stepsize);
-
-	pll_state->mg_pll_tdc_coldst_bias = MG_PLL_TDC_COLDST_COLDSTART |
-					    MG_PLL_TDC_COLDST_IREFINT_EN |
-					    MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
-					    MG_PLL_TDC_TDCOVCCORR_EN |
-					    MG_PLL_TDC_TDCSEL(3);
-
-	pll_state->mg_pll_bias = MG_PLL_BIAS_BIAS_GB_SEL(3) |
-				 MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
-				 MG_PLL_BIAS_BIAS_BONUS(10) |
-				 MG_PLL_BIAS_BIASCAL_EN |
-				 MG_PLL_BIAS_CTRIM(12) |
-				 MG_PLL_BIAS_VREF_RDAC(4) |
-				 MG_PLL_BIAS_IREFTRIM(iref_trim);
-
-	if (refclk_khz == 38400) {
-		pll_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART;
-		pll_state->mg_pll_bias_mask = 0;
-	} else {
-		pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
-		pll_state->mg_pll_bias_mask = -1U;
-	}
+	/* write pll_state calculations */
+	{
+		pll_state->mg_pll_div0 =
+			(m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
+			MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
+			MG_PLL_DIV0_FBDIV_INT(m2div_int);
+
+		pll_state->mg_pll_div1 =
+			MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
+			MG_PLL_DIV1_DITHER_DIV_2 |
+			MG_PLL_DIV1_NDIVRATIO(1) |
+			MG_PLL_DIV1_FBPREDIV(m1div);
+
+		pll_state->mg_pll_lf =
+			MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
+			MG_PLL_LF_AFCCNTSEL_512 |
+			MG_PLL_LF_GAINCTRL(1) |
+			MG_PLL_LF_INT_COEFF(int_coeff) |
+			MG_PLL_LF_PROP_COEFF(prop_coeff);
+
+		pll_state->mg_pll_frac_lock =
+			MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
+			MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
+			MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
+			MG_PLL_FRAC_LOCK_DCODITHEREN |
+			MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
+		if (use_ssc || m2div_rem > 0)
+			pll_state->mg_pll_frac_lock |=
+				MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
+
+		pll_state->mg_pll_ssc =
+			(use_ssc ? MG_PLL_SSC_EN : 0) |
+			MG_PLL_SSC_TYPE(2) |
+			MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
+			MG_PLL_SSC_STEPNUM(ssc_steplog) |
+			MG_PLL_SSC_FLLEN |
+			MG_PLL_SSC_STEPSIZE(ssc_stepsize);
+
+		pll_state->mg_pll_tdc_coldst_bias =
+			MG_PLL_TDC_COLDST_COLDSTART |
+			MG_PLL_TDC_COLDST_IREFINT_EN |
+			MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
+			MG_PLL_TDC_TDCOVCCORR_EN |
+			MG_PLL_TDC_TDCSEL(3);
+
+		pll_state->mg_pll_bias =
+			MG_PLL_BIAS_BIAS_GB_SEL(3) |
+			MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
+			MG_PLL_BIAS_BIAS_BONUS(10) |
+			MG_PLL_BIAS_BIASCAL_EN |
+			MG_PLL_BIAS_CTRIM(12) |
+			MG_PLL_BIAS_VREF_RDAC(4) |
+			MG_PLL_BIAS_IREFTRIM(iref_trim);
+
+		if (refclk_khz == 38400) {
+			pll_state->mg_pll_tdc_coldst_bias_mask =
+				MG_PLL_TDC_COLDST_COLDSTART;
+			pll_state->mg_pll_bias_mask = 0;
+		} else {
+			pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
+			pll_state->mg_pll_bias_mask = -1U;
+		}
 
-	pll_state->mg_pll_tdc_coldst_bias &= pll_state->mg_pll_tdc_coldst_bias_mask;
-	pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
+		pll_state->mg_pll_tdc_coldst_bias &=
+			pll_state->mg_pll_tdc_coldst_bias_mask;
+		pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
+	}
 
 	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] 27+ messages in thread

* [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (4 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 22:04   ` Lucas De Marchi
  2019-09-23 19:55 ` [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training José Roberto de Souza
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx

Extending ICL mg calculations to also support dkl calculations.

v3:
Fixing iref_trim calculation for 38400 refclock

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 | 45 ++++++++++++++++---
 2 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 1ab3e0c4c0a1..938639675529 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1440,11 +1440,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 9bae6f2d0f36..496df4095a21 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2607,7 +2607,8 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
 
 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)
+				     struct intel_dpll_hw_state *state,
+				     bool is_dkl)
 {
 	u32 dco_min_freq, dco_max_freq;
 	int div1_vals[] = {7, 5, 3, 2};
@@ -2629,8 +2630,13 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
 				continue;
 
 			if (div2 >= 2) {
-				a_divratio = is_dp ? 10 : 5;
-				tlinedrv = 2;
+				if (is_dkl) {
+					a_divratio = 5;
+					tlinedrv = 1;
+				} else {
+					a_divratio = is_dp ? 10 : 5;
+					tlinedrv = 2;
+				}
 			} else {
 				a_divratio = 5;
 				tlinedrv = 0;
@@ -2693,11 +2699,12 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 	u64 tmp;
 	bool use_ssc = false;
 	bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
+	bool is_dkl = INTEL_GEN(dev_priv) >= 12;
 
 	memset(pll_state, 0, sizeof(*pll_state));
 
 	if (!icl_mg_pll_find_divisors(clock, is_dp, use_ssc, &dco_khz,
-				      pll_state)) {
+				      pll_state, is_dkl)) {
 		DRM_DEBUG_KMS("Failed to find divisors for clock %d\n", clock);
 		return false;
 	}
@@ -2705,8 +2712,11 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 	m1div = 2;
 	m2div_int = dco_khz / (refclk_khz * m1div);
 	if (m2div_int > 255) {
-		m1div = 4;
-		m2div_int = dco_khz / (refclk_khz * m1div);
+		if (!is_dkl) {
+			m1div = 4;
+			m2div_int = dco_khz / (refclk_khz * m1div);
+		}
+
 		if (m2div_int > 255) {
 			DRM_DEBUG_KMS("Failed to find mdiv for clock %d\n",
 				      clock);
@@ -2787,7 +2797,28 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
 	ssc_steplog = 4;
 
 	/* write pll_state calculations */
-	{
+	if (is_dkl) {
+		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 ? 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);
+
+	} else {
 		pll_state->mg_pll_div0 =
 			(m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
 			MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
-- 
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] 27+ messages in thread

* [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (5 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-24 15:58   ` Imre Deak
  2019-09-23 19:55 ` [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports José Roberto de Souza
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx

Link training is failling when running link at 2.7GHz and 1.62GHz and
following BSpec pll algorithm.

Comparing the values calculated and the ones from the reference table
it looks like MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO should not always set
to 5. For DP ports ICL mg pll algorithm sets it to 10 or 5 based on
div2 value, that matches with dkl hardcoded table.

So implementing this way as it proved to work in HW and leaving a
comment so we know why it do not match BSpec.

Issue reported on BSpec 49204.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 ++++++-
 1 file changed, 6 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 496df4095a21..3c881d4cf973 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2631,7 +2631,12 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
 
 			if (div2 >= 2) {
 				if (is_dkl) {
-					a_divratio = 5;
+					/*
+					 * Note: a_divratio not matching TGL
+					 * BSpec algorithm but matching
+					 * hardcoded values and working on HW
+					 */
+					a_divratio = 10;
 					tlinedrv = 1;
 				} else {
 					a_divratio = is_dp ? 10 : 5;
-- 
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] 27+ messages in thread

* [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (6 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 22:07   ` Lucas De Marchi
  2019-09-23 19:55 ` [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

TGL added 2 more TC ports that currently are not being handled by
icl_pll_to_ddi_clk_sel(), so adding those.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reported-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 938639675529..e50f492b3100 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1076,6 +1076,8 @@ static u32 icl_pll_to_ddi_clk_sel(struct intel_encoder *encoder,
 	case DPLL_ID_ICL_MGPLL2:
 	case DPLL_ID_ICL_MGPLL3:
 	case DPLL_ID_ICL_MGPLL4:
+	case DPLL_ID_TGL_MGPLL5:
+	case DPLL_ID_TGL_MGPLL6:
 		return DDI_CLK_SEL_MG;
 	}
 }
-- 
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] 27+ messages in thread

* [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (7 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports José Roberto de Souza
@ 2019-09-23 19:55 ` José Roberto de Souza
  2019-09-23 22:08   ` Lucas De Marchi
  2019-09-23 21:20 ` ✓ Fi.CI.BAT: success for TGL TC enabling (rev3) Patchwork
  2019-09-24 10:48 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 1 reply; 27+ messages in thread
From: José Roberto de Souza @ 2019-09-23 19:55 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>
Signed-off-by: José Roberto de Souza <jose.souza@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 5ecf54270181..43876fe8fa1e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15343,9 +15343,14 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 		return;
 
 	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] 27+ messages in thread

* ✓ Fi.CI.BAT: success for TGL TC enabling (rev3)
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (8 preceding siblings ...)
  2019-09-23 19:55 ` [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
@ 2019-09-23 21:20 ` Patchwork
  2019-09-24 10:48 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2019-09-23 21:20 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6945 -> Patchwork_14507
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@core_auth@basic-auth:
    - 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_6945/fi-icl-u3/igt@core_auth@basic-auth.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/fi-icl-u3/igt@core_auth@basic-auth.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-read-write-distinct:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html

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

  * igt@i915_selftest@live_execlists:
    - fi-icl-u2:          [DMESG-FAIL][9] ([fdo#111108]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/fi-icl-u2/igt@i915_selftest@live_execlists.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/fi-icl-u2/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-7500u:       [INCOMPLETE][11] ([fdo#108744]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111407]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/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#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [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#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (54 -> 45)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6945 -> Patchwork_14507

  CI-20190529: 20190529
  CI_DRM_6945: f11d819264a3fab210498a4920ef34a891da39e0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5197: aa534ff47fd2f455c8be9e59eae807695b87fcdd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14507: c7814e1ebc2c52bc5f31f279835132ca87d1ed40 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c7814e1ebc2c drm/i915/tgl: initialize TC and TBT ports
b42588f6e646 drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports
809a52cca2ce drm/i915/tgl: Fix dkl link training
1b15c7de0be9 drm/i915/tgl: Add dkl phy pll calculations
49cde1b25771 drm/i915/tgl: re-indent code to prepare for DKL changes
69267836573f drm/i915/tgl: Add dkl phy programming sequences
400a0a64d801 drm/i915/tgl: TC helper function to return pin mapping
62b64fc3a2b3 drm/i915/tgl: Add support for dkl pll write
6d31580da419 drm/i915/tgl: Add initial dkl pll support

== Logs ==

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

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

* Re: [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-23 19:55 ` [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
@ 2019-09-23 22:02   ` Lucas De Marchi
  2019-09-24 13:00     ` Imre Deak
  0 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-23 22:02 UTC (permalink / raw)
  To: José Roberto de Souza, Imre Deak; +Cc: Intel Graphics

On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> From: Clinton A Taylor <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.
>
> v2:
> Setting the right HIP_INDEX_REG bits (José)
>
> v3:
> Adding the meaning of each column of tgl_dkl_phy_ddi_translations
> Adding if gen >= 12 on intel_ddi_hdmi_level() and
> intel_ddi_pre_enable_hdmi() instead of reuse part of gen >= 11 if

we should treat these patch versioning the same way of the commit messages,
avoiding the gerund.

>
> BSpec: 49292
> BSpec: 49190
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 249 ++++++++++++++++++++++-
>  1 file changed, 240 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 33cd766f9eea..1ab3e0c4c0a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -586,6 +586,26 @@ 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[] = {
> +                               /* VS   pre-emp Non-trans mV    Pre-emph dB */
> +       { 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)
>  {
> @@ -872,7 +892,14 @@ 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_GEN(dev_priv) >= 12) {
> +               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(tgl_dkl_phy_ddi_translations);
> +               default_entry = n_entries - 1;
> +       } else if (INTEL_GEN(dev_priv) == 11) {
>                 if (intel_phy_is_combo(dev_priv, phy))
>                         icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI,
>                                                 0, &n_entries);
> @@ -2313,7 +2340,13 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>         enum phy phy = intel_port_to_phy(dev_priv, port);
>         int n_entries;
>
> -       if (INTEL_GEN(dev_priv) >= 11) {
> +       if (INTEL_GEN(dev_priv) >= 12) {
> +               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(tgl_dkl_phy_ddi_translations);
> +       } else if (INTEL_GEN(dev_priv) == 11) {
>                 if (intel_phy_is_combo(dev_priv, phy))
>                         icl_get_combo_buf_trans(dev_priv, encoder->type,
>                                                 intel_dp->link_rate, &n_entries);
> @@ -2755,6 +2788,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
> +        */

this comment is bogus as it's actually HIP_INDEX_VAL we are talking about,
and that's pretty clear by the loop.

> +       for (ln = 0; ln < 2; ln++) {
> +               I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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;
> @@ -2786,7 +2879,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))
> @@ -3033,6 +3129,34 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
>         }
>  }
>
> +static void
> +tgl_phy_set_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, bits;
> +       int ln;
> +
> +       if (tc_port == PORT_TC_NONE)
> +               return;
> +
> +       bits = 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), HIP_INDEX_VAL(tc_port, ln));
> +
> +               val = I915_READ(DKL_DP_MODE(tc_port));
> +               if (enable)
> +                       val |= bits;
> +               else
> +                       val &= ~bits;
> +               I915_WRITE(DKL_DP_MODE(tc_port), val);
> +       }
> +}
> +
>  static void
>  icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable)
>  {
> @@ -3132,6 +3256,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;

In icl_program_mg_dp_mode() this is

        if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)

if intel_dig_port is not backed by a TC phy, tc_mode should be
TC_PORT_TBT_ALT since that's 0.

> +
> +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0));
> +       ln0 = I915_READ(DKL_DP_MODE(tc_port));
> +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1));
> +       ln1 = I915_READ(DKL_DP_MODE(tc_port));
> +
> +       num_lanes = intel_dig_port->dp.lane_count;

in this file this is often called width

> +
> +       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);

typo: the second one should be DKL_DP_MODE_CFG_DP_X2_MODE

but.... for DKL we have the exact same table as for MG. Why do we need
these changes here while
we don't for ICL?

> +               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) {

From a quick look to the table, we don't change based on pin_mask, but
rather based on lane_mask, just like for MG.
Btw, lane_mask is unused in this function now and we don't get a
warning just because it's misused in a
MISSING_CASE() below.

The values do change based on width, but I'm not sure why MG doesn't
need to take that into account.
+Imre Deak

Lucas De Marchi

> +               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), HIP_INDEX_VAL(tc_port, 0x0));
> +       I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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)
>  {
> @@ -3218,7 +3431,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 ov´er MST
> @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
>         intel_ddi_config_transcoder_func(crtc_state);
>
>         /* 7.d */
> -       icl_phy_set_clock_gating(dig_port, false);
> +       tgl_phy_set_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 */
> @@ -3266,6 +3479,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 now.
> +        */
> +
>         /* 7.l */
>         intel_ddi_enable_fec(encoder, crtc_state);
>         intel_dsc_enable(encoder, crtc_state);
> @@ -3371,9 +3593,15 @@ 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_phy_set_clock_gating(dig_port, false);
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               tgl_phy_set_clock_gating(dig_port, false);
> +       else
> +               icl_phy_set_clock_gating(dig_port, false);
>
> -       if (INTEL_GEN(dev_priv) >= 11)
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
> +                                       level, INTEL_OUTPUT_HDMI);
> +       else if (INTEL_GEN(dev_priv) == 11)
>                 icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>                                         level, INTEL_OUTPUT_HDMI);
>         else if (IS_CANNONLAKE(dev_priv))
> @@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>         else
>                 intel_prepare_hdmi_ddi_buffers(encoder, level);
>
> -       icl_phy_set_clock_gating(dig_port, true);
> +       if (INTEL_GEN(dev_priv) >= 12)
> +               tgl_phy_set_clock_gating(dig_port, true);
> +       else
> +               icl_phy_set_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



-- 
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] 27+ messages in thread

* Re: [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes
  2019-09-23 19:55 ` [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes José Roberto de Souza
@ 2019-09-23 22:04   ` Lucas De Marchi
  2019-09-24 20:58     ` Matt Roper
  0 siblings, 1 reply; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-23 22:04 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Lucas De Marchi

On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> From: Lucas De Marchi <lucas.demarchi@intel.com>
>
> The final save operation into pll_state of the calculations done will
> be different for DKL PHY. Prepare for that by reindenting code so it's
> easier to check for correctness. This one has no change in behavior.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

this is trivial enough for an added r-b, too?

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 119 ++++++++++--------
>  1 file changed, 66 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 21249997940d..9bae6f2d0f36 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2786,60 +2786,73 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>         }
>         ssc_steplog = 4;
>
> -       pll_state->mg_pll_div0 = (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
> -                                 MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
> -                                 MG_PLL_DIV0_FBDIV_INT(m2div_int);
> -
> -       pll_state->mg_pll_div1 = MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> -                                MG_PLL_DIV1_DITHER_DIV_2 |
> -                                MG_PLL_DIV1_NDIVRATIO(1) |
> -                                MG_PLL_DIV1_FBPREDIV(m1div);
> -
> -       pll_state->mg_pll_lf = MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> -                              MG_PLL_LF_AFCCNTSEL_512 |
> -                              MG_PLL_LF_GAINCTRL(1) |
> -                              MG_PLL_LF_INT_COEFF(int_coeff) |
> -                              MG_PLL_LF_PROP_COEFF(prop_coeff);
> -
> -       pll_state->mg_pll_frac_lock = MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> -                                     MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
> -                                     MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
> -                                     MG_PLL_FRAC_LOCK_DCODITHEREN |
> -                                     MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
> -       if (use_ssc || m2div_rem > 0)
> -               pll_state->mg_pll_frac_lock |= MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> -
> -       pll_state->mg_pll_ssc = (use_ssc ? MG_PLL_SSC_EN : 0) |
> -                               MG_PLL_SSC_TYPE(2) |
> -                               MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
> -                               MG_PLL_SSC_STEPNUM(ssc_steplog) |
> -                               MG_PLL_SSC_FLLEN |
> -                               MG_PLL_SSC_STEPSIZE(ssc_stepsize);
> -
> -       pll_state->mg_pll_tdc_coldst_bias = MG_PLL_TDC_COLDST_COLDSTART |
> -                                           MG_PLL_TDC_COLDST_IREFINT_EN |
> -                                           MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
> -                                           MG_PLL_TDC_TDCOVCCORR_EN |
> -                                           MG_PLL_TDC_TDCSEL(3);
> -
> -       pll_state->mg_pll_bias = MG_PLL_BIAS_BIAS_GB_SEL(3) |
> -                                MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
> -                                MG_PLL_BIAS_BIAS_BONUS(10) |
> -                                MG_PLL_BIAS_BIASCAL_EN |
> -                                MG_PLL_BIAS_CTRIM(12) |
> -                                MG_PLL_BIAS_VREF_RDAC(4) |
> -                                MG_PLL_BIAS_IREFTRIM(iref_trim);
> -
> -       if (refclk_khz == 38400) {
> -               pll_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART;
> -               pll_state->mg_pll_bias_mask = 0;
> -       } else {
> -               pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
> -               pll_state->mg_pll_bias_mask = -1U;
> -       }
> +       /* write pll_state calculations */
> +       {
> +               pll_state->mg_pll_div0 =
> +                       (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
> +                       MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
> +                       MG_PLL_DIV0_FBDIV_INT(m2div_int);
> +
> +               pll_state->mg_pll_div1 =
> +                       MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> +                       MG_PLL_DIV1_DITHER_DIV_2 |
> +                       MG_PLL_DIV1_NDIVRATIO(1) |
> +                       MG_PLL_DIV1_FBPREDIV(m1div);
> +
> +               pll_state->mg_pll_lf =
> +                       MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> +                       MG_PLL_LF_AFCCNTSEL_512 |
> +                       MG_PLL_LF_GAINCTRL(1) |
> +                       MG_PLL_LF_INT_COEFF(int_coeff) |
> +                       MG_PLL_LF_PROP_COEFF(prop_coeff);
> +
> +               pll_state->mg_pll_frac_lock =
> +                       MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> +                       MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
> +                       MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
> +                       MG_PLL_FRAC_LOCK_DCODITHEREN |
> +                       MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
> +               if (use_ssc || m2div_rem > 0)
> +                       pll_state->mg_pll_frac_lock |=
> +                               MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> +
> +               pll_state->mg_pll_ssc =
> +                       (use_ssc ? MG_PLL_SSC_EN : 0) |
> +                       MG_PLL_SSC_TYPE(2) |
> +                       MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
> +                       MG_PLL_SSC_STEPNUM(ssc_steplog) |
> +                       MG_PLL_SSC_FLLEN |
> +                       MG_PLL_SSC_STEPSIZE(ssc_stepsize);
> +
> +               pll_state->mg_pll_tdc_coldst_bias =
> +                       MG_PLL_TDC_COLDST_COLDSTART |
> +                       MG_PLL_TDC_COLDST_IREFINT_EN |
> +                       MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
> +                       MG_PLL_TDC_TDCOVCCORR_EN |
> +                       MG_PLL_TDC_TDCSEL(3);
> +
> +               pll_state->mg_pll_bias =
> +                       MG_PLL_BIAS_BIAS_GB_SEL(3) |
> +                       MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
> +                       MG_PLL_BIAS_BIAS_BONUS(10) |
> +                       MG_PLL_BIAS_BIASCAL_EN |
> +                       MG_PLL_BIAS_CTRIM(12) |
> +                       MG_PLL_BIAS_VREF_RDAC(4) |
> +                       MG_PLL_BIAS_IREFTRIM(iref_trim);
> +
> +               if (refclk_khz == 38400) {
> +                       pll_state->mg_pll_tdc_coldst_bias_mask =
> +                               MG_PLL_TDC_COLDST_COLDSTART;
> +                       pll_state->mg_pll_bias_mask = 0;
> +               } else {
> +                       pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
> +                       pll_state->mg_pll_bias_mask = -1U;
> +               }
>
> -       pll_state->mg_pll_tdc_coldst_bias &= pll_state->mg_pll_tdc_coldst_bias_mask;
> -       pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
> +               pll_state->mg_pll_tdc_coldst_bias &=
> +                       pll_state->mg_pll_tdc_coldst_bias_mask;
> +               pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
> +       }
>
>         return true;
>  }
> --
> 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] 27+ messages in thread

* Re: [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations
  2019-09-23 19:55 ` [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations José Roberto de Souza
@ 2019-09-23 22:04   ` Lucas De Marchi
  0 siblings, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-23 22:04 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics

On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> Extending ICL mg calculations to also support dkl calculations.
>
> v3:
> Fixing iref_trim calculation for 38400 refclock
>
> BSpec: 49204
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

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

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 29 +++++++++---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 45 ++++++++++++++++---
>  2 files changed, 62 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 1ab3e0c4c0a1..938639675529 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1440,11 +1440,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 9bae6f2d0f36..496df4095a21 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2607,7 +2607,8 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
>
>  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)
> +                                    struct intel_dpll_hw_state *state,
> +                                    bool is_dkl)
>  {
>         u32 dco_min_freq, dco_max_freq;
>         int div1_vals[] = {7, 5, 3, 2};
> @@ -2629,8 +2630,13 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
>                                 continue;
>
>                         if (div2 >= 2) {
> -                               a_divratio = is_dp ? 10 : 5;
> -                               tlinedrv = 2;
> +                               if (is_dkl) {
> +                                       a_divratio = 5;
> +                                       tlinedrv = 1;
> +                               } else {
> +                                       a_divratio = is_dp ? 10 : 5;
> +                                       tlinedrv = 2;
> +                               }
>                         } else {
>                                 a_divratio = 5;
>                                 tlinedrv = 0;
> @@ -2693,11 +2699,12 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>         u64 tmp;
>         bool use_ssc = false;
>         bool is_dp = !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
> +       bool is_dkl = INTEL_GEN(dev_priv) >= 12;
>
>         memset(pll_state, 0, sizeof(*pll_state));
>
>         if (!icl_mg_pll_find_divisors(clock, is_dp, use_ssc, &dco_khz,
> -                                     pll_state)) {
> +                                     pll_state, is_dkl)) {
>                 DRM_DEBUG_KMS("Failed to find divisors for clock %d\n", clock);
>                 return false;
>         }
> @@ -2705,8 +2712,11 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>         m1div = 2;
>         m2div_int = dco_khz / (refclk_khz * m1div);
>         if (m2div_int > 255) {
> -               m1div = 4;
> -               m2div_int = dco_khz / (refclk_khz * m1div);
> +               if (!is_dkl) {
> +                       m1div = 4;
> +                       m2div_int = dco_khz / (refclk_khz * m1div);
> +               }
> +
>                 if (m2div_int > 255) {
>                         DRM_DEBUG_KMS("Failed to find mdiv for clock %d\n",
>                                       clock);
> @@ -2787,7 +2797,28 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>         ssc_steplog = 4;
>
>         /* write pll_state calculations */
> -       {
> +       if (is_dkl) {
> +               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 ? 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);
> +
> +       } else {
>                 pll_state->mg_pll_div0 =
>                         (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
>                         MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
> --
> 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] 27+ messages in thread

* Re: [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports
  2019-09-23 19:55 ` [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports José Roberto de Souza
@ 2019-09-23 22:07   ` Lucas De Marchi
  0 siblings, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-23 22:07 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Lucas De Marchi

On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> TGL added 2 more TC ports that currently are not being handled by
> icl_pll_to_ddi_clk_sel(), so adding those.
>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Reported-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

I don't think we use them, but for completeness seem ok (so we don't
keep thinking why we didn't
add it to match the spec)... maybe future sku's make use of them, so
better match the spec:

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

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 938639675529..e50f492b3100 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1076,6 +1076,8 @@ static u32 icl_pll_to_ddi_clk_sel(struct intel_encoder *encoder,
>         case DPLL_ID_ICL_MGPLL2:
>         case DPLL_ID_ICL_MGPLL3:
>         case DPLL_ID_ICL_MGPLL4:
> +       case DPLL_ID_TGL_MGPLL5:
> +       case DPLL_ID_TGL_MGPLL6:
>                 return DDI_CLK_SEL_MG;
>         }
>  }
> --
> 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] 27+ messages in thread

* Re: [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports
  2019-09-23 19:55 ` [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
@ 2019-09-23 22:08   ` Lucas De Marchi
  0 siblings, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-23 22:08 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Intel Graphics, Lucas De Marchi

On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> 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>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Another trivial patch that you could also add a r-b?

Lucas De Marchi

> ---
>  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 5ecf54270181..43876fe8fa1e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15343,9 +15343,14 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>                 return;
>
>         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



-- 
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] 27+ messages in thread

* ✓ Fi.CI.IGT: success for TGL TC enabling (rev3)
  2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
                   ` (9 preceding siblings ...)
  2019-09-23 21:20 ` ✓ Fi.CI.BAT: success for TGL TC enabling (rev3) Patchwork
@ 2019-09-24 10:48 ` Patchwork
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2019-09-24 10:48 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6945_full -> Patchwork_14507_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

  Here are the changes found in Patchwork_14507_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_6945/shard-iclb5/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +15 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb4/igt@gem_exec_schedule@independent-bsd2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb6/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +9 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +6 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][9] -> [INCOMPLETE][10] ([fdo#103665])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([fdo#105363])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-iclb:         [PASS][13] -> [INCOMPLETE][14] ([fdo#107713] / [fdo#109507])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb3/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([fdo#111609])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-glk6/igt@kms_flip@modeset-vs-vblank-race.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-glk8/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-badstride:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-badstride.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-badstride.html

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-blt:
    - shard-apl:          [PASS][19] -> [INCOMPLETE][20] ([fdo#103927])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-apl8/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-blt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-apl6/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145] / [fdo#110403])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][25] -> [FAIL][26] ([fdo#103166])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#108341])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb8/igt@kms_psr@no_drrs.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@perf@blocking:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([fdo#110728])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-skl1/igt@perf@blocking.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-skl6/igt@perf@blocking.html

  
#### Possible fixes ####

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

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-skl:          [FAIL][33] ([fdo#111609]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-skl5/igt@kms_flip@modeset-vs-vblank-race.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-skl10/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][35] ([fdo#103167]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +5 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

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

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb5/igt@kms_psr@psr2_sprite_blt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][43] ([fdo#99912]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-apl6/igt@kms_setmode@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-apl6/igt@kms_setmode@basic.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +23 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6945/shard-iclb7/igt@prime_busy@hang-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb4/igt@prime_busy@hang-bsd2.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_6945/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14507/shard-iclb7/igt@gem_ctx_isolation@vcs1-nonpriv.html

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

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [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#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
  [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
  [fdo#111609]: https://bugs.freedesktop.org/show_bug.cgi?id=111609
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (9 -> 8)
------------------------------

  Missing    (1): pig-hsw-4770r 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6945 -> Patchwork_14507

  CI-20190529: 20190529
  CI_DRM_6945: f11d819264a3fab210498a4920ef34a891da39e0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5197: aa534ff47fd2f455c8be9e59eae807695b87fcdd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14507: c7814e1ebc2c52bc5f31f279835132ca87d1ed40 @ 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_14507/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-23 22:02   ` Lucas De Marchi
@ 2019-09-24 13:00     ` Imre Deak
  2019-09-24 23:21       ` Souza, Jose
  0 siblings, 1 reply; 27+ messages in thread
From: Imre Deak @ 2019-09-24 13:00 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Intel Graphics

On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote:
> On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > [...]
> 
> > +               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) {
> 
> From a quick look to the table, we don't change based on pin_mask, but
> rather based on lane_mask, just like for MG.

There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3 vs.
pin_mask=0x2/lane_mask=0x3.

> Btw, lane_mask is unused in this function now and we don't get a
> warning just because it's misused in a
> MISSING_CASE() below.
> 
> The values do change based on width, but I'm not sure why MG doesn't
> need to take that into account.

Yes, looks like crtc_state->lane_count should be considered. Not sure
why the MG version doesn't do that either, I assume it was an addition
to BSpec only after the function was added. The same applies to
pin_mask.

> +Imre Deak
> 
> Lucas De Marchi
> 
> > +               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), HIP_INDEX_VAL(tc_port, 0x0));
> > +       I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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)
> >  {
> > @@ -3218,7 +3431,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 ov´er MST
> > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >         intel_ddi_config_transcoder_func(crtc_state);
> >
> >         /* 7.d */
> > -       icl_phy_set_clock_gating(dig_port, false);
> > +       tgl_phy_set_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 */
> > @@ -3266,6 +3479,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 now.
> > +        */
> > +
> >         /* 7.l */
> >         intel_ddi_enable_fec(encoder, crtc_state);
> >         intel_dsc_enable(encoder, crtc_state);
> > @@ -3371,9 +3593,15 @@ 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_phy_set_clock_gating(dig_port, false);
> > +       if (INTEL_GEN(dev_priv) >= 12)
> > +               tgl_phy_set_clock_gating(dig_port, false);
> > +       else
> > +               icl_phy_set_clock_gating(dig_port, false);
> >
> > -       if (INTEL_GEN(dev_priv) >= 11)
> > +       if (INTEL_GEN(dev_priv) >= 12)
> > +               tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
> > +                                       level, INTEL_OUTPUT_HDMI);
> > +       else if (INTEL_GEN(dev_priv) == 11)
> >                 icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
> >                                         level, INTEL_OUTPUT_HDMI);
> >         else if (IS_CANNONLAKE(dev_priv))
> > @@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> >         else
> >                 intel_prepare_hdmi_ddi_buffers(encoder, level);
> >
> > -       icl_phy_set_clock_gating(dig_port, true);
> > +       if (INTEL_GEN(dev_priv) >= 12)
> > +               tgl_phy_set_clock_gating(dig_port, true);
> > +       else
> > +               icl_phy_set_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
> 
> 
> 
> -- 
> 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] 27+ messages in thread

* Re: [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support
  2019-09-23 19:55 ` [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
@ 2019-09-24 14:20   ` Imre Deak
  2019-09-24 20:20     ` Souza, Jose
  0 siblings, 1 reply; 27+ messages in thread
From: Imre Deak @ 2019-09-24 14:20 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx, Lucas De Marchi

On Mon, Sep 23, 2019 at 12:55:05PM -0700, José Roberto de Souza wrote:
> 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.
> 
> v2:
> Setting the right HIP_INDEX_REG bits (José)
> 
> Acked-by: Lucas De Marchi <lucas.demarchi@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 | 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..46dde614bfb5 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), HIP_INDEX_VAL(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));

For the above masking the computed fields is missing.

> +
> +	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.

Imo BSpec Index/49204 should state it explicitly that the MG and DKL
enable/disable sequence are the same, but let's assume that.

> +	 */
> +	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);

Nit: reusing mg_pll_enable() instead with a platform check here would
make things more symmetric.

With the above fix:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +
> +	/*
> +	 * 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training
  2019-09-23 19:55 ` [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training José Roberto de Souza
@ 2019-09-24 15:58   ` Imre Deak
  2019-09-24 23:59     ` Souza, Jose
  0 siblings, 1 reply; 27+ messages in thread
From: Imre Deak @ 2019-09-24 15:58 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Mon, Sep 23, 2019 at 12:55:11PM -0700, José Roberto de Souza wrote:
> Link training is failling when running link at 2.7GHz and 1.62GHz and
> following BSpec pll algorithm.
> 
> Comparing the values calculated and the ones from the reference table
> it looks like MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO should not always set
> to 5. For DP ports ICL mg pll algorithm sets it to 10 or 5 based on
> div2 value, that matches with dkl hardcoded table.
> 
> So implementing this way as it proved to work in HW and leaving a
> comment so we know why it do not match BSpec.
> 
> Issue reported on BSpec 49204.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 ++++++-
>  1 file changed, 6 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 496df4095a21..3c881d4cf973 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2631,7 +2631,12 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
>  
>  			if (div2 >= 2) {
>  				if (is_dkl) {
> -					a_divratio = 5;
> +					/*
> +					 * Note: a_divratio not matching TGL
> +					 * BSpec algorithm but matching
> +					 * hardcoded values and working on HW
> +					 */
> +					a_divratio = 10;

Yes, matches the hardcoded values for CLKTOP2_HSCLKCTL/CORECLKCTL1, with
the exception that the hardcoded value for CLKTOP2_HSCLKCTL 5p4 is also
incorrect wrt. div2 within that (should be 1 but it's 2).

However we don't have any hardcoded values in Bspec for HDMI except for
the 5.94Gbps/div2=1 case. So until otherwise proven, for HDMI/div2>=2 I
wouldn't change the value, rather have the same logic as for ICL that is
is_dp ? 10 : 5;

>  					tlinedrv = 1;
>  				} else {
>  					a_divratio = is_dp ? 10 : 5;
> -- 
> 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] 27+ messages in thread

* Re: [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support
  2019-09-24 14:20   ` Imre Deak
@ 2019-09-24 20:20     ` Souza, Jose
  0 siblings, 0 replies; 27+ messages in thread
From: Souza, Jose @ 2019-09-24 20:20 UTC (permalink / raw)
  To: Deak, Imre; +Cc: intel-gfx, De Marchi, Lucas

On Tue, 2019-09-24 at 17:20 +0300, Imre Deak wrote:
> On Mon, Sep 23, 2019 at 12:55:05PM -0700, José Roberto de Souza
> wrote:
> > 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.
> > 
> > v2:
> > Setting the right HIP_INDEX_REG bits (José)
> > 
> > Acked-by: Lucas De Marchi <lucas.demarchi@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 | 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..46dde614bfb5 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_DISPL
> > AY_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), HIP_INDEX_VAL(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));
> 
> For the above masking the computed fields is missing.

Done, thanks for catching this

> 
> > +
> > +	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.
> 
> Imo BSpec Index/49204 should state it explicitly that the MG and DKL
> enable/disable sequence are the same, but let's assume that.
> 
> > +	 */
> > +	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);
> 
> Nit: reusing mg_pll_enable() instead with a platform check here would
> make things more symmetric.

Done

> 
> With the above fix:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks

> 
> > +
> > +	/*
> > +	 * 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes
  2019-09-23 22:04   ` Lucas De Marchi
@ 2019-09-24 20:58     ` Matt Roper
  0 siblings, 0 replies; 27+ messages in thread
From: Matt Roper @ 2019-09-24 20:58 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Intel Graphics, Lucas De Marchi

On Mon, Sep 23, 2019 at 03:04:06PM -0700, Lucas De Marchi wrote:
> On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> >
> > From: Lucas De Marchi <lucas.demarchi@intel.com>
> >
> > The final save operation into pll_state of the calculations done will
> > be different for DKL PHY. Prepare for that by reindenting code so it's
> > easier to check for correctness. This one has no change in behavior.
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> 
> this is trivial enough for an added r-b, too?
> 
> Lucas De Marchi

Confirmed that this is formatting-only with no functional change.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 119 ++++++++++--------
> >  1 file changed, 66 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index 21249997940d..9bae6f2d0f36 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -2786,60 +2786,73 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
> >         }
> >         ssc_steplog = 4;
> >
> > -       pll_state->mg_pll_div0 = (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
> > -                                 MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
> > -                                 MG_PLL_DIV0_FBDIV_INT(m2div_int);
> > -
> > -       pll_state->mg_pll_div1 = MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> > -                                MG_PLL_DIV1_DITHER_DIV_2 |
> > -                                MG_PLL_DIV1_NDIVRATIO(1) |
> > -                                MG_PLL_DIV1_FBPREDIV(m1div);
> > -
> > -       pll_state->mg_pll_lf = MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> > -                              MG_PLL_LF_AFCCNTSEL_512 |
> > -                              MG_PLL_LF_GAINCTRL(1) |
> > -                              MG_PLL_LF_INT_COEFF(int_coeff) |
> > -                              MG_PLL_LF_PROP_COEFF(prop_coeff);
> > -
> > -       pll_state->mg_pll_frac_lock = MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> > -                                     MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
> > -                                     MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
> > -                                     MG_PLL_FRAC_LOCK_DCODITHEREN |
> > -                                     MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
> > -       if (use_ssc || m2div_rem > 0)
> > -               pll_state->mg_pll_frac_lock |= MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> > -
> > -       pll_state->mg_pll_ssc = (use_ssc ? MG_PLL_SSC_EN : 0) |
> > -                               MG_PLL_SSC_TYPE(2) |
> > -                               MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
> > -                               MG_PLL_SSC_STEPNUM(ssc_steplog) |
> > -                               MG_PLL_SSC_FLLEN |
> > -                               MG_PLL_SSC_STEPSIZE(ssc_stepsize);
> > -
> > -       pll_state->mg_pll_tdc_coldst_bias = MG_PLL_TDC_COLDST_COLDSTART |
> > -                                           MG_PLL_TDC_COLDST_IREFINT_EN |
> > -                                           MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
> > -                                           MG_PLL_TDC_TDCOVCCORR_EN |
> > -                                           MG_PLL_TDC_TDCSEL(3);
> > -
> > -       pll_state->mg_pll_bias = MG_PLL_BIAS_BIAS_GB_SEL(3) |
> > -                                MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
> > -                                MG_PLL_BIAS_BIAS_BONUS(10) |
> > -                                MG_PLL_BIAS_BIASCAL_EN |
> > -                                MG_PLL_BIAS_CTRIM(12) |
> > -                                MG_PLL_BIAS_VREF_RDAC(4) |
> > -                                MG_PLL_BIAS_IREFTRIM(iref_trim);
> > -
> > -       if (refclk_khz == 38400) {
> > -               pll_state->mg_pll_tdc_coldst_bias_mask = MG_PLL_TDC_COLDST_COLDSTART;
> > -               pll_state->mg_pll_bias_mask = 0;
> > -       } else {
> > -               pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
> > -               pll_state->mg_pll_bias_mask = -1U;
> > -       }
> > +       /* write pll_state calculations */
> > +       {
> > +               pll_state->mg_pll_div0 =
> > +                       (m2div_rem > 0 ? MG_PLL_DIV0_FRACNEN_H : 0) |
> > +                       MG_PLL_DIV0_FBDIV_FRAC(m2div_frac) |
> > +                       MG_PLL_DIV0_FBDIV_INT(m2div_int);
> > +
> > +               pll_state->mg_pll_div1 =
> > +                       MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> > +                       MG_PLL_DIV1_DITHER_DIV_2 |
> > +                       MG_PLL_DIV1_NDIVRATIO(1) |
> > +                       MG_PLL_DIV1_FBPREDIV(m1div);
> > +
> > +               pll_state->mg_pll_lf =
> > +                       MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> > +                       MG_PLL_LF_AFCCNTSEL_512 |
> > +                       MG_PLL_LF_GAINCTRL(1) |
> > +                       MG_PLL_LF_INT_COEFF(int_coeff) |
> > +                       MG_PLL_LF_PROP_COEFF(prop_coeff);
> > +
> > +               pll_state->mg_pll_frac_lock =
> > +                       MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> > +                       MG_PLL_FRAC_LOCK_EARLYLOCK_CRIT_32 |
> > +                       MG_PLL_FRAC_LOCK_LOCKTHRESH(10) |
> > +                       MG_PLL_FRAC_LOCK_DCODITHEREN |
> > +                       MG_PLL_FRAC_LOCK_FEEDFWRDGAIN(feedfwgain);
> > +               if (use_ssc || m2div_rem > 0)
> > +                       pll_state->mg_pll_frac_lock |=
> > +                               MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> > +
> > +               pll_state->mg_pll_ssc =
> > +                       (use_ssc ? MG_PLL_SSC_EN : 0) |
> > +                       MG_PLL_SSC_TYPE(2) |
> > +                       MG_PLL_SSC_STEPLENGTH(ssc_steplen) |
> > +                       MG_PLL_SSC_STEPNUM(ssc_steplog) |
> > +                       MG_PLL_SSC_FLLEN |
> > +                       MG_PLL_SSC_STEPSIZE(ssc_stepsize);
> > +
> > +               pll_state->mg_pll_tdc_coldst_bias =
> > +                       MG_PLL_TDC_COLDST_COLDSTART |
> > +                       MG_PLL_TDC_COLDST_IREFINT_EN |
> > +                       MG_PLL_TDC_COLDST_REFBIAS_START_PULSE_W(iref_pulse_w) |
> > +                       MG_PLL_TDC_TDCOVCCORR_EN |
> > +                       MG_PLL_TDC_TDCSEL(3);
> > +
> > +               pll_state->mg_pll_bias =
> > +                       MG_PLL_BIAS_BIAS_GB_SEL(3) |
> > +                       MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
> > +                       MG_PLL_BIAS_BIAS_BONUS(10) |
> > +                       MG_PLL_BIAS_BIASCAL_EN |
> > +                       MG_PLL_BIAS_CTRIM(12) |
> > +                       MG_PLL_BIAS_VREF_RDAC(4) |
> > +                       MG_PLL_BIAS_IREFTRIM(iref_trim);
> > +
> > +               if (refclk_khz == 38400) {
> > +                       pll_state->mg_pll_tdc_coldst_bias_mask =
> > +                               MG_PLL_TDC_COLDST_COLDSTART;
> > +                       pll_state->mg_pll_bias_mask = 0;
> > +               } else {
> > +                       pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
> > +                       pll_state->mg_pll_bias_mask = -1U;
> > +               }
> >
> > -       pll_state->mg_pll_tdc_coldst_bias &= pll_state->mg_pll_tdc_coldst_bias_mask;
> > -       pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
> > +               pll_state->mg_pll_tdc_coldst_bias &=
> > +                       pll_state->mg_pll_tdc_coldst_bias_mask;
> > +               pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
> > +       }
> >
> >         return true;
> >  }
> > --
> > 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

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-24 13:00     ` Imre Deak
@ 2019-09-24 23:21       ` Souza, Jose
  2019-09-25 14:26         ` Imre Deak
  2019-09-25 16:12         ` Lucas De Marchi
  0 siblings, 2 replies; 27+ messages in thread
From: Souza, Jose @ 2019-09-24 23:21 UTC (permalink / raw)
  To: lucas.de.marchi, Deak, Imre; +Cc: intel-gfx

On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote:
> On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote:
> > On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
> > <jose.souza@intel.com> wrote:
> > > [...]
> > > +               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) {
> > 
> > From a quick look to the table, we don't change based on pin_mask,
> > but
> > rather based on lane_mask, just like for MG.
> 
> There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3
> vs.
> pin_mask=0x2/lane_mask=0x3.
> 
> > Btw, lane_mask is unused in this function now and we don't get a
> > warning just because it's misused in a
> > MISSING_CASE() below.
> > 
> > The values do change based on width, but I'm not sure why MG
> > doesn't
> > need to take that into account.
> 
> Yes, looks like crtc_state->lane_count should be considered. Not sure
> why the MG version doesn't do that either, I assume it was an
> addition
> to BSpec only after the function was added. The same applies to
> pin_mask.

Looking more carefully, FIA takes care of flip/reversal lanes, so for
us just matter what lanes can be used.

So dropping the patch adding intel_tc_port_get_pin_assignment_mask(),
dropping tgl_program_dkl_dp_mode and tgl_phy_set_clock_gating() and
reusing ICL versions with a couple of "if gen > 12" to access dkl
register.

One odd thing that I notice is that we use port instead of tc_port in
most MG registers, those MG registers uses a macro that subtract port
and PORT_C to get the right register, thinking in send a patch changing
all of those to receive as parameter tc_port to make it consistent,
what you guys think?


> 
> > +Imre Deak
> > 
> > Lucas De Marchi
> > 
> > > +               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), HIP_INDEX_VAL(tc_port,
> > > 0x0));
> > > +       I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> > > +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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)
> > >  {
> > > @@ -3218,7 +3431,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
> > > ov´er MST
> > > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct
> > > intel_encoder *encoder,
> > >         intel_ddi_config_transcoder_func(crtc_state);
> > > 
> > >         /* 7.d */
> > > -       icl_phy_set_clock_gating(dig_port, false);
> > > +       tgl_phy_set_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 */
> > > @@ -3266,6 +3479,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 now.
> > > +        */
> > > +
> > >         /* 7.l */
> > >         intel_ddi_enable_fec(encoder, crtc_state);
> > >         intel_dsc_enable(encoder, crtc_state);
> > > @@ -3371,9 +3593,15 @@ 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_phy_set_clock_gating(dig_port, false);
> > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > +               tgl_phy_set_clock_gating(dig_port, false);
> > > +       else
> > > +               icl_phy_set_clock_gating(dig_port, false);
> > > 
> > > -       if (INTEL_GEN(dev_priv) >= 11)
> > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > +               tgl_ddi_vswing_sequence(encoder, crtc_state-
> > > >port_clock,
> > > +                                       level,
> > > INTEL_OUTPUT_HDMI);
> > > +       else if (INTEL_GEN(dev_priv) == 11)
> > >                 icl_ddi_vswing_sequence(encoder, crtc_state-
> > > >port_clock,
> > >                                         level,
> > > INTEL_OUTPUT_HDMI);
> > >         else if (IS_CANNONLAKE(dev_priv))
> > > @@ -3383,7 +3611,10 @@ static void
> > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > >         else
> > >                 intel_prepare_hdmi_ddi_buffers(encoder, level);
> > > 
> > > -       icl_phy_set_clock_gating(dig_port, true);
> > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > +               tgl_phy_set_clock_gating(dig_port, true);
> > > +       else
> > > +               icl_phy_set_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
> > 
> > 
> > -- 
> > 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] 27+ messages in thread

* Re: [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training
  2019-09-24 15:58   ` Imre Deak
@ 2019-09-24 23:59     ` Souza, Jose
  2019-09-25 15:25       ` Imre Deak
  0 siblings, 1 reply; 27+ messages in thread
From: Souza, Jose @ 2019-09-24 23:59 UTC (permalink / raw)
  To: Deak, Imre; +Cc: intel-gfx

On Tue, 2019-09-24 at 18:58 +0300, Imre Deak wrote:
> On Mon, Sep 23, 2019 at 12:55:11PM -0700, José Roberto de Souza
> wrote:
> > Link training is failling when running link at 2.7GHz and 1.62GHz
> > and
> > following BSpec pll algorithm.
> > 
> > Comparing the values calculated and the ones from the reference
> > table
> > it looks like MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO should not always
> > set
> > to 5. For DP ports ICL mg pll algorithm sets it to 10 or 5 based on
> > div2 value, that matches with dkl hardcoded table.
> > 
> > So implementing this way as it proved to work in HW and leaving a
> > comment so we know why it do not match BSpec.
> > 
> > Issue reported on BSpec 49204.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 ++++++-
> >  1 file changed, 6 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 496df4095a21..3c881d4cf973 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -2631,7 +2631,12 @@ static bool icl_mg_pll_find_divisors(int
> > clock_khz, bool is_dp, bool use_ssc,
> >  
> >  			if (div2 >= 2) {
> >  				if (is_dkl) {
> > -					a_divratio = 5;
> > +					/*
> > +					 * Note: a_divratio not
> > matching TGL
> > +					 * BSpec algorithm but matching
> > +					 * hardcoded values and working
> > on HW
> > +					 */
> > +					a_divratio = 10;
> 
> Yes, matches the hardcoded values for CLKTOP2_HSCLKCTL/CORECLKCTL1,
> with
> the exception that the hardcoded value for CLKTOP2_HSCLKCTL 5p4 is
> also
> incorrect wrt. div2 within that (should be 1 but it's 2).
> 
> However we don't have any hardcoded values in Bspec for HDMI except
> for
> the 5.94Gbps/div2=1 case. So until otherwise proven, for HDMI/div2>=2
> I
> wouldn't change the value, rather have the same logic as for ICL that
> is
> is_dp ? 10 : 5;

I was trying to avoid touch any HDMI calculation as I can't test it but
it looks sane, so I will keep the if is_dkl, replace to "a_divratio =
is_dp ? 10 : 5;" and add to the comment that this is the supposed value
for HDMI but it is not tested.

> 
> >  					tlinedrv = 1;
> >  				} else {
> >  					a_divratio = is_dp ? 10 : 5;
> > -- 
> > 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] 27+ messages in thread

* Re: [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-24 23:21       ` Souza, Jose
@ 2019-09-25 14:26         ` Imre Deak
  2019-09-25 16:12         ` Lucas De Marchi
  1 sibling, 0 replies; 27+ messages in thread
From: Imre Deak @ 2019-09-25 14:26 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Wed, Sep 25, 2019 at 02:21:52AM +0300, Souza, Jose wrote:
> On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote:
> > On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote:
> > > On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza
> > > <jose.souza@intel.com> wrote:
> > > > [...]
> > > > +               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) {
> > > 
> > > From a quick look to the table, we don't change based on pin_mask,
> > > but
> > > rather based on lane_mask, just like for MG.
> > 
> > There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3
> > vs.
> > pin_mask=0x2/lane_mask=0x3.
> > 
> > > Btw, lane_mask is unused in this function now and we don't get a
> > > warning just because it's misused in a
> > > MISSING_CASE() below.
> > > 
> > > The values do change based on width, but I'm not sure why MG
> > > doesn't
> > > need to take that into account.
> > 
> > Yes, looks like crtc_state->lane_count should be considered. Not sure
> > why the MG version doesn't do that either, I assume it was an
> > addition
> > to BSpec only after the function was added. The same applies to
> > pin_mask.
> 
> Looking more carefully, FIA takes care of flip/reversal lanes, so for
> us just matter what lanes can be used.

I think both the pin assignment and crtc_state->lane_count must be taken
into account, see the above example with different pin assignment but
same lane mask, where you have to program the DP_MODE registers
differently. ICL would need to be fixed and in the end TGL and ICL would
program DP_MODE the same way.

> 
> So dropping the patch adding intel_tc_port_get_pin_assignment_mask(),
> dropping tgl_program_dkl_dp_mode and tgl_phy_set_clock_gating() and
> reusing ICL versions with a couple of "if gen > 12" to access dkl
> register.
> 
> One odd thing that I notice is that we use port instead of tc_port in
> most MG registers, those MG registers uses a macro that subtract port
> and PORT_C to get the right register, thinking in send a patch changing
> all of those to receive as parameter tc_port to make it consistent,
> what you guys think?
> 
> 
> > 
> > > +Imre Deak
> > > 
> > > Lucas De Marchi
> > > 
> > > > +               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), HIP_INDEX_VAL(tc_port,
> > > > 0x0));
> > > > +       I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> > > > +       I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(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)
> > > >  {
> > > > @@ -3218,7 +3431,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
> > > > ov´er MST
> > > > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct
> > > > intel_encoder *encoder,
> > > >         intel_ddi_config_transcoder_func(crtc_state);
> > > > 
> > > >         /* 7.d */
> > > > -       icl_phy_set_clock_gating(dig_port, false);
> > > > +       tgl_phy_set_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 */
> > > > @@ -3266,6 +3479,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 now.
> > > > +        */
> > > > +
> > > >         /* 7.l */
> > > >         intel_ddi_enable_fec(encoder, crtc_state);
> > > >         intel_dsc_enable(encoder, crtc_state);
> > > > @@ -3371,9 +3593,15 @@ 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_phy_set_clock_gating(dig_port, false);
> > > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > > +               tgl_phy_set_clock_gating(dig_port, false);
> > > > +       else
> > > > +               icl_phy_set_clock_gating(dig_port, false);
> > > > 
> > > > -       if (INTEL_GEN(dev_priv) >= 11)
> > > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > > +               tgl_ddi_vswing_sequence(encoder, crtc_state-
> > > > >port_clock,
> > > > +                                       level,
> > > > INTEL_OUTPUT_HDMI);
> > > > +       else if (INTEL_GEN(dev_priv) == 11)
> > > >                 icl_ddi_vswing_sequence(encoder, crtc_state-
> > > > >port_clock,
> > > >                                         level,
> > > > INTEL_OUTPUT_HDMI);
> > > >         else if (IS_CANNONLAKE(dev_priv))
> > > > @@ -3383,7 +3611,10 @@ static void
> > > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > > >         else
> > > >                 intel_prepare_hdmi_ddi_buffers(encoder, level);
> > > > 
> > > > -       icl_phy_set_clock_gating(dig_port, true);
> > > > +       if (INTEL_GEN(dev_priv) >= 12)
> > > > +               tgl_phy_set_clock_gating(dig_port, true);
> > > > +       else
> > > > +               icl_phy_set_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
> > > 
> > > 
> > > -- 
> > > 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] 27+ messages in thread

* Re: [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training
  2019-09-24 23:59     ` Souza, Jose
@ 2019-09-25 15:25       ` Imre Deak
  0 siblings, 0 replies; 27+ messages in thread
From: Imre Deak @ 2019-09-25 15:25 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Wed, Sep 25, 2019 at 02:59:02AM +0300, Souza, Jose wrote:
> On Tue, 2019-09-24 at 18:58 +0300, Imre Deak wrote:
> > On Mon, Sep 23, 2019 at 12:55:11PM -0700, José Roberto de Souza
> > wrote:
> > > Link training is failling when running link at 2.7GHz and 1.62GHz
> > > and
> > > following BSpec pll algorithm.
> > > 
> > > Comparing the values calculated and the ones from the reference
> > > table
> > > it looks like MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO should not always
> > > set
> > > to 5. For DP ports ICL mg pll algorithm sets it to 10 or 5 based on
> > > div2 value, that matches with dkl hardcoded table.
> > > 
> > > So implementing this way as it proved to work in HW and leaving a
> > > comment so we know why it do not match BSpec.
> > > 
> > > Issue reported on BSpec 49204.
> > > 
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 ++++++-
> > >  1 file changed, 6 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 496df4095a21..3c881d4cf973 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -2631,7 +2631,12 @@ static bool icl_mg_pll_find_divisors(int
> > > clock_khz, bool is_dp, bool use_ssc,
> > >  
> > >  			if (div2 >= 2) {
> > >  				if (is_dkl) {
> > > -					a_divratio = 5;
> > > +					/*
> > > +					 * Note: a_divratio not
> > > matching TGL
> > > +					 * BSpec algorithm but matching
> > > +					 * hardcoded values and working
> > > on HW
> > > +					 */
> > > +					a_divratio = 10;
> > 
> > Yes, matches the hardcoded values for CLKTOP2_HSCLKCTL/CORECLKCTL1,
> > with
> > the exception that the hardcoded value for CLKTOP2_HSCLKCTL 5p4 is
> > also
> > incorrect wrt. div2 within that (should be 1 but it's 2).
> > 
> > However we don't have any hardcoded values in Bspec for HDMI except
> > for
> > the 5.94Gbps/div2=1 case. So until otherwise proven, for HDMI/div2>=2
> > I
> > wouldn't change the value, rather have the same logic as for ICL that
> > is
> > is_dp ? 10 : 5;
> 
> I was trying to avoid touch any HDMI calculation as I can't test it but
> it looks sane, so I will keep the if is_dkl, replace to "a_divratio =
> is_dp ? 10 : 5;" and add to the comment that this is the supposed value
> for HDMI but it is not tested.

Ok, you could also assign only tlinedrv based on the platform.

> 
> > 
> > >  					tlinedrv = 1;
> > >  				} else {
> > >  					a_divratio = is_dp ? 10 : 5;
> > > -- 
> > > 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] 27+ messages in thread

* Re: [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences
  2019-09-24 23:21       ` Souza, Jose
  2019-09-25 14:26         ` Imre Deak
@ 2019-09-25 16:12         ` Lucas De Marchi
  1 sibling, 0 replies; 27+ messages in thread
From: Lucas De Marchi @ 2019-09-25 16:12 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Tue, Sep 24, 2019 at 4:21 PM Souza, Jose <jose.souza@intel.com> wrote:
>
> On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote:
> > On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote:
> One odd thing that I notice is that we use port instead of tc_port in
> most MG registers, those MG registers uses a macro that subtract port
> and PORT_C to get the right register, thinking in send a patch changing
> all of those to receive as parameter tc_port to make it consistent,
> what you guys think?

I think it's a leftover from previous implementation. For use them in
TGL we do have to change
to tc_port since TC ports in TGL start with PORT_D.

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] 27+ messages in thread

end of thread, other threads:[~2019-09-25 16:12 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23 19:55 [PATCH v3 0/9] TGL TC enabling v3 José Roberto de Souza
2019-09-23 19:55 ` [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support José Roberto de Souza
2019-09-24 14:20   ` Imre Deak
2019-09-24 20:20     ` Souza, Jose
2019-09-23 19:55 ` [PATCH v3 2/9] drm/i915/tgl: Add support for dkl pll write José Roberto de Souza
2019-09-23 19:55 ` [PATCH v3 3/9] drm/i915/tgl: TC helper function to return pin mapping José Roberto de Souza
2019-09-23 19:55 ` [PATCH v3 4/9] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
2019-09-23 22:02   ` Lucas De Marchi
2019-09-24 13:00     ` Imre Deak
2019-09-24 23:21       ` Souza, Jose
2019-09-25 14:26         ` Imre Deak
2019-09-25 16:12         ` Lucas De Marchi
2019-09-23 19:55 ` [PATCH v3 5/9] drm/i915/tgl: re-indent code to prepare for DKL changes José Roberto de Souza
2019-09-23 22:04   ` Lucas De Marchi
2019-09-24 20:58     ` Matt Roper
2019-09-23 19:55 ` [PATCH v3 6/9] drm/i915/tgl: Add dkl phy pll calculations José Roberto de Souza
2019-09-23 22:04   ` Lucas De Marchi
2019-09-23 19:55 ` [PATCH v3 7/9] drm/i915/tgl: Fix dkl link training José Roberto de Souza
2019-09-24 15:58   ` Imre Deak
2019-09-24 23:59     ` Souza, Jose
2019-09-25 15:25       ` Imre Deak
2019-09-23 19:55 ` [PATCH v3 8/9] drm/i915/tgl: Return the mg/dkl pll as DDI clock for new TC ports José Roberto de Souza
2019-09-23 22:07   ` Lucas De Marchi
2019-09-23 19:55 ` [PATCH v3 9/9] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
2019-09-23 22:08   ` Lucas De Marchi
2019-09-23 21:20 ` ✓ Fi.CI.BAT: success for TGL TC enabling (rev3) Patchwork
2019-09-24 10:48 ` ✓ 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.