| Message ID | 20230921143028.483008-1-shekhar.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add Wa_18028616096 | expand |
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@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 >+ 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. -- Gustavo Sousa > } > > if (IS_DG2_G11(i915)) { >-- >2.34.1 >
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@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/ 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 >>
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@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
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 */ + wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, UGM_FRAGMENT_THRESHOLD_TO_3); } if (IS_DG2_G11(i915)) {
Drop UGM per set fragment threshold to 3 BSpec: 54833 Signed-off-by: Shekhar Chauhan <shekhar.chauhan@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(+)