diff mbox

[08/17] drm/i915/icl: Implement voltage swing programming sequence for Combo PHY DDI

Message ID 20180222035519.13486-9-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Feb. 22, 2018, 3:55 a.m. UTC
From: Manasi Navare <manasi.d.navare@intel.com>

This is an important part of the DDI initalization as well as
for changing the voltage during DisplayPort link training.

The Voltage swing seqeuence is similar to Cannonlake.
However it has different register definitions and hence
it makes sense to create a separate vswing sequence and
program functions for ICL to leave room for more changes
in case the Bspec changes later and deviates from CNL sequence.

v2:
Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)

v3:
* Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)

v4:
* Call it combo_vswing, use switch statement (Paulo)

v5 (from Paulo):
* Fix a typo.
* s/rate < 600000/rate <= 600000/.
* Don't remove blank lines that should be there.

v6:
* Rebased by Rodrigo on top of Cannonlake changes
  where non vswing sequences are not aligned with iboost
  anymore.

v7: Another rebase after an upstream rework.

v8 (from Paulo):
* Adjust the code to the upstream output type changes.
* Squash the patch that moved some functions up.
* Merge both get_combo_buf_trans functions in order to simplify the
  code.
* Change the changelog format.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 189 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 3 deletions(-)

Comments

Zanoni, Paulo R March 22, 2018, 10:23 p.m. UTC | #1
Em Qui, 2018-02-22 às 00:55 -0300, Paulo Zanoni escreveu:
> From: Manasi Navare <manasi.d.navare@intel.com>
> 
> This is an important part of the DDI initalization as well as
> for changing the voltage during DisplayPort link training.
> 
> The Voltage swing seqeuence is similar to Cannonlake.
> However it has different register definitions and hence
> it makes sense to create a separate vswing sequence and
> program functions for ICL to leave room for more changes
> in case the Bspec changes later and deviates from CNL sequence.
> 
> v2:
> Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)
> 
> v3:
> * Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)
> 
> v4:
> * Call it combo_vswing, use switch statement (Paulo)
> 
> v5 (from Paulo):
> * Fix a typo.
> * s/rate < 600000/rate <= 600000/.
> * Don't remove blank lines that should be there.
> 
> v6:
> * Rebased by Rodrigo on top of Cannonlake changes
>   where non vswing sequences are not aligned with iboost
>   anymore.
> 
> v7: Another rebase after an upstream rework.
> 
> v8 (from Paulo):
> * Adjust the code to the upstream output type changes.
> * Squash the patch that moved some functions up.
> * Merge both get_combo_buf_trans functions in order to simplify the
>   code.
> * Change the changelog format.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 189
> ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 186 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0a4683991ec2..c38873cb98ca 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -849,6 +849,45 @@ cnl_get_buf_trans_edp(struct drm_i915_private
> *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static const struct icl_combo_phy_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;
> +		}
> +	}
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv,
> enum port port)
>  {
>  	int n_entries, level, default_entry;
> @@ -2178,6 +2217,144 @@ static void cnl_ddi_vswing_sequence(struct
> intel_encoder *encoder,
>  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
>  }
>  
> +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;
> +	u32 n_entries, val;
> +	int ln;
> +
> +	ddi_translations = icl_get_combo_buf_trans(dev_priv, port,
> type,
> +						   &n_entries);
> +	if (!ddi_translations)
> +		return;
> +
> +	if (level >= n_entries) {
> +		DRM_DEBUG_KMS("DDI translation not found for level
> %d. Using %d instead.", level, n_entries - 1);
> +		level = n_entries - 1;
> +	}
> +
> +	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val &= ~SCALING_MODE_SEL_MASK;
> +	val |= SCALING_MODE_SEL(0x6);

This should be RTERM_SEL.


> +	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;
> +	}
> +	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);
> +	/* Program Rcomp scalar for every table entry */
> +	val |=
> RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> +	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> +
> +	/* Program PORT_TX_DW4 */
> +	/* We cannot write to GRP. It would overwrite individual
> loadgen. */
> +	for (ln = 0; ln <= 3; ln++) {
> +		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;
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +	}
> +}
> +
> +static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder
> *encoder, u32 level)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> +	enum port port = encoder->port;
> +	int type = encoder->type;
> +	int width = 0;
> +	int rate = 0;
> +	u32 val;
> +	int ln = 0;
> +
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		width = 4;
> +		/* Rate is always < than 6GHz for HDMI */
> +	} else {
> +		struct intel_dp *intel_dp =
> enc_to_intel_dp(&encoder->base);
> +
> +		width = intel_dp->lane_count;
> +		rate = intel_dp->link_rate;
> +	}
> +
> +	/*
> +	 * 1. If port type is eDP or DP,
> +	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> +	 * else clear to 0b.
> +	 */
> +	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> +	if (type == INTEL_OUTPUT_HDMI)
> +		val &= ~COMMON_KEEPER_EN;
> +	else
> +		val |= COMMON_KEEPER_EN;
> +	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
> +
> +	/* 2. Program loadgen select */
> +	/*
> +	 * Program PORT_TX_DW4_LN depending on Bit rate and used
> lanes
> +	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
> +	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
> +	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
> +	 */
> +	for (ln = 0; ln <= 3; ln++) {
> +		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> +		val &= ~LOADGEN_SELECT;
> +
> +		if ((rate <= 600000 && width == 4 && ln >= 1) ||
> +		    (rate <= 600000 && width < 4 && (ln == 1 || ln
> == 2))) {
> +			val |= LOADGEN_SELECT;
> +		}
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +	}
> +
> +	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> +	val = I915_READ(ICL_PORT_CL_DW5(port));
> +	val |= SUS_CLOCK_CONFIG;
> +	I915_WRITE(ICL_PORT_CL_DW5(port), val);
> +
> +	/* 4. Clear training enable to change swing values */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val &= ~TX_TRAINING_EN;
> +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> +
> +	/* 5. Program swing and de-emphasis */
> +	icl_ddi_combo_vswing_program(dev_priv, level, port, type);
> +
> +	/* 6. Set training enable to trigger update */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val |= TX_TRAINING_EN;
> +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> +}
> +
> +static void icl_ddi_vswing_sequence(struct intel_encoder *encoder,
> u32 level)
> +{
> +	enum port port = encoder->port;
> +
> +	if (port == PORT_A || port == PORT_B)
> +		icl_combo_phy_ddi_vswing_sequence(encoder, level);
> +	else
> +		/* Not Implemented Yet */
> +		WARN_ON(1);
> +}
> +
>  static uint32_t translate_signal_level(int signal_levels)
>  {
>  	int i;
> @@ -2209,7 +2386,9 @@ u32 bxt_signal_levels(struct intel_dp
> *intel_dp)
>  	struct intel_encoder *encoder = &dport->base;
>  	int level = intel_ddi_dp_level(intel_dp);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);

I was 100% sure I had already reworked the patch to follow the new
standard for the other platforms, but I guess I was wrong.

I'll update this patch based on my review, but I guess now we'll be at
a point where I changed the patch so much that we'll need a new
reviewer.

> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> >type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> >type);
> @@ -2389,7 +2568,9 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port-
> >ddi_io_power_domain);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> >type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> >type);
> @@ -2420,7 +2601,9 @@ static void intel_ddi_pre_enable_hdmi(struct
> intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port-
> >ddi_io_power_domain);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level,
> INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level,
> INTEL_OUTPUT_HDMI);
Rodrigo Vivi April 6, 2018, 12:20 a.m. UTC | #2
On Thu, Feb 22, 2018 at 12:55:10AM -0300, Paulo Zanoni wrote:
> From: Manasi Navare <manasi.d.navare@intel.com>
> 
> This is an important part of the DDI initalization as well as
> for changing the voltage during DisplayPort link training.
> 
> The Voltage swing seqeuence is similar to Cannonlake.
> However it has different register definitions and hence
> it makes sense to create a separate vswing sequence and
> program functions for ICL to leave room for more changes
> in case the Bspec changes later and deviates from CNL sequence.
> 
> v2:
> Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)
> 
> v3:
> * Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)
> 
> v4:
> * Call it combo_vswing, use switch statement (Paulo)
> 
> v5 (from Paulo):
> * Fix a typo.
> * s/rate < 600000/rate <= 600000/.
> * Don't remove blank lines that should be there.
> 
> v6:
> * Rebased by Rodrigo on top of Cannonlake changes
>   where non vswing sequences are not aligned with iboost
>   anymore.
> 
> v7: Another rebase after an upstream rework.
> 
> v8 (from Paulo):
> * Adjust the code to the upstream output type changes.
> * Squash the patch that moved some functions up.
> * Merge both get_combo_buf_trans functions in order to simplify the
>   code.
> * Change the changelog format.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 189 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 186 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0a4683991ec2..c38873cb98ca 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -849,6 +849,45 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static const struct icl_combo_phy_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 {

DP ends up here on HDMI?
This is strange...

Also, for clarity, why not to split this into separated functions
like all other platforms?

> +		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;
> +		}
> +	}
> +}
> +
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	int n_entries, level, default_entry;
> @@ -2178,6 +2217,144 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
>  }
>  
> +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;
> +	u32 n_entries, val;
> +	int ln;
> +
> +	ddi_translations = icl_get_combo_buf_trans(dev_priv, port, type,
> +						   &n_entries);
> +	if (!ddi_translations)
> +		return;
> +
> +	if (level >= n_entries) {
> +		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> +		level = n_entries - 1;
> +	}
> +
> +	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val &= ~SCALING_MODE_SEL_MASK;
> +	val |= SCALING_MODE_SEL(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;
> +	}
> +	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);
> +	/* Program Rcomp scalar for every table entry */
> +	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> +	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> +
> +	/* Program PORT_TX_DW4 */
> +	/* We cannot write to GRP. It would overwrite individual loadgen. */
> +	for (ln = 0; ln <= 3; ln++) {
> +		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;
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +	}
> +}
> +
> +static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum port port = encoder->port;
> +	int type = encoder->type;
> +	int width = 0;
> +	int rate = 0;
> +	u32 val;
> +	int ln = 0;
> +
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		width = 4;
> +		/* Rate is always < than 6GHz for HDMI */
> +	} else {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +		width = intel_dp->lane_count;
> +		rate = intel_dp->link_rate;
> +	}
> +
> +	/*
> +	 * 1. If port type is eDP or DP,
> +	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> +	 * else clear to 0b.
> +	 */
> +	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> +	if (type == INTEL_OUTPUT_HDMI)
> +		val &= ~COMMON_KEEPER_EN;
> +	else
> +		val |= COMMON_KEEPER_EN;
> +	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
> +
> +	/* 2. Program loadgen select */
> +	/*
> +	 * Program PORT_TX_DW4_LN depending on Bit rate and used lanes
> +	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
> +	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
> +	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
> +	 */
> +	for (ln = 0; ln <= 3; ln++) {
> +		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> +		val &= ~LOADGEN_SELECT;
> +
> +		if ((rate <= 600000 && width == 4 && ln >= 1) ||
> +		    (rate <= 600000 && width < 4 && (ln == 1 || ln == 2))) {
> +			val |= LOADGEN_SELECT;
> +		}
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +	}
> +
> +	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> +	val = I915_READ(ICL_PORT_CL_DW5(port));
> +	val |= SUS_CLOCK_CONFIG;
> +	I915_WRITE(ICL_PORT_CL_DW5(port), val);
> +
> +	/* 4. Clear training enable to change swing values */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val &= ~TX_TRAINING_EN;
> +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> +
> +	/* 5. Program swing and de-emphasis */
> +	icl_ddi_combo_vswing_program(dev_priv, level, port, type);
> +
> +	/* 6. Set training enable to trigger update */
> +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> +	val |= TX_TRAINING_EN;
> +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> +}
> +
> +static void icl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> +{
> +	enum port port = encoder->port;
> +
> +	if (port == PORT_A || port == PORT_B)
> +		icl_combo_phy_ddi_vswing_sequence(encoder, level);
> +	else
> +		/* Not Implemented Yet */
> +		WARN_ON(1);
> +}
> +
>  static uint32_t translate_signal_level(int signal_levels)
>  {
>  	int i;
> @@ -2209,7 +2386,9 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_encoder *encoder = &dport->base;
>  	int level = intel_ddi_dp_level(intel_dp);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2389,7 +2568,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2420,7 +2601,9 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		icl_ddi_vswing_sequence(encoder, level);
> +	else if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> -- 
> 2.14.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R April 25, 2018, 12:34 a.m. UTC | #3
Em Qui, 2018-04-05 às 17:20 -0700, Rodrigo Vivi escreveu:
> On Thu, Feb 22, 2018 at 12:55:10AM -0300, Paulo Zanoni wrote:
> > From: Manasi Navare <manasi.d.navare@intel.com>
> > 
> > This is an important part of the DDI initalization as well as
> > for changing the voltage during DisplayPort link training.
> > 
> > The Voltage swing seqeuence is similar to Cannonlake.
> > However it has different register definitions and hence
> > it makes sense to create a separate vswing sequence and
> > program functions for ICL to leave room for more changes
> > in case the Bspec changes later and deviates from CNL sequence.
> > 
> > v2:
> > Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)
> > 
> > v3:
> > * Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)
> > 
> > v4:
> > * Call it combo_vswing, use switch statement (Paulo)
> > 
> > v5 (from Paulo):
> > * Fix a typo.
> > * s/rate < 600000/rate <= 600000/.
> > * Don't remove blank lines that should be there.
> > 
> > v6:
> > * Rebased by Rodrigo on top of Cannonlake changes
> >   where non vswing sequences are not aligned with iboost
> >   anymore.
> > 
> > v7: Another rebase after an upstream rework.
> > 
> > v8 (from Paulo):
> > * Adjust the code to the upstream output type changes.
> > * Squash the patch that moved some functions up.
> > * Merge both get_combo_buf_trans functions in order to simplify the
> >   code.
> > * Change the changelog format.
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 189
> > ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 186 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0a4683991ec2..c38873cb98ca 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -849,6 +849,45 @@ cnl_get_buf_trans_edp(struct drm_i915_private
> > *dev_priv, int *n_entries)
> >  	}
> >  }
> >  
> > +static const struct icl_combo_phy_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 {
> 
> DP ends up here on HDMI?
> This is strange...

DP ends on the "dp_hdmi" part, while edp ends on the "edp" part. What
is strange about that?

> 
> Also, for clarity, why not to split this into separated functions
> like all other platforms?

So we don't end up reading PORT_COMP_DW3 multiple times (like we do for
CNL), and as a bonus the code looks better IMHO.



> 
> > +		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;
> > +		}
> > +	}
> > +}
> > +
> >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv,
> > enum port port)
> >  {
> >  	int n_entries, level, default_entry;
> > @@ -2178,6 +2217,144 @@ static void cnl_ddi_vswing_sequence(struct
> > intel_encoder *encoder,
> >  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
> >  }
> >  
> > +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;
> > +	u32 n_entries, val;
> > +	int ln;
> > +
> > +	ddi_translations = icl_get_combo_buf_trans(dev_priv, port,
> > type,
> > +						   &n_entries);
> > +	if (!ddi_translations)
> > +		return;
> > +
> > +	if (level >= n_entries) {
> > +		DRM_DEBUG_KMS("DDI translation not found for level
> > %d. Using %d instead.", level, n_entries - 1);
> > +		level = n_entries - 1;
> > +	}
> > +
> > +	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
> > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > +	val &= ~SCALING_MODE_SEL_MASK;
> > +	val |= SCALING_MODE_SEL(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;
> > +	}
> > +	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);
> > +	/* Program Rcomp scalar for every table entry */
> > +	val |=
> > RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > +	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> > +
> > +	/* Program PORT_TX_DW4 */
> > +	/* We cannot write to GRP. It would overwrite individual
> > loadgen. */
> > +	for (ln = 0; ln <= 3; ln++) {
> > +		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;
> > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > +	}
> > +}
> > +
> > +static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder
> > *encoder, u32 level)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > >base.dev);
> > +	enum port port = encoder->port;
> > +	int type = encoder->type;
> > +	int width = 0;
> > +	int rate = 0;
> > +	u32 val;
> > +	int ln = 0;
> > +
> > +	if (type == INTEL_OUTPUT_HDMI) {
> > +		width = 4;
> > +		/* Rate is always < than 6GHz for HDMI */
> > +	} else {
> > +		struct intel_dp *intel_dp =
> > enc_to_intel_dp(&encoder->base);
> > +
> > +		width = intel_dp->lane_count;
> > +		rate = intel_dp->link_rate;
> > +	}
> > +
> > +	/*
> > +	 * 1. If port type is eDP or DP,
> > +	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> > +	 * else clear to 0b.
> > +	 */
> > +	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> > +	if (type == INTEL_OUTPUT_HDMI)
> > +		val &= ~COMMON_KEEPER_EN;
> > +	else
> > +		val |= COMMON_KEEPER_EN;
> > +	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
> > +
> > +	/* 2. Program loadgen select */
> > +	/*
> > +	 * Program PORT_TX_DW4_LN depending on Bit rate and used
> > lanes
> > +	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
> > +	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
> > +	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
> > +	 */
> > +	for (ln = 0; ln <= 3; ln++) {
> > +		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> > +		val &= ~LOADGEN_SELECT;
> > +
> > +		if ((rate <= 600000 && width == 4 && ln >= 1) ||
> > +		    (rate <= 600000 && width < 4 && (ln == 1 || ln
> > == 2))) {
> > +			val |= LOADGEN_SELECT;
> > +		}
> > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > +	}
> > +
> > +	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> > +	val = I915_READ(ICL_PORT_CL_DW5(port));
> > +	val |= SUS_CLOCK_CONFIG;
> > +	I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > +
> > +	/* 4. Clear training enable to change swing values */
> > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > +	val &= ~TX_TRAINING_EN;
> > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > +
> > +	/* 5. Program swing and de-emphasis */
> > +	icl_ddi_combo_vswing_program(dev_priv, level, port, type);
> > +
> > +	/* 6. Set training enable to trigger update */
> > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > +	val |= TX_TRAINING_EN;
> > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > +}
> > +
> > +static void icl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > u32 level)
> > +{
> > +	enum port port = encoder->port;
> > +
> > +	if (port == PORT_A || port == PORT_B)
> > +		icl_combo_phy_ddi_vswing_sequence(encoder, level);
> > +	else
> > +		/* Not Implemented Yet */
> > +		WARN_ON(1);
> > +}
> > +
> >  static uint32_t translate_signal_level(int signal_levels)
> >  {
> >  	int i;
> > @@ -2209,7 +2386,9 @@ u32 bxt_signal_levels(struct intel_dp
> > *intel_dp)
> >  	struct intel_encoder *encoder = &dport->base;
> >  	int level = intel_ddi_dp_level(intel_dp);
> >  
> > -	if (IS_CANNONLAKE(dev_priv))
> > +	if (IS_ICELAKE(dev_priv))
> > +		icl_ddi_vswing_sequence(encoder, level);
> > +	else if (IS_CANNONLAKE(dev_priv))
> >  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> > >type);
> >  	else
> >  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> > >type);
> > @@ -2389,7 +2568,9 @@ static void intel_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >  
> >  	intel_display_power_get(dev_priv, dig_port-
> > >ddi_io_power_domain);
> >  
> > -	if (IS_CANNONLAKE(dev_priv))
> > +	if (IS_ICELAKE(dev_priv))
> > +		icl_ddi_vswing_sequence(encoder, level);
> > +	else if (IS_CANNONLAKE(dev_priv))
> >  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> > >type);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> > >type);
> > @@ -2420,7 +2601,9 @@ static void intel_ddi_pre_enable_hdmi(struct
> > intel_encoder *encoder,
> >  
> >  	intel_display_power_get(dev_priv, dig_port-
> > >ddi_io_power_domain);
> >  
> > -	if (IS_CANNONLAKE(dev_priv))
> > +	if (IS_ICELAKE(dev_priv))
> > +		icl_ddi_vswing_sequence(encoder, level);
> > +	else if (IS_CANNONLAKE(dev_priv))
> >  		cnl_ddi_vswing_sequence(encoder, level,
> > INTEL_OUTPUT_HDMI);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		bxt_ddi_vswing_sequence(encoder, level,
> > INTEL_OUTPUT_HDMI);
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi April 25, 2018, 6:01 p.m. UTC | #4
On Tue, Apr 24, 2018 at 05:34:14PM -0700, Paulo Zanoni wrote:
> Em Qui, 2018-04-05 às 17:20 -0700, Rodrigo Vivi escreveu:
> > On Thu, Feb 22, 2018 at 12:55:10AM -0300, Paulo Zanoni wrote:
> > > From: Manasi Navare <manasi.d.navare@intel.com>
> > > 
> > > This is an important part of the DDI initalization as well as
> > > for changing the voltage during DisplayPort link training.
> > > 
> > > The Voltage swing seqeuence is similar to Cannonlake.
> > > However it has different register definitions and hence
> > > it makes sense to create a separate vswing sequence and
> > > program functions for ICL to leave room for more changes
> > > in case the Bspec changes later and deviates from CNL sequence.
> > > 
> > > v2:
> > > Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)
> > > 
> > > v3:
> > > * Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)
> > > 
> > > v4:
> > > * Call it combo_vswing, use switch statement (Paulo)
> > > 
> > > v5 (from Paulo):
> > > * Fix a typo.
> > > * s/rate < 600000/rate <= 600000/.
> > > * Don't remove blank lines that should be there.
> > > 
> > > v6:
> > > * Rebased by Rodrigo on top of Cannonlake changes
> > >   where non vswing sequences are not aligned with iboost
> > >   anymore.
> > > 
> > > v7: Another rebase after an upstream rework.
> > > 
> > > v8 (from Paulo):
> > > * Adjust the code to the upstream output type changes.
> > > * Squash the patch that moved some functions up.
> > > * Merge both get_combo_buf_trans functions in order to simplify the
> > >   code.
> > > * Change the changelog format.
> > > 
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 189
> > > ++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 186 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 0a4683991ec2..c38873cb98ca 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -849,6 +849,45 @@ cnl_get_buf_trans_edp(struct drm_i915_private
> > > *dev_priv, int *n_entries)
> > >  	}
> > >  }
> > >  
> > > +static const struct icl_combo_phy_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 {
> > 
> > DP ends up here on HDMI?
> > This is strange...
> 
> DP ends on the "dp_hdmi" part, while edp ends on the "edp" part. What
> is strange about that?

Oh... actually spec is strange...
They have 2 tables, 1 for DP and 1 for HDMI.
I just read them again and they are the same indeed.
So, nothing wrong here.

> 
> > 
> > Also, for clarity, why not to split this into separated functions
> > like all other platforms?
> 
> So we don't end up reading PORT_COMP_DW3 multiple times (like we do for
> CNL), and as a bonus the code looks better IMHO.

Agree.

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

> 
> 
> 
> > 
> > > +		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;
> > > +		}
> > > +	}
> > > +}
> > > +
> > >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv,
> > > enum port port)
> > >  {
> > >  	int n_entries, level, default_entry;
> > > @@ -2178,6 +2217,144 @@ static void cnl_ddi_vswing_sequence(struct
> > > intel_encoder *encoder,
> > >  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
> > >  }
> > >  
> > > +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;
> > > +	u32 n_entries, val;
> > > +	int ln;
> > > +
> > > +	ddi_translations = icl_get_combo_buf_trans(dev_priv, port,
> > > type,
> > > +						   &n_entries);
> > > +	if (!ddi_translations)
> > > +		return;
> > > +
> > > +	if (level >= n_entries) {
> > > +		DRM_DEBUG_KMS("DDI translation not found for level
> > > %d. Using %d instead.", level, n_entries - 1);
> > > +		level = n_entries - 1;
> > > +	}
> > > +
> > > +	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
> > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > +	val &= ~SCALING_MODE_SEL_MASK;
> > > +	val |= SCALING_MODE_SEL(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;
> > > +	}
> > > +	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);
> > > +	/* Program Rcomp scalar for every table entry */
> > > +	val |=
> > > RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > > +	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> > > +
> > > +	/* Program PORT_TX_DW4 */
> > > +	/* We cannot write to GRP. It would overwrite individual
> > > loadgen. */
> > > +	for (ln = 0; ln <= 3; ln++) {
> > > +		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;
> > > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > > +	}
> > > +}
> > > +
> > > +static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder
> > > *encoder, u32 level)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > > >base.dev);
> > > +	enum port port = encoder->port;
> > > +	int type = encoder->type;
> > > +	int width = 0;
> > > +	int rate = 0;
> > > +	u32 val;
> > > +	int ln = 0;
> > > +
> > > +	if (type == INTEL_OUTPUT_HDMI) {
> > > +		width = 4;
> > > +		/* Rate is always < than 6GHz for HDMI */
> > > +	} else {
> > > +		struct intel_dp *intel_dp =
> > > enc_to_intel_dp(&encoder->base);
> > > +
> > > +		width = intel_dp->lane_count;
> > > +		rate = intel_dp->link_rate;
> > > +	}
> > > +
> > > +	/*
> > > +	 * 1. If port type is eDP or DP,
> > > +	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> > > +	 * else clear to 0b.
> > > +	 */
> > > +	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> > > +	if (type == INTEL_OUTPUT_HDMI)
> > > +		val &= ~COMMON_KEEPER_EN;
> > > +	else
> > > +		val |= COMMON_KEEPER_EN;
> > > +	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
> > > +
> > > +	/* 2. Program loadgen select */
> > > +	/*
> > > +	 * Program PORT_TX_DW4_LN depending on Bit rate and used
> > > lanes
> > > +	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
> > > +	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
> > > +	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
> > > +	 */
> > > +	for (ln = 0; ln <= 3; ln++) {
> > > +		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> > > +		val &= ~LOADGEN_SELECT;
> > > +
> > > +		if ((rate <= 600000 && width == 4 && ln >= 1) ||
> > > +		    (rate <= 600000 && width < 4 && (ln == 1 || ln
> > > == 2))) {
> > > +			val |= LOADGEN_SELECT;
> > > +		}
> > > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > > +	}
> > > +
> > > +	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> > > +	val = I915_READ(ICL_PORT_CL_DW5(port));
> > > +	val |= SUS_CLOCK_CONFIG;
> > > +	I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > > +
> > > +	/* 4. Clear training enable to change swing values */
> > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > +	val &= ~TX_TRAINING_EN;
> > > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > +
> > > +	/* 5. Program swing and de-emphasis */
> > > +	icl_ddi_combo_vswing_program(dev_priv, level, port, type);
> > > +
> > > +	/* 6. Set training enable to trigger update */
> > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > +	val |= TX_TRAINING_EN;
> > > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > +}
> > > +
> > > +static void icl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > u32 level)
> > > +{
> > > +	enum port port = encoder->port;
> > > +
> > > +	if (port == PORT_A || port == PORT_B)
> > > +		icl_combo_phy_ddi_vswing_sequence(encoder, level);
> > > +	else
> > > +		/* Not Implemented Yet */
> > > +		WARN_ON(1);
> > > +}
> > > +
> > >  static uint32_t translate_signal_level(int signal_levels)
> > >  {
> > >  	int i;
> > > @@ -2209,7 +2386,9 @@ u32 bxt_signal_levels(struct intel_dp
> > > *intel_dp)
> > >  	struct intel_encoder *encoder = &dport->base;
> > >  	int level = intel_ddi_dp_level(intel_dp);
> > >  
> > > -	if (IS_CANNONLAKE(dev_priv))
> > > +	if (IS_ICELAKE(dev_priv))
> > > +		icl_ddi_vswing_sequence(encoder, level);
> > > +	else if (IS_CANNONLAKE(dev_priv))
> > >  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> > > >type);
> > >  	else
> > >  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> > > >type);
> > > @@ -2389,7 +2568,9 @@ static void intel_ddi_pre_enable_dp(struct
> > > intel_encoder *encoder,
> > >  
> > >  	intel_display_power_get(dev_priv, dig_port-
> > > >ddi_io_power_domain);
> > >  
> > > -	if (IS_CANNONLAKE(dev_priv))
> > > +	if (IS_ICELAKE(dev_priv))
> > > +		icl_ddi_vswing_sequence(encoder, level);
> > > +	else if (IS_CANNONLAKE(dev_priv))
> > >  		cnl_ddi_vswing_sequence(encoder, level, encoder-
> > > >type);
> > >  	else if (IS_GEN9_LP(dev_priv))
> > >  		bxt_ddi_vswing_sequence(encoder, level, encoder-
> > > >type);
> > > @@ -2420,7 +2601,9 @@ static void intel_ddi_pre_enable_hdmi(struct
> > > intel_encoder *encoder,
> > >  
> > >  	intel_display_power_get(dev_priv, dig_port-
> > > >ddi_io_power_domain);
> > >  
> > > -	if (IS_CANNONLAKE(dev_priv))
> > > +	if (IS_ICELAKE(dev_priv))
> > > +		icl_ddi_vswing_sequence(encoder, level);
> > > +	else if (IS_CANNONLAKE(dev_priv))
> > >  		cnl_ddi_vswing_sequence(encoder, level,
> > > INTEL_OUTPUT_HDMI);
> > >  	else if (IS_GEN9_LP(dev_priv))
> > >  		bxt_ddi_vswing_sequence(encoder, level,
> > > INTEL_OUTPUT_HDMI);
> > > -- 
> > > 2.14.3
> > > 
> > > _______________________________________________
> > > 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
Zanoni, Paulo R April 25, 2018, 11:33 p.m. UTC | #5
Em Qua, 2018-04-25 às 11:01 -0700, Rodrigo Vivi escreveu:
> On Tue, Apr 24, 2018 at 05:34:14PM -0700, Paulo Zanoni wrote:
> > Em Qui, 2018-04-05 às 17:20 -0700, Rodrigo Vivi escreveu:
> > > On Thu, Feb 22, 2018 at 12:55:10AM -0300, Paulo Zanoni wrote:
> > > > From: Manasi Navare <manasi.d.navare@intel.com>
> > > > 
> > > > This is an important part of the DDI initalization as well as
> > > > for changing the voltage during DisplayPort link training.
> > > > 
> > > > The Voltage swing seqeuence is similar to Cannonlake.
> > > > However it has different register definitions and hence
> > > > it makes sense to create a separate vswing sequence and
> > > > program functions for ICL to leave room for more changes
> > > > in case the Bspec changes later and deviates from CNL sequence.
> > > > 
> > > > v2:
> > > > Use ~TAP3_DISABLE for enbaling that bit (Jani Nikula)
> > > > 
> > > > v3:
> > > > * Use dw4_scaling column for PORT_TX_DW4 values (Rodrigo)
> > > > 
> > > > v4:
> > > > * Call it combo_vswing, use switch statement (Paulo)
> > > > 
> > > > v5 (from Paulo):
> > > > * Fix a typo.
> > > > * s/rate < 600000/rate <= 600000/.
> > > > * Don't remove blank lines that should be there.
> > > > 
> > > > v6:
> > > > * Rebased by Rodrigo on top of Cannonlake changes
> > > >   where non vswing sequences are not aligned with iboost
> > > >   anymore.
> > > > 
> > > > v7: Another rebase after an upstream rework.
> > > > 
> > > > v8 (from Paulo):
> > > > * Adjust the code to the upstream output type changes.
> > > > * Squash the patch that moved some functions up.
> > > > * Merge both get_combo_buf_trans functions in order to simplify
> > > > the
> > > >   code.
> > > > * Change the changelog format.
> > > > 
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v5)
> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ddi.c | 189
> > > > ++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 186 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index 0a4683991ec2..c38873cb98ca 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -849,6 +849,45 @@ cnl_get_buf_trans_edp(struct
> > > > drm_i915_private
> > > > *dev_priv, int *n_entries)
> > > >  	}
> > > >  }
> > > >  
> > > > +static const struct icl_combo_phy_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 {
> > > 
> > > DP ends up here on HDMI?
> > > This is strange...
> > 
> > DP ends on the "dp_hdmi" part, while edp ends on the "edp" part.
> > What
> > is strange about that?
> 
> Oh... actually spec is strange...
> They have 2 tables, 1 for DP and 1 for HDMI.
> I just read them again and they are the same indeed.
> So, nothing wrong here.
> 
> > 
> > > 
> > > Also, for clarity, why not to split this into separated functions
> > > like all other platforms?
> > 
> > So we don't end up reading PORT_COMP_DW3 multiple times (like we do
> > for
> > CNL), and as a bonus the code looks better IMHO.
> 
> Agree.
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

You're reviewing v8 of the patch. This thread already has v9 (as a
reply to v8). Does the r-b also apply to it?

v9 was also resent as https://patchwork.freedesktop.org/patch/213515/

> 
> > 
> > 
> > 
> > > 
> > > > +		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;
> > > > +		}
> > > > +	}
> > > > +}
> > > > +
> > > >  static int intel_ddi_hdmi_level(struct drm_i915_private
> > > > *dev_priv,
> > > > enum port port)
> > > >  {
> > > >  	int n_entries, level, default_entry;
> > > > @@ -2178,6 +2217,144 @@ static void
> > > > cnl_ddi_vswing_sequence(struct
> > > > intel_encoder *encoder,
> > > >  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
> > > >  }
> > > >  
> > > > +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;
> > > > +	u32 n_entries, val;
> > > > +	int ln;
> > > > +
> > > > +	ddi_translations = icl_get_combo_buf_trans(dev_priv,
> > > > port,
> > > > type,
> > > > +						   &n_entries)
> > > > ;
> > > > +	if (!ddi_translations)
> > > > +		return;
> > > > +
> > > > +	if (level >= n_entries) {
> > > > +		DRM_DEBUG_KMS("DDI translation not found for
> > > > level
> > > > %d. Using %d instead.", level, n_entries - 1);
> > > > +		level = n_entries - 1;
> > > > +	}
> > > > +
> > > > +	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
> > > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > > +	val &= ~SCALING_MODE_SEL_MASK;
> > > > +	val |= SCALING_MODE_SEL(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;
> > > > +	}
> > > > +	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);
> > > > +	/* Program Rcomp scalar for every table entry */
> > > > +	val |=
> > > > RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
> > > > +	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
> > > > +
> > > > +	/* Program PORT_TX_DW4 */
> > > > +	/* We cannot write to GRP. It would overwrite
> > > > individual
> > > > loadgen. */
> > > > +	for (ln = 0; ln <= 3; ln++) {
> > > > +		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;
> > > > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > > > +	}
> > > > +}
> > > > +
> > > > +static void icl_combo_phy_ddi_vswing_sequence(struct
> > > > intel_encoder
> > > > *encoder, u32 level)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> > > > > base.dev);
> > > > 
> > > > +	enum port port = encoder->port;
> > > > +	int type = encoder->type;
> > > > +	int width = 0;
> > > > +	int rate = 0;
> > > > +	u32 val;
> > > > +	int ln = 0;
> > > > +
> > > > +	if (type == INTEL_OUTPUT_HDMI) {
> > > > +		width = 4;
> > > > +		/* Rate is always < than 6GHz for HDMI */
> > > > +	} else {
> > > > +		struct intel_dp *intel_dp =
> > > > enc_to_intel_dp(&encoder->base);
> > > > +
> > > > +		width = intel_dp->lane_count;
> > > > +		rate = intel_dp->link_rate;
> > > > +	}
> > > > +
> > > > +	/*
> > > > +	 * 1. If port type is eDP or DP,
> > > > +	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
> > > > +	 * else clear to 0b.
> > > > +	 */
> > > > +	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> > > > +	if (type == INTEL_OUTPUT_HDMI)
> > > > +		val &= ~COMMON_KEEPER_EN;
> > > > +	else
> > > > +		val |= COMMON_KEEPER_EN;
> > > > +	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
> > > > +
> > > > +	/* 2. Program loadgen select */
> > > > +	/*
> > > > +	 * Program PORT_TX_DW4_LN depending on Bit rate and
> > > > used
> > > > lanes
> > > > +	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
> > > > +	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
> > > > +	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
> > > > +	 */
> > > > +	for (ln = 0; ln <= 3; ln++) {
> > > > +		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> > > > +		val &= ~LOADGEN_SELECT;
> > > > +
> > > > +		if ((rate <= 600000 && width == 4 && ln >= 1)
> > > > ||
> > > > +		    (rate <= 600000 && width < 4 && (ln == 1
> > > > || ln
> > > > == 2))) {
> > > > +			val |= LOADGEN_SELECT;
> > > > +		}
> > > > +		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> > > > +	}
> > > > +
> > > > +	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> > > > +	val = I915_READ(ICL_PORT_CL_DW5(port));
> > > > +	val |= SUS_CLOCK_CONFIG;
> > > > +	I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > > > +
> > > > +	/* 4. Clear training enable to change swing values */
> > > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > > +	val &= ~TX_TRAINING_EN;
> > > > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > > +
> > > > +	/* 5. Program swing and de-emphasis */
> > > > +	icl_ddi_combo_vswing_program(dev_priv, level, port,
> > > > type);
> > > > +
> > > > +	/* 6. Set training enable to trigger update */
> > > > +	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
> > > > +	val |= TX_TRAINING_EN;
> > > > +	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
> > > > +}
> > > > +
> > > > +static void icl_ddi_vswing_sequence(struct intel_encoder
> > > > *encoder,
> > > > u32 level)
> > > > +{
> > > > +	enum port port = encoder->port;
> > > > +
> > > > +	if (port == PORT_A || port == PORT_B)
> > > > +		icl_combo_phy_ddi_vswing_sequence(encoder,
> > > > level);
> > > > +	else
> > > > +		/* Not Implemented Yet */
> > > > +		WARN_ON(1);
> > > > +}
> > > > +
> > > >  static uint32_t translate_signal_level(int signal_levels)
> > > >  {
> > > >  	int i;
> > > > @@ -2209,7 +2386,9 @@ u32 bxt_signal_levels(struct intel_dp
> > > > *intel_dp)
> > > >  	struct intel_encoder *encoder = &dport->base;
> > > >  	int level = intel_ddi_dp_level(intel_dp);
> > > >  
> > > > -	if (IS_CANNONLAKE(dev_priv))
> > > > +	if (IS_ICELAKE(dev_priv))
> > > > +		icl_ddi_vswing_sequence(encoder, level);
> > > > +	else if (IS_CANNONLAKE(dev_priv))
> > > >  		cnl_ddi_vswing_sequence(encoder, level,
> > > > encoder-
> > > > > type);
> > > > 
> > > >  	else
> > > >  		bxt_ddi_vswing_sequence(encoder, level,
> > > > encoder-
> > > > > type);
> > > > 
> > > > @@ -2389,7 +2568,9 @@ static void
> > > > intel_ddi_pre_enable_dp(struct
> > > > intel_encoder *encoder,
> > > >  
> > > >  	intel_display_power_get(dev_priv, dig_port-
> > > > > ddi_io_power_domain);
> > > > 
> > > >  
> > > > -	if (IS_CANNONLAKE(dev_priv))
> > > > +	if (IS_ICELAKE(dev_priv))
> > > > +		icl_ddi_vswing_sequence(encoder, level);
> > > > +	else if (IS_CANNONLAKE(dev_priv))
> > > >  		cnl_ddi_vswing_sequence(encoder, level,
> > > > encoder-
> > > > > type);
> > > > 
> > > >  	else if (IS_GEN9_LP(dev_priv))
> > > >  		bxt_ddi_vswing_sequence(encoder, level,
> > > > encoder-
> > > > > type);
> > > > 
> > > > @@ -2420,7 +2601,9 @@ static void
> > > > intel_ddi_pre_enable_hdmi(struct
> > > > intel_encoder *encoder,
> > > >  
> > > >  	intel_display_power_get(dev_priv, dig_port-
> > > > > ddi_io_power_domain);
> > > > 
> > > >  
> > > > -	if (IS_CANNONLAKE(dev_priv))
> > > > +	if (IS_ICELAKE(dev_priv))
> > > > +		icl_ddi_vswing_sequence(encoder, level);
> > > > +	else if (IS_CANNONLAKE(dev_priv))
> > > >  		cnl_ddi_vswing_sequence(encoder, level,
> > > > INTEL_OUTPUT_HDMI);
> > > >  	else if (IS_GEN9_LP(dev_priv))
> > > >  		bxt_ddi_vswing_sequence(encoder, level,
> > > > INTEL_OUTPUT_HDMI);
> > > > -- 
> > > > 2.14.3
> > > > 
> > > > _______________________________________________
> > > > 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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0a4683991ec2..c38873cb98ca 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -849,6 +849,45 @@  cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static const struct icl_combo_phy_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;
+		}
+	}
+}
+
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
 	int n_entries, level, default_entry;
@@ -2178,6 +2217,144 @@  static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
 	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
 }
 
+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;
+	u32 n_entries, val;
+	int ln;
+
+	ddi_translations = icl_get_combo_buf_trans(dev_priv, port, type,
+						   &n_entries);
+	if (!ddi_translations)
+		return;
+
+	if (level >= n_entries) {
+		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
+		level = n_entries - 1;
+	}
+
+	/* Set PORT_TX_DW5 Scaling Mode Sel to 110b. */
+	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+	val &= ~SCALING_MODE_SEL_MASK;
+	val |= SCALING_MODE_SEL(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;
+	}
+	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);
+	/* Program Rcomp scalar for every table entry */
+	val |= RCOMP_SCALAR(ddi_translations[level].dw2_swing_scalar);
+	I915_WRITE(ICL_PORT_TX_DW2_GRP(port), val);
+
+	/* Program PORT_TX_DW4 */
+	/* We cannot write to GRP. It would overwrite individual loadgen. */
+	for (ln = 0; ln <= 3; ln++) {
+		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;
+		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
+	}
+}
+
+static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = encoder->port;
+	int type = encoder->type;
+	int width = 0;
+	int rate = 0;
+	u32 val;
+	int ln = 0;
+
+	if (type == INTEL_OUTPUT_HDMI) {
+		width = 4;
+		/* Rate is always < than 6GHz for HDMI */
+	} else {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+		width = intel_dp->lane_count;
+		rate = intel_dp->link_rate;
+	}
+
+	/*
+	 * 1. If port type is eDP or DP,
+	 * set PORT_PCS_DW1 cmnkeeper_enable to 1b,
+	 * else clear to 0b.
+	 */
+	val = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
+	if (type == INTEL_OUTPUT_HDMI)
+		val &= ~COMMON_KEEPER_EN;
+	else
+		val |= COMMON_KEEPER_EN;
+	I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), val);
+
+	/* 2. Program loadgen select */
+	/*
+	 * Program PORT_TX_DW4_LN depending on Bit rate and used lanes
+	 * <= 6 GHz and 4 lanes (LN0=0, LN1=1, LN2=1, LN3=1)
+	 * <= 6 GHz and 1,2 lanes (LN0=0, LN1=1, LN2=1, LN3=0)
+	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
+	 */
+	for (ln = 0; ln <= 3; ln++) {
+		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
+		val &= ~LOADGEN_SELECT;
+
+		if ((rate <= 600000 && width == 4 && ln >= 1) ||
+		    (rate <= 600000 && width < 4 && (ln == 1 || ln == 2))) {
+			val |= LOADGEN_SELECT;
+		}
+		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
+	}
+
+	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
+	val = I915_READ(ICL_PORT_CL_DW5(port));
+	val |= SUS_CLOCK_CONFIG;
+	I915_WRITE(ICL_PORT_CL_DW5(port), val);
+
+	/* 4. Clear training enable to change swing values */
+	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+	val &= ~TX_TRAINING_EN;
+	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
+
+	/* 5. Program swing and de-emphasis */
+	icl_ddi_combo_vswing_program(dev_priv, level, port, type);
+
+	/* 6. Set training enable to trigger update */
+	val = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+	val |= TX_TRAINING_EN;
+	I915_WRITE(ICL_PORT_TX_DW5_GRP(port), val);
+}
+
+static void icl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
+{
+	enum port port = encoder->port;
+
+	if (port == PORT_A || port == PORT_B)
+		icl_combo_phy_ddi_vswing_sequence(encoder, level);
+	else
+		/* Not Implemented Yet */
+		WARN_ON(1);
+}
+
 static uint32_t translate_signal_level(int signal_levels)
 {
 	int i;
@@ -2209,7 +2386,9 @@  u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_encoder *encoder = &dport->base;
 	int level = intel_ddi_dp_level(intel_dp);
 
-	if (IS_CANNONLAKE(dev_priv))
+	if (IS_ICELAKE(dev_priv))
+		icl_ddi_vswing_sequence(encoder, level);
+	else if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2389,7 +2568,9 @@  static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	if (IS_CANNONLAKE(dev_priv))
+	if (IS_ICELAKE(dev_priv))
+		icl_ddi_vswing_sequence(encoder, level);
+	else if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2420,7 +2601,9 @@  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	if (IS_CANNONLAKE(dev_priv))
+	if (IS_ICELAKE(dev_priv))
+		icl_ddi_vswing_sequence(encoder, level);
+	else if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);