Re: [Intel-gfx] [PATCH] drm/i915/dg2: Add performance workaround 18019455067
On Wed, Jun 22, 2022 at 09:38:36PM +0300, Lionel Landwerlin wrote: This is the recommended value for optimal performance. Signed-off-by: Lionel Landwerlin --- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++ drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 07ef111947b8c..a50b5790e434e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1112,6 +1112,9 @@ #define GEN12_PUSH_CONST_DEREF_HOLD_DIS REG_BIT(8) #define RT_CTRL _MMIO(0xe530) +#define NUMBER_OF_STACKIDS_512 (2 << 5) +#define NUMBER_OF_STACKIDS_1024 (1 << 5) +#define NUMBER_OF_STACKIDS_2048 (0 << 5) Should be using REG_BIT / REG_FIELD_PREP #define DIS_NULL_QUERYREG_BIT(10) #define EU_PERF_CNTL1 _MMIO(0xe558) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 3213c593a55f4..a8a389d36986c 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -2106,6 +2106,9 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) * performance guide section. */ wa_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS); + +/* Wa_18019455067:dg2 / BSpec 68331/54402 */ +wa_write_or(wal, RT_CTRL, NUMBER_OF_STACKIDS_512); this is not a workaround, it's a tuning value. See functions with "tuning" in the name. Do not keep bspec reference in comment. If at all, this should be in the commit message. This is also not setting the correct value if it was previously programmed with NUMBER_OF_STACKIDS_1024 since it will just OR the bits. Use wa_write_clr_set(). Lucas De Marchi
Re: [Intel-gfx] [PATCH] drm/i915/dg2: Add performance workaround 18019455067
On Wed, Jun 22, 2022 at 09:38:36PM +0300, Lionel Landwerlin wrote:
> This is the recommended value for optimal performance.
>
> Signed-off-by: Lionel Landwerlin
> ---
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 07ef111947b8c..a50b5790e434e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1112,6 +1112,9 @@
> #define GEN12_PUSH_CONST_DEREF_HOLD_DISREG_BIT(8)
>
> #define RT_CTRL _MMIO(0xe530)
> +#define NUMBER_OF_STACKIDS_512 (2 << 5)
> +#define NUMBER_OF_STACKIDS_1024(1 << 5)
> +#define NUMBER_OF_STACKIDS_2048(0 << 5)
Preferred notation these days would be to use REG_* macros. I.e.,
#define NUMBER_OF_STACKIDSREG_GENMASK(6, 5)
#define NUMBER_OF_STACKIDS_512REG_FIELD_PREP(NUMBER_OF_STACKIDS, 0x2)
It's also probably not worth defining the other values that we're not
using. If we wind up needing one of them on a future platform, we'll
want to double check at that point anyway to make sure the meaning
hasn't changed.
> #define DIS_NULL_QUERY REG_BIT(10)
>
> #define EU_PERF_CNTL1_MMIO(0xe558)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 3213c593a55f4..a8a389d36986c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -2106,6 +2106,9 @@ rcs_engine_wa_init(struct intel_engine_cs *engine,
> struct i915_wa_list *wal)
>* performance guide section.
>*/
> wa_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
> +
> +/* Wa_18019455067:dg2 / BSpec 68331/54402 */
Generally we just stick the bspec page numbers in the commit message
above the Signed-off-by line and don't put them in the code itself.
> +wa_write_or(wal, RT_CTRL, NUMBER_OF_STACKIDS_512);
This will bit-wise OR the STACKIDS_512 into register's existing value.
Since the hardware default for the field is 0 that would probably work
out okay in this case, but in general when we need to change the value
of a multi-bit field we want to define the workaround in a way that will
clear all bits of the field before OR'ing in the new value so that you
don't wind up with any leftover garbage. You can do that with
wa_write_clr_set(wal, RT_CTRL, NUMBER_OF_STACKIDS,
NUMBER_OF_STACKIDS_512);
Looks like there might be some whitespace issues here too (spaces
where we should have tabs according to the kernel coding style).
Matt
> }
>
> if (IS_DG2_GRAPHICS_STEP(i915, G11, STEP_A0, STEP_B0)) {
> --
> 2.32.0
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
[Intel-gfx] [PATCH] drm/i915/dg2: Add performance workaround 18019455067
This is the recommended value for optimal performance.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 07ef111947b8c..a50b5790e434e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1112,6 +1112,9 @@
#define GEN12_PUSH_CONST_DEREF_HOLD_DIS REG_BIT(8)
#define RT_CTRL_MMIO(0xe530)
+#define NUMBER_OF_STACKIDS_512 (2 << 5)
+#define NUMBER_OF_STACKIDS_1024 (1 << 5)
+#define NUMBER_OF_STACKIDS_2048 (0 << 5)
#define DIS_NULL_QUERY REG_BIT(10)
#define EU_PERF_CNTL1 _MMIO(0xe558)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 3213c593a55f4..a8a389d36986c 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2106,6 +2106,9 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct
i915_wa_list *wal)
* performance guide section.
*/
wa_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
+
+/* Wa_18019455067:dg2 / BSpec 68331/54402 */
+wa_write_or(wal, RT_CTRL, NUMBER_OF_STACKIDS_512);
}
if (IS_DG2_GRAPHICS_STEP(i915, G11, STEP_A0, STEP_B0)) {
--
2.32.0
