Re: [PATCH 22/71] drm/i915/chv: Add phy supports for Cherryview

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 13991 bytes --]

On Wed, 2014-04-09 at 13:28 +0300, ville.syrjala@linux.intel.com wrote:
> From: Chon Ming Lee <chon.ming.lee@intel.com>
> 
> Added programming phy layer for CHV based on "Application note for 1273
> CHV Display phy".
> 
> v2: Rebase the code and do some cleanup.
> v3: Rework based on Ville review.
>     -Fix the macro where the ch info need to swap, and add parens to ?
> 	 operator.
> 	-Fix wrong bit define for DPIO_PCS_SWING_CALC_0 and
> 	 DPIO_PCS_SWING_CALC_1 and rename for meaningful.
>     -Add some comments for CHV specific DPIO registers.
>     -Change the dp margin registery value to decimal to align with the
> 	 doc.
> 	-Fix the not clearing some value in vlv_dpio_read before write again.
>     -Create new hdmi/dp encoder function for chv instead of share with
> 	valleyview.
> v4: Rebase the code after rename the DPIO registers define and upstream
> 	change.
>     Based on Ville review.
>     -For unique transition scale selection, after Ville point out, look
> 	 like the doc might wrong for the bit 26.  Use bit 27 for ch0 and
> 	 ch1.
> 	-Break up some dpio write value into two/three steps for readability.
> 	-Remove unrelated change.
>     -Add some shift define for some registers instead just give the hex
> 	value.
>     -Fix a bug where write to wrong VLV_TX_DW3.
> v5: Based on Ville review.
> 	- Move tx lane latency optimal setting from chv_dp_pre_pll_enable to
> 	  chv_pre_enable_dp, and chv_hdmi_pre_pll_enable to
> 	  chv_hdmi_pre_enable respectively.
>  	- Fix typo in one margin_reg_value for DP_TRAIN_VOLTAGE_SWING_400.
> 	- Clear DPIO_TX_UNIQ_TRANS_SCALE_EN for DP and HDMI.
> 	- Mask the old deemph and swing bits for hdmi.
> v6: Remove stub for pre_pll_enable for dp and hdmi.
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> [vsyrjala: Don't touch panel power sequencing on DP]
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks ok, so:
Reviewed-by: Imre Deak <imre.deak@intel.com>

Some nitpicks follow, fixing them is optional.

> ---
>  drivers/gpu/drm/i915/intel_dp.c   | 188 +++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_hdmi.c |  84 ++++++++++++++++-
>  2 files changed, 270 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6be7b35..71a4fa2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1932,6 +1932,50 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> +static void chv_pre_enable_dp(struct intel_encoder *encoder)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct edp_power_seq power_seq;
> +	struct intel_crtc *intel_crtc =
> +		to_intel_crtc(encoder->base.crtc);
> +	enum dpio_channel ch = vlv_dport_to_channel(dport);
> +	int pipe = intel_crtc->pipe;
> +	int data, i;
> +
> +	/* Program Tx lane latency optimal setting*/
> +	mutex_lock(&dev_priv->dpio_lock);
> +	for (i = 0; i < 4; i++) {
> +		/* Set the latency optimal bit */
> +		data = (i == 1) ? 0x0 : 0x6;
> +		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW11(ch, i),
> +				data << DPIO_FRC_LATENCY_SHFIT);
> +
> +		/* Set the upar bit */
> +		data = (i == 1) ? 0x0 : 0x1;
> +		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW14(ch, i),
> +				data << DPIO_UPAR_SHIFT);
> +	}
> +
> +	/* Data lane stagger programming */
> +	/* FIXME: Fix up value only after power analysis */
> +
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	if (is_edp(intel_dp)) {
> +		/* init power sequencer on this pipe and port */
> +		intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> +		intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> +							      &power_seq);
> +	}
> +
> +	intel_enable_dp(encoder);
> +
> +	vlv_wait_port_ready(dev_priv, dport);
> +}
> +
>  /*
>   * Native read with retry for link status and receiver capability reads for
>   * cases where the sink may still be asleep.
> @@ -2156,6 +2200,142 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp)
>  	return 0;
>  }
>  
> +static uint32_t intel_chv_signal_levels(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(dport->base.base.crtc);
> +	u32 deemph_reg_value, margin_reg_value, val, tx_dw2;
> +	uint8_t train_set = intel_dp->train_set[0];
> +	enum dpio_channel ch = vlv_dport_to_channel(dport);
> +	int pipe = intel_crtc->pipe;
> +
> +	switch (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) {
> +	case DP_TRAIN_PRE_EMPHASIS_0:
> +		switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_400:
> +			deemph_reg_value = 128;
> +			margin_reg_value = 52;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_600:
> +			deemph_reg_value = 128;
> +			margin_reg_value = 77;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_800:
> +			deemph_reg_value = 128;
> +			margin_reg_value = 102;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_1200:
> +			deemph_reg_value = 128;
> +			margin_reg_value = 154;
> +			/* FIXME extra to set for 1200 */
> +			break;
> +		default:

We could throw a WARN at these spots, as they would all point to a bug
in the driver.

> +			return 0;
> +		}
> +		break;
> +	case DP_TRAIN_PRE_EMPHASIS_3_5:
> +		switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_400:
> +			deemph_reg_value = 85;
> +			margin_reg_value = 78;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_600:
> +			deemph_reg_value = 85;
> +			margin_reg_value = 116;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_800:
> +			deemph_reg_value = 85;
> +			margin_reg_value = 154;
> +			break;
> +		default:
> +			return 0;
> +		}
> +		break;
> +	case DP_TRAIN_PRE_EMPHASIS_6:
> +		switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_400:
> +			deemph_reg_value = 64;
> +			margin_reg_value = 104;
> +			break;
> +		case DP_TRAIN_VOLTAGE_SWING_600:
> +			deemph_reg_value = 64;
> +			margin_reg_value = 154;
> +			break;
> +		default:
> +			return 0;
> +		}
> +		break;
> +	case DP_TRAIN_PRE_EMPHASIS_9_5:
> +		switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) {
> +		case DP_TRAIN_VOLTAGE_SWING_400:
> +			deemph_reg_value = 43;
> +			margin_reg_value = 154;
> +			break;
> +		default:
> +			return 0;
> +		}
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +
> +	/* Clear calc init */
> +	vlv_dpio_write(dev_priv, pipe, CHV_PCS_DW10(ch), 0);
> +
> +	/* Program swing deemph */
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW4(ch));
> +	val &= ~DPIO_SWING_DEEMPH9P5_MASK;
> +	val |= deemph_reg_value << DPIO_SWING_DEEMPH9P5_SHIFT;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW4(ch), val);
> +
> +	/* Program swing margin */
> +	tx_dw2 = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW2(ch));
> +	tx_dw2 &= ~DPIO_SWING_MARGIN_MASK;
> +	tx_dw2 |= margin_reg_value << DPIO_SWING_MARGIN_SHIFT;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(ch), tx_dw2);
> +
> +	/* Disable unique transition scale */
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW3(ch));
> +	val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(ch), val);
> +
> +	if (((train_set & DP_TRAIN_PRE_EMPHASIS_MASK)
> +			== DP_TRAIN_PRE_EMPHASIS_0) &&
> +		((train_set & DP_TRAIN_VOLTAGE_SWING_MASK)
> +			== DP_TRAIN_VOLTAGE_SWING_1200)) {

It works, but the check for pre-emphasis is redundant.

> +
> +		/*
> +		 * 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.
> +		 */
> +		val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW3(ch));
> +		val |= DPIO_TX_UNIQ_TRANS_SCALE_EN;
> +		vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(ch), val);
> +
> +		tx_dw2 |= (0x9a << DPIO_UNIQ_TRANS_SCALE_SHIFT);
> +		vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(ch), tx_dw2);
> +	}
> +
> +	/* Start swing calculation */
> +	vlv_dpio_write(dev_priv, pipe, CHV_PCS_DW10(ch),
> +		(DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3));
> +
> +	/* LRC Bypass */
> +	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
> +	val |= DPIO_LRC_BYPASS;
> +	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
> +
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	return 0;
> +}
> +
>  static void
>  intel_get_adjust_train(struct intel_dp *intel_dp,
>  		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
> @@ -2370,6 +2550,9 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
>  	} else if (IS_HASWELL(dev)) {
>  		signal_levels = intel_hsw_signal_levels(train_set);
>  		mask = DDI_BUF_EMP_MASK;
> +	} else if (IS_CHERRYVIEW(dev)) {
> +		signal_levels = intel_chv_signal_levels(intel_dp);
> +		mask = 0;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		signal_levels = intel_vlv_signal_levels(intel_dp);
>  		mask = 0;
> @@ -3865,7 +4048,10 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_encoder->disable = intel_disable_dp;
>  	intel_encoder->get_hw_state = intel_dp_get_hw_state;
>  	intel_encoder->get_config = intel_dp_get_config;
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_CHERRYVIEW(dev)) {
> +		intel_encoder->pre_enable = chv_pre_enable_dp;
> +		intel_encoder->enable = vlv_enable_dp;
> +	} else if (IS_VALLEYVIEW(dev)) {
>  		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = vlv_pre_enable_dp;
>  		intel_encoder->enable = vlv_enable_dp;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index b0413e1..bbda011 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1198,6 +1198,85 @@ static void vlv_hdmi_post_disable(struct intel_encoder *encoder)
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> +static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
> +{
> +	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =
> +		to_intel_crtc(encoder->base.crtc);
> +	enum dpio_channel ch = vlv_dport_to_channel(dport);
> +	int pipe = intel_crtc->pipe;
> +	int data, i;
> +	u32 val;
> +
> +	/* Program Tx latency optimal setting */
> +	mutex_lock(&dev_priv->dpio_lock);
> +	for (i = 0; i < 4; i++) {
> +		/* Set the latency optimal bit */
> +		data = (i == 1) ? 0x0 : 0x6;
> +		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW11(ch, i),
> +				data << DPIO_FRC_LATENCY_SHFIT);
> +
> +		/* Set the upar bit */
> +		data = (i == 1) ? 0x0 : 0x1;
> +		vlv_dpio_write(dev_priv, pipe, CHV_TX_DW14(ch, i),
> +				data << DPIO_UPAR_SHIFT);
> +	}
> +
> +	/* Data lane stagger programming */
> +	/* FIXME: Fix up value only after power analysis */
> +
> +	/* Clear calc init */
> +	vlv_dpio_write(dev_priv, pipe, CHV_PCS_DW10(ch), 0);
> +
> +	/* FIXME: Program the support xxx V-dB */
> +	/* Use 800mV-0dB */
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW4(ch));
> +	val &= ~DPIO_SWING_DEEMPH9P5_MASK;
> +	val |= 128 << DPIO_SWING_DEEMPH9P5_SHIFT;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW4(ch), val);
> +
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW2(ch));
> +	val &= ~DPIO_SWING_MARGIN_MASK;
> +	val |= 102 << DPIO_SWING_MARGIN_SHIFT;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(ch), val);
> +
> +	/* Disable unique transition scale */
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_TX_DW3(ch));
> +	val &= ~DPIO_TX_UNIQ_TRANS_SCALE_EN;
> +	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(ch), 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

Could remove this chunk.

> +	/* Start swing calculation */
> +	vlv_dpio_write(dev_priv, pipe, CHV_PCS_DW10(ch),
> +			DPIO_PCS_SWING_CALC_TX0_TX2 |
> +			DPIO_PCS_SWING_CALC_TX1_TX3);
> +
> +	/* LRC Bypass */
> +	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
> +	val |= DPIO_LRC_BYPASS;
> +	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
> +
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	intel_enable_hdmi(encoder);
> +
> +	vlv_wait_port_ready(dev_priv, dport);
> +}
> +
>  static void intel_hdmi_destroy(struct drm_connector *connector)
>  {
>  	drm_connector_cleanup(connector);
> @@ -1332,7 +1411,10 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  	intel_encoder->disable = intel_disable_hdmi;
>  	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
>  	intel_encoder->get_config = intel_hdmi_get_config;
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_CHERRYVIEW(dev)) {
> +		intel_encoder->pre_enable = chv_hdmi_pre_enable;
> +		intel_encoder->enable = vlv_enable_hdmi;
> +	} else if (IS_VALLEYVIEW(dev)) {
>  		intel_encoder->pre_pll_enable = vlv_hdmi_pre_pll_enable;
>  		intel_encoder->pre_enable = vlv_hdmi_pre_enable;
>  		intel_encoder->enable = vlv_enable_hdmi;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx