Re: [PATCH v3 2/7] drm/xe: Consolidate setting PTE_AE into one place

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

On 4/15/2024 4:52 PM, Nirmoy Das wrote:
> Currently decision to set PTE_AE is spread between xe_pt
> and xe_vm files and there is no reason to be keep it that
> way. Consolidate the logic for better maintainability.
>
> Atomics is not expected on userptr memory so this patch
> also making sure PTE_AE is only applied when a buffer object
> exist.

Synced with Lionel on this and the expectation is not correct for mesa. KMD

have to allow device atomics if UMD ask for it using the VM bind flag on 
userptr buffers too. I will correct this patch.


Regards,

Nirmoy

>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_pt.c | 4 +---
>   drivers/gpu/drm/xe/xe_vm.c | 7 ++++---
>   2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 5b7930f46cf3..7dc13a8bb44f 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -597,7 +597,6 @@ static int
>   xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
>   		 struct xe_vm_pgtable_update *entries, u32 *num_entries)
>   {
> -	struct xe_device *xe = tile_to_xe(tile);
>   	struct xe_bo *bo = xe_vma_bo(vma);
>   	bool is_devmem = !xe_vma_is_userptr(vma) && bo &&
>   		(xe_bo_is_vram(bo) || xe_bo_is_stolen_devmem(bo));
> @@ -619,8 +618,7 @@ 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;
>   
> -	if ((vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT) &&
> -	    (is_devmem || !IS_DGFX(xe)))
> +	if (vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT)
>   		xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
>   
>   	if (is_devmem) {
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 2dbba55e7785..b1dcaa35b6cc 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -806,9 +806,6 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
>   	for_each_tile(tile, vm->xe, id)
>   		vma->tile_mask |= 0x1 << id;
>   
> -	if (GRAPHICS_VER(vm->xe) >= 20 || vm->xe->info.platform == XE_PVC)
> -		vma->gpuva.flags |= XE_VMA_ATOMIC_PTE_BIT;
> -
>   	vma->pat_index = pat_index;
>   
>   	if (bo) {
> @@ -816,6 +813,10 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
>   
>   		xe_bo_assert_held(bo);
>   
> +		if (vm->xe->info.has_atomic_enable_pte_bit &&
> +		    (xe_bo_is_vram(bo) || !IS_DGFX(vm->xe)))
> +			vma->gpuva.flags |= XE_VMA_ATOMIC_PTE_BIT;
> +
>   		vm_bo = drm_gpuvm_bo_obtain(vma->gpuva.vm, &bo->ttm.base);
>   		if (IS_ERR(vm_bo)) {
>   			xe_vma_free(vma);