Re: [PATCH v5 5/5] drm/xe: Refactor default device atomic settings

["Souza, Jose" <jose.souza@intel.com>]

On Fri, 2024-04-26 at 12:56 +0200, Nirmoy Das wrote:
> The default behavior of device atomics depends on the
> VM type and buffer allocation types. Device atomics are
> expected to function with all types of allocations for
> traditional applications/APIs. Additionally, in compute/SVM
> API scenarios with fault mode or LR mode VMs, device atomics
> must work with single-region allocations. In all other cases
> device atomics should be disabled by default also on platforms
> where we know device atomics doesn't on work on particular
> allocations types.
> 
> v2: Fix platform checks to correct atomics behaviour on PVC.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c | 27 ++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_vm.c |  2 +-
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 5b7930f46cf3..237e4a4985a4 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -619,9 +619,30 @@ 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)))
> -		xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
> +	/**
> +	 * Default atomic expectations for different allocation scenarios are as follows:
> +	 *
> +	 * 1. Traditional API: When the VM is not in fault mode or LR mode:
> +	 *    - Device atomics are expected to function with all allocations.
> +	 *
> +	 * 2. Compute/SVM API: When the VM is either in fault mode or LR mode:
> +	 *    - Device atomics are the default behavior when the bo is placed in a single region.
> +	 *    - In all other cases device atomics will be disabled with AE=0 until an application
> +	 *      request differently using a ioctl like madvise.
> +	 */
> +	if (vma->gpuva.flags & XE_VMA_ATOMIC_PTE_BIT) {
> +		if (xe_vm_in_fault_mode(xe_vma_vm(vma)) ||
> +		    xe_vm_in_lr_mode(xe_vma_vm(vma))) {

nit: xe_vm_in_fault_mode requires xe_vm_in_lr_mode, so you can just check for the later.

> +			if (bo && xe_bo_has_single_placement(bo))
> +				xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
> +		} else {
> +			xe_walk.default_pte |= XE_USM_PPGTT_PTE_AE;
> +		}
> +
> +		/* Unset AE if the platform(PVC) doesn't support it */
> +		if (!xe->info.has_device_atomics_on_smem && !is_devmem)
> +			xe_walk.default_pte &= ~XE_USM_PPGTT_PTE_AE;
> +	}
>  
>  	if (is_devmem) {
>  		xe_walk.default_pte |= XE_PPGTT_PTE_DM;
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 8fc37c5a0196..f795016a80d5 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -805,7 +805,7 @@ 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)
> +	if (vm->xe->info.has_atomic_enable_pte_bit)
>  		vma->gpuva.flags |= XE_VMA_ATOMIC_PTE_BIT;
>  
>  	vma->pat_index = pat_index;