On Wed, 2014-04-09 at 13:28 +0300, ville.syrj...@linux.intel.com wrote:
> From: Chon Ming Lee <chon.ming....@intel.com>
> 
> Added programming PLL for CHV based on "Application note for 1273 CHV
> Display phy".
> 
> v2:  -Break the common lane reset into another patch.
>      -Break the clock calculation into another patch.
> 
>     -The changes are based on Ville review.
>     -Rework based on DPIO register define naming convention change.
>     -Break the dpio write into few lines to improve readability.
>     -Correct the udelay during chv_enable_pll.
>     -clean up some magic numbers with some new define.
>     -program the afc recal bit which was missed.
> 
> v3: Based on Ville review
>       -  minor correction of the bit defination
>     - add deassert/propagate data lane reset
> 
> v4: Corrected the udelay between dclkp enable and pll enable.
>       Minor comment and better way to clear the TX lane reset.
> 
> v5: Squash in fixup from Rafael Barbalho.
> 
> Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Chon Ming Lee <chon.ming....@intel.com>

A couple of nitpicks below, fixing any/all of those is optional. This
patch is
Reviewed-by: Imre Deak <imre.d...@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  70 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 133 
> ++++++++++++++++++++++++++++++++++-
>  2 files changed, 201 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8fcf4ea..75f31f5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -668,6 +668,12 @@ enum punit_power_well {
>  #define _VLV_PCS_DW9_CH1             0x8424
>  #define      VLV_PCS_DW9(ch) _PORT(ch, _VLV_PCS_DW9_CH0, _VLV_PCS_DW9_CH1)
>  
> +#define _CHV_PCS_DW10_CH0            0x8228
> +#define _CHV_PCS_DW10_CH1            0x8428

Note that these are atm unused (the code uses instead the single
addressing versions), although these group addressing versions could be
used too.

> +#define   DPIO_PCS_SWING_CALC_TX0_TX2        (1<<30)
> +#define   DPIO_PCS_SWING_CALC_TX1_TX3        (1<<31)
> +#define CHV_PCS_DW10(ch) _PORT(ch, _CHV_PCS_DW10_CH0, _CHV_PCS_DW10_CH1)
> +
>  #define _VLV_PCS_DW11_CH0            0x822c
>  #define _VLV_PCS_DW11_CH1            0x842c
>  #define VLV_PCS_DW11(ch) _PORT(ch, _VLV_PCS_DW11_CH0, _VLV_PCS_DW11_CH1)
> @@ -686,14 +692,21 @@ enum punit_power_well {
>  
>  #define _VLV_TX_DW2_CH0                      0x8288
>  #define _VLV_TX_DW2_CH1                      0x8488
> +#define   DPIO_UNIQ_TRANS_SCALE_SHIFT        8
> +#define   DPIO_SWING_MARGIN_SHIFT    16
> +#define   DPIO_SWING_MARGIN_MASK     (0xff << DPIO_SWING_MARGIN_SHIFT)
>  #define VLV_TX_DW2(ch) _PORT(ch, _VLV_TX_DW2_CH0, _VLV_TX_DW2_CH1)
>  
>  #define _VLV_TX_DW3_CH0                      0x828c
>  #define _VLV_TX_DW3_CH1                      0x848c
> +/* The following bit for CHV phy */
> +#define   DPIO_TX_UNIQ_TRANS_SCALE_EN        (1<<27)
>  #define VLV_TX_DW3(ch) _PORT(ch, _VLV_TX_DW3_CH0, _VLV_TX_DW3_CH1)
>  
>  #define _VLV_TX_DW4_CH0                      0x8290
>  #define _VLV_TX_DW4_CH1                      0x8490
> +#define   DPIO_SWING_DEEMPH9P5_SHIFT 24
> +#define   DPIO_SWING_DEEMPH9P5_MASK  (0xff << DPIO_SWING_DEEMPH9P5_SHIFT)
>  #define VLV_TX_DW4(ch) _PORT(ch, _VLV_TX_DW4_CH0, _VLV_TX_DW4_CH1)
>  
>  #define _VLV_TX3_DW4_CH0             0x690
> @@ -713,6 +726,62 @@ enum punit_power_well {
>  #define _VLV_TX_DW14_CH1             0x84b8
>  #define VLV_TX_DW14(ch) _PORT(ch, _VLV_TX_DW14_CH0, _VLV_TX_DW14_CH1)
>  
> +/* CHV dpPhy registers */
> +#define _CHV_PLL_DW0_CH0             0x8000
> +#define _CHV_PLL_DW0_CH1             0x8180
> +#define CHV_PLL_DW0(ch) _PIPE(ch, _CHV_PLL_DW0_CH0, _CHV_PLL_DW0_CH1)
> +
> +#define _CHV_PLL_DW1_CH0             0x8004
> +#define _CHV_PLL_DW1_CH1             0x8184
> +#define   DPIO_CHV_M1_DIV_BY_2               (0 << 0)
> +#define   DPIO_CHV_N_DIV_SHIFT               (8)

Here and below for simple constants, no need for the (). 

> +#define CHV_PLL_DW1(ch) _PIPE(ch, _CHV_PLL_DW1_CH0, _CHV_PLL_DW1_CH1)
> +
> +#define _CHV_PLL_DW2_CH0             0x8008
> +#define _CHV_PLL_DW2_CH1             0x8188
> +#define CHV_PLL_DW2(ch) _PIPE(ch, _CHV_PLL_DW2_CH0, _CHV_PLL_DW2_CH1)
> +
> +#define _CHV_PLL_DW3_CH0             0x800c
> +#define _CHV_PLL_DW3_CH1             0x818c
> +#define  DPIO_CHV_FEEDFWD_GAIN               2
> +#define  DPIO_CHV_FIRST_MOD          (0 << 8)
> +#define  DPIO_CHV_SECOND_MOD         (1 << 8)
> +#define  DPIO_CHV_FRAC_DIV_EN                (1 << 16)
> +#define CHV_PLL_DW3(ch) _PIPE(ch, _CHV_PLL_DW3_CH0, _CHV_PLL_DW3_CH1)
> +
> +#define _CHV_PLL_DW6_CH0             0x8018
> +#define _CHV_PLL_DW6_CH1             0x8198
> +#define   DPIO_CHV_PROP_COEFF                (5 << 0)
> +#define   DPIO_CHV_GAIN_CTRL         (2 << 16)

These macros combine the field shift and field value, for consistency
I'd define here only the field shift and use the field value inlined in
the code. 

> +#define        DPIO_CHV_INT_COEFF_SHIFT      8
> +#define CHV_PLL_DW6(ch) _PIPE(ch, _CHV_PLL_DW6_CH0, _CHV_PLL_DW6_CH1)
> +
> +#define _CHV_CMN_DW13_CH0            0x8134
> +#define _CHV_CMN_DW0_CH1             0x8080
> +#define   DPIO_CHV_S1_DIV_SELECT     (21)
> +#define   DPIO_CHV_P1_SHIFT          (13) /* 3 bits */
> +#define   DPIO_CHV_P2_SHIFT          (8)  /* 5 bits */
> +#define   DPIO_CHV_K_DIV_SHIFT               (4)
> +#define   DPIO_PLL_LOCK                      0x1

(1 << 0) for consistency.

> +#define   DPIO_PLL_FREQLOCK          0x2

(1 << 1)

> +#define CHV_CMN_DW13(ch) _PIPE(ch, _CHV_CMN_DW13_CH0, _CHV_CMN_DW0_CH1)
> +
> +#define _CHV_CMN_DW14_CH0            0x8138
> +#define _CHV_CMN_DW1_CH1             0x8084
> +#define   DPIO_AFC_RECAL             (1 << 14)
> +#define   DPIO_DCLKP_EN                      (1 << 13)
> +#define CHV_CMN_DW14(ch) _PIPE(ch, _CHV_CMN_DW14_CH0, _CHV_CMN_DW1_CH1)
> +
> +#define CHV_CMN_DW30                 0x8178
> +#define   DPIO_LRC_BYPASS            (1 << 3)
> +
> +#define _TXLANE(ch, lane, offset) ((ch ? 0x2400 : 0) + \
> +                                     (lane) * 0x200 + (offset))

This works too, but according to the PHY spec the base addresses would
be more precisely written as
((ch ? 0x2480 : 0x80) + (lane) * 0x200 + (offset))

... and then

> +
> +#define CHV_TX_DW11(ch, lane) _TXLANE(ch, lane, 0xac)

_TXLANE(ch, lane, 0x2c)

> +#define   DPIO_FRC_LATENCY_SHFIT     (8)
> +#define CHV_TX_DW14(ch, lane) _TXLANE(ch, lane, 0xb8)

_TXLANE(ch, lane, 0x38)

--Imre

> +#define   DPIO_UPAR_SHIFT            (30)

>  /*
>   * Fence registers
>   */
> @@ -1383,6 +1452,7 @@ enum punit_power_well {
>  #define   DPLL_LOCK_VLV                      (1<<15)
>  #define   DPLL_INTEGRATED_CRI_CLK_VLV        (1<<14)
>  #define   DPLL_INTEGRATED_CLOCK_VLV  (1<<13)
> +#define   DPLL_SSC_REF_CLOCK_CHV     (1<<13)
>  #define   DPLL_PORTC_READY_MASK              (0xf << 4)
>  #define   DPLL_PORTB_READY_MASK              (0xf)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index d73fec5..36d6e212 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1549,6 +1549,49 @@ static void vlv_enable_pll(struct intel_crtc *crtc)
>       udelay(150); /* wait for warmup */
>  }
>  
> +static void chv_enable_pll(struct intel_crtc *crtc)
> +{
> +     struct drm_device *dev = crtc->base.dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     int pipe = crtc->pipe;
> +     enum dpio_channel port = vlv_pipe_to_channel(pipe);
> +     int dpll = DPLL(crtc->pipe);
> +     u32 tmp;
> +
> +     assert_pipe_disabled(dev_priv, crtc->pipe);
> +
> +     BUG_ON(!IS_CHERRYVIEW(dev_priv->dev));
> +
> +     mutex_lock(&dev_priv->dpio_lock);
> +
> +     /* Enable back the 10bit clock to display controller */
> +     tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> +     tmp |= DPIO_DCLKP_EN;
> +     vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), tmp);
> +
> +     /*
> +      * Need to wait > 100ns between dclkp clock enable bit and PLL enable.
> +      */
> +     udelay(1);
> +
> +     /* Enable PLL */
> +     tmp = I915_READ(dpll);
> +     tmp |= DPLL_VCO_ENABLE;
> +     I915_WRITE(dpll, tmp);
> +
> +     /* Check PLL is locked */
> +     if (wait_for(((I915_READ(dpll) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> +             DRM_ERROR("PLL %d failed to lock\n", pipe);
> +
> +     /* Deassert soft data lane reset*/
> +     tmp = vlv_dpio_read(dev_priv, pipe, VLV_PCS_DW0(port));
> +     tmp |= (DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +     vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW0(port), tmp);
> +
> +
> +     mutex_unlock(&dev_priv->dpio_lock);
> +}
> +
>  static void i9xx_enable_pll(struct intel_crtc *crtc)
>  {
>       struct drm_device *dev = crtc->base.dev;
> @@ -4503,8 +4546,12 @@ static void valleyview_crtc_enable(struct drm_crtc 
> *crtc)
>  
>       is_dsi = intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI);
>  
> -     if (!is_dsi)
> -             vlv_enable_pll(intel_crtc);
> +     if (!is_dsi) {
> +             if (IS_CHERRYVIEW(dev))
> +                     chv_enable_pll(intel_crtc);
> +             else
> +                     vlv_enable_pll(intel_crtc);
> +     }
>  
>       for_each_encoder_on_crtc(dev, crtc, encoder)
>               if (encoder->pre_enable)
> @@ -5385,6 +5432,86 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>       mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> +static void chv_update_pll(struct intel_crtc *crtc)
> +{
> +     struct drm_device *dev = crtc->base.dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     int pipe = crtc->pipe;
> +     int dpll_reg = DPLL(crtc->pipe);
> +     enum dpio_channel port = vlv_pipe_to_channel(pipe);
> +     u32 val, loopfilter, intcoeff;
> +     u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
> +     int refclk;
> +
> +     mutex_lock(&dev_priv->dpio_lock);
> +
> +     bestn = crtc->config.dpll.n;
> +     bestm2_frac = crtc->config.dpll.m2 & 0x3fffff;
> +     bestm1 = crtc->config.dpll.m1;
> +     bestm2 = crtc->config.dpll.m2 >> 22;
> +     bestp1 = crtc->config.dpll.p1;
> +     bestp2 = crtc->config.dpll.p2;
> +
> +     /*
> +      * Enable Refclk and SSC
> +      */
> +     val = I915_READ(dpll_reg);
> +     val |= (DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV);
> +     I915_WRITE(dpll_reg, val);
> +
> +     /* Propagate soft reset to data lane reset */
> +     val = vlv_dpio_read(dev_priv, pipe, VLV_PCS_DW0(port));
> +     val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +     vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW0(port), val);
> +
> +     /* Disable 10bit clock to display controller */
> +     val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
> +     val &= ~DPIO_DCLKP_EN;
> +     vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val);
> +
> +     /* p1 and p2 divider */
> +     vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW13(port),
> +                     5 << DPIO_CHV_S1_DIV_SELECT |
> +                     bestp1 << DPIO_CHV_P1_SHIFT |
> +                     bestp2 << DPIO_CHV_P2_SHIFT |
> +                     1 << DPIO_CHV_K_DIV_SHIFT);
> +
> +     /* Feedback post-divider - m2 */
> +     vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW0(port), bestm2);
> +
> +     /* Feedback refclk divider - n and m1 */
> +     vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),
> +                     DPIO_CHV_M1_DIV_BY_2 |
> +                     1 << DPIO_CHV_N_DIV_SHIFT);
> +
> +     /* M2 fraction division */
> +     vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
> +
> +     /* M2 fraction division enable */
> +     vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port),
> +                     DPIO_CHV_FRAC_DIV_EN |
> +                     DPIO_CHV_FEEDFWD_GAIN);
> +
> +     /* Loop filter */
> +     refclk = i9xx_get_refclk(&crtc->base, 0);
> +     loopfilter = DPIO_CHV_PROP_COEFF | DPIO_CHV_GAIN_CTRL;
> +     if (refclk == 100000)
> +             intcoeff = 11;
> +     else if (refclk == 38400)
> +             intcoeff = 10;
> +     else
> +             intcoeff = 9;
> +     loopfilter |= (intcoeff << DPIO_CHV_INT_COEFF_SHIFT);
> +     vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW6(port), loopfilter);
> +
> +     /* AFC Recal */
> +     vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port),
> +                     vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port)) |
> +                     DPIO_AFC_RECAL);
> +
> +     mutex_unlock(&dev_priv->dpio_lock);
> +}
> +
>  static void i9xx_update_pll(struct intel_crtc *crtc,
>                           intel_clock_t *reduced_clock,
>                           int num_connectors)
> @@ -5768,6 +5895,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>               i8xx_update_pll(intel_crtc,
>                               has_reduced_clock ? &reduced_clock : NULL,
>                               num_connectors);
> +     } else if (IS_CHERRYVIEW(dev)) {
> +             chv_update_pll(intel_crtc);
>       } else if (IS_VALLEYVIEW(dev)) {
>               vlv_update_pll(intel_crtc);
>       } else {


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

Reply via email to