Re: [PATCH] drm/i915: vlv: clamp minimum RPS frequency to what Punit allows

Imre Deak
2015-12-08T19:53:05Z

On ma, 2015-12-07 at 16:40 +0200, Ville Syrjälä wrote:
> On Fri, Dec 05, 2014 at 11:23:29PM +0200, Ville Syrjälä wrote:
> > On Thu, Dec 04, 2014 at 06:39:35PM +0200, Imre Deak wrote:
> > > As described in the code comment, I couldn't set the minimum RPS
> > > frequency on my BYT-M B0 to the minimum allowed as reported by
> > > Punit.
> > > Fix this by clamping the minimum value to the first one that was
> > > accepted on my machine. This fixes at least the pm_rpm basic-api
> > > and
> > > min-max-config subtests.
> > >
> > > Testcase: igt/pm_rps
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 45c786f..7a1112f 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -5039,7 +5039,17 @@ static int valleyview_rps_rpe_freq(struct
> > > drm_i915_private *dev_priv)
> > >
> > >  static int valleyview_rps_min_freq(struct drm_i915_private
> > > *dev_priv)
> > >  {
> > > - return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) &
> > > 0xff;
> > > + u32 val;
> > > +
> > > + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) &
> > > 0xff;
> > > + /*
> > > +  * According to the BYT Punit GPU turbo HAS 1.1.6.3 the
> > > minimum value
> > > +  * for the minimum frequency in GPLL mode is 0xc1.
> > > Contrary to this on
> > > +  * a BYT-M B0 the above register contains 0xbf. Moreover
> > > when setting
> > > +  * a frequency Punit will not allow values below 0xc0.
> > > Clamp it 0xc0
> > > +  * to make sure it matches what Punit accepts.
> > > +  */
> > > + return max_t(u32, val, 0xc0);
> >
> > Matches what I see on this ffrd as well. But this too has
> > czclk==266.
> > Would be interesting to see what happens with other czclks. Anyone
> > have a byt w/ 1333 memory?
>
> I guess we never found a 333 SKU. Since the patch is at least correct
> for every VLV machine I've ever seen, I think we can go with it.
>
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, I pushed the patch to dinq. The igt/pm_rps subtest that failed
because of this issue doesn't fail any more due to a change in the
driver that caches the current frequency that was last set. But the
actual frequency is something that we also want to check eventually, so
this fix is needed for that. I updated the commit message accordingly
while merging it.

> > >  }
> > >
> > >  /* Check that the pctx buffer wasn't move under us. */
> > > --
> > > 1.8.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC