[02/15] drm/i915: Always program unique transition scale for CHV
diff mbox

Message ID 1436388361-11130-3-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä July 8, 2015, 8:45 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The docs give you the impression that the unique transition scale
value shouldn't matter when unique transition scale is enabled. But
as Imre found on BXT (and I verfied also on BSW) the value does
matter. So from now on just program the same value 0x9a always.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   | 53 +++++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_hdmi.c | 30 +++++++++++-----------
 2 files changed, 42 insertions(+), 41 deletions(-)

Comments

deepak.s@linux.intel.com Aug. 17, 2015, 2:31 a.m. UTC | #1
On 07/09/2015 02:15 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The docs give you the impression that the unique transition scale
> value shouldn't matter when unique transition scale is enabled. But
> as Imre found on BXT (and I verfied also on BSW) the value does
> matter. So from now on just program the same value 0x9a always.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c   | 53 +++++++++++++++++++--------------------
>   drivers/gpu/drm/i915/intel_hdmi.c | 30 +++++++++++-----------
>   2 files changed, 42 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f424833..32d7e43 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3142,6 +3142,12 @@ static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
>   	return 0;
>   }
>   
> +static bool chv_need_uniq_trans_scale(uint8_t train_set)
> +{
> +	return (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) == DP_TRAIN_PRE_EMPH_LEVEL_0 &&
> +		(train_set & DP_TRAIN_VOLTAGE_SWING_MASK) == DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> +}
> +
>   static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> @@ -3260,41 +3266,34 @@ static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
>   	/* Program swing margin */
>   	for (i = 0; i < 4; i++) {
>   		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
> +
>   		val &= ~DPIO_SWING_MARGIN000_MASK;
>   		val |= margin_reg_value << DPIO_SWING_MARGIN000_SHIFT;
> +
> +		/*
> +		 * Supposedly this value shouldn't matter when unique transition
> +		 * scale is disabled, but in fact it does matter. Let's just
> +		 * always program the same value and hope it's OK.
> +		 */
> +		val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
> +		val |= 0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT;
> +
>   		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
>   	}
>   
> -	/* Disable unique transition scale */
> +	/*
> +	 * The document said it needs to set bit 27 for ch0 and bit 26
> +	 * for ch1. Might be a typo in the doc.
> +	 * For now, for this unique transition scale selection, set bit
> +	 * 27 for ch0 and ch1.
> +	 */
>   	for (i = 0; i < 4; i++) {
>   		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
> -		val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
> -		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
> -	}
> -
> -	if (((train_set & DP_TRAIN_PRE_EMPHASIS_MASK)
> -			== DP_TRAIN_PRE_EMPH_LEVEL_0) &&
> -		((train_set & DP_TRAIN_VOLTAGE_SWING_MASK)
> -			== DP_TRAIN_VOLTAGE_SWING_LEVEL_3)) {
> -
> -		/*
> -		 * The document said it needs to set bit 27 for ch0 and bit 26
> -		 * for ch1. Might be a typo in the doc.
> -		 * For now, for this unique transition scale selection, set bit
> -		 * 27 for ch0 and ch1.
> -		 */
> -		for (i = 0; i < 4; i++) {
> -			val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
> +		if (chv_need_uniq_trans_scale(train_set))
>   			val |= DPIO_TX_UNIQ_TRANS_SCALE_EN;
> -			vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
> -		}
> -
> -		for (i = 0; i < 4; i++) {
> -			val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
> -			val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
> -			val |= (0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT);
> -			vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
> -		}
> +		else
> +			val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
> +		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
>   	}
>   
>   	/* Start swing calculation */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ba845f7..9f79afb 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1846,31 +1846,33 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   
>   	for (i = 0; i < 4; i++) {
>   		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
> +
>   		val &= ~DPIO_SWING_MARGIN000_MASK;
>   		val |= 102 << DPIO_SWING_MARGIN000_SHIFT;
> +
> +		/*
> +		 * Supposedly this value shouldn't matter when unique transition
> +		 * scale is disabled, but in fact it does matter. Let's just
> +		 * always program the same value and hope it's OK.
> +		 */
> +		val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
> +		val |= 0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT;
> +
>   		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
>   	}
>   
> -	/* Disable unique transition scale */
> +	/*
> +	 * The document said it needs to set bit 27 for ch0 and bit 26
> +	 * for ch1. Might be a typo in the doc.
> +	 * For now, for this unique transition scale selection, set bit
> +	 * 27 for ch0 and ch1.
> +	 */
>   	for (i = 0; i < 4; i++) {
>   		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
>   		val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
>   		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
>   	}
>   
> -	/* Additional steps for 1200mV-0dB */
> -#if 0
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW3(ch));
> -	if (ch)
> -		val |= DPIO_TX_UNIQ_TRANS_SCALE_CH1;
> -	else
> -		val |= DPIO_TX_UNIQ_TRANS_SCALE_CH0;
> -	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(ch), val);
> -
> -	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(ch),
> -			vlv_dpio_read(dev_priv, pipe, VLV_TX_DW2(ch)) |
> -				(0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT));
> -#endif
>   	/* Start swing calculation */
>   	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW10(ch));
>   	val |= DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3;
Looks fine to me
Reviewed-by: Deepak S <deepak.s@linux.intel.com>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f424833..32d7e43 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3142,6 +3142,12 @@  static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
 	return 0;
 }
 
+static bool chv_need_uniq_trans_scale(uint8_t train_set)
+{
+	return (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) == DP_TRAIN_PRE_EMPH_LEVEL_0 &&
+		(train_set & DP_TRAIN_VOLTAGE_SWING_MASK) == DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
+}
+
 static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3260,41 +3266,34 @@  static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
 	/* Program swing margin */
 	for (i = 0; i < 4; i++) {
 		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
+
 		val &= ~DPIO_SWING_MARGIN000_MASK;
 		val |= margin_reg_value << DPIO_SWING_MARGIN000_SHIFT;
+
+		/*
+		 * Supposedly this value shouldn't matter when unique transition
+		 * scale is disabled, but in fact it does matter. Let's just
+		 * always program the same value and hope it's OK.
+		 */
+		val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
+		val |= 0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT;
+
 		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
 	}
 
-	/* Disable unique transition scale */
+	/*
+	 * The document said it needs to set bit 27 for ch0 and bit 26
+	 * for ch1. Might be a typo in the doc.
+	 * For now, for this unique transition scale selection, set bit
+	 * 27 for ch0 and ch1.
+	 */
 	for (i = 0; i < 4; i++) {
 		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
-		val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
-		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
-	}
-
-	if (((train_set & DP_TRAIN_PRE_EMPHASIS_MASK)
-			== DP_TRAIN_PRE_EMPH_LEVEL_0) &&
-		((train_set & DP_TRAIN_VOLTAGE_SWING_MASK)
-			== DP_TRAIN_VOLTAGE_SWING_LEVEL_3)) {
-
-		/*
-		 * The document said it needs to set bit 27 for ch0 and bit 26
-		 * for ch1. Might be a typo in the doc.
-		 * For now, for this unique transition scale selection, set bit
-		 * 27 for ch0 and ch1.
-		 */
-		for (i = 0; i < 4; i++) {
-			val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
+		if (chv_need_uniq_trans_scale(train_set))
 			val |= DPIO_TX_UNIQ_TRANS_SCALE_EN;
-			vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
-		}
-
-		for (i = 0; i < 4; i++) {
-			val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
-			val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
-			val |= (0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT);
-			vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
-		}
+		else
+			val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
+		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
 	}
 
 	/* Start swing calculation */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ba845f7..9f79afb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1846,31 +1846,33 @@  static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 
 	for (i = 0; i < 4; i++) {
 		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW2(ch, i));
+
 		val &= ~DPIO_SWING_MARGIN000_MASK;
 		val |= 102 << DPIO_SWING_MARGIN000_SHIFT;
+
+		/*
+		 * Supposedly this value shouldn't matter when unique transition
+		 * scale is disabled, but in fact it does matter. Let's just
+		 * always program the same value and hope it's OK.
+		 */
+		val &= ~(0xff << DPIO_UNIQ_TRANS_SCALE_SHIFT);
+		val |= 0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT;
+
 		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW2(ch, i), val);
 	}
 
-	/* Disable unique transition scale */
+	/*
+	 * The document said it needs to set bit 27 for ch0 and bit 26
+	 * for ch1. Might be a typo in the doc.
+	 * For now, for this unique transition scale selection, set bit
+	 * 27 for ch0 and ch1.
+	 */
 	for (i = 0; i < 4; i++) {
 		val = vlv_dpio_read(dev_priv, pipe, CHV_TX_DW3(ch, i));
 		val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
 		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW3(ch, i), val);
 	}
 
-	/* Additional steps for 1200mV-0dB */
-#if 0
-	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW3(ch));
-	if (ch)
-		val |= DPIO_TX_UNIQ_TRANS_SCALE_CH1;
-	else
-		val |= DPIO_TX_UNIQ_TRANS_SCALE_CH0;
-	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(ch), val);
-
-	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(ch),
-			vlv_dpio_read(dev_priv, pipe, VLV_TX_DW2(ch)) |
-				(0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT));
-#endif
 	/* Start swing calculation */
 	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW10(ch));
 	val |= DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3;