diff mbox series

[v3] drm/i915/icl: combo port vswing programming changes per BSPEC

Message ID 1543966869-9759-1-git-send-email-clinton.a.taylor@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3] drm/i915/icl: combo port vswing programming changes per BSPEC | expand

Commit Message

Clint Taylor Dec. 4, 2018, 11:41 p.m. UTC 
From: Clint Taylor <clinton.a.taylor@intel.com>

In August 2018 the BSPEC changed the ICL port programming sequence to
closely resemble earlier gen programming sequence.

v2: remove debug code that Imre found
v3: simplify translation table if-else

BSpec: 21257
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |   4 +
 drivers/gpu/drm/i915/intel_ddi.c | 224 ++++++++++++++-------------------------
 2 files changed, 85 insertions(+), 143 deletions(-)

Comments

Imre Deak Dec. 5, 2018, 4:32 p.m. UTC | #1 
On Tue, Dec 04, 2018 at 03:41:09PM -0800, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> In August 2018 the BSPEC changed the ICL port programming sequence to
> closely resemble earlier gen programming sequence.
> 
> v2: remove debug code that Imre found
> v3: simplify translation table if-else
> 
> BSpec: 21257
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |   4 +
>  drivers/gpu/drm/i915/intel_ddi.c | 224 ++++++++++++++-------------------------
>  2 files changed, 85 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0a7d605..29acdb9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1866,6 +1866,10 @@ enum i915_power_well_id {
>  
>  #define CNL_PORT_TX_DW7_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP((port), 7))
>  #define CNL_PORT_TX_DW7_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0((port), 7))
> +#define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
> +#define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
> +#define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
> +#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))

Looks like _CNL_PORT_TX_DW_GRP() is inconsistent with the ICL
counterpart and CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are broken atm,
they need to be fixed as a follow-up.

>  #define   N_SCALAR(x)			((x) << 24)
>  #define   N_SCALAR_MASK			(0x7F << 24)
>  
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f3e1d6a..d78ec17 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -494,103 +494,63 @@ struct cnl_ddi_buf_trans {
>  	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
>  };
>  
> -struct icl_combo_phy_ddi_buf_trans {
> -	u32 dw2_swing_select;
> -	u32 dw2_swing_scalar;
> -	u32 dw4_scaling;
> -};
> -
> -/* Voltage Swing Programming for VccIO 0.85V for DP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_85V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> -	{ 0xB, 0x70, 0x0018 },	/* 600         0.0   */
> -	{ 0xB, 0x70, 0x3015 },	/* 600         3.5   */
> -	{ 0xB, 0x70, 0x6012 },	/* 600         6.0   */
> -	{ 0x5, 0x00, 0x0018 },	/* 800         0.0   */
> -	{ 0x5, 0x00, 0x3015 },	/* 800         3.5   */
> -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> -};
> -
> -/* FIXME - After table is updated in Bspec */
> -/* Voltage Swing Programming for VccIO 0.85V for eDP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_85V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> +/* icl_combo_phy_ddi_translations */
> +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_dp[] = {
> +						/* NT mV Trans mV db    */
> +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   705      0.6   */
                                                     ----^ 700

> +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
>  };
>  
> -/* Voltage Swing Programming for VccIO 0.95V for DP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_95V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> -	{ 0x5, 0x76, 0x0018 },	/* 800         0.0   */
> -	{ 0x5, 0x76, 0x3015 },	/* 800         3.5   */
> -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_lowswing[] = {
> +						/* NT mV Trans mV db    */
> +	{ 0x0, 0x7F, 0x3F, 0x00, 0x00 },	/* 200   200      0.0   */
> +	{ 0x8, 0x7F, 0x38, 0x00, 0x07 },	/* 200   250      1.9   */
> +	{ 0x1, 0x7F, 0x33, 0x00, 0x0C },	/* 200   300      3.5   */
> +	{ 0x9, 0x7F, 0x31, 0x00, 0x0E },	/* 200   350      4.9   */
> +	{ 0x8, 0x7F, 0x3F, 0x00, 0x00 },	/* 250   250      0.0   */
> +	{ 0x1, 0x7F, 0x38, 0x00, 0x07 },	/* 250   300      1.6   */
> +	{ 0x9, 0x7F, 0x35, 0x00, 0x0A },	/* 250   350      2.9   */
> +	{ 0x1, 0x7F, 0x3F, 0x00, 0x00 },	/* 300   300      0.0   */
> +	{ 0x9, 0x7F, 0x38, 0x00, 0x07 },	/* 300   350      1.3   */
> +	{ 0x9, 0x7F, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
>  };
>  
> -/* FIXME - After table is updated in Bspec */
> -/* Voltage Swing Programming for VccIO 0.95V for eDP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_95V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_hbr3[] = {
> +						/* NT mV Trans mV db    */
> +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   700      0.6   */
> +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
>  };
>  
> -/* Voltage Swing Programming for VccIO 1.05V for DP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_1_05V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> -	{ 0x5, 0x71, 0x0018 },	/* 800         0.0   */
> -	{ 0x5, 0x71, 0x3015 },	/* 800         3.5   */
> -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_hdmi[] = {
> +						/* NT mV Trans mV db    */
> +	{ 0xA, 0x60, 0x3F, 0x00, 0x00 },	/* 450   450      0.0   */
> +	{ 0xB, 0x73, 0x36, 0x00, 0x09 },	/* 450   650      3.2   */
> +	{ 0x6, 0x7F, 0x31, 0x00, 0x0E },	/* 450   850      5.5   */
> +	{ 0xB, 0x73, 0x3F, 0x00, 0x00 },	/* 650   650      0.0   ALS */
> +	{ 0x6, 0x7F, 0x37, 0x00, 0x08 },	/* 650   850      2.3   */
> +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 850   850      0.0   */
> +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   850      3.0   */
>  };
>  
> -/* FIXME - After table is updated in Bspec */
> -/* Voltage Swing Programming for VccIO 1.05V for eDP */
> -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_1_05V[] = {
> -				/* Voltage mV  db    */
> -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_mipi[] = {
> +						/* NT mV Trans mV db    */
> +	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */

The DSI code has its own PHY programming atm, and this table here is
unused so no need adding it.

Unrelated but looks like the DSI code doesn't program ICL_PORT_TX_DW7,
that should be fixed as a follow-up.

>  };
>  
>  struct icl_mg_phy_ddi_buf_trans {
> @@ -871,43 +831,24 @@ static int skl_buf_trans_num_entries(enum port port, int n_entries)
>  	}
>  }
>  
> -static const struct icl_combo_phy_ddi_buf_trans *
> +static const struct cnl_ddi_buf_trans *
>  icl_get_combo_buf_trans(struct drm_i915_private *dev_priv, enum port port,
>  			int type, int *n_entries)
>  {
> -	u32 voltage = I915_READ(ICL_PORT_COMP_DW3(port)) & VOLTAGE_INFO_MASK;
> -
> -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> -		switch (voltage) {
> -		case VOLTAGE_INFO_0_85V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_85V);
> -			return icl_combo_phy_ddi_translations_edp_0_85V;
> -		case VOLTAGE_INFO_0_95V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_95V);
> -			return icl_combo_phy_ddi_translations_edp_0_95V;
> -		case VOLTAGE_INFO_1_05V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_1_05V);
> -			return icl_combo_phy_ddi_translations_edp_1_05V;
> -		default:
> -			MISSING_CASE(voltage);
> -			return NULL;
> -		}
> -	} else {
> -		switch (voltage) {
> -		case VOLTAGE_INFO_0_85V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_85V);
> -			return icl_combo_phy_ddi_translations_dp_hdmi_0_85V;
> -		case VOLTAGE_INFO_0_95V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_95V);
> -			return icl_combo_phy_ddi_translations_dp_hdmi_0_95V;
> -		case VOLTAGE_INFO_1_05V:
> -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_1_05V);
> -			return icl_combo_phy_ddi_translations_dp_hdmi_1_05V;
> -		default:
> -			MISSING_CASE(voltage);
> -			return NULL;
> -		}
> +
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> +		return icl_combo_phy_ddi_translations_hdmi;
> +	} else if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_lowswing);
> +		return icl_combo_phy_ddi_translations_edp_lowswing;

Hm, both in the CNL and this ICL code, not sure why we would use the
HBR2 table when the low_vswing param is set. It would be more logical to
me that HBR2 is to be used when the actual link rate is HBR2, else we'd
use the HBR3 table. BXT, ICL still defined these low vswing tables but
CNL and ICL don't. We should at least ask for clarification about this
in BSpec.

Other than the above the patch looks ok, but I'd wait for the BSpec
clarification until adding R-b.

> +	} else if (type == INTEL_OUTPUT_EDP) {
> +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> +		return icl_combo_phy_ddi_translations_edp_hbr3;
>  	}
> +
> +	*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp);
> +	return icl_combo_phy_ddi_translations_dp;
>  }
>  
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> @@ -2464,7 +2405,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
>  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
>  					 u32 level, enum port port, int type)
>  {
> -	const struct icl_combo_phy_ddi_buf_trans *ddi_translations = NULL;
> +	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
>  	u32 n_entries, val;
>  	int ln;
>  
> @@ -2478,34 +2419,23 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
>  		level = n_entries - 1;
>  	}
>  
> -	/* Set PORT_TX_DW5 Rterm Sel to 110b. */
> +	/* Set PORT_TX_DW5 */
>  	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> -	val &= ~RTERM_SELECT_MASK;
> +	val &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK |
> +		  TAP2_DISABLE | TAP3_DISABLE);
> +	val |= SCALING_MODE_SEL(0x2);
>  	val |= RTERM_SELECT(0x6);
> -	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> -
> -	/* Program PORT_TX_DW5 */
> -	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> -	/* Set DisableTap2 and DisableTap3 if MIPI DSI
> -	 * Clear DisableTap2 and DisableTap3 for all other Ports
> -	 */
> -	if (type == INTEL_OUTPUT_DSI) {
> -		val |= TAP2_DISABLE;
> -		val |= TAP3_DISABLE;
> -	} else {
> -		val &= ~TAP2_DISABLE;
> -		val &= ~TAP3_DISABLE;
> -	}
> +	val |= TAP3_DISABLE;
>  	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
>  
>  	/* Program PORT_TX_DW2 */
>  	val = I915_READ(ICL_PORT_TX_DW2_LN0(port));
>  	val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
>  		 RCOMP_SCALAR_MASK);
> -	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_select);
> -	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_select);
> +	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel);
> +	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel);
>  	/* Program Rcomp scalar for every table entry */
> -	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> +	val |= RCOMP_SCALAR(0x98);
>  	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
>  
>  	/* Program PORT_TX_DW4 */
> @@ -2514,9 +2444,17 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
>  		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
>  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>  			 CURSOR_COEFF_MASK);
> -		val |= ddi_translations[level].dw4_scaling;
> +		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
> +		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
> +		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
>  		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
>  	}
> +
> +	/* Program PORT_TX_DW7 */
> +	val = I915_READ(ICL_PORT_TX_DW7_LN0(port));
> +	val &= ~N_SCALAR_MASK;
> +	val |= N_SCALAR(ddi_translations[level].dw7_n_scalar);
> +	I915_WRITE(ICL_PORT_TX_DW7_GRP(port), val);
>  }
>  
>  static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -- 
> 1.9.1
>
Imre Deak Dec. 11, 2018, 9:40 a.m. UTC | #2 
On Wed, Dec 05, 2018 at 06:32:22PM +0200, Imre Deak wrote:
> On Tue, Dec 04, 2018 at 03:41:09PM -0800, clinton.a.taylor@intel.com wrote:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> > 
> > In August 2018 the BSPEC changed the ICL port programming sequence to
> > closely resemble earlier gen programming sequence.
> > 
> > v2: remove debug code that Imre found
> > v3: simplify translation table if-else
> > 
> > BSpec: 21257
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |   4 +
> >  drivers/gpu/drm/i915/intel_ddi.c | 224 ++++++++++++++-------------------------
> >  2 files changed, 85 insertions(+), 143 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 0a7d605..29acdb9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1866,6 +1866,10 @@ enum i915_power_well_id {
> >  
> >  #define CNL_PORT_TX_DW7_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP((port), 7))
> >  #define CNL_PORT_TX_DW7_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0((port), 7))
> > +#define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
> > +#define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
> > +#define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
> > +#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
> 
> Looks like _CNL_PORT_TX_DW_GRP() is inconsistent with the ICL
> counterpart and CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are broken atm,
> they need to be fixed as a follow-up.
> 
> >  #define   N_SCALAR(x)			((x) << 24)
> >  #define   N_SCALAR_MASK			(0x7F << 24)
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f3e1d6a..d78ec17 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -494,103 +494,63 @@ struct cnl_ddi_buf_trans {
> >  	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> >  };
> >  
> > -struct icl_combo_phy_ddi_buf_trans {
> > -	u32 dw2_swing_select;
> > -	u32 dw2_swing_scalar;
> > -	u32 dw4_scaling;
> > -};
> > -
> > -/* Voltage Swing Programming for VccIO 0.85V for DP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_85V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > -	{ 0xB, 0x70, 0x0018 },	/* 600         0.0   */
> > -	{ 0xB, 0x70, 0x3015 },	/* 600         3.5   */
> > -	{ 0xB, 0x70, 0x6012 },	/* 600         6.0   */
> > -	{ 0x5, 0x00, 0x0018 },	/* 800         0.0   */
> > -	{ 0x5, 0x00, 0x3015 },	/* 800         3.5   */
> > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > -};
> > -
> > -/* FIXME - After table is updated in Bspec */
> > -/* Voltage Swing Programming for VccIO 0.85V for eDP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_85V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > +/* icl_combo_phy_ddi_translations */
> > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_dp[] = {
> > +						/* NT mV Trans mV db    */
> > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   705      0.6   */
>                                                      ----^ 700
> 
> > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> >  };
> >  
> > -/* Voltage Swing Programming for VccIO 0.95V for DP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_95V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > -	{ 0x5, 0x76, 0x0018 },	/* 800         0.0   */
> > -	{ 0x5, 0x76, 0x3015 },	/* 800         3.5   */
> > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_lowswing[] = {
> > +						/* NT mV Trans mV db    */
> > +	{ 0x0, 0x7F, 0x3F, 0x00, 0x00 },	/* 200   200      0.0   */
> > +	{ 0x8, 0x7F, 0x38, 0x00, 0x07 },	/* 200   250      1.9   */
> > +	{ 0x1, 0x7F, 0x33, 0x00, 0x0C },	/* 200   300      3.5   */
> > +	{ 0x9, 0x7F, 0x31, 0x00, 0x0E },	/* 200   350      4.9   */
> > +	{ 0x8, 0x7F, 0x3F, 0x00, 0x00 },	/* 250   250      0.0   */
> > +	{ 0x1, 0x7F, 0x38, 0x00, 0x07 },	/* 250   300      1.6   */
> > +	{ 0x9, 0x7F, 0x35, 0x00, 0x0A },	/* 250   350      2.9   */
> > +	{ 0x1, 0x7F, 0x3F, 0x00, 0x00 },	/* 300   300      0.0   */
> > +	{ 0x9, 0x7F, 0x38, 0x00, 0x07 },	/* 300   350      1.3   */
> > +	{ 0x9, 0x7F, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> >  };
> >  
> > -/* FIXME - After table is updated in Bspec */
> > -/* Voltage Swing Programming for VccIO 0.95V for eDP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_95V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_hbr3[] = {
> > +						/* NT mV Trans mV db    */
> > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   700      0.6   */
> > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> >  };
> >  
> > -/* Voltage Swing Programming for VccIO 1.05V for DP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_1_05V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > -	{ 0x5, 0x71, 0x0018 },	/* 800         0.0   */
> > -	{ 0x5, 0x71, 0x3015 },	/* 800         3.5   */
> > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_hdmi[] = {
> > +						/* NT mV Trans mV db    */
> > +	{ 0xA, 0x60, 0x3F, 0x00, 0x00 },	/* 450   450      0.0   */
> > +	{ 0xB, 0x73, 0x36, 0x00, 0x09 },	/* 450   650      3.2   */
> > +	{ 0x6, 0x7F, 0x31, 0x00, 0x0E },	/* 450   850      5.5   */
> > +	{ 0xB, 0x73, 0x3F, 0x00, 0x00 },	/* 650   650      0.0   ALS */
> > +	{ 0x6, 0x7F, 0x37, 0x00, 0x08 },	/* 650   850      2.3   */
> > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 850   850      0.0   */
> > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   850      3.0   */
> >  };
> >  
> > -/* FIXME - After table is updated in Bspec */
> > -/* Voltage Swing Programming for VccIO 1.05V for eDP */
> > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_1_05V[] = {
> > -				/* Voltage mV  db    */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_mipi[] = {
> > +						/* NT mV Trans mV db    */
> > +	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> 
> The DSI code has its own PHY programming atm, and this table here is
> unused so no need adding it.
> 
> Unrelated but looks like the DSI code doesn't program ICL_PORT_TX_DW7,
> that should be fixed as a follow-up.
> 
> >  };
> >  
> >  struct icl_mg_phy_ddi_buf_trans {
> > @@ -871,43 +831,24 @@ static int skl_buf_trans_num_entries(enum port port, int n_entries)
> >  	}
> >  }
> >  
> > -static const struct icl_combo_phy_ddi_buf_trans *
> > +static const struct cnl_ddi_buf_trans *
> >  icl_get_combo_buf_trans(struct drm_i915_private *dev_priv, enum port port,
> >  			int type, int *n_entries)
> >  {
> > -	u32 voltage = I915_READ(ICL_PORT_COMP_DW3(port)) & VOLTAGE_INFO_MASK;
> > -
> > -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > -		switch (voltage) {
> > -		case VOLTAGE_INFO_0_85V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_85V);
> > -			return icl_combo_phy_ddi_translations_edp_0_85V;
> > -		case VOLTAGE_INFO_0_95V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_95V);
> > -			return icl_combo_phy_ddi_translations_edp_0_95V;
> > -		case VOLTAGE_INFO_1_05V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_1_05V);
> > -			return icl_combo_phy_ddi_translations_edp_1_05V;
> > -		default:
> > -			MISSING_CASE(voltage);
> > -			return NULL;
> > -		}
> > -	} else {
> > -		switch (voltage) {
> > -		case VOLTAGE_INFO_0_85V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_85V);
> > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_85V;
> > -		case VOLTAGE_INFO_0_95V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_95V);
> > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_95V;
> > -		case VOLTAGE_INFO_1_05V:
> > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_1_05V);
> > -			return icl_combo_phy_ddi_translations_dp_hdmi_1_05V;
> > -		default:
> > -			MISSING_CASE(voltage);
> > -			return NULL;
> > -		}
> > +
> > +	if (type == INTEL_OUTPUT_HDMI) {
> > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > +		return icl_combo_phy_ddi_translations_hdmi;
> > +	} else if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_lowswing);
> > +		return icl_combo_phy_ddi_translations_edp_lowswing;
> 
> Hm, both in the CNL and this ICL code, not sure why we would use the
> HBR2 table when the low_vswing param is set. It would be more logical to
> me that HBR2 is to be used when the actual link rate is HBR2, else we'd
> use the HBR3 table. BXT, ICL still defined these low vswing tables but
> CNL and ICL don't. We should at least ask for clarification about this
> in BSpec.

Based on the recent BSpec update, we should just ignore
dev_priv->vbt.edp.low_vswing and use the "edp up to HBR2" table if the
link rate is <= HBR2 and the "edp HBR3" table otherwise.

Could you resend the patch with that changed (and the 2 other issues
above addressed)?

Thanks,
Imre

> 
> Other than the above the patch looks ok, but I'd wait for the BSpec
> clarification until adding R-b.
> 
> > +	} else if (type == INTEL_OUTPUT_EDP) {
> > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > +		return icl_combo_phy_ddi_translations_edp_hbr3;
> >  	}
> > +
> > +	*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp);
> > +	return icl_combo_phy_ddi_translations_dp;
> >  }
> >  
> >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> > @@ -2464,7 +2405,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> >  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> >  					 u32 level, enum port port, int type)
> >  {
> > -	const struct icl_combo_phy_ddi_buf_trans *ddi_translations = NULL;
> > +	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> >  	u32 n_entries, val;
> >  	int ln;
> >  
> > @@ -2478,34 +2419,23 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> >  		level = n_entries - 1;
> >  	}
> >  
> > -	/* Set PORT_TX_DW5 Rterm Sel to 110b. */
> > +	/* Set PORT_TX_DW5 */
> >  	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > -	val &= ~RTERM_SELECT_MASK;
> > +	val &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK |
> > +		  TAP2_DISABLE | TAP3_DISABLE);
> > +	val |= SCALING_MODE_SEL(0x2);
> >  	val |= RTERM_SELECT(0x6);
> > -	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > -
> > -	/* Program PORT_TX_DW5 */
> > -	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > -	/* Set DisableTap2 and DisableTap3 if MIPI DSI
> > -	 * Clear DisableTap2 and DisableTap3 for all other Ports
> > -	 */
> > -	if (type == INTEL_OUTPUT_DSI) {
> > -		val |= TAP2_DISABLE;
> > -		val |= TAP3_DISABLE;
> > -	} else {
> > -		val &= ~TAP2_DISABLE;
> > -		val &= ~TAP3_DISABLE;
> > -	}
> > +	val |= TAP3_DISABLE;
> >  	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> >  
> >  	/* Program PORT_TX_DW2 */
> >  	val = I915_READ(ICL_PORT_TX_DW2_LN0(port));
> >  	val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> >  		 RCOMP_SCALAR_MASK);
> > -	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_select);
> > -	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_select);
> > +	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel);
> > +	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel);
> >  	/* Program Rcomp scalar for every table entry */
> > -	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > +	val |= RCOMP_SCALAR(0x98);
> >  	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> >  
> >  	/* Program PORT_TX_DW4 */
> > @@ -2514,9 +2444,17 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> >  		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> >  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> >  			 CURSOR_COEFF_MASK);
> > -		val |= ddi_translations[level].dw4_scaling;
> > +		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
> > +		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
> > +		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
> >  		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> >  	}
> > +
> > +	/* Program PORT_TX_DW7 */
> > +	val = I915_READ(ICL_PORT_TX_DW7_LN0(port));
> > +	val &= ~N_SCALAR_MASK;
> > +	val |= N_SCALAR(ddi_translations[level].dw7_n_scalar);
> > +	I915_WRITE(ICL_PORT_TX_DW7_GRP(port), val);
> >  }
> >  
> >  static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > -- 
> > 1.9.1
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjala Dec. 11, 2018, 2:18 p.m. UTC | #3 
On Tue, Dec 11, 2018 at 11:40:43AM +0200, Imre Deak wrote:
> On Wed, Dec 05, 2018 at 06:32:22PM +0200, Imre Deak wrote:
> > On Tue, Dec 04, 2018 at 03:41:09PM -0800, clinton.a.taylor@intel.com wrote:
> > > From: Clint Taylor <clinton.a.taylor@intel.com>
> > > 
> > > In August 2018 the BSPEC changed the ICL port programming sequence to
> > > closely resemble earlier gen programming sequence.
> > > 
> > > v2: remove debug code that Imre found
> > > v3: simplify translation table if-else
> > > 
> > > BSpec: 21257
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  |   4 +
> > >  drivers/gpu/drm/i915/intel_ddi.c | 224 ++++++++++++++-------------------------
> > >  2 files changed, 85 insertions(+), 143 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 0a7d605..29acdb9 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -1866,6 +1866,10 @@ enum i915_power_well_id {
> > >  
> > >  #define CNL_PORT_TX_DW7_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP((port), 7))
> > >  #define CNL_PORT_TX_DW7_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0((port), 7))
> > > +#define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
> > > +#define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
> > > +#define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
> > > +#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
> > 
> > Looks like _CNL_PORT_TX_DW_GRP() is inconsistent with the ICL
> > counterpart and CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are broken atm,
> > they need to be fixed as a follow-up.
> > 
> > >  #define   N_SCALAR(x)			((x) << 24)
> > >  #define   N_SCALAR_MASK			(0x7F << 24)
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index f3e1d6a..d78ec17 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -494,103 +494,63 @@ struct cnl_ddi_buf_trans {
> > >  	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> > >  };
> > >  
> > > -struct icl_combo_phy_ddi_buf_trans {
> > > -	u32 dw2_swing_select;
> > > -	u32 dw2_swing_scalar;
> > > -	u32 dw4_scaling;
> > > -};
> > > -
> > > -/* Voltage Swing Programming for VccIO 0.85V for DP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_85V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > -	{ 0xB, 0x70, 0x0018 },	/* 600         0.0   */
> > > -	{ 0xB, 0x70, 0x3015 },	/* 600         3.5   */
> > > -	{ 0xB, 0x70, 0x6012 },	/* 600         6.0   */
> > > -	{ 0x5, 0x00, 0x0018 },	/* 800         0.0   */
> > > -	{ 0x5, 0x00, 0x3015 },	/* 800         3.5   */
> > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > -};
> > > -
> > > -/* FIXME - After table is updated in Bspec */
> > > -/* Voltage Swing Programming for VccIO 0.85V for eDP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_85V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > +/* icl_combo_phy_ddi_translations */
> > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_dp[] = {
> > > +						/* NT mV Trans mV db    */
> > > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   705      0.6   */
> >                                                      ----^ 700
> > 
> > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> > >  };
> > >  
> > > -/* Voltage Swing Programming for VccIO 0.95V for DP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_95V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > > -	{ 0x5, 0x76, 0x0018 },	/* 800         0.0   */
> > > -	{ 0x5, 0x76, 0x3015 },	/* 800         3.5   */
> > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_lowswing[] = {
> > > +						/* NT mV Trans mV db    */
> > > +	{ 0x0, 0x7F, 0x3F, 0x00, 0x00 },	/* 200   200      0.0   */
> > > +	{ 0x8, 0x7F, 0x38, 0x00, 0x07 },	/* 200   250      1.9   */
> > > +	{ 0x1, 0x7F, 0x33, 0x00, 0x0C },	/* 200   300      3.5   */
> > > +	{ 0x9, 0x7F, 0x31, 0x00, 0x0E },	/* 200   350      4.9   */
> > > +	{ 0x8, 0x7F, 0x3F, 0x00, 0x00 },	/* 250   250      0.0   */
> > > +	{ 0x1, 0x7F, 0x38, 0x00, 0x07 },	/* 250   300      1.6   */
> > > +	{ 0x9, 0x7F, 0x35, 0x00, 0x0A },	/* 250   350      2.9   */
> > > +	{ 0x1, 0x7F, 0x3F, 0x00, 0x00 },	/* 300   300      0.0   */
> > > +	{ 0x9, 0x7F, 0x38, 0x00, 0x07 },	/* 300   350      1.3   */
> > > +	{ 0x9, 0x7F, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > >  };
> > >  
> > > -/* FIXME - After table is updated in Bspec */
> > > -/* Voltage Swing Programming for VccIO 0.95V for eDP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_95V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_hbr3[] = {
> > > +						/* NT mV Trans mV db    */
> > > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   700      0.6   */
> > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> > >  };
> > >  
> > > -/* Voltage Swing Programming for VccIO 1.05V for DP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_1_05V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > > -	{ 0x5, 0x71, 0x0018 },	/* 800         0.0   */
> > > -	{ 0x5, 0x71, 0x3015 },	/* 800         3.5   */
> > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_hdmi[] = {
> > > +						/* NT mV Trans mV db    */
> > > +	{ 0xA, 0x60, 0x3F, 0x00, 0x00 },	/* 450   450      0.0   */
> > > +	{ 0xB, 0x73, 0x36, 0x00, 0x09 },	/* 450   650      3.2   */
> > > +	{ 0x6, 0x7F, 0x31, 0x00, 0x0E },	/* 450   850      5.5   */
> > > +	{ 0xB, 0x73, 0x3F, 0x00, 0x00 },	/* 650   650      0.0   ALS */
> > > +	{ 0x6, 0x7F, 0x37, 0x00, 0x08 },	/* 650   850      2.3   */
> > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 850   850      0.0   */
> > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   850      3.0   */
> > >  };
> > >  
> > > -/* FIXME - After table is updated in Bspec */
> > > -/* Voltage Swing Programming for VccIO 1.05V for eDP */
> > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_1_05V[] = {
> > > -				/* Voltage mV  db    */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_mipi[] = {
> > > +						/* NT mV Trans mV db    */
> > > +	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> > 
> > The DSI code has its own PHY programming atm, and this table here is
> > unused so no need adding it.
> > 
> > Unrelated but looks like the DSI code doesn't program ICL_PORT_TX_DW7,
> > that should be fixed as a follow-up.
> > 
> > >  };
> > >  
> > >  struct icl_mg_phy_ddi_buf_trans {
> > > @@ -871,43 +831,24 @@ static int skl_buf_trans_num_entries(enum port port, int n_entries)
> > >  	}
> > >  }
> > >  
> > > -static const struct icl_combo_phy_ddi_buf_trans *
> > > +static const struct cnl_ddi_buf_trans *
> > >  icl_get_combo_buf_trans(struct drm_i915_private *dev_priv, enum port port,
> > >  			int type, int *n_entries)
> > >  {
> > > -	u32 voltage = I915_READ(ICL_PORT_COMP_DW3(port)) & VOLTAGE_INFO_MASK;
> > > -
> > > -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > > -		switch (voltage) {
> > > -		case VOLTAGE_INFO_0_85V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_85V);
> > > -			return icl_combo_phy_ddi_translations_edp_0_85V;
> > > -		case VOLTAGE_INFO_0_95V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_95V);
> > > -			return icl_combo_phy_ddi_translations_edp_0_95V;
> > > -		case VOLTAGE_INFO_1_05V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_1_05V);
> > > -			return icl_combo_phy_ddi_translations_edp_1_05V;
> > > -		default:
> > > -			MISSING_CASE(voltage);
> > > -			return NULL;
> > > -		}
> > > -	} else {
> > > -		switch (voltage) {
> > > -		case VOLTAGE_INFO_0_85V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_85V);
> > > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_85V;
> > > -		case VOLTAGE_INFO_0_95V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_95V);
> > > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_95V;
> > > -		case VOLTAGE_INFO_1_05V:
> > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_1_05V);
> > > -			return icl_combo_phy_ddi_translations_dp_hdmi_1_05V;
> > > -		default:
> > > -			MISSING_CASE(voltage);
> > > -			return NULL;
> > > -		}
> > > +
> > > +	if (type == INTEL_OUTPUT_HDMI) {
> > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > +		return icl_combo_phy_ddi_translations_hdmi;
> > > +	} else if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_lowswing);
> > > +		return icl_combo_phy_ddi_translations_edp_lowswing;
> > 
> > Hm, both in the CNL and this ICL code, not sure why we would use the
> > HBR2 table when the low_vswing param is set. It would be more logical to
> > me that HBR2 is to be used when the actual link rate is HBR2, else we'd
> > use the HBR3 table. BXT, ICL still defined these low vswing tables but
> > CNL and ICL don't. We should at least ask for clarification about this
> > in BSpec.
> 
> Based on the recent BSpec update, we should just ignore
> dev_priv->vbt.edp.low_vswing and use the "edp up to HBR2" table if the
> link rate is <= HBR2 and the "edp HBR3" table otherwise.

After Art's clarification I think the correct logic will be:

if (hdmi) {
	return ...;
else if (rate == 8.1) // could check for edp too here i suppose
	return icl_combo_phy_ddi_translations_edp_hbr3;
else if (edp && low_vswing)
	return icl_combo_phy_ddi_translations_edp_hbr2;
else
	return icl_combo_phy_ddi_translations_dp_hbr2;


And I believe icl_max_source_rate() needs to be changed to
something like:

if (is_combo_phy() && !edp)
	return 5.4;
return 8.1;

> 
> Could you resend the patch with that changed (and the 2 other issues
> above addressed)?
> 
> Thanks,
> Imre
> 
> > 
> > Other than the above the patch looks ok, but I'd wait for the BSpec
> > clarification until adding R-b.
> > 
> > > +	} else if (type == INTEL_OUTPUT_EDP) {
> > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > > +		return icl_combo_phy_ddi_translations_edp_hbr3;
> > >  	}
> > > +
> > > +	*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp);
> > > +	return icl_combo_phy_ddi_translations_dp;
> > >  }
> > >  
> > >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> > > @@ -2464,7 +2405,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > >  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > >  					 u32 level, enum port port, int type)
> > >  {
> > > -	const struct icl_combo_phy_ddi_buf_trans *ddi_translations = NULL;
> > > +	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> > >  	u32 n_entries, val;
> > >  	int ln;
> > >  
> > > @@ -2478,34 +2419,23 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > >  		level = n_entries - 1;
> > >  	}
> > >  
> > > -	/* Set PORT_TX_DW5 Rterm Sel to 110b. */
> > > +	/* Set PORT_TX_DW5 */
> > >  	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > -	val &= ~RTERM_SELECT_MASK;
> > > +	val &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK |
> > > +		  TAP2_DISABLE | TAP3_DISABLE);
> > > +	val |= SCALING_MODE_SEL(0x2);
> > >  	val |= RTERM_SELECT(0x6);
> > > -	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > -
> > > -	/* Program PORT_TX_DW5 */
> > > -	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > -	/* Set DisableTap2 and DisableTap3 if MIPI DSI
> > > -	 * Clear DisableTap2 and DisableTap3 for all other Ports
> > > -	 */
> > > -	if (type == INTEL_OUTPUT_DSI) {
> > > -		val |= TAP2_DISABLE;
> > > -		val |= TAP3_DISABLE;
> > > -	} else {
> > > -		val &= ~TAP2_DISABLE;
> > > -		val &= ~TAP3_DISABLE;
> > > -	}
> > > +	val |= TAP3_DISABLE;
> > >  	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > >  
> > >  	/* Program PORT_TX_DW2 */
> > >  	val = I915_READ(ICL_PORT_TX_DW2_LN0(port));
> > >  	val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> > >  		 RCOMP_SCALAR_MASK);
> > > -	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_select);
> > > -	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_select);
> > > +	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel);
> > > +	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel);
> > >  	/* Program Rcomp scalar for every table entry */
> > > -	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > > +	val |= RCOMP_SCALAR(0x98);
> > >  	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> > >  
> > >  	/* Program PORT_TX_DW4 */
> > > @@ -2514,9 +2444,17 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > >  		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> > >  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> > >  			 CURSOR_COEFF_MASK);
> > > -		val |= ddi_translations[level].dw4_scaling;
> > > +		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
> > > +		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
> > > +		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
> > >  		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > >  	}
> > > +
> > > +	/* Program PORT_TX_DW7 */
> > > +	val = I915_READ(ICL_PORT_TX_DW7_LN0(port));
> > > +	val &= ~N_SCALAR_MASK;
> > > +	val |= N_SCALAR(ddi_translations[level].dw7_n_scalar);
> > > +	I915_WRITE(ICL_PORT_TX_DW7_GRP(port), val);
> > >  }
> > >  
> > >  static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > -- 
> > > 1.9.1
> > > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak Dec. 11, 2018, 2:25 p.m. UTC | #4 
On Tue, Dec 11, 2018 at 04:18:47PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 11, 2018 at 11:40:43AM +0200, Imre Deak wrote:
> > On Wed, Dec 05, 2018 at 06:32:22PM +0200, Imre Deak wrote:
> > > On Tue, Dec 04, 2018 at 03:41:09PM -0800, clinton.a.taylor@intel.com wrote:
> > > > From: Clint Taylor <clinton.a.taylor@intel.com>
> > > > 
> > > > In August 2018 the BSPEC changed the ICL port programming sequence to
> > > > closely resemble earlier gen programming sequence.
> > > > 
> > > > v2: remove debug code that Imre found
> > > > v3: simplify translation table if-else
> > > > 
> > > > BSpec: 21257
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h  |   4 +
> > > >  drivers/gpu/drm/i915/intel_ddi.c | 224 ++++++++++++++-------------------------
> > > >  2 files changed, 85 insertions(+), 143 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 0a7d605..29acdb9 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -1866,6 +1866,10 @@ enum i915_power_well_id {
> > > >  
> > > >  #define CNL_PORT_TX_DW7_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP((port), 7))
> > > >  #define CNL_PORT_TX_DW7_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0((port), 7))
> > > > +#define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
> > > > +#define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
> > > > +#define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
> > > > +#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
> > > 
> > > Looks like _CNL_PORT_TX_DW_GRP() is inconsistent with the ICL
> > > counterpart and CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are broken atm,
> > > they need to be fixed as a follow-up.
> > > 
> > > >  #define   N_SCALAR(x)			((x) << 24)
> > > >  #define   N_SCALAR_MASK			(0x7F << 24)
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index f3e1d6a..d78ec17 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -494,103 +494,63 @@ struct cnl_ddi_buf_trans {
> > > >  	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> > > >  };
> > > >  
> > > > -struct icl_combo_phy_ddi_buf_trans {
> > > > -	u32 dw2_swing_select;
> > > > -	u32 dw2_swing_scalar;
> > > > -	u32 dw4_scaling;
> > > > -};
> > > > -
> > > > -/* Voltage Swing Programming for VccIO 0.85V for DP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_85V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > > -	{ 0xB, 0x70, 0x0018 },	/* 600         0.0   */
> > > > -	{ 0xB, 0x70, 0x3015 },	/* 600         3.5   */
> > > > -	{ 0xB, 0x70, 0x6012 },	/* 600         6.0   */
> > > > -	{ 0x5, 0x00, 0x0018 },	/* 800         0.0   */
> > > > -	{ 0x5, 0x00, 0x3015 },	/* 800         3.5   */
> > > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > > -};
> > > > -
> > > > -/* FIXME - After table is updated in Bspec */
> > > > -/* Voltage Swing Programming for VccIO 0.85V for eDP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_85V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > > +/* icl_combo_phy_ddi_translations */
> > > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_dp[] = {
> > > > +						/* NT mV Trans mV db    */
> > > > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > > > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > > > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > > > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > > > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > > > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > > > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > > > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   705      0.6   */
> > >                                                      ----^ 700
> > > 
> > > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> > > >  };
> > > >  
> > > > -/* Voltage Swing Programming for VccIO 0.95V for DP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_95V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > > > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > > > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > > > -	{ 0x5, 0x76, 0x0018 },	/* 800         0.0   */
> > > > -	{ 0x5, 0x76, 0x3015 },	/* 800         3.5   */
> > > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_lowswing[] = {
> > > > +						/* NT mV Trans mV db    */
> > > > +	{ 0x0, 0x7F, 0x3F, 0x00, 0x00 },	/* 200   200      0.0   */
> > > > +	{ 0x8, 0x7F, 0x38, 0x00, 0x07 },	/* 200   250      1.9   */
> > > > +	{ 0x1, 0x7F, 0x33, 0x00, 0x0C },	/* 200   300      3.5   */
> > > > +	{ 0x9, 0x7F, 0x31, 0x00, 0x0E },	/* 200   350      4.9   */
> > > > +	{ 0x8, 0x7F, 0x3F, 0x00, 0x00 },	/* 250   250      0.0   */
> > > > +	{ 0x1, 0x7F, 0x38, 0x00, 0x07 },	/* 250   300      1.6   */
> > > > +	{ 0x9, 0x7F, 0x35, 0x00, 0x0A },	/* 250   350      2.9   */
> > > > +	{ 0x1, 0x7F, 0x3F, 0x00, 0x00 },	/* 300   300      0.0   */
> > > > +	{ 0x9, 0x7F, 0x38, 0x00, 0x07 },	/* 300   350      1.3   */
> > > > +	{ 0x9, 0x7F, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > > >  };
> > > >  
> > > > -/* FIXME - After table is updated in Bspec */
> > > > -/* Voltage Swing Programming for VccIO 0.95V for eDP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_95V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_hbr3[] = {
> > > > +						/* NT mV Trans mV db    */
> > > > +	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
> > > > +	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
> > > > +	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
> > > > +	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
> > > > +	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
> > > > +	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
> > > > +	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
> > > > +	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   700      0.6   */
> > > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
> > > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
> > > >  };
> > > >  
> > > > -/* Voltage Swing Programming for VccIO 1.05V for DP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_1_05V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
> > > > -	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
> > > > -	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
> > > > -	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
> > > > -	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
> > > > -	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
> > > > -	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
> > > > -	{ 0x5, 0x71, 0x0018 },	/* 800         0.0   */
> > > > -	{ 0x5, 0x71, 0x3015 },	/* 800         3.5   */
> > > > -	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
> > > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_hdmi[] = {
> > > > +						/* NT mV Trans mV db    */
> > > > +	{ 0xA, 0x60, 0x3F, 0x00, 0x00 },	/* 450   450      0.0   */
> > > > +	{ 0xB, 0x73, 0x36, 0x00, 0x09 },	/* 450   650      3.2   */
> > > > +	{ 0x6, 0x7F, 0x31, 0x00, 0x0E },	/* 450   850      5.5   */
> > > > +	{ 0xB, 0x73, 0x3F, 0x00, 0x00 },	/* 650   650      0.0   ALS */
> > > > +	{ 0x6, 0x7F, 0x37, 0x00, 0x08 },	/* 650   850      2.3   */
> > > > +	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 850   850      0.0   */
> > > > +	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   850      3.0   */
> > > >  };
> > > >  
> > > > -/* FIXME - After table is updated in Bspec */
> > > > -/* Voltage Swing Programming for VccIO 1.05V for eDP */
> > > > -static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_1_05V[] = {
> > > > -				/* Voltage mV  db    */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
> > > > -	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
> > > > +static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_mipi[] = {
> > > > +						/* NT mV Trans mV db    */
> > > > +	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
> > > 
> > > The DSI code has its own PHY programming atm, and this table here is
> > > unused so no need adding it.
> > > 
> > > Unrelated but looks like the DSI code doesn't program ICL_PORT_TX_DW7,
> > > that should be fixed as a follow-up.
> > > 
> > > >  };
> > > >  
> > > >  struct icl_mg_phy_ddi_buf_trans {
> > > > @@ -871,43 +831,24 @@ static int skl_buf_trans_num_entries(enum port port, int n_entries)
> > > >  	}
> > > >  }
> > > >  
> > > > -static const struct icl_combo_phy_ddi_buf_trans *
> > > > +static const struct cnl_ddi_buf_trans *
> > > >  icl_get_combo_buf_trans(struct drm_i915_private *dev_priv, enum port port,
> > > >  			int type, int *n_entries)
> > > >  {
> > > > -	u32 voltage = I915_READ(ICL_PORT_COMP_DW3(port)) & VOLTAGE_INFO_MASK;
> > > > -
> > > > -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > > > -		switch (voltage) {
> > > > -		case VOLTAGE_INFO_0_85V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_85V);
> > > > -			return icl_combo_phy_ddi_translations_edp_0_85V;
> > > > -		case VOLTAGE_INFO_0_95V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_95V);
> > > > -			return icl_combo_phy_ddi_translations_edp_0_95V;
> > > > -		case VOLTAGE_INFO_1_05V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_1_05V);
> > > > -			return icl_combo_phy_ddi_translations_edp_1_05V;
> > > > -		default:
> > > > -			MISSING_CASE(voltage);
> > > > -			return NULL;
> > > > -		}
> > > > -	} else {
> > > > -		switch (voltage) {
> > > > -		case VOLTAGE_INFO_0_85V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_85V);
> > > > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_85V;
> > > > -		case VOLTAGE_INFO_0_95V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_95V);
> > > > -			return icl_combo_phy_ddi_translations_dp_hdmi_0_95V;
> > > > -		case VOLTAGE_INFO_1_05V:
> > > > -			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_1_05V);
> > > > -			return icl_combo_phy_ddi_translations_dp_hdmi_1_05V;
> > > > -		default:
> > > > -			MISSING_CASE(voltage);
> > > > -			return NULL;
> > > > -		}
> > > > +
> > > > +	if (type == INTEL_OUTPUT_HDMI) {
> > > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > > +		return icl_combo_phy_ddi_translations_hdmi;
> > > > +	} else if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> > > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_lowswing);
> > > > +		return icl_combo_phy_ddi_translations_edp_lowswing;
> > > 
> > > Hm, both in the CNL and this ICL code, not sure why we would use the
> > > HBR2 table when the low_vswing param is set. It would be more logical to
> > > me that HBR2 is to be used when the actual link rate is HBR2, else we'd
> > > use the HBR3 table. BXT, ICL still defined these low vswing tables but
> > > CNL and ICL don't. We should at least ask for clarification about this
> > > in BSpec.
> > 
> > Based on the recent BSpec update, we should just ignore
> > dev_priv->vbt.edp.low_vswing and use the "edp up to HBR2" table if the
> > link rate is <= HBR2 and the "edp HBR3" table otherwise.
> 
> After Art's clarification I think the correct logic will be:
> 
> if (hdmi) {
> 	return ...;
> else if (rate == 8.1) // could check for edp too here i suppose
> 	return icl_combo_phy_ddi_translations_edp_hbr3;
> else if (edp && low_vswing)
> 	return icl_combo_phy_ddi_translations_edp_hbr2;
> else
> 	return icl_combo_phy_ddi_translations_dp_hbr2;
> 
> 
> And I believe icl_max_source_rate() needs to be changed to
> something like:
> 
> if (is_combo_phy() && !edp)
> 	return 5.4;
> return 8.1;

Yep, correct, I misread in his comment thinking 'DP up to HBR2' is
'eDP up to HBR2'. The above logic you describe seems correct to me.

> 
> > 
> > Could you resend the patch with that changed (and the 2 other issues
> > above addressed)?
> > 
> > Thanks,
> > Imre
> > 
> > > 
> > > Other than the above the patch looks ok, but I'd wait for the BSpec
> > > clarification until adding R-b.
> > > 
> > > > +	} else if (type == INTEL_OUTPUT_EDP) {
> > > > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > > > +		return icl_combo_phy_ddi_translations_edp_hbr3;
> > > >  	}
> > > > +
> > > > +	*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp);
> > > > +	return icl_combo_phy_ddi_translations_dp;
> > > >  }
> > > >  
> > > >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> > > > @@ -2464,7 +2405,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > >  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > > >  					 u32 level, enum port port, int type)
> > > >  {
> > > > -	const struct icl_combo_phy_ddi_buf_trans *ddi_translations = NULL;
> > > > +	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> > > >  	u32 n_entries, val;
> > > >  	int ln;
> > > >  
> > > > @@ -2478,34 +2419,23 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > > >  		level = n_entries - 1;
> > > >  	}
> > > >  
> > > > -	/* Set PORT_TX_DW5 Rterm Sel to 110b. */
> > > > +	/* Set PORT_TX_DW5 */
> > > >  	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > > -	val &= ~RTERM_SELECT_MASK;
> > > > +	val &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK |
> > > > +		  TAP2_DISABLE | TAP3_DISABLE);
> > > > +	val |= SCALING_MODE_SEL(0x2);
> > > >  	val |= RTERM_SELECT(0x6);
> > > > -	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > > -
> > > > -	/* Program PORT_TX_DW5 */
> > > > -	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > > -	/* Set DisableTap2 and DisableTap3 if MIPI DSI
> > > > -	 * Clear DisableTap2 and DisableTap3 for all other Ports
> > > > -	 */
> > > > -	if (type == INTEL_OUTPUT_DSI) {
> > > > -		val |= TAP2_DISABLE;
> > > > -		val |= TAP3_DISABLE;
> > > > -	} else {
> > > > -		val &= ~TAP2_DISABLE;
> > > > -		val &= ~TAP3_DISABLE;
> > > > -	}
> > > > +	val |= TAP3_DISABLE;
> > > >  	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > >  
> > > >  	/* Program PORT_TX_DW2 */
> > > >  	val = I915_READ(ICL_PORT_TX_DW2_LN0(port));
> > > >  	val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> > > >  		 RCOMP_SCALAR_MASK);
> > > > -	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_select);
> > > > -	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_select);
> > > > +	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel);
> > > > +	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel);
> > > >  	/* Program Rcomp scalar for every table entry */
> > > > -	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > > > +	val |= RCOMP_SCALAR(0x98);
> > > >  	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> > > >  
> > > >  	/* Program PORT_TX_DW4 */
> > > > @@ -2514,9 +2444,17 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
> > > >  		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> > > >  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> > > >  			 CURSOR_COEFF_MASK);
> > > > -		val |= ddi_translations[level].dw4_scaling;
> > > > +		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
> > > > +		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
> > > > +		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
> > > >  		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > > >  	}
> > > > +
> > > > +	/* Program PORT_TX_DW7 */
> > > > +	val = I915_READ(ICL_PORT_TX_DW7_LN0(port));
> > > > +	val &= ~N_SCALAR_MASK;
> > > > +	val |= N_SCALAR(ddi_translations[level].dw7_n_scalar);
> > > > +	I915_WRITE(ICL_PORT_TX_DW7_GRP(port), val);
> > > >  }
> > > >  
> > > >  static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > > -- 
> > > > 1.9.1
> > > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d605..29acdb9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1866,6 +1866,10 @@  enum i915_power_well_id {
 
 #define CNL_PORT_TX_DW7_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP((port), 7))
 #define CNL_PORT_TX_DW7_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0((port), 7))
+#define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
+#define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
+#define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
+#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
 #define   N_SCALAR(x)			((x) << 24)
 #define   N_SCALAR_MASK			(0x7F << 24)
 
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f3e1d6a..d78ec17 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -494,103 +494,63 @@  struct cnl_ddi_buf_trans {
 	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
 };
 
-struct icl_combo_phy_ddi_buf_trans {
-	u32 dw2_swing_select;
-	u32 dw2_swing_scalar;
-	u32 dw4_scaling;
-};
-
-/* Voltage Swing Programming for VccIO 0.85V for DP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_85V[] = {
-				/* Voltage mV  db    */
-	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
-	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
-	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
-	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
-	{ 0xB, 0x70, 0x0018 },	/* 600         0.0   */
-	{ 0xB, 0x70, 0x3015 },	/* 600         3.5   */
-	{ 0xB, 0x70, 0x6012 },	/* 600         6.0   */
-	{ 0x5, 0x00, 0x0018 },	/* 800         0.0   */
-	{ 0x5, 0x00, 0x3015 },	/* 800         3.5   */
-	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
-};
-
-/* FIXME - After table is updated in Bspec */
-/* Voltage Swing Programming for VccIO 0.85V for eDP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_85V[] = {
-				/* Voltage mV  db    */
-	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
+/* icl_combo_phy_ddi_translations */
+static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_dp[] = {
+						/* NT mV Trans mV db    */
+	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
+	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
+	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
+	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
+	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
+	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
+	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
+	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   705      0.6   */
+	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
+	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
 };
 
-/* Voltage Swing Programming for VccIO 0.95V for DP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_0_95V[] = {
-				/* Voltage mV  db    */
-	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
-	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
-	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
-	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
-	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
-	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
-	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
-	{ 0x5, 0x76, 0x0018 },	/* 800         0.0   */
-	{ 0x5, 0x76, 0x3015 },	/* 800         3.5   */
-	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
+static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_lowswing[] = {
+						/* NT mV Trans mV db    */
+	{ 0x0, 0x7F, 0x3F, 0x00, 0x00 },	/* 200   200      0.0   */
+	{ 0x8, 0x7F, 0x38, 0x00, 0x07 },	/* 200   250      1.9   */
+	{ 0x1, 0x7F, 0x33, 0x00, 0x0C },	/* 200   300      3.5   */
+	{ 0x9, 0x7F, 0x31, 0x00, 0x0E },	/* 200   350      4.9   */
+	{ 0x8, 0x7F, 0x3F, 0x00, 0x00 },	/* 250   250      0.0   */
+	{ 0x1, 0x7F, 0x38, 0x00, 0x07 },	/* 250   300      1.6   */
+	{ 0x9, 0x7F, 0x35, 0x00, 0x0A },	/* 250   350      2.9   */
+	{ 0x1, 0x7F, 0x3F, 0x00, 0x00 },	/* 300   300      0.0   */
+	{ 0x9, 0x7F, 0x38, 0x00, 0x07 },	/* 300   350      1.3   */
+	{ 0x9, 0x7F, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
 };
 
-/* FIXME - After table is updated in Bspec */
-/* Voltage Swing Programming for VccIO 0.95V for eDP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_0_95V[] = {
-				/* Voltage mV  db    */
-	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
+static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_edp_hbr3[] = {
+						/* NT mV Trans mV db    */
+	{ 0xA, 0x35, 0x3F, 0x00, 0x00 },	/* 350   350      0.0   */
+	{ 0xA, 0x4F, 0x37, 0x00, 0x08 },	/* 350   500      3.1   */
+	{ 0xC, 0x71, 0x2F, 0x00, 0x10 },	/* 350   700      6.0   */
+	{ 0x6, 0x7F, 0x2B, 0x00, 0x14 },	/* 350   900      8.2   */
+	{ 0xA, 0x4C, 0x3F, 0x00, 0x00 },	/* 500   500      0.0   */
+	{ 0xC, 0x73, 0x34, 0x00, 0x0B },	/* 500   700      2.9   */
+	{ 0x6, 0x7F, 0x2F, 0x00, 0x10 },	/* 500   900      5.1   */
+	{ 0xC, 0x6C, 0x3C, 0x00, 0x03 },	/* 650   700      0.6   */
+	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   900      3.5   */
+	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 900   900      0.0   */
 };
 
-/* Voltage Swing Programming for VccIO 1.05V for DP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_dp_hdmi_1_05V[] = {
-				/* Voltage mV  db    */
-	{ 0x2, 0x98, 0x0018 },	/* 400         0.0   */
-	{ 0x2, 0x98, 0x3015 },	/* 400         3.5   */
-	{ 0x2, 0x98, 0x6012 },	/* 400         6.0   */
-	{ 0x2, 0x98, 0x900F },	/* 400         9.5   */
-	{ 0x4, 0x98, 0x0018 },	/* 600         0.0   */
-	{ 0x4, 0x98, 0x3015 },	/* 600         3.5   */
-	{ 0x4, 0x98, 0x6012 },	/* 600         6.0   */
-	{ 0x5, 0x71, 0x0018 },	/* 800         0.0   */
-	{ 0x5, 0x71, 0x3015 },	/* 800         3.5   */
-	{ 0x6, 0x98, 0x0018 },	/* 1200        0.0   */
+static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_hdmi[] = {
+						/* NT mV Trans mV db    */
+	{ 0xA, 0x60, 0x3F, 0x00, 0x00 },	/* 450   450      0.0   */
+	{ 0xB, 0x73, 0x36, 0x00, 0x09 },	/* 450   650      3.2   */
+	{ 0x6, 0x7F, 0x31, 0x00, 0x0E },	/* 450   850      5.5   */
+	{ 0xB, 0x73, 0x3F, 0x00, 0x00 },	/* 650   650      0.0   ALS */
+	{ 0x6, 0x7F, 0x37, 0x00, 0x08 },	/* 650   850      2.3   */
+	{ 0x6, 0x7F, 0x3F, 0x00, 0x00 },	/* 850   850      0.0   */
+	{ 0x6, 0x7F, 0x35, 0x00, 0x0A },	/* 600   850      3.0   */
 };
 
-/* FIXME - After table is updated in Bspec */
-/* Voltage Swing Programming for VccIO 1.05V for eDP */
-static const struct icl_combo_phy_ddi_buf_trans icl_combo_phy_ddi_translations_edp_1_05V[] = {
-				/* Voltage mV  db    */
-	{ 0x0, 0x00, 0x00 },	/* 200         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 200         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 200         6.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 250         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 250         4.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         0.0   */
-	{ 0x0, 0x00, 0x00 },	/* 300         1.5   */
-	{ 0x0, 0x00, 0x00 },	/* 350         0.0   */
+static const struct cnl_ddi_buf_trans icl_combo_phy_ddi_translations_mipi[] = {
+						/* NT mV Trans mV db    */
+	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
 };
 
 struct icl_mg_phy_ddi_buf_trans {
@@ -871,43 +831,24 @@  static int skl_buf_trans_num_entries(enum port port, int n_entries)
 	}
 }
 
-static const struct icl_combo_phy_ddi_buf_trans *
+static const struct cnl_ddi_buf_trans *
 icl_get_combo_buf_trans(struct drm_i915_private *dev_priv, enum port port,
 			int type, int *n_entries)
 {
-	u32 voltage = I915_READ(ICL_PORT_COMP_DW3(port)) & VOLTAGE_INFO_MASK;
-
-	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
-		switch (voltage) {
-		case VOLTAGE_INFO_0_85V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_85V);
-			return icl_combo_phy_ddi_translations_edp_0_85V;
-		case VOLTAGE_INFO_0_95V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_0_95V);
-			return icl_combo_phy_ddi_translations_edp_0_95V;
-		case VOLTAGE_INFO_1_05V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_1_05V);
-			return icl_combo_phy_ddi_translations_edp_1_05V;
-		default:
-			MISSING_CASE(voltage);
-			return NULL;
-		}
-	} else {
-		switch (voltage) {
-		case VOLTAGE_INFO_0_85V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_85V);
-			return icl_combo_phy_ddi_translations_dp_hdmi_0_85V;
-		case VOLTAGE_INFO_0_95V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_0_95V);
-			return icl_combo_phy_ddi_translations_dp_hdmi_0_95V;
-		case VOLTAGE_INFO_1_05V:
-			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hdmi_1_05V);
-			return icl_combo_phy_ddi_translations_dp_hdmi_1_05V;
-		default:
-			MISSING_CASE(voltage);
-			return NULL;
-		}
+
+	if (type == INTEL_OUTPUT_HDMI) {
+		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
+		return icl_combo_phy_ddi_translations_hdmi;
+	} else if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
+		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_lowswing);
+		return icl_combo_phy_ddi_translations_edp_lowswing;
+	} else if (type == INTEL_OUTPUT_EDP) {
+		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
+		return icl_combo_phy_ddi_translations_edp_hbr3;
 	}
+
+	*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp);
+	return icl_combo_phy_ddi_translations_dp;
 }
 
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
@@ -2464,7 +2405,7 @@  static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
 static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
 					 u32 level, enum port port, int type)
 {
-	const struct icl_combo_phy_ddi_buf_trans *ddi_translations = NULL;
+	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
 	u32 n_entries, val;
 	int ln;
 
@@ -2478,34 +2419,23 @@  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
 		level = n_entries - 1;
 	}
 
-	/* Set PORT_TX_DW5 Rterm Sel to 110b. */
+	/* Set PORT_TX_DW5 */
 	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
-	val &= ~RTERM_SELECT_MASK;
+	val &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK |
+		  TAP2_DISABLE | TAP3_DISABLE);
+	val |= SCALING_MODE_SEL(0x2);
 	val |= RTERM_SELECT(0x6);
-	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
-
-	/* Program PORT_TX_DW5 */
-	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
-	/* Set DisableTap2 and DisableTap3 if MIPI DSI
-	 * Clear DisableTap2 and DisableTap3 for all other Ports
-	 */
-	if (type == INTEL_OUTPUT_DSI) {
-		val |= TAP2_DISABLE;
-		val |= TAP3_DISABLE;
-	} else {
-		val &= ~TAP2_DISABLE;
-		val &= ~TAP3_DISABLE;
-	}
+	val |= TAP3_DISABLE;
 	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
 
 	/* Program PORT_TX_DW2 */
 	val = I915_READ(ICL_PORT_TX_DW2_LN0(port));
 	val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
 		 RCOMP_SCALAR_MASK);
-	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_select);
-	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_select);
+	val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel);
+	val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel);
 	/* Program Rcomp scalar for every table entry */
-	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
+	val |= RCOMP_SCALAR(0x98);
 	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
 
 	/* Program PORT_TX_DW4 */
@@ -2514,9 +2444,17 @@  static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
 		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
 		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
 			 CURSOR_COEFF_MASK);
-		val |= ddi_translations[level].dw4_scaling;
+		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
+		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
+		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
 		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
 	}
+
+	/* Program PORT_TX_DW7 */
+	val = I915_READ(ICL_PORT_TX_DW7_LN0(port));
+	val &= ~N_SCALAR_MASK;
+	val |= N_SCALAR(ddi_translations[level].dw7_n_scalar);
+	I915_WRITE(ICL_PORT_TX_DW7_GRP(port), val);
 }
 
 static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,