diff mbox series

[v6] drm/i915: Add Wa_18022495364

Message ID 20230914194032.1066442-1-dnyaneshwar.bhadane@intel.com (mailing list archive)
State New, archived
Headers show
Series [v6] drm/i915: Add Wa_18022495364 | expand

Commit Message

Bhadane, Dnyaneshwar Sept. 14, 2023, 7:40 p.m. UTC 
Invalidate instruction and State cache bit using INDIRECT_CTX on
every gpu context switch for gen12.
The goal of this workaround is to actually perform an explicit
invalidation of that cache (by re-writing the register) during every GPU
context switch, which is accomplished via a "workaround batchbuffer"
that's attached to the context via INDIRECT_CTX. (Matt Ropper)
BSpec: 11354

Please refer [1] for more reviews and comment on the same patch

[1] https://patchwork.freedesktop.org/patch/556154/

v2:
- Remove extra parentheses from the condition (Lucas)
- Align spacing and new line (Lucas)

v3:
- Fix commit message.

v4:
- Only GEN12 changes are kept  (Matt Ropper)
- Fix the commit message for r-b (Matt Ropper)
- Rename the register bit in define

v5:
- Move out this workaround from golden context init (Matt Roper)
- Use INDIRECT_CTX to set bit on each GPU context switch (Matt Roper)

v6:
- Change IP Version base condition for Gen12 (Matt Ropper)
- Made imperative form of commit version messages (Suraj)
- s/Added/Add in patch header (Suraj)

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>

---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h |  2 ++
 drivers/gpu/drm/i915/gt/intel_lrc.c     | 13 +++++++++++++
 2 files changed, 15 insertions(+)

Comments

Matt Atwood Sept. 14, 2023, 7:45 p.m. UTC | #1 
On Fri, Sep 15, 2023 at 01:10:32AM +0530, Dnyaneshwar Bhadane wrote:
> Invalidate instruction and State cache bit using INDIRECT_CTX on
> every gpu context switch for gen12.
> The goal of this workaround is to actually perform an explicit
> invalidation of that cache (by re-writing the register) during every GPU
> context switch, which is accomplished via a "workaround batchbuffer"
> that's attached to the context via INDIRECT_CTX. (Matt Ropper)
> BSpec: 11354
> 
> Please refer [1] for more reviews and comment on the same patch
> 
> [1] https://patchwork.freedesktop.org/patch/556154/
> 
> v2:
> - Remove extra parentheses from the condition (Lucas)
> - Align spacing and new line (Lucas)
> 
> v3:
> - Fix commit message.
> 
> v4:
> - Only GEN12 changes are kept  (Matt Ropper)
Not clear to me what this means, also its Matt Roper
> - Fix the commit message for r-b (Matt Ropper)
> - Rename the register bit in define
> 
> v5:
> - Move out this workaround from golden context init (Matt Roper)
> - Use INDIRECT_CTX to set bit on each GPU context switch (Matt Roper)
> 
> v6:
> - Change IP Version base condition for Gen12 (Matt Ropper)
Matt Roper
> - Made imperative form of commit version messages (Suraj)
> - s/Added/Add in patch header (Suraj)
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> 
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  2 ++
>  drivers/gpu/drm/i915/gt/intel_lrc.c     | 13 +++++++++++++
>  2 files changed, 15 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..0d5260d126d8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -164,6 +164,8 @@
>  #define GEN9_CSFE_CHICKEN1_RCS			_MMIO(0x20d4)
>  #define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE	(1 << 2)
>  #define   GEN11_ENABLE_32_PLANE_MODE		(1 << 7)
> +#define GEN12_CS_DEBUG_MODE2			_MMIO(0x20d8)
> +#define   INSTRUCTION_STATE_CACHE_INVALIDATE	REG_BIT(6)
>  
>  #define GEN7_FF_SLICE_CS_CHICKEN1		_MMIO(0x20e0)
>  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL		(1 << 14)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index b99efa348ad1..56c916730e9b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1333,6 +1333,15 @@ dg2_emit_draw_watermark_setting(u32 *cs)
>  	return cs;
>  }
>  
> +static u32 *
> +gen12_invalidate_state_cache(u32 *cs)
> +{
> +	*cs++ = MI_LOAD_REGISTER_IMM(1);
> +	*cs++ = i915_mmio_reg_offset(GEN12_CS_DEBUG_MODE2);
> +	*cs++ = _MASKED_BIT_ENABLE(INSTRUCTION_STATE_CACHE_INVALIDATE);
> +	return cs;
> +}
> +
>  static u32 *
>  gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>  {
> @@ -1346,6 +1355,10 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>  
>  	cs = gen12_emit_aux_table_inv(ce->engine, cs);
>  
> +	/* Wa_18022495364 */
> +	if (IS_GFX_GT_IP_RANGE(ce->engine->gt, IP_VER(12, 0), IP_VER(12, 10)))
> +		cs = gen12_invalidate_state_cache(cs);
> +
>  	/* Wa_16014892111 */
>  	if (IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) ||
>  	    IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_B0) ||
> -- 
> 2.34.1
>
Bhadane, Dnyaneshwar Sept. 14, 2023, 8:01 p.m. UTC | #2 
> -----Original Message-----
> From: Atwood, Matthew S <matthew.s.atwood@intel.com>
> Sent: Friday, September 15, 2023 1:15 AM
> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Garg, Nemesa <nemesa.garg@intel.com>; Roper,
> Matthew D <matthew.d.roper@intel.com>; Kandpal, Suraj
> <suraj.kandpal@intel.com>
> Subject: Re: [PATCH v6] drm/i915: Add Wa_18022495364
> 
> On Fri, Sep 15, 2023 at 01:10:32AM +0530, Dnyaneshwar Bhadane wrote:
> > Invalidate instruction and State cache bit using INDIRECT_CTX on every
> > gpu context switch for gen12.
> > The goal of this workaround is to actually perform an explicit
> > invalidation of that cache (by re-writing the register) during every
> > GPU context switch, which is accomplished via a "workaround batchbuffer"
> > that's attached to the context via INDIRECT_CTX. (Matt Ropper)
> > BSpec: 11354
> >
> > Please refer [1] for more reviews and comment on the same patch
> >
> > [1] https://patchwork.freedesktop.org/patch/556154/
> >
> > v2:
> > - Remove extra parentheses from the condition (Lucas)
> > - Align spacing and new line (Lucas)
> >
> > v3:
> > - Fix commit message.
> >
> > v4:
> > - Only GEN12 changes are kept  (Matt Ropper)
> Not clear to me what this means, 
Matt has corrected me by saying that this workaround should only be applied to Gen12 platforms.
Initially, the patch was combined for the Gen12 and DG+ conditions, which was an incorrect understanding.
Dnyaneshwar

> > also its Matt Roper
> > - Fix the commit message for r-b (Matt Ropper)
> > - Rename the register bit in define
> >
> > v5:
> > - Move out this workaround from golden context init (Matt Roper)
> > - Use INDIRECT_CTX to set bit on each GPU context switch (Matt Roper)
> >
> > v6:
> > - Change IP Version base condition for Gen12 (Matt Ropper)
> Matt Roper
> > - Made imperative form of commit version messages (Suraj)
> > - s/Added/Add in patch header (Suraj)
> >
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> >
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  2 ++
> >  drivers/gpu/drm/i915/gt/intel_lrc.c     | 13 +++++++++++++
> >  2 files changed, 15 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..0d5260d126d8 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -164,6 +164,8 @@
> >  #define GEN9_CSFE_CHICKEN1_RCS			_MMIO(0x20d4)
> >  #define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE	(1 << 2)
> >  #define   GEN11_ENABLE_32_PLANE_MODE		(1 << 7)
> > +#define GEN12_CS_DEBUG_MODE2			_MMIO(0x20d8)
> > +#define   INSTRUCTION_STATE_CACHE_INVALIDATE	REG_BIT(6)
> >
> >  #define GEN7_FF_SLICE_CS_CHICKEN1		_MMIO(0x20e0)
> >  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL		(1 << 14)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index b99efa348ad1..56c916730e9b 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -1333,6 +1333,15 @@ dg2_emit_draw_watermark_setting(u32 *cs)
> >  	return cs;
> >  }
> >
> > +static u32 *
> > +gen12_invalidate_state_cache(u32 *cs) {
> > +	*cs++ = MI_LOAD_REGISTER_IMM(1);
> > +	*cs++ = i915_mmio_reg_offset(GEN12_CS_DEBUG_MODE2);
> > +	*cs++ =
> _MASKED_BIT_ENABLE(INSTRUCTION_STATE_CACHE_INVALIDATE);
> > +	return cs;
> > +}
> > +
> >  static u32 *
> >  gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
> > { @@ -1346,6 +1355,10 @@ gen12_emit_indirect_ctx_rcs(const struct
> > intel_context *ce, u32 *cs)
> >
> >  	cs = gen12_emit_aux_table_inv(ce->engine, cs);
> >
> > +	/* Wa_18022495364 */
> > +	if (IS_GFX_GT_IP_RANGE(ce->engine->gt, IP_VER(12, 0), IP_VER(12, 10)))
> > +		cs = gen12_invalidate_state_cache(cs);
> > +
> >  	/* Wa_16014892111 */
> >  	if (IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0,
> STEP_B0) ||
> >  	    IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0,
> > STEP_B0) ||
> > --
> > 2.34.1
> >
Matt Roper Sept. 14, 2023, 8:09 p.m. UTC | #3 
On Fri, Sep 15, 2023 at 01:10:32AM +0530, Dnyaneshwar Bhadane wrote:
> Invalidate instruction and State cache bit using INDIRECT_CTX on
> every gpu context switch for gen12.
> The goal of this workaround is to actually perform an explicit
> invalidation of that cache (by re-writing the register) during every GPU
> context switch, which is accomplished via a "workaround batchbuffer"
> that's attached to the context via INDIRECT_CTX. (Matt Ropper)
> BSpec: 11354
> 
> Please refer [1] for more reviews and comment on the same patch
> 
> [1] https://patchwork.freedesktop.org/patch/556154/
> 
> v2:
> - Remove extra parentheses from the condition (Lucas)
> - Align spacing and new line (Lucas)
> 
> v3:
> - Fix commit message.
> 
> v4:
> - Only GEN12 changes are kept  (Matt Ropper)
> - Fix the commit message for r-b (Matt Ropper)
> - Rename the register bit in define
> 
> v5:
> - Move out this workaround from golden context init (Matt Roper)
> - Use INDIRECT_CTX to set bit on each GPU context switch (Matt Roper)
> 
> v6:
> - Change IP Version base condition for Gen12 (Matt Ropper)
> - Made imperative form of commit version messages (Suraj)
> - s/Added/Add in patch header (Suraj)
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> 
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  2 ++
>  drivers/gpu/drm/i915/gt/intel_lrc.c     | 13 +++++++++++++
>  2 files changed, 15 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..0d5260d126d8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -164,6 +164,8 @@
>  #define GEN9_CSFE_CHICKEN1_RCS			_MMIO(0x20d4)
>  #define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE	(1 << 2)
>  #define   GEN11_ENABLE_32_PLANE_MODE		(1 << 7)
> +#define GEN12_CS_DEBUG_MODE2			_MMIO(0x20d8)
> +#define   INSTRUCTION_STATE_CACHE_INVALIDATE	REG_BIT(6)
>  
>  #define GEN7_FF_SLICE_CS_CHICKEN1		_MMIO(0x20e0)
>  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL		(1 << 14)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index b99efa348ad1..56c916730e9b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1333,6 +1333,15 @@ dg2_emit_draw_watermark_setting(u32 *cs)
>  	return cs;
>  }
>  
> +static u32 *
> +gen12_invalidate_state_cache(u32 *cs)
> +{
> +	*cs++ = MI_LOAD_REGISTER_IMM(1);
> +	*cs++ = i915_mmio_reg_offset(GEN12_CS_DEBUG_MODE2);
> +	*cs++ = _MASKED_BIT_ENABLE(INSTRUCTION_STATE_CACHE_INVALIDATE);
> +	return cs;
> +}
> +
>  static u32 *
>  gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>  {
> @@ -1346,6 +1355,10 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>  
>  	cs = gen12_emit_aux_table_inv(ce->engine, cs);
>  
> +	/* Wa_18022495364 */
> +	if (IS_GFX_GT_IP_RANGE(ce->engine->gt, IP_VER(12, 0), IP_VER(12, 10)))
> +		cs = gen12_invalidate_state_cache(cs);
> +
>  	/* Wa_16014892111 */
>  	if (IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) ||
>  	    IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_B0) ||
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index a00ff51c681d..0d5260d126d8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -164,6 +164,8 @@ 
 #define GEN9_CSFE_CHICKEN1_RCS			_MMIO(0x20d4)
 #define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE	(1 << 2)
 #define   GEN11_ENABLE_32_PLANE_MODE		(1 << 7)
+#define GEN12_CS_DEBUG_MODE2			_MMIO(0x20d8)
+#define   INSTRUCTION_STATE_CACHE_INVALIDATE	REG_BIT(6)
 
 #define GEN7_FF_SLICE_CS_CHICKEN1		_MMIO(0x20e0)
 #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL		(1 << 14)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index b99efa348ad1..56c916730e9b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1333,6 +1333,15 @@  dg2_emit_draw_watermark_setting(u32 *cs)
 	return cs;
 }
 
+static u32 *
+gen12_invalidate_state_cache(u32 *cs)
+{
+	*cs++ = MI_LOAD_REGISTER_IMM(1);
+	*cs++ = i915_mmio_reg_offset(GEN12_CS_DEBUG_MODE2);
+	*cs++ = _MASKED_BIT_ENABLE(INSTRUCTION_STATE_CACHE_INVALIDATE);
+	return cs;
+}
+
 static u32 *
 gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
 {
@@ -1346,6 +1355,10 @@  gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
 
 	cs = gen12_emit_aux_table_inv(ce->engine, cs);
 
+	/* Wa_18022495364 */
+	if (IS_GFX_GT_IP_RANGE(ce->engine->gt, IP_VER(12, 0), IP_VER(12, 10)))
+		cs = gen12_invalidate_state_cache(cs);
+
 	/* Wa_16014892111 */
 	if (IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 70), STEP_A0, STEP_B0) ||
 	    IS_GFX_GT_IP_STEP(ce->engine->gt, IP_VER(12, 71), STEP_A0, STEP_B0) ||