On Thu, Feb 27, 2020 at 02:01:01PM -0800, José Roberto de Souza wrote:
> This will fix a memory coherence issue.
>
> v3: using whitespace to make easy to read WA (Chris)
>
> BSpec: 52890
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++++
> drivers/gpu/drm/i915/i915_reg.h | 20 +++++++++++---------
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 3e375a3b7714..c59e1a604ab8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -601,6 +601,14 @@ static void tgl_ctx_workarounds_init(struct
> intel_engine_cs *engine,
> */
> wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
> FF_MODE2_TDS_TIMER_128, 0);
> +
> + /* Wa_1407901919:tgl */
> + wa_add(wal, ICL_HDC_MODE,
> + HDC_COHERENT_ACCESS_L1_CACHE_DIS |
> + HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W,
I'm not sure if this is what the workaround is asking for. The way I
understood the workaround, the 2-dword STATE_COMPUTE_MODE instruction
has a couple bits that must be left at 0. STATE_COMPUTE_MODE is
basically how we ultimately load the HDC_MODE registers (rather than
using a simple LRI like we do for a bunch of other registers), but the
workaround isn't asking us to worry about bits 13+14 in the HDC_MODE
register itself, but rather those flags bits on the instruction that
manipulates the register.
Every time there's a context switch, the hardware will generate a copy
of this instruction as part of the context image in writes to RAM; I'm
assuming these bits aren't set on those hardware-created instructions?
Assuming that's true, then I think this workaround would just be
userspace's responsibility --- if they submit an explicit
STATE_COMPUTE_MODE instruction that isn't just part of the context
image, they need to follow the workaround guidance here and leave two of
those bits set to 0.
Matt
> + 0,
> + HDC_COHERENT_ACCESS_L1_CACHE_DIS |
> + HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W);
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 80cf02a6eec1..28822585537b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7883,15 +7883,17 @@ enum {
> #define GEN8_LQSC_FLUSH_COHERENT_LINES (1 << 21)
>
> /* GEN8 chicken */
> -#define HDC_CHICKEN0 _MMIO(0x7300)
> -#define CNL_HDC_CHICKEN0 _MMIO(0xE5F0)
> -#define ICL_HDC_MODE _MMIO(0xE5F4)
> -#define HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE (1 << 15)
> -#define HDC_FENCE_DEST_SLM_DISABLE (1 << 14)
> -#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1 << 11)
> -#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT (1 << 5)
> -#define HDC_FORCE_NON_COHERENT (1 << 4)
> -#define HDC_BARRIER_PERFORMANCE_DISABLE (1 << 10)
> +#define HDC_CHICKEN0 _MMIO(0x7300)
> +#define CNL_HDC_CHICKEN0 _MMIO(0xE5F0)
> +#define ICL_HDC_MODE _MMIO(0xE5F4)
> +#define HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE REG_BIT(15)
> +#define HDC_FENCE_DEST_SLM_DISABLE REG_BIT(14)
> +#define HDC_DIS_L1_INVAL_FOR_NON_L1_CACHEABLE_W REG_BIT(13)
> +#define HDC_COHERENT_ACCESS_L1_CACHE_DIS REG_BIT(12)
> +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED REG_BIT(11)
> +#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT REG_BIT(5)
> +#define HDC_FORCE_NON_COHERENT REG_BIT(4)
> +#define HDC_BARRIER_PERFORMANCE_DISABLE REG_BIT(10)
>
> #define GEN8_HDC_CHICKEN1 _MMIO(0x7304)
>
> --
> 2.25.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
