From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: fix i915_frequency_info on BDW Date: Mon, 25 Aug 2014 23:03:13 +0200 Message-ID: <20140825210313.GP15520@phenom.ffwll.local> References: <1406927688-2111-1-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 2D2A36E363 for ; Mon, 25 Aug 2014 14:02:52 -0700 (PDT) Received: by mail-we0-f177.google.com with SMTP id w62so13558370wes.8 for ; Mon, 25 Aug 2014 14:02:50 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Rodrigo Vivi Cc: intel-gfx , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Mon, Aug 18, 2014 at 11:11:50AM -0700, Rodrigo Vivi wrote: > On Fri, Aug 15, 2014 at 10:12 AM, Paulo Zanoni wrote: > > > 2014-08-15 13:50 GMT-03:00 Rodrigo Vivi : > > > > > > > > > > > > On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni wrote: > > >> > > >> From: Paulo Zanoni > > >> > > >> 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 > > >> --- > > >> 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 Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch