* [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
@ 2015-11-09 23:28 Bob Paauwe
2015-11-10 9:04 ` Mika Kuoppala
0 siblings, 1 reply; 6+ messages in thread
From: Bob Paauwe @ 2015-11-09 23:28 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Bob Paauwe <bob.j.paauwe@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;
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
2015-11-09 23:28 [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies Bob Paauwe
@ 2015-11-10 9:04 ` Mika Kuoppala
2015-11-11 21:36 ` Bob Paauwe
0 siblings, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2015-11-10 9:04 UTC (permalink / raw)
To: Bob Paauwe, intel-gfx
Bob Paauwe <bob.j.paauwe@intel.com> writes:
> Signed-off-by: Bob Paauwe <bob.j.paauwe@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
> 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@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
2015-11-10 9:04 ` Mika Kuoppala
@ 2015-11-11 21:36 ` Bob Paauwe
2015-11-12 8:35 ` Imre Deak
0 siblings, 1 reply; 6+ messages in thread
From: Bob Paauwe @ 2015-11-11 21:36 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Tue, 10 Nov 2015 11:04:22 +0200
Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Bob Paauwe <bob.j.paauwe@intel.com> writes:
>
> > Signed-off-by: Bob Paauwe <bob.j.paauwe@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.
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.
>
> > 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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
--
Bob Paauwe
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
2015-11-11 21:36 ` Bob Paauwe
@ 2015-11-12 8:35 ` Imre Deak
2015-11-12 18:14 ` Bob Paauwe
0 siblings, 1 reply; 6+ messages in thread
From: Imre Deak @ 2015-11-12 8:35 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
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@linux.intel.com> wrote:
>
> > Bob Paauwe <bob.j.paauwe@intel.com> writes:
> >
> > > Signed-off-by: Bob Paauwe <bob.j.paauwe@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.
> 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.
> > > 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@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
2015-11-12 8:35 ` Imre Deak
@ 2015-11-12 18:14 ` Bob Paauwe
2015-11-13 15:21 ` Imre Deak
0 siblings, 1 reply; 6+ messages in thread
From: Bob Paauwe @ 2015-11-12 18:14 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Thu, 12 Nov 2015 10:35:00 +0200
Imre Deak <imre.deak@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@linux.intel.com> wrote:
> >
> > > Bob Paauwe <bob.j.paauwe@intel.com> writes:
> > >
> > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@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@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
>
>
--
--
Bob Paauwe
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies.
2015-11-12 18:14 ` Bob Paauwe
@ 2015-11-13 15:21 ` Imre Deak
0 siblings, 0 replies; 6+ messages in thread
From: Imre Deak @ 2015-11-13 15:21 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
On to, 2015-11-12 at 10:14 -0800, Bob Paauwe wrote:
> On Thu, 12 Nov 2015 10:35:00 +0200
> Imre Deak <imre.deak@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@linux.intel.com> wrote:
> > >
> > > > Bob Paauwe <bob.j.paauwe@intel.com> writes:
> > > >
> > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@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.
For SKL:
RP_STATE_CAP in configdb (offset 0x5998)
RP_FREQ_NORMAL in bspec (0xA008)
For BXT:
GRAPHICS_FREQUENCY_CAPABILITIES in configdb (offset 0x8170)
RP_FREQ_NORMAL in bspec (0xA008)
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-13 15:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09 23:28 [PATCH] drm/i915/bxt: Broxton doesn't use gen9 scaling for rps frequencies Bob Paauwe
2015-11-10 9:04 ` Mika Kuoppala
2015-11-11 21:36 ` Bob Paauwe
2015-11-12 8:35 ` Imre Deak
2015-11-12 18:14 ` Bob Paauwe
2015-11-13 15:21 ` Imre Deak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.