[Intel-gfx] [PATCH 2/2] drm/i915/skl: Ring frequency table programming changes

[Akash Goel]

On Wed, 2015-06-03 at 14:24 -0700, Rodrigo Vivi wrote:
> On Tue, May 5, 2015 at 4:30 AM,  <akash.goel at intel.com> wrote:
> > From: Akash Goel <akash.goel at intel.com>
> >
> > Ring frequency table programming changes for SKL. No need for a
> > floor on ring frequency, as the issue of performance impact with
> > ring running below DDR frequency, is believed to be fixed on SKL
> >
> > Issue: VIZ-5144
> > Signed-off-by: Akash Goel <akash.goel at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++-----
> >  1 file changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 421b78d..d1bdea7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4582,6 +4582,7 @@ static void __gen6_update_ring_freq(struct drm_device *dev)
> >         int min_freq = 15;
> >         unsigned int gpu_freq;
> >         unsigned int max_ia_freq, min_ring_freq;
> > +       unsigned int max_gpu_freq, min_gpu_freq;
> >         int scaling_factor = 180;
> >         struct cpufreq_policy *policy;
> >
> > @@ -4606,17 +4607,31 @@ static void __gen6_update_ring_freq(struct drm_device *dev)
> >         /* convert DDR frequency from units of 266.6MHz to bandwidth */
> >         min_ring_freq = mult_frac(min_ring_freq, 8, 3);
> >
> > +       if (IS_SKYLAKE(dev)) {
> > +               /* Convert GT frequency to 50 HZ units */
> > +               min_gpu_freq = dev_priv->rps.min_freq / GEN9_FREQ_SCALER;
> > +               max_gpu_freq = dev_priv->rps.max_freq / GEN9_FREQ_SCALER;
> 
> I got even more confused here with the scale differences... Doesn't
> look so consistent..

For Ring frequency table programming, as an input GT frequency has to be
specified in units of 50 MHZ only. For SKL natural hardware unit is
16.667 MHZ and Driver is internally storing in the same unit only, hence
an extra conversion required here.

> 
> > +       } else {
> > +               min_gpu_freq = dev_priv->rps.min_freq;
> > +               max_gpu_freq = dev_priv->rps.max_freq;
> > +       }
> > +
> >         /*
> >          * For each potential GPU frequency, load a ring frequency we'd like
> >          * to use for memory access.  We do this by specifying the IA frequency
> >          * the PCU should use as a reference to determine the ring frequency.
> >          */
> > -       for (gpu_freq = dev_priv->rps.max_freq; gpu_freq >= dev_priv->rps.min_freq;
> > -            gpu_freq--) {
> > -               int diff = dev_priv->rps.max_freq - gpu_freq;
> > +       for (gpu_freq = max_gpu_freq; gpu_freq >= min_gpu_freq; gpu_freq--) {
> > +               int diff = max_gpu_freq - gpu_freq;
> >                 unsigned int ia_freq = 0, ring_freq = 0;
> >
> > -               if (INTEL_INFO(dev)->gen >= 8) {
> > +               if (IS_SKYLAKE(dev)) {
> > +                       /*
> > +                        * ring_freq = 2 * GT. ring_freq is in 100MHz units
> > +                        * No floor required for ring frequency on SKL.
> > +                        */
> > +                       ring_freq = gpu_freq;
> 
> Aren't we using the 16.6 now? Shouldn't it be converted anyway?
> 
> > +               } else if (INTEL_INFO(dev)->gen >= 8) {
> >                         /* max(2 * GT, DDR). NB: GT is 50MHz units */
> >                         ring_freq = max(min_ring_freq, gpu_freq);
> >                 } else if (IS_HASWELL(dev)) {
> > @@ -5770,7 +5785,8 @@ static void intel_gen6_powersave_work(struct work_struct *work)
> >         } else if (INTEL_INFO(dev)->gen >= 9) {
> >                 gen9_enable_rc6(dev);
> >                 gen9_enable_rps(dev);
> > -               __gen6_update_ring_freq(dev);
> > +               if (IS_SKYLAKE(dev))
> why this if here?
> 
> If it is a bxt restriction shouldn't it be in a separated patch?

Yes for BXT, its not supported. Would move it to separate patch.

> 
> > +                       __gen6_update_ring_freq(dev);
> >         } else if (IS_BROADWELL(dev)) {
> >                 gen8_enable_rps(dev);
> >                 __gen6_update_ring_freq(dev);
> > --
> > 1.9.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
>