[Intel-gfx] [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.

[Bob Paauwe]

On Thu, 12 Nov 2015 10:35:00 +0200
Imre Deak <imre.deak at intel.com> wrote:

> On Wed, 2015-11-11 at 13:36 -0800, Bob Paauwe wrote:
> > On Tue, 10 Nov 2015 11:04:22 +0200
> > Mika Kuoppala <mika.kuoppala at linux.intel.com> wrote:
> > 
> > > Bob Paauwe <bob.j.paauwe at intel.com> writes:
> > > 
> > > > Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92768
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index 647c0ff..fc5097f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -7138,7 +7138,7 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> > > >  
> > > >  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)
> > > >  {
> > > > -	if (IS_GEN9(dev_priv->dev))
> > > > +	if (IS_GEN9(dev_priv->dev) && !IS_BROXTON(dev_priv->dev))
> > > >  		return (val * GT_FREQUENCY_MULTIPLIER) /
> > > >  GEN9_FREQ_SCALER;
> > > 
> > > Documentation disagrees with this patch. The units are 16.67Mhz
> > > thus we should use 50/3.
> > > 
> > > -Mika
> > 
> > I'm not sure I trust the documentation in this case.  Elsewhere, in
> > gen6_init_rps_frequencies() we use GEN9_FREQ_SCALER for SKL only, not for BXT.
> 
> On SKL the frequency _capability_ register uses 50MHz units, but the
> frequency _request_ register uses 16.67MHz units. We store the frequency
> limits in 16.67MHz units in rps.rp*_freq, hence the use of
> GEN9_FREQ_SCALER in gen6_init_rps_frequencies(). When requesting a
> frequency we only use GEN9_FREQ_SCALER to get the 16.67MHz constant
> value.
> 
> On BXT both the capability and request registers use 16.67MHz units, so
> we don't need to convert in gen6_init_rps_frequencies(), but we still
> use GEN9_FREQ_SCALER when requesting the frequency to get the 16.67MHz
> constant value.
> 
> Confusing like hell, but it works.

That makes it a bit less confusing, thanks!   Is this all documented
somewhere?  I'd like to read up on it so if you have any pointers
please send me an email.

> 
> > I tried changing gen6_init_rps_frequencies() to use it for both SKL and
> > BXT but that didn't resolve the problem when running the ps_rps igt
> > test. So if we really should be using the 50/3 scale for BXT, there's
> > another bug somewhere.
> 
> The problem is that atm we can't set the min/max frequencies via sysfs
> to the same value that we read out from these same files, due to the
> rounding down we do in the driver. Using round-to-closest is one way to
> fix it I posted a patch to do this to the bug report.

I saw your patch there, I'll give that a try. 

> 
> > > >  	else if (IS_CHERRYVIEW(dev_priv->dev))
> > > >  		return chv_gpu_freq(dev_priv, val);
> > > > @@ -7150,7 +7150,7 @@ int intel_gpu_freq(struct drm_i915_private *dev_priv, int val)
> > > >  
> > > >  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
> > > >  {
> > > > -	if (IS_GEN9(dev_priv->dev))
> > > > +	if (IS_GEN9(dev_priv->dev) && !IS_BROXTON(dev_priv->dev))
> > > >  		return (val * GEN9_FREQ_SCALER) / GT_FREQUENCY_MULTIPLIER;
> > > >  	else if (IS_CHERRYVIEW(dev_priv->dev))
> > > >  		return chv_freq_opcode(dev_priv, val);
> > > > -- 
> > > > 2.4.3
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> 
> 



-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193