Re: [PATCH 3/3] drm/i915: Cache HPLL frequency on VLV/CHV

Imre Deak
2014-11-06T14:03:36Z

On Tue, 2014-10-07 at 17:41 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We need the HPLL frequency when calculating cdclk. Currently we read
> that out from the hardware every single time, which isn't going to fly
> very well if the device is runtime suspended. So cache the HPLL
> frequency in dev_priv and use the cached value.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

You could add:
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=82939

where this fixes some subtest failures on VLV.
On 1/3 and 3/3 of this patchset:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1e476b5..68a3509 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1545,6 +1545,7 @@ struct drm_i915_private {
>
> unsigned int fsb_freq, mem_freq, is_ddr3;
> unsigned int vlv_cdclk_freq;
> + unsigned int hpll_freq;
>
> /**
> * wq - Driver workqueue for GEM.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 149310b..6fbae00 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4660,10 +4660,9 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> if (cdclk == 400000) {
> - u32 divider, vco;
> + u32 divider;
>
> - vco = valleyview_get_vco(dev_priv);
> - divider = DIV_ROUND_CLOSEST(vco << 1, cdclk) - 1;
> + divider = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
>
> mutex_lock(&dev_priv->dpio_lock);
> /* adjust cdclk divider */
> @@ -4742,8 +4741,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
> static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> int max_pixclk)
> {
> - int vco = valleyview_get_vco(dev_priv);
> - int freq_320 = (vco << 1) % 320000 != 0 ? 333333 : 320000;
> + int freq_320 = (dev_priv->hpll_freq << 1) % 320000 != 0 ? 333333 : 320000;
>
> /* FIXME: Punit isn't quite ready yet */
> if (IS_CHERRYVIEW(dev_priv->dev))
> @@ -5452,7 +5450,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> static int valleyview_get_display_clock_speed(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int vco = valleyview_get_vco(dev_priv);
> u32 val;
> int divider;
>
> @@ -5460,6 +5457,9 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
> if (IS_CHERRYVIEW(dev))
> return 400000;
>
> + if (dev_priv->hpll_freq == 0)
> + dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> +
> mutex_lock(&dev_priv->dpio_lock);
> val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> mutex_unlock(&dev_priv->dpio_lock);
> @@ -5470,7 +5470,7 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
> (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> "cdclk change in progress\n");
>
> - return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> + return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> }
>
> static int i945_get_display_clock_speed(struct drm_device *dev)