Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx> writes:
> This WA is implemented in init_context as well as WA batch init.
> There are also some dependent bits need to be set in other registers
> for this to be complete.
>
> v2: behaviour of disable gather at set shader bit can be specified by
> two different registers, use a better option (Ben).
>
For me it looks like there are 2 orthogonal goals for this patch.
I think the actual workaround should be one patch, the resetting
of the set shader bit and the patch named accordingly.
Then the set shader initialization in a different patch,
if there is justification for it (that I have not managed yet
to find).
But lets concentrate on the workaround itself...
> Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++++
> drivers/gpu/drm/i915/intel_lrc.c | 8 ++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++++++++++++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8991cd5..8719a5a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1721,6 +1721,10 @@ enum skl_disp_power_wells {
> #define MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
> #define FW_BLC 0x020d8
> #define FW_BLC2 0x020dc
> +#define GEN7_RS_CHICKEN 0x20DC
> +#define GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
> +#define GEN7_FF_SLICE_CHICKEN1 0x20E0
> +#define GEN9_PER_CTX_DISABLE_GATHER_CONTROL (1<<15)
> #define FW_BLC_SELF 0x020e0 /* 915+ only */
> #define FW_BLC_SELF_EN_MASK (1<<31)
> #define FW_BLC_SELF_FIFO_MASK (1<<16) /* 945 only */
> @@ -5836,6 +5840,7 @@ enum skl_disp_power_wells {
> # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26))
> # define GEN9_RHWO_OPTIMIZATION_DISABLE (1<<14)
> #define COMMON_SLICE_CHICKEN2 0x7014
> +#define GEN9_DISABLE_GATHER_SET_SHADER_SLICE (1<<12)
> # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (1<<0)
>
> #define HIZ_CHICKEN 0x7018
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 9faad82..d3a03f3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1292,6 +1292,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
> struct drm_device *dev = ring->dev;
> uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>
> + /* WA to reset "disable gather at set shader slice" bit */
I am thinking how we could also alert the reader that this workaround
needs to be revisited when it has been given a name.
By adding WaNoName:skl,bxt along with the comment above?
> + if (IS_SKYLAKE(dev)) {
As Ben noted, documentation is rather sparse. But the reference
to previous problems with this bit being save/restored in wrong order,
we can conclude that this should be for BXT also.
> + wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
> + wa_ctx_emit(batch, index, COMMON_SLICE_CHICKEN2);
> + wa_ctx_emit(batch, index,
> + _MASKED_BIT_DISABLE(GEN9_DISABLE_GATHER_SET_SHADER_SLICE));
> + }
> +
The actual value of 'disable set shader' is orthogonal and beyond scope
of this Workaround so the rest should be strip out to different patch.
-Mika
> /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
> if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
> (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index dcd1b8f..5e8e5f9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -985,6 +985,17 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
> tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
> WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
>
> + /* WA to gather at set shader - skl,bxt
> + * These are dependent bits need to be set for the WA.
> + */
> + if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) > SKL_REVID_D0) ||
> + (IS_BROXTON(dev) && INTEL_REVID(dev) > BXT_REVID_A0)) {
> + WA_SET_BIT_MASKED(GEN7_FF_SLICE_CHICKEN1,
> + GEN9_PER_CTX_DISABLE_GATHER_CONTROL);
> + WA_SET_BIT_MASKED(GEN7_RS_CHICKEN,
> + GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
> + }
> +
> return 0;
> }
>
> @@ -1068,6 +1079,13 @@ static int skl_init_workarounds(struct intel_engine_cs *ring)
> HDC_FENCE_DEST_SLM_DISABLE |
> HDC_BARRIER_PERFORMANCE_DISABLE);
>
> + /* WA to Disable gather at set shader - skl
> + * This bit needs to be reset in Per ctx WA batch and it is also
> + * dependent on other bits in different register, all of them need
> + * be set for the WA to be complete.
> + */
> + WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
> +
> return skl_tune_iz_hashing(ring);
> }
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
