Re: [PATCH] drm/i915/icl: Read the correct Gen11 interrupt registers

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qui, 2018-05-17 às 10:04 -0700, Oscar Mateo Lozano escreveu:
> 
> On 5/17/2018 9:55 AM, Michel Thierry wrote:
> > On 5/16/2018 4:39 PM, Paulo Zanoni wrote:
> > > Em Qui, 2018-05-10 às 14:59 -0700, Oscar Mateo escreveu:
> > > > Stop reading some now deprecated interrupt registers in both
> > > > debugfs and error state. Instead, read the new equivalents in
> > > > the
> > > > Gen11 interrupt repartitioning scheme.
> > > > 
> > > > Note that the equivalent to the PM ISR & IIR cannot be read
> > > > without
> > > > affecting the current state of the system, so I've opted for
> > > > leaving
> > > > them out. See gen11_service_one_iir() for more info.
> > > 
> > > I can't find this function. Did you mean something else?
> > > 
> > 
> > s/gen11_service_one_iir/gen11_reset_one_iir/
> > 
> 
> Yup, that's right. Thanks Michel, I didn't realize this had been
> renamed 
> in upstream.

I fixed this in the commit message, did a small bikeshed on the single-
if-with-braces and merged the patch.

I downloaded your patch through patchwork and it changed your name
(patch author) from "Oscar Mateo" to "Oscar Mateo Lozano" (then dim
push-queued complained the patch was missing author Signed-off-by due
to the name difference, which made me notice it). You may want to
either fix your name in patchwork so it appears as "Oscar Mateo" or
change your git configuration to add the missing name.

Thanks for the patch and reviews.

Thanks,
Paulo

> 
> > > 
> > > 
> > > > 
> > > > v2: else if !!! (Paulo)
> > > > v3: another else if (Vinay)
> > > > v4:
> > > >    - Rebased
> > > >    - Renamed patch
> > > >    - Improved the ordering of GENs
> > > >    - Improved the printing of per-GEN info
> > > > v5: Avoid maybe-unitialized & add comment explaining the lack
> > > >      of PM ISR & IIR
> > > > 
> > > > Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/i915_debugfs.c   | 34
> > > > ++++++++++++++++++++++++-
> > > > ---------
> > > >   drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++++++++++-
> > > >   drivers/gpu/drm/i915/i915_gpu_error.h |  2 +-
> > > >   3 files changed, 35 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > index d663a9e0..d992dd2 100644
> > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > @@ -1170,19 +1170,28 @@ static int i915_frequency_info(struct
> > > > seq_file *m, void *unused)
> > > >             intel_uncore_forcewake_put(dev_priv,
> > > > FORCEWAKE_ALL);
> > > >   -        if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
> > > > -            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 {
> > > > +        if (INTEL_GEN(dev_priv) >= 11) {
> > > > +            pm_ier =
> > > > I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE);
> > > > +            pm_imr =
> > > > I915_READ(GEN11_GPM_WGBOXPERF_INTR_MASK);
> > > > +            /*
> > > > +             * The equivalent to the PM ISR & IIR cannot
> > > > be read
> > > > +             * without affecting the current state of
> > > > the system
> > > > +             */
> > > > +            pm_isr = 0;
> > > > +            pm_iir = 0;
> > > > +        } else if (INTEL_GEN(dev_priv) >= 8) {
> > > >               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);
> > > > +        } else {
> > > > +            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);
> > > > +
> > > >           seq_printf(m, "Video Turbo Mode: %s\n",
> > > >                  yesno(rpmodectl & GEN6_RP_MEDIA_TURBO));
> > > >           seq_printf(m, "HW control enabled: %s\n",
> > > > @@ -1190,8 +1199,13 @@ static int i915_frequency_info(struct
> > > > seq_file
> > > > *m, void *unused)
> > > >           seq_printf(m, "SW control enabled: %s\n",
> > > >                  yesno((rpmodectl &
> > > > GEN6_RP_MEDIA_MODE_MASK) ==
> > > >                     GEN6_RP_MEDIA_SW_MODE));
> > > > -        seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x
> > > > IIR=0x%08x, MASK=0x%08x\n",
> > > > -               pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
> > > > +
> > > > +        seq_printf(m, "PM IER=0x%08x IMR=0x%08x,
> > > > MASK=0x%08x\n",
> > > > +               pm_ier, pm_imr, pm_mask);
> > > > +        if (INTEL_GEN(dev_priv) < 11) {
> > > > +            seq_printf(m, "PM ISR=0x%08x IIR=0x%08x\n",
> > > > +                   pm_isr, pm_iir);
> > > > +        }
> > > >           seq_printf(m, "pm_intrmsk_mbz: 0x%08x\n",
> > > >                  rps->pm_intrmsk_mbz);
> > > >           seq_printf(m, "GT_PERF_STATUS: 0x%08x\n",
> > > > gt_perf_status);
> > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > index b98cd44..d9f2f69 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > @@ -1675,7 +1675,16 @@ static void capture_reg_state(struct
> > > > i915_gpu_state *error)
> > > >       }
> > > >         /* 4: Everything else */
> > > > -    if (INTEL_GEN(dev_priv) >= 8) {
> > > > +    if (INTEL_GEN(dev_priv) >= 11) {
> > > > +        error->ier = I915_READ(GEN8_DE_MISC_IER);
> > > > +        error->gtier[0] =
> > > > I915_READ(GEN11_RENDER_COPY_INTR_ENABLE);
> > > > +        error->gtier[1] =
> > > > I915_READ(GEN11_VCS_VECS_INTR_ENABLE);
> > > > +        error->gtier[2] =
> > > > I915_READ(GEN11_GUC_SG_INTR_ENABLE);
> > > > +        error->gtier[3] =
> > > > I915_READ(GEN11_GPM_WGBOXPERF_INTR_ENABLE);
> > > > +        error->gtier[4] =
> > > > I915_READ(GEN11_CRYPTO_RSVD_INTR_ENABLE);
> > > > +        error->gtier[5] =
> > > > I915_READ(GEN11_GUNIT_CSME_INTR_ENABLE);
> > > > +        error->ngtier = 6;
> > > > +    } else if (INTEL_GEN(dev_priv) >= 8) {
> > > >           error->ier = I915_READ(GEN8_DE_MISC_IER);
> > > >           for (i = 0; i < 4; i++)
> > > >               error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h
> > > > b/drivers/gpu/drm/i915/i915_gpu_error.h
> > > > index dac0f8c..58910f1 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> > > > @@ -58,7 +58,7 @@ struct i915_gpu_state {
> > > >       u32 eir;
> > > >       u32 pgtbl_er;
> > > >       u32 ier;
> > > > -    u32 gtier[4], ngtier;
> > > > +    u32 gtier[6], ngtier;
> > > >       u32 ccid;
> > > >       u32 derrmr;
> > > >       u32 forcewake;
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx