Re: [Intel-xe] [PATCH v3 33/43] drm/xe/uapi: Convert tile_mask to a pt_placement_hint

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Nov 09, 2023 at 03:44:47PM +0000, Francois Dugast wrote:
> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> The previous tile_mask was also an optional hint, and only used
> for the page-table tree placement. However, it was so tied
> with the tile concept itself. Let's clarify things up and make
> this generic enough. So accept any valid memory region mask.
> It could even be a direct near_mem_region gotten from the engine_info.
> pt stands for page table.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>

I thought we landed on converting tile_mask to sched_group_mask? I do
not like pt_placement_hint at all as I've statede what we actually care
about is creating mappings for exec queues. The sched_group_mask is
still a hint basically saying at minimum you must create a mapping for
these sched groups perhaps more. The driver is free to place a PPGTT (or
multiple) anywhere it wants to based on the platform.

e.g. On PVC we have two scheduling groups, and two PPGTT (one per tile in VRAM)
e.g. On MTL we have two scheduling groups, and one PPGTT (sysmem)
e.g. On a unified memory hypothetical future platform we have two scheduling groups, and one PPGGT (shared across tiles in unified VRAM)

Matt

> ---
>  drivers/gpu/drm/xe/xe_vm.c | 14 ++++++++++----
>  include/uapi/drm/xe_drm.h  | 16 +++++++++++++---
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index f8559ebad9bc..ad3b5ea6f91a 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3018,11 +3018,16 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  			goto release_vm_lock;
>  		}
>  
> -		if (bind_ops[i].tile_mask) {
> +		if (bind_ops[i].pt_placement_hint) {
>  			u64 valid_tiles = BIT(xe->info.tile_count) - 1;
> +			/*
> +			 * System memory is currently ignored from this hint,
> +			 * which gets entirely converted to a tile_mask
> +			 */
> +			u8 system_memory = 0x1;
>  
> -			if (XE_IOCTL_DBG(xe, bind_ops[i].tile_mask &
> -					 ~valid_tiles)) {
> +			if (XE_IOCTL_DBG(xe, bind_ops[i].pt_placement_hint &
> +					 ~valid_tiles & ~system_memory)) {
>  				err = -EINVAL;
>  				goto release_vm_lock;
>  			}
> @@ -3099,7 +3104,8 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  		u32 op = bind_ops[i].op;
>  		u32 flags = bind_ops[i].flags;
>  		u64 obj_offset = bind_ops[i].obj_offset;
> -		u8 tile_mask = bind_ops[i].tile_mask;
> +		/* Remove the system memory bit when converting to tiles */
> +		u8 tile_mask = bind_ops[i].pt_placement_hint & ~0x1;
>  		u32 prefetch_region = bind_ops[i].prefetch_mem_region_instance;
>  
>  		ops[i] = vm_bind_ioctl_ops_create(vm, bos[i], obj_offset,
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 3cbfc17d9ffa..144a423868cf 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -853,10 +853,20 @@ struct drm_xe_vm_bind_op {
>  	__u64 addr;
>  
>  	/**
> -	 * @tile_mask: Mask for which tiles to create binds for, 0 == All tiles,
> -	 * only applies to creating new VMAs
> +	 * @pt_placement_hint: An optional memory_region bit-mask hint, which
> +	 * only applies when creating new VMAs. Default value '0' is the
> +	 * recommended value.
> +	 *
> +	 * It hints the optimal placement for the page-table tree for this VMA.
> +	 * For instance, when userspace is using engines living in a secondary
> +	 * tile with allocated BOs near those engines, that same
> +	 * @near_mem_region could be used in this hint field.
> +	 *
> +	 * Since it is a hint, the Xe kernel driver is free to ignore this mask
> +	 * and choose the best location for the page-table, taking into
> +	 * consideration the running hardware and runtime constrains.
>  	 */
> -	__u64 tile_mask;
> +	__u64 pt_placement_hint;
>  
>  #define DRM_XE_VM_BIND_OP_MAP		0x0
>  #define DRM_XE_VM_BIND_OP_UNMAP		0x1
> -- 
> 2.34.1
>