[PATCH] tests/intel/xe_exec_store: Modify test for Priority Mem Read feature

[Mishra, Pallavi]

-----Original Message-----
From: Roper, Matthew D <matthew.d.roper at intel.com> 
Sent: Friday, July 26, 2024 2:29 PM
To: Mishra, Pallavi <pallavi.mishra at intel.com>
Cc: igt-dev at lists.freedesktop.org
Subject: Re: [PATCH] tests/intel/xe_exec_store: Modify test for Priority Mem Read feature

On Thu, Jul 25, 2024 at 01:10:06AM +0530, Pallavi Mishra wrote:
> Now that KMD supports Priority Mem Read feature, care needs to be 
> taken to avoid RAW hazards which may get introduced due to unordered 
> read and writes. Inorder to prevent this insert MI_MEM_FENCE which 
> will ensure data coherency in such scenarios.
> 
> KMD patch to enable Priority Mem Read:
> https://patchwork.freedesktop.org/series/134038/
> 
> Signed-off-by: Pallavi Mishra <pallavi.mishra at intel.com>
> ---
>  include/intel_gpu_commands.h | 2 ++
>  tests/intel/xe_exec_store.c  | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/include/intel_gpu_commands.h 
> b/include/intel_gpu_commands.h index fe734c4bb..cd281ba89 100644
> --- a/include/intel_gpu_commands.h
> +++ b/include/intel_gpu_commands.h
> @@ -89,6 +89,7 @@
>  #define   MI_DISPLAY_FLIP_SKL_PLANE_3_A	(7 << 8)
>  #define   MI_DISPLAY_FLIP_SKL_PLANE_3_B	(8 << 8)
>  #define   MI_DISPLAY_FLIP_SKL_PLANE_3_C	(9 << 8)
> +#define   MI_MEM_FENCE		MI_INSTR(0x09, 0)
>  #define MI_SEMAPHORE_MBOX	MI_INSTR(0x16, 1) /* gen6, gen7 */
>  #define   MI_SEMAPHORE_GLOBAL_GTT    (1<<22)
>  #define   MI_SEMAPHORE_UPDATE	    (1<<21)
> @@ -192,6 +193,7 @@
>  #define MI_OPCODE(x)		(((x) >> 23) & 0x3f)
>  #define IS_MI_LRI_CMD(x)	(MI_OPCODE(x) == MI_OPCODE(MI_INSTR(0x22, 0)))
>  #define MI_LRI_LEN(x)		(((x) & 0xff) + 1)
> +#define MI_WRITE_FENCE			(3 << 0)
>  
>  /*
>   * 3D instructions used by the kernel diff --git 
> a/tests/intel/xe_exec_store.c b/tests/intel/xe_exec_store.c index 
> c872c22d5..fb99e53b7 100644
> --- a/tests/intel/xe_exec_store.c
> +++ b/tests/intel/xe_exec_store.c
> @@ -63,6 +63,7 @@ static void cond_batch(struct data *data, uint64_t addr, int value)
>  	data->batch[b++] = MI_ATOMIC | MI_ATOMIC_INC;
>  	data->batch[b++] = sdi_addr;
>  	data->batch[b++] = sdi_addr >> 32;
> +	data->batch[b++] = MI_MEM_FENCE | MI_WRITE_FENCE;

Does this need to be conditional?  I don't think the MI_MEM_FENCE instruction existed before PVC, so I'm not sure what will happen if the test executes this opcode on an older platform.

Will add a platform check here.

Thanks,
Pallavi


Matt

>  	data->batch[b++] = MI_CONDITIONAL_BATCH_BUFFER_END | MI_DO_COMPARE | 5 << 12 | 2;
>  	data->batch[b++] = value;
>  	data->batch[b++] = sdi_addr;
> --
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation