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@xxxxxxxxx> >>> --- >>> 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@xxxxxxxxx >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@xxxxxxxxx/ 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
