Re: [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers

[Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>]

On 24/07/2020 12:26, Chris Wilson wrote:
> Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59)
> > From: Piotr Maciejewski <piotr.maciejewski@xxxxxxxxx>
> >
> > OA reports can be triggered into the OA buffer by writing into the
> > OAREPORTTRIG registers. Whitelist the registers to allow user to trigger
> > reports.
> >
> > v2:
> > - Move related change to this patch (Lionel)
> > - Bump up perf revision (Lionel)
> >
> > v3: Pardon whitelisted registers for selftest (Umesh)
> > v4: Document supported gens for the feature (Lionel)
> >
> > Signed-off-by: Piotr Maciejewski <piotr.maciejewski@xxxxxxxxx>
> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
> > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 26 +++++++++++++++++++
> >   .../gpu/drm/i915/gt/selftest_workarounds.c    |  8 ++++++
> >   drivers/gpu/drm/i915/i915_perf.c              | 11 +++++---
> >   3 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index cef1c122696f..a72ebfd115e5 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -1387,6 +1387,20 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
> >          whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW);
> >   }
> >   
> > +static void gen9_whitelist_build_performance_counters(struct i915_wa_list *w)
> > +{
> > +       /* OA buffer trigger report 2/6 used by performance query */
> > +       whitelist_reg(w, OAREPORTTRIG2);
> > +       whitelist_reg(w, OAREPORTTRIG6);
> The other question is: are you sure these are per-context registers?
All the registers exposed in this series are global.
> > +}
> > +
> > +static void gen12_whitelist_build_performance_counters(struct i915_wa_list *w)
> > +{
> > +       /* OA buffer trigger report 2/6 used by performance query */
> > +       whitelist_reg(w, GEN12_OAG_OAREPORTTRIG2);
> > +       whitelist_reg(w, GEN12_OAG_OAREPORTTRIG6);
> > +}
> > +
> >   static void gen9_whitelist_build(struct i915_wa_list *w)
> >   {
> >          /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
> > @@ -1400,6 +1414,9 @@ static void gen9_whitelist_build(struct i915_wa_list *w)
> >   
> >          /* WaSendPushConstantsFromMMIO:skl,bxt */
> >          whitelist_reg(w, COMMON_SLICE_CHICKEN2);
> > +
> > +       /* Performance counters support */
> > +       gen9_whitelist_build_performance_counters(w);
> >   }
> >   
> >   static void skl_whitelist_build(struct intel_engine_cs *engine)
> > @@ -1493,6 +1510,9 @@ static void cnl_whitelist_build(struct intel_engine_cs *engine)
> >   
> >          /* WaEnablePreemptionGranularityControlByUMD:cnl */
> >          whitelist_reg(w, GEN8_CS_CHICKEN1);
> > +
> > +       /* Performance counters support */
> > +       gen9_whitelist_build_performance_counters(w);
> >   }
> >   
> >   static void icl_whitelist_build(struct intel_engine_cs *engine)
> > @@ -1522,6 +1542,9 @@ static void icl_whitelist_build(struct intel_engine_cs *engine)
> >                  whitelist_reg_ext(w, PS_INVOCATION_COUNT,
> >                                    RING_FORCE_TO_NONPRIV_ACCESS_RD |
> >                                    RING_FORCE_TO_NONPRIV_RANGE_4);
> > +
> > +               /* Performance counters support */
> > +               gen9_whitelist_build_performance_counters(w);
> >                  break;
> >   
> >          case VIDEO_DECODE_CLASS:
> > @@ -1572,6 +1595,9 @@ static void tgl_whitelist_build(struct intel_engine_cs *engine)
> >   
> >                  /* Wa_1806527549:tgl */
> >                  whitelist_reg(w, HIZ_CHICKEN);
> > +
> > +               /* Performance counters support */
> > +               gen12_whitelist_build_performance_counters(w);
> >                  break;
> >          default:
> >                  whitelist_reg_ext(w,
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > index febc9e6692ba..3b1d3dbcd477 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > @@ -934,6 +934,10 @@ static bool pardon_reg(struct drm_i915_private *i915, i915_reg_t reg)
> >          static const struct regmask pardon[] = {
> >                  { GEN9_CTX_PREEMPT_REG, INTEL_GEN_MASK(9, 9) },
> >                  { GEN8_L3SQCREG4, INTEL_GEN_MASK(9, 9) },
> > +               { OAREPORTTRIG2, INTEL_GEN_MASK(8, 11) },
> > +               { OAREPORTTRIG6, INTEL_GEN_MASK(8, 11) },
> > +               { GEN12_OAG_OAREPORTTRIG2, INTEL_GEN_MASK(12, 12) },
> > +               { GEN12_OAG_OAREPORTTRIG6, INTEL_GEN_MASK(12, 12) },
> Because we are not making the mistake of exposing more globals, and the
> pardon is a list of our past sins, not an excuse for more.

I'm afraid the HW design leave us no choice on Gen12 :(


-Lionel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx