Re: [PATCH 1/3] drm/i915: Use GPLL ref clock to calculate GPU freqs on VLV/CHV

Imre Deak
2016-03-16T17:18:07Z

On Fri, 2016-03-04 at 21:43 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the GPLL reference frequency from CCK and use it in the
> GPU freq<->opcode conversions on VLV/CHV. This eliminates all the
> assumptions we have about which divider is used for which czclk
> frequency.
>
> Note that unlike most clocks from CCK, the GPLL ref clock is a divided
> down version of the CZ clock rather than the HPLL clock. CZ clock itself
> is a divided down version of the HPLL clock though, so in effect it just
> gets divided down twice.
>
> While at it, throw in a few comments explaining the remaining constants
> for anyone who later wants to compare this to the spreadsheets.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks ok. The relevant info is spread across multiple documents but
based on what I gathered this is ok and the old and new way of
calculation matches, so:
Reviewed-by: Imre Deak <imre.deak@intel.com>

Some notes:
For clarity, I'd rename *_gpu_freq to *_gpu_opcode_to_freq and
*_freq_opcode to *_gpu_freq_to_opcode and mention that the CHV CU/CU2X
clocks are also known as slow/fast clock, so it better matches the VLV
code.

The entries for VLV 320/400MHz CZ clock are missing from the tables
that I saw, which is one justification for this change, since we don't
know if vlv_gpu_freq_div() was correct.

--Imre

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 19 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  drivers/gpu/drm/i915/intel_pm.c      | 74 +++++++++++++-----------------------
>  5 files changed, 44 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f37ac120a29d..95f77cd0ce17 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1166,6 +1166,7 @@ struct intel_gen6_power_mgmt {
>   u8 efficient_freq; /* AKA RPe. Pre-determined balanced frequency */
>   u8 rp1_freq; /* "less than" RP0 power/freqency */
>   u8 rp0_freq; /* Non-overclocked max frequency. */
> + u16 gpll_ref_freq; /* vlv/chv GPLL reference frequency */
>
>   u8 up_threshold; /* Current %busy required to uplock */
>   u8 down_threshold; /* Current %busy required to downclock */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc4007f3fa..aa21bfc06a24 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -785,6 +785,7 @@ enum skl_disp_power_wells {
>  #define  DSI_PLL_M1_DIV_SHIFT 0
>  #define  DSI_PLL_M1_DIV_MASK (0x1ff << 0)
>  #define CCK_CZ_CLOCK_CONTROL 0x62
> +#define CCK_GPLL_CLOCK_CONTROL 0x67
>  #define CCK_DISPLAY_CLOCK_CONTROL 0x6b
>  #define CCK_DISPLAY_REF_CLOCK_CONTROL 0x6c
>  #define  CCK_TRUNK_FORCE_ON (1 << 17)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 62d36a7b3398..ba50de534f9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -147,15 +147,12 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>   return vco_freq[hpll_freq] * 1000;
>  }
>
> -static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> -   const char *name, u32 reg)
> +int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> +       const char *name, u32 reg, int ref_freq)
>  {
>   u32 val;
>   int divider;
>
> - if (dev_priv->hpll_freq == 0)
> - dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> -
>   mutex_lock(&dev_priv->sb_lock);
>   val = vlv_cck_read(dev_priv, reg);
>   mutex_unlock(&dev_priv->sb_lock);
> @@ -166,7 +163,17 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
>        (divider << CCK_FREQUENCY_STATUS_SHIFT),
>        "%s change in progress\n", name);
>
> - return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> + return DIV_ROUND_CLOSEST(ref_freq << 1, divider + 1);
> +}
> +
> +static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> +   const char *name, u32 reg)
> +{
> + if (dev_priv->hpll_freq == 0)
> + dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> +
> + return vlv_get_cck_clock(dev_priv, name, reg,
> +  dev_priv->hpll_freq);
>  }
>
>  static int
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd0b4eacbddf..c416f0adae48 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1116,6 +1116,8 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv);
>  void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
>
>  /* intel_display.c */
> +int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> +       const char *name, u32 reg, int ref_freq);
>  extern const struct drm_plane_funcs intel_plane_funcs;
>  unsigned int intel_rotation_info_size(const struct intel_rotation_info *rot_info);
>  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f65e84137060..c53b8c4d381c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5364,6 +5364,17 @@ static void valleyview_cleanup_pctx(struct drm_device *dev)
>   dev_priv->vlv_pctx = NULL;
>  }
>
> +static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
> +{
> + dev_priv->rps.gpll_ref_freq =
> + vlv_get_cck_clock(dev_priv, "GPLL ref",
> +   CCK_GPLL_CLOCK_CONTROL,
> +   dev_priv->czclk_freq);
> +
> + DRM_DEBUG_DRIVER("GPLL reference freq: %d kHz\n",
> +  dev_priv->rps.gpll_ref_freq);
> +}
> +
>  static void valleyview_init_gt_powersave(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5371,6 +5382,8 @@ static void valleyview_init_gt_powersave(struct drm_device *dev)
>
>   valleyview_setup_pctx(dev);
>
> + vlv_init_gpll_ref_freq(dev_priv);
> +
>   mutex_lock(&dev_priv->rps.hw_lock);
>
>   val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> @@ -5428,6 +5441,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>
>   cherryview_setup_pctx(dev);
>
> + vlv_init_gpll_ref_freq(dev_priv);
> +
>   mutex_lock(&dev_priv->rps.hw_lock);
>
>   mutex_lock(&dev_priv->sb_lock);
> @@ -7261,68 +7276,33 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val
>   return 0;
>  }
>
> -static int vlv_gpu_freq_div(unsigned int czclk_freq)
> -{
> - switch (czclk_freq) {
> - case 200:
> - return 10;
> - case 267:
> - return 12;
> - case 320:
> - case 333:
> - return 16;
> - case 400:
> - return 20;
> - default:
> - return -1;
> - }
> -}
> -
>  static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  {
> - int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
> -
> - div = vlv_gpu_freq_div(czclk_freq);
> - if (div < 0)
> - return div;
> -
> - return DIV_ROUND_CLOSEST(czclk_freq * (val + 6 - 0xbd), div);
> + /*
> +  * N = val - 0xb7
> +  * Slow = Fast = GPLL ref * N
> +  */
> + return DIV_ROUND_CLOSEST(dev_priv->rps.gpll_ref_freq * (val - 0xb7), 1000);
>  }
>
>  static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  {
> - int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
> -
> - mul = vlv_gpu_freq_div(czclk_freq);
> - if (mul < 0)
> - return mul;
> -
> - return DIV_ROUND_CLOSEST(mul * val, czclk_freq) + 0xbd - 6;
> + return DIV_ROUND_CLOSEST(1000 * val, dev_priv->rps.gpll_ref_freq) + 0xb7;
>  }
>
>  static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
>  {
> - int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
> -
> - div = vlv_gpu_freq_div(czclk_freq);
> - if (div < 0)
> - return div;
> - div /= 2;
> -
> - return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2;
> + /*
> +  * N = val / 2
> +  * CU = CU2x / 2 = GPLL ref * N / 2
> +  */
> + return DIV_ROUND_CLOSEST(dev_priv->rps.gpll_ref_freq * val, 2 * 2 * 1000);
>  }
>
>  static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  {
> - int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
> -
> - mul = vlv_gpu_freq_div(czclk_freq);
> - if (mul < 0)
> - return mul;
> - mul /= 2;
> -
>   /* CHV needs even values */
> - return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
> + return DIV_ROUND_CLOSEST(2 * 1000 * val, dev_priv->rps.gpll_ref_freq) * 2;
>  }
>
>  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)