[Intel-gfx] [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

Rodrigo Vivi rodrigo.vivi at gmail.com
Wed Jul 30 20:20:49 CEST 2014


On Wed, Jul 30, 2014 at 11:09 AM, Paulo Zanoni <przanoni at gmail.com> wrote:

> 2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> > 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 <paulo.r.zanoni at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  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.
>

oh, for a moment I thought it was messy as the other code and it was
writing twice...
mainly when you mentioned that you could reproduce the unclaimed when
reading debugfs...
but probably was related to the powerwell you mentioned below...


>
>
> > 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.
>

thanks! I'll add it...
Also also need to be added to debugfs interface.


>
> I also just noticed that this function contains the "/* 4: Everything
> else */" comment twice. You could remove one :)
>

indeed.



>
>
> > +               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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140730/2f3f1e9b/attachment.html>


More information about the Intel-gfx mailing list