On Thu, 2015-09-24 at 23:29 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> As with the cdclk, read out czclk from CCK as well. This gives us the
> real current value and avoids having to decode fuses and whatnot.
>
> Also store it in kHz under dev_priv like we do for cdlck since it's not
> just an rps related clock, and having it in kHz is more
> standard/convenient for some things.
>
> Imre also pointed out that we currently fail to read czclk on VLV, which
> means the PFI credit programming isn't working as expected.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Yep, much better to have a generic way to read out all the CCK clocks,
removes a lot of special casing. It's also true that these clocks can
change, though let's note that czclk is fixed. Looks ok to me:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
Below some nitpicks about naming.
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 88 ++++++++++++++++++++++--------------
> drivers/gpu/drm/i915/intel_pm.c | 25 ++--------
> 4 files changed, 60 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2b5d587..77c9312 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1146,7 +1146,6 @@ 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. */
> - u32 cz_freq;
>
> u8 up_threshold; /* Current %busy required to uplock */
> u8 down_threshold; /* Current %busy required to downclock */
> @@ -1810,6 +1809,7 @@ struct drm_i915_private {
> unsigned int cdclk_freq, max_cdclk_freq;
> unsigned int max_dotclk_freq;
> unsigned int hpll_freq;
> + unsigned int czclk_freq;
>
> /**
> * wq - Driver workqueue for GEM.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f22fb80..83218032 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -728,6 +728,7 @@ enum skl_disp_power_wells {
> #define DSI_PLL_N1_DIV_MASK (3 << 16)
> #define DSI_PLL_M1_DIV_SHIFT 0
> #define DSI_PLL_M1_DIV_MASK (0x1ff << 0)
> +#define CCK_CZ_CLOCK_CONTROL 0x62
> #define CCK_DISPLAY_CLOCK_CONTROL 0x6b
> #define CCK_TRUNK_FORCE_ON (1 << 17)
> #define CCK_TRUNK_FORCE_OFF (1 << 16)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8567b46..d668897 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -132,6 +132,42 @@ struct intel_limit {
> intel_p2_t p2;
> };
>
> +/* returns HPLL frequency in kHz */
> +static int valleyview_get_vco(struct drm_i915_private *dev_priv)
This wasn't added in this patch, but I would call it get_cck_ref or
similar. VCO is too generic and HPLL isn't accurate, since the 800MHz
clock is a bypass clock directly from iCLK.
> +{
> + int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> +
> + /* Obtain SKU information */
> + mutex_lock(&dev_priv->sb_lock);
> + hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
> + CCK_FUSE_HPLL_FREQ_MASK;
> + mutex_unlock(&dev_priv->sb_lock);
> +
> + return vco_freq[hpll_freq] * 1000;
> +}
> +
> +static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> + const char *name, u32 reg)
I would have called it simply get_cck_clock for the above reason.
> +{
> + 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);
> +
> + divider = val & CCK_FREQUENCY_VALUES;
> +
> + WARN((val & CCK_FREQUENCY_STATUS) !=
> + (divider << CCK_FREQUENCY_STATUS_SHIFT),
> + "%s change in progress\n", name);
> +
> + return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> +}
> +
> int
> intel_pch_rawclk(struct drm_device *dev)
> {
> @@ -175,6 +211,17 @@ int intel_hrawclk(struct drm_device *dev)
> }
> }
>
> +static void intel_update_czclk(struct drm_i915_private *dev_priv)
> +{
> + if (!IS_VALLEYVIEW(dev_priv))
> + return;
> +
> + dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> + CCK_CZ_CLOCK_CONTROL);
> +
> + DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> +}
> +
> static inline u32 /* units of 100MHz */
> intel_fdi_link_freq(struct drm_device *dev)
> {
> @@ -5749,20 +5796,6 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
> DRM_ERROR("DBuf power enable timeout\n");
> }
>
> -/* returns HPLL frequency in kHz */
> -static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> -{
> - int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> -
> - /* Obtain SKU information */
> - mutex_lock(&dev_priv->sb_lock);
> - hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
> - CCK_FUSE_HPLL_FREQ_MASK;
> - mutex_unlock(&dev_priv->sb_lock);
> -
> - return vco_freq[hpll_freq] * 1000;
> -}
> -
> /* Adjust CDclk dividers to allow high res or save power if possible */
> static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> {
> @@ -5983,7 +6016,7 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> else
> default_credits = PFI_CREDIT(8);
>
> - if (DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 1000) >= dev_priv->rps.cz_freq) {
> + if (dev_priv->cdclk_freq >= dev_priv->czclk_freq) {
> /* CHV suggested value is 31 or 63 */
> if (IS_CHERRYVIEW(dev_priv))
> credits = PFI_CREDIT_63;
> @@ -6715,24 +6748,8 @@ static int haswell_get_display_clock_speed(struct drm_device *dev)
>
> static int valleyview_get_display_clock_speed(struct drm_device *dev)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - 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, CCK_DISPLAY_CLOCK_CONTROL);
> - mutex_unlock(&dev_priv->sb_lock);
> -
> - divider = val & CCK_FREQUENCY_VALUES;
> -
> - WARN((val & CCK_FREQUENCY_STATUS) !=
> - (divider << CCK_FREQUENCY_STATUS_SHIFT),
> - "cdclk change in progress\n");
> -
> - return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> + return vlv_get_cck_clock_hpll(to_i915(dev), "cdclk",
> + CCK_DISPLAY_CLOCK_CONTROL);
> }
>
> static int ilk_get_display_clock_speed(struct drm_device *dev)
> @@ -13323,8 +13340,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - intel_update_cdclk(dev);
> -
> if (HAS_DDI(dev))
> intel_ddi_pll_init(dev);
> else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> @@ -14836,6 +14851,9 @@ void intel_modeset_init(struct drm_device *dev)
> }
> }
>
> + intel_update_czclk(dev_priv);
> + intel_update_cdclk(dev);
> +
> intel_shared_dpll_init(dev);
>
> /* Just disable it once at startup */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab5ac5e..896a95c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5482,25 +5482,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> mutex_unlock(&dev_priv->sb_lock);
>
> switch ((val >> 2) & 0x7) {
> - case 0:
> - case 1:
> - dev_priv->rps.cz_freq = 200;
> - dev_priv->mem_freq = 1600;
> - break;
> - case 2:
> - dev_priv->rps.cz_freq = 267;
> - dev_priv->mem_freq = 1600;
> - break;
> case 3:
> - dev_priv->rps.cz_freq = 333;
> dev_priv->mem_freq = 2000;
> break;
> - case 4:
> - dev_priv->rps.cz_freq = 320;
> - dev_priv->mem_freq = 1600;
> - break;
> - case 5:
> - dev_priv->rps.cz_freq = 400;
> + default:
> dev_priv->mem_freq = 1600;
> break;
> }
> @@ -7327,7 +7312,7 @@ static int vlv_gpu_freq_div(unsigned int czclk_freq)
>
> static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
> {
> - int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
> + int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>
> div = vlv_gpu_freq_div(czclk_freq);
> if (div < 0)
> @@ -7338,7 +7323,7 @@ static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
>
> static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
> {
> - int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
> + int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>
> mul = vlv_gpu_freq_div(czclk_freq);
> if (mul < 0)
> @@ -7349,7 +7334,7 @@ static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
>
> static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
> {
> - int div, czclk_freq = dev_priv->rps.cz_freq;
> + int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>
> div = vlv_gpu_freq_div(czclk_freq) / 2;
> if (div < 0)
> @@ -7360,7 +7345,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
>
> static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> {
> - int mul, czclk_freq = dev_priv->rps.cz_freq;
> + int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->czclk_freq, 1000);
>
> mul = vlv_gpu_freq_div(czclk_freq) / 2;
> if (mul < 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
