On Wed, 2015-06-03 at 14:24 -0700, Rodrigo Vivi wrote:
> On Tue, May 5, 2015 at 4:30 AM, <akash.g...@intel.com> wrote:
> > From: Akash Goel <akash.g...@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.g...@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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
