Re: [Intel-gfx] [PATCH V4 3/6] drm/i915/gt: Set BLIT_CCTL reg to un-cached

2021-09-02 Thread Matt Roper
On Fri, Sep 03, 2021 at 12:26:32AM +0530, Ayaz A Siddiqui wrote:
> Blitter commands which do not have MOCS fields rely on
> cacheability of BlitterCacheControlRegister which was mapped
> to index 0 by default.Once we changed the MOCS value of
> index 0 to L3 WB, tests like gem_linear_blits started failing
> due to a change in cacheability from UC to WB.
> 
> Program and place the BlitterCacheControlRegister in
> build_aux_regs().
> 
> Cc: Matt Roper 
> Signed-off-by: Ayaz A Siddiqui 
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 43 -
>  drivers/gpu/drm/i915/i915_reg.h |  9 +
>  2 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 38c66765ff94c..04fc977ec27fc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -675,6 +675,41 @@ static void fakewa_disable_nestedbb_mode(struct 
> intel_engine_cs *engine,
>   wa_masked_dis(wal, RING_MI_MODE(engine->mmio_base), TGL_NESTED_BB_EN);
>  }
>  
> +static void gen12_ctx_gt_mocs_init(struct intel_engine_cs *engine,
> +struct i915_wa_list *wal)
> +{
> + u8 mocs;
> +
> + if (engine->class == COPY_ENGINE_CLASS) {
> + /*
> +  * Some blitter commands do not have a field for MOCS, those
> +  * commands will use MOCS index pointed by BLIT_CCTL.
> +  * BLIT_CCTL registers are needed to be programmed to un-cached.
> +  */
> + mocs = engine->gt->mocs.uc_index;

As on the previous patch, the indentation of the comment here is unusual.

> + wa_masked_field_set(wal,

Unlike CMD_CCTL, BLIT_CCTL is _not_ a masked register so we don't want
to use wa_masked_field_set.  Instead this should be a wa_write_clr_set.

> + BLIT_CCTL(engine->mmio_base),
> + BLIT_CCTL_MASK,
> + BLIT_CCTL_MOCS(mocs, mocs));
> + }
> +}
> +
> +/*
> + * gen12_ctx_gt_fake_wa_init() aren't programming actual workarounds,
> + * but it programming general context registers.
> + * Adding those context register programming in context workaround
> + * allow us to use the wa framework for proper application and validation.
> + */
> +static void
> +gen12_ctx_gt_fake_wa_init(struct intel_engine_cs *engine,
> +   struct i915_wa_list *wal)
> +{
> + if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
> + fakewa_disable_nestedbb_mode(engine, wal);
> +
> + gen12_ctx_gt_mocs_init(engine, wal);
> +}

In the future we can move over WaDisable3DMidCmdPreemption,
WaDisableGPGPUMidCmdPreemption, and probably several others, but we can
do that in a separate series down the road.


After applying the s/wa_masked_field_set/wa_write_clr_set/ fix above,
the rest of the patch looks correct so

Reviewed-by: Matt Roper 

> +
>  static void
>  __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
>  struct i915_wa_list *wal,
> @@ -685,8 +720,12 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs 
> *engine,
>   wa_init_start(wal, name, engine->name);
>  
>   /* Applies to all engines */
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
> - fakewa_disable_nestedbb_mode(engine, wal);
> + /*
> +  * Fake workarounds are not the actual workaround but
> +  * programming of context registers using workaround framework.
> +  */
> + if (GRAPHICS_VER(i915) >= 12)
> + gen12_ctx_gt_fake_wa_init(engine, wal);
>  
>   if (engine->class != RENDER_CLASS)
>   goto done;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 92fda75751eef..99cb9321adac9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2568,6 +2568,15 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   (REG_FIELD_PREP(CMD_CCTL_WRITE_OVERRIDE_MASK, (write) << 1) | \
>REG_FIELD_PREP(CMD_CCTL_READ_OVERRIDE_MASK, (read) << 1))
>  
> +#define BLIT_CCTL(base) _MMIO((base) + 0x204)
> +#define   BLIT_CCTL_DST_MOCS_MASK   REG_GENMASK(14, 8)
> +#define   BLIT_CCTL_SRC_MOCS_MASK   REG_GENMASK(6, 0)
> +#define   BLIT_CCTL_MASK (BLIT_CCTL_DST_MOCS_MASK | \
> +   BLIT_CCTL_SRC_MOCS_MASK)
> +#define   BLIT_CCTL_MOCS(dst, src)  \
> + (REG_FIELD_PREP(BLIT_CCTL_DST_MOCS_MASK, (dst) << 1) | \
> +  REG_FIELD_PREP(BLIT_CCTL_SRC_MOCS_MASK, (src) << 1))
> +
>  #define RING_RESET_CTL(base) _MMIO((base) + 0xd0)
>  #define   RESET_CTL_CAT_ERROR   REG_BIT(2)
>  #define   RESET_CTL_READY_TO_RESET REG_BIT(1)
> -- 
> 2.26.2
> 

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


[Intel-gfx] [PATCH V4 3/6] drm/i915/gt: Set BLIT_CCTL reg to un-cached

2021-09-02 Thread Ayaz A Siddiqui
Blitter commands which do not have MOCS fields rely on
cacheability of BlitterCacheControlRegister which was mapped
to index 0 by default.Once we changed the MOCS value of
index 0 to L3 WB, tests like gem_linear_blits started failing
due to a change in cacheability from UC to WB.

Program and place the BlitterCacheControlRegister in
build_aux_regs().

Cc: Matt Roper 
Signed-off-by: Ayaz A Siddiqui 
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 43 -
 drivers/gpu/drm/i915/i915_reg.h |  9 +
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 38c66765ff94c..04fc977ec27fc 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -675,6 +675,41 @@ static void fakewa_disable_nestedbb_mode(struct 
intel_engine_cs *engine,
wa_masked_dis(wal, RING_MI_MODE(engine->mmio_base), TGL_NESTED_BB_EN);
 }
 
+static void gen12_ctx_gt_mocs_init(struct intel_engine_cs *engine,
+  struct i915_wa_list *wal)
+{
+   u8 mocs;
+
+   if (engine->class == COPY_ENGINE_CLASS) {
+   /*
+* Some blitter commands do not have a field for MOCS, those
+* commands will use MOCS index pointed by BLIT_CCTL.
+* BLIT_CCTL registers are needed to be programmed to un-cached.
+*/
+   mocs = engine->gt->mocs.uc_index;
+   wa_masked_field_set(wal,
+   BLIT_CCTL(engine->mmio_base),
+   BLIT_CCTL_MASK,
+   BLIT_CCTL_MOCS(mocs, mocs));
+   }
+}
+
+/*
+ * gen12_ctx_gt_fake_wa_init() aren't programming actual workarounds,
+ * but it programming general context registers.
+ * Adding those context register programming in context workaround
+ * allow us to use the wa framework for proper application and validation.
+ */
+static void
+gen12_ctx_gt_fake_wa_init(struct intel_engine_cs *engine,
+ struct i915_wa_list *wal)
+{
+   if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 55))
+   fakewa_disable_nestedbb_mode(engine, wal);
+
+   gen12_ctx_gt_mocs_init(engine, wal);
+}
+
 static void
 __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
   struct i915_wa_list *wal,
@@ -685,8 +720,12 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
wa_init_start(wal, name, engine->name);
 
/* Applies to all engines */
-   if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
-   fakewa_disable_nestedbb_mode(engine, wal);
+   /*
+* Fake workarounds are not the actual workaround but
+* programming of context registers using workaround framework.
+*/
+   if (GRAPHICS_VER(i915) >= 12)
+   gen12_ctx_gt_fake_wa_init(engine, wal);
 
if (engine->class != RENDER_CLASS)
goto done;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 92fda75751eef..99cb9321adac9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2568,6 +2568,15 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
(REG_FIELD_PREP(CMD_CCTL_WRITE_OVERRIDE_MASK, (write) << 1) | \
 REG_FIELD_PREP(CMD_CCTL_READ_OVERRIDE_MASK, (read) << 1))
 
+#define BLIT_CCTL(base) _MMIO((base) + 0x204)
+#define   BLIT_CCTL_DST_MOCS_MASK   REG_GENMASK(14, 8)
+#define   BLIT_CCTL_SRC_MOCS_MASK   REG_GENMASK(6, 0)
+#define   BLIT_CCTL_MASK (BLIT_CCTL_DST_MOCS_MASK | \
+ BLIT_CCTL_SRC_MOCS_MASK)
+#define   BLIT_CCTL_MOCS(dst, src)\
+   (REG_FIELD_PREP(BLIT_CCTL_DST_MOCS_MASK, (dst) << 1) | \
+REG_FIELD_PREP(BLIT_CCTL_SRC_MOCS_MASK, (src) << 1))
+
 #define RING_RESET_CTL(base)   _MMIO((base) + 0xd0)
 #define   RESET_CTL_CAT_ERROR REG_BIT(2)
 #define   RESET_CTL_READY_TO_RESET REG_BIT(1)
-- 
2.26.2