Re: [PATCH 1/4] drm/i915: Support flags in whitlist WAs

[John Harrison <John.C.Harrison@Intel.com>]

On 6/13/2019 23:48, Tvrtko Ursulin wrote:
>
> On 14/06/2019 01:28, Robert M. Fosha wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Newer hardware adds flags to the whitelist work-around register. These
>> allow per access direction privileges and ranges.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> Signed-off-by: Robert M. Fosha <robert.m.fosha@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 ++++++++-
>>   drivers/gpu/drm/i915/i915_reg.h             | 7 +++++++
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
>> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> index 165b0a45e009..ae82340fff45 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> @@ -1012,7 +1012,7 @@ bool intel_gt_verify_workarounds(struct 
>> drm_i915_private *i915,
>>   }
>>     static void
>> -whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
>> +whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
>>   {
>>       struct i915_wa wa = {
>>           .reg = reg
>> @@ -1021,9 +1021,16 @@ whitelist_reg(struct i915_wa_list *wal, 
>> i915_reg_t reg)
>>       if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
>>           return;
>>   +    wa.reg.reg |= flags;
>>       _wa_add(wal, &wa);
>>   }
>>   +static void
>> +whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
>> +{
>> +    whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_RW);
>> +}
>> +
>>   static void gen9_whitelist_build(struct i915_wa_list *w)
>>   {
>>       /* 
>> WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index edf9f93934a1..10fea5ab3fc3 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2513,6 +2513,13 @@ enum i915_power_well_id {
>>   #define   RING_WAIT_SEMAPHORE    (1 << 10) /* gen6+ */
>>     #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + 
>> (i) * 4)
>> +#define   RING_FORCE_TO_NONPRIV_RW        (0 << 28)    /* CFL+ & 
>> Gen11+ */
>
> Shouldn't this comment be against the RD and WR flags, while the above 
> is the legacy one?

Technically, this field does not exist at all on older hardware. However 
these bits are MBZ hence are equivalent to having the value of RW being 
set. To me, it makes more sense to be accurate and label the entire 
field as newer hardware only.

John.

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