From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paulo Zanoni Subject: Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW. Date: Wed, 30 Jul 2014 15:09:49 -0300 Message-ID: References: <1406560776-29622-1-git-send-email-rodrigo.vivi@intel.com> <1406560776-29622-2-git-send-email-rodrigo.vivi@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ob0-f170.google.com (mail-ob0-f170.google.com [209.85.214.170]) by gabe.freedesktop.org (Postfix) with ESMTP id F23446E29A for ; Wed, 30 Jul 2014 11:09:49 -0700 (PDT) Received: by mail-ob0-f170.google.com with SMTP id wp4so802992obc.29 for ; Wed, 30 Jul 2014 11:09:49 -0700 (PDT) In-Reply-To: <1406560776-29622-2-git-send-email-rodrigo.vivi@intel.com> 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 Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org 2014-07-28 12:19 GMT-03:00 Rodrigo Vivi : > BDW has many other Display Engine interrupts and GT interrupts registers. > Collecting it properly on gpu_error_state. > > On debugfs all was properly listed already but besides we were also listing old > DEIER and GTIER that doesn't exist on BDW anymore. This was causing > unclaimed register messages: > > https://bugs.freedesktop.org/show_bug.cgi?id=81701 > > Cc: Paulo Zanoni > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 4 +++- > drivers/gpu/drm/i915/i915_gpu_error.c | 23 ++++++++++++++++++++--- > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 9e737b7..679cda6 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -783,7 +783,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data) > seq_printf(m, "Pipe %c stat: %08x\n", > pipe_name(pipe), > I915_READ(PIPESTAT(pipe))); > - } else { > + } else if (!IS_BROADWELL(dev)) { > seq_printf(m, "North Display Interrupt enable: %08x\n", > I915_READ(DEIER)); > seq_printf(m, "North Display Interrupt identity: %08x\n", This chunk is not needed since we already have a check for "gen >= 8" at the top. > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ccb97f1..ee28cd7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -314,7 +314,9 @@ struct drm_i915_error_state { > u32 eir; > u32 pgtbl_er; > u32 ier; > - u32 gtier; > + u32 gtier[4]; > + u32 deier[3]; > + u32 de_misc_ier; > u32 ccid; > u32 derrmr; > u32 forcewake; > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index 372fea3..f865d1d 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -358,9 +358,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, > err_printf(m, "Suspend count: %u\n", error->suspend_count); > err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device); > err_printf(m, "EIR: 0x%08x\n", error->eir); > - err_printf(m, "IER: 0x%08x\n", error->ier); > + if (IS_BROADWELL(dev)) { > + for_each_pipe(i) > + err_printf(m, "DEIER pipe %c: 0x%08x\n", pipe_name(i), > + error->deier[i]); > + for (i = 0; i < 4; i++) > + err_printf(m, "GTIER gt %d: 0x%08x\n", i, > + error->gtier[i]); > + err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier); > + } else { > + err_printf(m, "IER: 0x%08x\n", error->ier); > + } > if (IS_HASWELL(dev)) > - err_printf(m, "GTIER: 0x%08x\n", error->gtier); > + err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]); > err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er); > err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake); > err_printf(m, "DERRMR: 0x%08x\n", error->derrmr); > @@ -1093,6 +1103,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, > struct drm_i915_error_state *error) > { > struct drm_device *dev = dev_priv->dev; > + int i; > > /* General organization > * 1. Registers specific to a single generation > @@ -1139,7 +1150,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, > > if (IS_HASWELL(dev)) { > error->ier = I915_READ(DEIER); > - error->gtier = I915_READ(GTIER); > + error->gtier[0] = I915_READ(GTIER); > + } else if (IS_BROADWELL(dev)) { > + for_each_pipe(i) > + error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i)); for_each_pipe(i) if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_PIPE(i))) error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i)); If the pipe's power well is disabled, the IER register is powered off and we can get an unclaimed register errors too. I also just noticed that this function contains the "/* 4: Everything else */" comment twice. You could remove one :) > + for (i = 0; i < 4; i++) > + error->gtier[3] = I915_READ(GEN8_GT_IER(i)); > + error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER); > } else if (HAS_PCH_SPLIT(dev)) { > error->ier = I915_READ(DEIER) | I915_READ(GTIER); > } else if (IS_GEN2(dev)) { > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni