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

From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Chris Wilson <chris.p.wilson@intel.com>,
	Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/4] drm/i915/perf: Whitelist OA report trigger registers
Date: Fri, 24 Jul 2020 13:07:18 +0300	[thread overview]
Message-ID: <dd661523-be0f-e90d-f450-2079fb109ce0@intel.com> (raw)
In-Reply-To: <159558277976.21069.2416672457874030480@build.alporthouse.com>

On 24/07/2020 12:26, Chris Wilson wrote:
> Quoting Umesh Nerlige Ramappa (2020-07-24 01:18:59)
>> From: Piotr Maciejewski <piotr.maciejewski@intel.com>
>>
>> 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@intel.com>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx