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

Imre Deak
2014-04-30T12:14:00Z

[-- 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 --]