> -----Original Message-----
> From: De Marchi, Lucas <lucas.demar...@intel.com>
> Sent: Friday, September 8, 2023 3:58 AM
> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhad...@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Roper, Matthew D
> <matthew.d.ro...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Added Wa_18022495364
>
> On Thu, Sep 07, 2023 at 12:27:34PM +0530, Dnyaneshwar Bhadane wrote:
> >This workaround has two different implementations, one for gen 12 to
> >DG2 and another for DG2 and later.
> >Bspec : 11354 , 56551.
>
> You seem to have extra spaces here. Please format it smilarly to other
> commits.
>
> >
> >Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhad...@intel.com>
> >---
> > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 4 ++++
> > drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 ++
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
> > 3 files changed, 9 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> >b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> >index 0143445dba83..fee2712f81e8 100644
> >--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> >+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> >@@ -271,6 +271,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq,
> u32 mode)
> > if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
> > bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
> >
> >+ /* Wa_18022495364 */
> >+ if ((GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70)) ||
>
> ^ why the extra parenthesis here?
>
>
> >+ IS_DG2(rq->i915))
> >+ bit_group_1 |=
> PIPE_CONTROL_CONST_CACHE_INVALIDATE;
>
> it's usually preferred to leave a newline after then if/else branches to help
> readability.
>
> > bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
> > bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
> > bit_group_1 |=
> PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> >diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> >b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> >index 0e4c638fcbbf..4c0cb3a3d0aa 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 GEN12_GLOBAL_DEBUG_ENABLE 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_workarounds.c
> >b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >index 864d41bcf6bb..1a026d4d7ac5 100644
> >--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >@@ -712,6 +712,9 @@ static void gen12_ctx_workarounds_init(struct
> intel_engine_cs *engine,
> > GEN9_PREEMPT_GPGPU_LEVEL_MASK,
> > GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
> >
> >+ /* Wa_18022495364 :tgl,rkl,dg1,adl-s,adl-p */
>
> ^ extra space here too.
>
> Actually there is no need to add the platforms names as comments.
>
>
> >+ wa_masked_en(wal, GEN12_CS_DEBUG_MODE2,
> >+ GEN12_GLOBAL_DEBUG_ENABLE);
>
> missing newline
>
Thanks Lucas, I will address all above suggestion in next revision.
>
> Lucas De Marchi
>
> > /*
> > * Wa_16011163337 - GS_TIMER
> > *
> >--
> >2.34.1
> >
