Re: [PATCH 02/15] drm/i915: Always program unique transition scale for CHV

[Deepak <deepak.s@linux.intel.com>]

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