[Intel-gfx] [PATCH] drm/i915: Add Wa_18028616096

[Gustavo Sousa]

Quoting Chauhan, Shekhar (2023-09-22 01:04:36-03:00)
>Quoting Gustavo Sousa:
>
>On 9/22/2023 02:31, Gustavo Sousa wrote:
>> Quoting Shekhar Chauhan (2023-09-21 11:30:28-03:00)
>>> Drop UGM per set fragment threshold to 3
>>>
>>> BSpec: 54833
>>> Signed-off-by: Shekhar Chauhan<shekhar.chauhan at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 1 +
>>> drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
>>> 2 files changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index a00ff51c681d..431c575c532b 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -1230,6 +1230,7 @@
>>> #define   DISABLE_D8_D16_COASLESCE                REG_BIT(30)
>>> #define   FORCE_1_SUB_MESSAGE_PER_FRAGMENT        REG_BIT(15)
>>> #define LSC_CHICKEN_BIT_0_UDW                        MCR_REG(0xe7c8 + 4)
>>> +#define   UGM_FRAGMENT_THRESHOLD_TO_3                REG_BIT(58 - 32)
>>> #define   DIS_CHAIN_2XSIMD8                        REG_BIT(55 - 32)
>>> #define   FORCE_SLM_FENCE_SCOPE_TO_TILE                REG_BIT(42 - 32)
>>> #define   FORCE_UGM_FENCE_SCOPE_TO_TILE                REG_BIT(41 - 32)
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> index 660d4f358eab..992041e3776c 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> @@ -2914,6 +2914,9 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li
>>>                   * Wa_22015475538:dg2
>>>                   */
>>>                  wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, DIS_CHAIN_2XSIMD8);
>>> +
>>> +                /* Wa_18028616096:dg2 */
>> This is not a blocker, but I would prefer to remove the ":dg2" suffix.
>>
>> There was an effort to remove them from our driver[1], but it kinda of
>> stalled. I myself agree that we would be better off without them.
>>
>> [1]https://lore.kernel.org/all/20221222082557.1364711-1-lucas.demarchi@intel.com
>Ack'ed in the new version.
>>
>>> +                wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, UGM_FRAGMENT_THRESHOLD_TO_3);
>> This workaround applies to (i) DG2 G10 from stepping C0 to forever and
>> (ii) any stepping of DG2 G12. Here you are applying this workaround to
>> any variant of DG2.
>>
>> It should be moved out of this "if" statement and rather be guarded by
>> something like:
>>
>>         if ((IS_DG2_G10(i915) && IS_GRAPHICS_STEP(i915, STEP_C0, STEP_FOREVER)) ||
>>             IS_DG2_G12(i915))
>>
>> Note that we are there is still a pending decision for G11, so we may
>> need to update this in the future.
>
>I believe we're only supporting production steppings for DG2, 
>henceforth, not really interacting with the "older" steppings.
>
>Please have a look:
>
>https://lore.kernel.org/intel-gfx/20230816214201.534095-7-matthew.d.roper@intel.com/

Oh, I missed that. Thanks!

So I believe we would have the following condition instead:

	if (IS_DG2_G10(i915) || IS_DG2_G12(i915))

, because we do not know yet if this will also apply to DG2 G11.

--
Gustavo Sousa

>
>Although, I could be wrong, if I am, I'll send in another version, 
>modifying the patch as you've suggested.
>
>> --
>> Gustavo Sousa
>>
>>>          }
>>>
>>>          if (IS_DG2_G11(i915)) {
>>> -- 
>>> 2.34.1
>>>
>-- 
>-shekhar