On Wed, 2016-03-16 at 19:47 +0200, Ville Syrjälä wrote:
> On Wed, Mar 16, 2016 at 07:17:51PM +0200, Imre Deak wrote:
> > On Fri, 2016-03-04 at 21:43 +0200, ville.syrj...@linux.intel.com
> > wrote:
> > > From: Ville Syrjälä <ville.syrj...@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.syrj...@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.d...@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.
> 
> Well I think the slow/fast names might be more of a leftover from
> VLV,
> ie. someone just forgot to update the names in bunch of the docs for
> CHV. But adding a note might be prudent to avoid people having to
> wonder about it.
> 
> > 
> > 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.
> 
> I don't think VLV had 320 or 400 SKUs.

Ah, ok so those exist only on CHV, I missed that.

--Imre

> 
> > 
> > --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)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to