* [PATCH] drm/i915: fix i915_frequency_info on BDW
@ 2014-08-01 21:14 Paulo Zanoni
2014-08-15 16:50 ` Rodrigo Vivi
0 siblings, 1 reply; 5+ messages in thread
From: Paulo Zanoni @ 2014-08-01 21:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The GEN6_PM* registers don't exist on BDW anymore, so when we read
this file we trigger unclaimed register errors. The equivalent BDW
register for PMs is GEN8_GT_I*R(2), so use it.
Testcase: igt/pm_rpm/debugfs-read
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e737b7..17bd20ff 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
u32 rpstat, cagf, reqf;
u32 rpupei, rpcurup, rpprevup;
u32 rpdownei, rpcurdown, rpprevdown;
+ u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
int max_freq;
/* RPSTAT1 is in the GT power well */
@@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
mutex_unlock(&dev->struct_mutex);
+ if (IS_GEN6(dev) || IS_GEN7(dev)) {
+ pm_ier = I915_READ(GEN6_PMIER);
+ pm_imr = I915_READ(GEN6_PMIMR);
+ pm_isr = I915_READ(GEN6_PMISR);
+ pm_iir = I915_READ(GEN6_PMIIR);
+ pm_mask = I915_READ(GEN6_PMINTRMSK);
+ } else {
+ pm_ier = I915_READ(GEN8_GT_IER(2));
+ pm_imr = I915_READ(GEN8_GT_IMR(2));
+ pm_isr = I915_READ(GEN8_GT_ISR(2));
+ pm_iir = I915_READ(GEN8_GT_IIR(2));
+ pm_mask = I915_READ(GEN6_PMINTRMSK);
+ }
seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x IIR=0x%08x, MASK=0x%08x\n",
- I915_READ(GEN6_PMIER),
- I915_READ(GEN6_PMIMR),
- I915_READ(GEN6_PMISR),
- I915_READ(GEN6_PMIIR),
- I915_READ(GEN6_PMINTRMSK));
+ pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
seq_printf(m, "Render p-state ratio: %d\n",
(gt_perf_status & 0xff00) >> 8);
--
2.0.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: fix i915_frequency_info on BDW
2014-08-01 21:14 [PATCH] drm/i915: fix i915_frequency_info on BDW Paulo Zanoni
@ 2014-08-15 16:50 ` Rodrigo Vivi
2014-08-15 17:12 ` Paulo Zanoni
0 siblings, 1 reply; 5+ messages in thread
From: Rodrigo Vivi @ 2014-08-15 16:50 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
[-- Attachment #1.1: Type: text/plain, Size: 3044 bytes --]
On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> The GEN6_PM* registers don't exist on BDW anymore, so when we read
> this file we trigger unclaimed register errors. The equivalent BDW
> register for PMs is GEN8_GT_I*R(2), so use it.
>
> Testcase: igt/pm_rpm/debugfs-read
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e737b7..17bd20ff 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
> void *unused)
> u32 rpstat, cagf, reqf;
> u32 rpupei, rpcurup, rpprevup;
> u32 rpdownei, rpcurdown, rpprevdown;
> + u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
> int max_freq;
>
> /* RPSTAT1 is in the GT power well */
> @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file *m,
> void *unused)
> gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> mutex_unlock(&dev->struct_mutex);
>
> + if (IS_GEN6(dev) || IS_GEN7(dev)) {
> + pm_ier = I915_READ(GEN6_PMIER);
> + pm_imr = I915_READ(GEN6_PMIMR);
> + pm_isr = I915_READ(GEN6_PMISR);
> + pm_iir = I915_READ(GEN6_PMIIR);
> + pm_mask = I915_READ(GEN6_PMINTRMSK);
> + } else {
> + pm_ier = I915_READ(GEN8_GT_IER(2));
> + pm_imr = I915_READ(GEN8_GT_IMR(2));
> + pm_isr = I915_READ(GEN8_GT_ISR(2));
> + pm_iir = I915_READ(GEN8_GT_IIR(2));
>
Why do we care only about GT(2) interrupt reg?
What about other 0, 1 and 3 regs?
Could this explain GT3 failures?
> + pm_mask = I915_READ(GEN6_PMINTRMSK);
> + }
> seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x
> IIR=0x%08x, MASK=0x%08x\n",
> - I915_READ(GEN6_PMIER),
> - I915_READ(GEN6_PMIMR),
> - I915_READ(GEN6_PMISR),
> - I915_READ(GEN6_PMIIR),
> - I915_READ(GEN6_PMINTRMSK));
> + pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
> seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
> seq_printf(m, "Render p-state ratio: %d\n",
> (gt_perf_status & 0xff00) >> 8);
> --
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
[-- Attachment #1.2: Type: text/html, Size: 4500 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: fix i915_frequency_info on BDW
2014-08-15 16:50 ` Rodrigo Vivi
@ 2014-08-15 17:12 ` Paulo Zanoni
2014-08-18 18:11 ` Rodrigo Vivi
0 siblings, 1 reply; 5+ messages in thread
From: Paulo Zanoni @ 2014-08-15 17:12 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
2014-08-15 13:50 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>
>
>
> On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>>
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> The GEN6_PM* registers don't exist on BDW anymore, so when we read
>> this file we trigger unclaimed register errors. The equivalent BDW
>> register for PMs is GEN8_GT_I*R(2), so use it.
>>
>> Testcase: igt/pm_rpm/debugfs-read
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 9e737b7..17bd20ff 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
>> void *unused)
>> u32 rpstat, cagf, reqf;
>> u32 rpupei, rpcurup, rpprevup;
>> u32 rpdownei, rpcurdown, rpprevdown;
>> + u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
>> int max_freq;
>>
>> /* RPSTAT1 is in the GT power well */
>> @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file *m,
>> void *unused)
>> gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
>> mutex_unlock(&dev->struct_mutex);
>>
>> + if (IS_GEN6(dev) || IS_GEN7(dev)) {
>> + pm_ier = I915_READ(GEN6_PMIER);
>> + pm_imr = I915_READ(GEN6_PMIMR);
>> + pm_isr = I915_READ(GEN6_PMISR);
>> + pm_iir = I915_READ(GEN6_PMIIR);
>> + pm_mask = I915_READ(GEN6_PMINTRMSK);
>> + } else {
>> + pm_ier = I915_READ(GEN8_GT_IER(2));
>> + pm_imr = I915_READ(GEN8_GT_IMR(2));
>> + pm_isr = I915_READ(GEN8_GT_ISR(2));
>> + pm_iir = I915_READ(GEN8_GT_IIR(2));
>
>
> Why do we care only about GT(2) interrupt reg?
> What about other 0, 1 and 3 regs?
Because, as far as I could see, the GEN8_GT(2) register is the one
that seems to be equivalent to the old GEN6_PM register in terms of
the functionality debugged by this function: it is the one that
contains all the RPS stuff. Another thing that influenced me to take
this decision was that, for example, snb_update_pm_irq() touches
GEN6_PM, while bdw_update_pm_irq() touches only GEN8_GT(2). But I'm
not a great user of this code, so maybe we do want more interrupts.
OTOH, if we want all, there's always i915_interrupt_info.
>
> Could this explain GT3 failures?
Which GT3 failures? I don't understand why you ask this.
>
>>
>> + pm_mask = I915_READ(GEN6_PMINTRMSK);
>> + }
>> seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x
>> IIR=0x%08x, MASK=0x%08x\n",
>> - I915_READ(GEN6_PMIER),
>> - I915_READ(GEN6_PMIMR),
>> - I915_READ(GEN6_PMISR),
>> - I915_READ(GEN6_PMIIR),
>> - I915_READ(GEN6_PMINTRMSK));
>> + pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
>> seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
>> seq_printf(m, "Render p-state ratio: %d\n",
>> (gt_perf_status & 0xff00) >> 8);
>> --
>> 2.0.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
>
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: fix i915_frequency_info on BDW
2014-08-15 17:12 ` Paulo Zanoni
@ 2014-08-18 18:11 ` Rodrigo Vivi
2014-08-25 21:03 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Rodrigo Vivi @ 2014-08-18 18:11 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
[-- Attachment #1.1: Type: text/plain, Size: 4276 bytes --]
On Fri, Aug 15, 2014 at 10:12 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2014-08-15 13:50 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> >
> >
> >
> > On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> >>
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> The GEN6_PM* registers don't exist on BDW anymore, so when we read
> >> this file we trigger unclaimed register errors. The equivalent BDW
> >> register for PMs is GEN8_GT_I*R(2), so use it.
> >>
> >> Testcase: igt/pm_rpm/debugfs-read
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_debugfs.c | 20 +++++++++++++++-----
> >> 1 file changed, 15 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> >> b/drivers/gpu/drm/i915/i915_debugfs.c
> >> index 9e737b7..17bd20ff 100644
> >> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >> @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
> >> void *unused)
> >> u32 rpstat, cagf, reqf;
> >> u32 rpupei, rpcurup, rpprevup;
> >> u32 rpdownei, rpcurdown, rpprevdown;
> >> + u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
> >> int max_freq;
> >>
> >> /* RPSTAT1 is in the GT power well */
> >> @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file
> *m,
> >> void *unused)
> >> gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> >> mutex_unlock(&dev->struct_mutex);
> >>
> >> + if (IS_GEN6(dev) || IS_GEN7(dev)) {
> >> + pm_ier = I915_READ(GEN6_PMIER);
> >> + pm_imr = I915_READ(GEN6_PMIMR);
> >> + pm_isr = I915_READ(GEN6_PMISR);
> >> + pm_iir = I915_READ(GEN6_PMIIR);
> >> + pm_mask = I915_READ(GEN6_PMINTRMSK);
> >> + } else {
> >> + pm_ier = I915_READ(GEN8_GT_IER(2));
> >> + pm_imr = I915_READ(GEN8_GT_IMR(2));
> >> + pm_isr = I915_READ(GEN8_GT_ISR(2));
> >> + pm_iir = I915_READ(GEN8_GT_IIR(2));
> >
> >
> > Why do we care only about GT(2) interrupt reg?
> > What about other 0, 1 and 3 regs?
>
> Because, as far as I could see, the GEN8_GT(2) register is the one
> that seems to be equivalent to the old GEN6_PM register in terms of
> the functionality debugged by this function: it is the one that
> contains all the RPS stuff. Another thing that influenced me to take
> this decision was that, for example, snb_update_pm_irq() touches
> GEN6_PM, while bdw_update_pm_irq() touches only GEN8_GT(2). But I'm
> not a great user of this code, so maybe we do want more interrupts.
> OTOH, if we want all, there's always i915_interrupt_info.
>
Yeah, makes sense.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> >
> > Could this explain GT3 failures?
>
> Which GT3 failures? I don't understand why you ask this.
>
forget about the gt3 on this, and thanks for the ideas!
>
> >
> >>
> >> + pm_mask = I915_READ(GEN6_PMINTRMSK);
> >> + }
> >> seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x
> >> IIR=0x%08x, MASK=0x%08x\n",
> >> - I915_READ(GEN6_PMIER),
> >> - I915_READ(GEN6_PMIMR),
> >> - I915_READ(GEN6_PMISR),
> >> - I915_READ(GEN6_PMIIR),
> >> - I915_READ(GEN6_PMINTRMSK));
> >> + pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
> >> seq_printf(m, "GT_PERF_STATUS: 0x%08x\n",
> gt_perf_status);
> >> seq_printf(m, "Render p-state ratio: %d\n",
> >> (gt_perf_status & 0xff00) >> 8);
> >> --
> >> 2.0.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
> >
> > --
> > Rodrigo Vivi
> > Blog: http://blog.vivi.eng.br
> >
>
>
>
> --
> Paulo Zanoni
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
[-- Attachment #1.2: Type: text/html, Size: 6775 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: fix i915_frequency_info on BDW
2014-08-18 18:11 ` Rodrigo Vivi
@ 2014-08-25 21:03 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2014-08-25 21:03 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Mon, Aug 18, 2014 at 11:11:50AM -0700, Rodrigo Vivi wrote:
> On Fri, Aug 15, 2014 at 10:12 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
>
> > 2014-08-15 13:50 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > >
> > >
> > >
> > > On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > >>
> > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >>
> > >> The GEN6_PM* registers don't exist on BDW anymore, so when we read
> > >> this file we trigger unclaimed register errors. The equivalent BDW
> > >> register for PMs is GEN8_GT_I*R(2), so use it.
> > >>
> > >> Testcase: igt/pm_rpm/debugfs-read
> > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >> ---
> > >> drivers/gpu/drm/i915/i915_debugfs.c | 20 +++++++++++++++-----
> > >> 1 file changed, 15 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > >> b/drivers/gpu/drm/i915/i915_debugfs.c
> > >> index 9e737b7..17bd20ff 100644
> > >> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > >> @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
> > >> void *unused)
> > >> u32 rpstat, cagf, reqf;
> > >> u32 rpupei, rpcurup, rpprevup;
> > >> u32 rpdownei, rpcurdown, rpprevdown;
> > >> + u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
> > >> int max_freq;
> > >>
> > >> /* RPSTAT1 is in the GT power well */
> > >> @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file
> > *m,
> > >> void *unused)
> > >> gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> > >> mutex_unlock(&dev->struct_mutex);
> > >>
> > >> + if (IS_GEN6(dev) || IS_GEN7(dev)) {
> > >> + pm_ier = I915_READ(GEN6_PMIER);
> > >> + pm_imr = I915_READ(GEN6_PMIMR);
> > >> + pm_isr = I915_READ(GEN6_PMISR);
> > >> + pm_iir = I915_READ(GEN6_PMIIR);
> > >> + pm_mask = I915_READ(GEN6_PMINTRMSK);
> > >> + } else {
> > >> + pm_ier = I915_READ(GEN8_GT_IER(2));
> > >> + pm_imr = I915_READ(GEN8_GT_IMR(2));
> > >> + pm_isr = I915_READ(GEN8_GT_ISR(2));
> > >> + pm_iir = I915_READ(GEN8_GT_IIR(2));
> > >
> > >
> > > Why do we care only about GT(2) interrupt reg?
> > > What about other 0, 1 and 3 regs?
> >
> > Because, as far as I could see, the GEN8_GT(2) register is the one
> > that seems to be equivalent to the old GEN6_PM register in terms of
> > the functionality debugged by this function: it is the one that
> > contains all the RPS stuff. Another thing that influenced me to take
> > this decision was that, for example, snb_update_pm_irq() touches
> > GEN6_PM, while bdw_update_pm_irq() touches only GEN8_GT(2). But I'm
> > not a great user of this code, so maybe we do want more interrupts.
> > OTOH, if we want all, there's always i915_interrupt_info.
> >
>
> Yeah, makes sense.
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-25 21:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-01 21:14 [PATCH] drm/i915: fix i915_frequency_info on BDW Paulo Zanoni
2014-08-15 16:50 ` Rodrigo Vivi
2014-08-15 17:12 ` Paulo Zanoni
2014-08-18 18:11 ` Rodrigo Vivi
2014-08-25 21:03 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox