Re: [Intel-gfx] [PATCH V3] drm/i915/dg2: add Wa_14014947963

2022-02-25 Thread Matt Roper
On Thu, Feb 17, 2022 at 03:54:09PM -0800, Matt Roper wrote:
> On Thu, Feb 10, 2022 at 09:23:33PM -0800, clinton.a.tay...@intel.com wrote:
> > From: Clint Taylor 
> > 
> > BSPEC: 46123
> > v2: Address review feedback [MattR]
> > v3: move register definition to gt_regs [MattR]
> > Cc: Matt Roper 
> > Signed-off-by: Clint Taylor 
> 
> Reviewed-by: Matt Roper 
> 
> although see below for a couple style tweaks we should make while
> applying this.
> 
> Also, I think we'll need to wait for the maintainers to do a
> backmerge/crossmerge in drm-intel-gt-next (which I think they're
> planning to do soon) before applying this since that branch is still
> missing all the recent register rework.

This is done now, so applied to drm-intel-gt-next (with the minor
whitespace and comment tweak mentioned).  Thanks for the patch.


Matt

> 
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index a6f0220c2e9f..5c8c3bc65acc 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -124,6 +124,9 @@
> >  #define   ECOCHK_PPGTT_WT_HSW  (0x2 << 3)
> >  #define   ECOCHK_PPGTT_WB_HSW  (0x3 << 3)
> >  
> > +#define VF_PREEMPTION  _MMIO(0x83a4)
> > +#define  PREEMPTION_VERTEX_COUNT   REG_GENMASK(15, 0)
> 
> While applying we'll hit some merge conflicts due to the cleanup of the
> file that just landed; while resolving the conflict, we should add an
> extra space between 'define' and 'PREEMPTION_VERTEX_COUNT' and tab over
> farther before the REG_GENMASK to match the style of the rest of the
> file.
> 
> > +
> >  #define GEN8_RC6_CTX_INFO  _MMIO(0x8504)
> >  
> >  #define GAC_ECO_BITS   _MMIO(0x14090)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index b146a393cd79..9416b1434c64 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -689,6 +689,11 @@ static void dg2_ctx_workarounds_init(struct 
> > intel_engine_cs *engine,
> > IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
> > wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7,
> >  DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA);
> > +
> > +   /* wa_14014947963: dg2_g10 [B0..NONE] dg2_g11 dg2_g12 */
> 
> We can probably just simplify the comment down to "Wa_14014947963:dg2"
> since that's what we've been doing with most of the other DG2
> workarounds lately.  Including the steppings in the comment isn't
> terribly useful since the next line shows them pretty clearly (and we
> run the risk of the comment becoming stale if we update the code later).
> We can tweak that while applying the patch as well.
> 
> 
> Matt
> 
> > +   if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_FOREVER) ||
> > +   IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
> > +   wa_masked_field_set(wal, VF_PREEMPTION, 
> > PREEMPTION_VERTEX_COUNT, 0x4000);
> >  }
> >  
> >  static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine,
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795


Re: [Intel-gfx] [PATCH V3] drm/i915/dg2: add Wa_14014947963

2022-02-17 Thread Matt Roper
On Thu, Feb 10, 2022 at 09:23:33PM -0800, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor 
> 
> BSPEC: 46123
> v2: Address review feedback [MattR]
> v3: move register definition to gt_regs [MattR]
> Cc: Matt Roper 
> Signed-off-by: Clint Taylor 

Reviewed-by: Matt Roper 

although see below for a couple style tweaks we should make while
applying this.

Also, I think we'll need to wait for the maintainers to do a
backmerge/crossmerge in drm-intel-gt-next (which I think they're
planning to do soon) before applying this since that branch is still
missing all the recent register rework.

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index a6f0220c2e9f..5c8c3bc65acc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -124,6 +124,9 @@
>  #define   ECOCHK_PPGTT_WT_HSW(0x2 << 3)
>  #define   ECOCHK_PPGTT_WB_HSW(0x3 << 3)
>  
> +#define VF_PREEMPTION_MMIO(0x83a4)
> +#define  PREEMPTION_VERTEX_COUNT REG_GENMASK(15, 0)

While applying we'll hit some merge conflicts due to the cleanup of the
file that just landed; while resolving the conflict, we should add an
extra space between 'define' and 'PREEMPTION_VERTEX_COUNT' and tab over
farther before the REG_GENMASK to match the style of the rest of the
file.

> +
>  #define GEN8_RC6_CTX_INFO_MMIO(0x8504)
>  
>  #define GAC_ECO_BITS _MMIO(0x14090)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index b146a393cd79..9416b1434c64 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -689,6 +689,11 @@ static void dg2_ctx_workarounds_init(struct 
> intel_engine_cs *engine,
>   IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
>   wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7,
>DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA);
> +
> + /* wa_14014947963: dg2_g10 [B0..NONE] dg2_g11 dg2_g12 */

We can probably just simplify the comment down to "Wa_14014947963:dg2"
since that's what we've been doing with most of the other DG2
workarounds lately.  Including the steppings in the comment isn't
terribly useful since the next line shows them pretty clearly (and we
run the risk of the comment becoming stale if we update the code later).
We can tweak that while applying the patch as well.


Matt

> + if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_FOREVER) ||
> + IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
> + wa_masked_field_set(wal, VF_PREEMPTION, 
> PREEMPTION_VERTEX_COUNT, 0x4000);
>  }
>  
>  static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine,
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795


[Intel-gfx] [PATCH V3] drm/i915/dg2: add Wa_14014947963

2022-02-10 Thread clinton . a . taylor
From: Clint Taylor 

BSPEC: 46123
v2: Address review feedback [MattR]
v3: move register definition to gt_regs [MattR]
Cc: Matt Roper 
Signed-off-by: Clint Taylor 
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index a6f0220c2e9f..5c8c3bc65acc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -124,6 +124,9 @@
 #define   ECOCHK_PPGTT_WT_HSW  (0x2 << 3)
 #define   ECOCHK_PPGTT_WB_HSW  (0x3 << 3)
 
+#define VF_PREEMPTION  _MMIO(0x83a4)
+#define  PREEMPTION_VERTEX_COUNT   REG_GENMASK(15, 0)
+
 #define GEN8_RC6_CTX_INFO  _MMIO(0x8504)
 
 #define GAC_ECO_BITS   _MMIO(0x14090)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index b146a393cd79..9416b1434c64 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -689,6 +689,11 @@ static void dg2_ctx_workarounds_init(struct 
intel_engine_cs *engine,
IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7,
 DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA);
+
+   /* wa_14014947963: dg2_g10 [B0..NONE] dg2_g11 dg2_g12 */
+   if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_FOREVER) ||
+   IS_DG2_G11(engine->i915) || IS_DG2_G12(engine->i915))
+   wa_masked_field_set(wal, VF_PREEMPTION, 
PREEMPTION_VERTEX_COUNT, 0x4000);
 }
 
 static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine,
-- 
2.34.1