* [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