Re: [PATCH] drm/xe: Set PTE_AE for xe2 dgfx platforms

[Nirmoy Das <nirmoy.das@intel.com>]

Hi Matt,

On 1/18/2024 1:49 AM, Matt Roper wrote:
> On Wed, Jan 17, 2024 at 03:48:51PM +0100, Nirmoy Das wrote:
>> Atomics on XE2 works for both type of memory so
>> extend setting PTE_AE for dgfx platforms as well.
> There are no Xe2 dgfx platforms yet, so it's kind of hard to review this
> fully,
I should've have sent it internally.
>   but my understanding from bspec 71539 is that in theory a dGPU
> atomic operation against system memory would probably only be atomic in
> device scope, not global scope.  I.e., it's atomic with other GPU
> operations,
This is my understand as well.
>   assuming the CPU isn't also accessing the buffer.  But if
> the buffer is shared between the CPU and GPU, then you'd want to set
> AE=0 to ensure that we get a page fault and can migrate the object into
> lmem first.

I think on system memory, AE=1 should be the default and opt-out by UMD 
with a uAPI. Basic operations like

MI_ATOMIC will fail otherwise which if understand correctly MI_ATOMIC 
should work on system memory except

on PVC because of a know bug. I added a small test to check MI_ATOMIC 
which works on dg2 system memory

https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10547/bat-dg2-oem2/igt@xe_exec_atomic@basic-dec-all@engine-drm_xe_engine_class_compute-instance-0-tile-0-system-memory.html

and also on discrete xe2 but requires this patch.



Regards,

Nirmoy

> Matt
>
>> Cc: Fei Yang<fei.yang@intel.com>
>> Cc: Jose Souza<jose.souza@intel.com>
>> Cc: Matt Roper<matthew.d.roper@intel.com>
>> Cc: Brian Welty<brian.welty@intel.com>
>> Signed-off-by: Nirmoy Das<nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_pt.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
>> index de1030a47588..3ace4b401369 100644
>> --- a/drivers/gpu/drm/xe/xe_pt.c
>> +++ b/drivers/gpu/drm/xe/xe_pt.c
>> @@ -602,8 +602,12 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>>   	struct xe_pt *pt = xe_vma_vm(vma)->pt_root[tile->id];
>>   	int ret;
>>   
>> +	/**
>> +	 * XE_USM_PPGTT_PTE_AE is available for igfx and dgfx from xe2 onwards
>> +	 * and also for PVC but atomics only works for PVC on device memory.
>> +	 */
>>   	if (vma && (vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT) &&
>> -	    (is_devmem || !IS_DGFX(xe)))
>> +	    (is_devmem || GRAPHICS_VER(xe) >= 20))
>>   		xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
>>   
>>   	if (is_devmem) {
>> -- 
>> 2.42.0
>>