[PATCH 1/3] drm/xe: Consolidate setting PTE_AE into one place

[Matt Roper]

On Wed, Apr 10, 2024 at 07:03:06PM +0200, 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.

Does this series bisect properly?  I.e., if we run the driver with this
patch applied, but the other two patches missing, isn't it going to turn
on the AE bit in the page table in some BMG SMEM cases where it
shouldn't?  It seems like we should at least mention that in the commit
message to avoid confusion.


Matt

> 
> This also remove the extra care needed for PVC which only
> allows setting PTE_AE for LMEM.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das at 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 a196dbe65252..8f3474c5f480 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -904,9 +904,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) {
> @@ -914,6 +911,10 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
>  
>  		xe_bo_assert_held(bo);
>  
> +		if (GRAPHICS_VER(vm->xe) >= 20 || 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);
> -- 
> 2.42.0
> 

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