[Intel-xe] [RFC v1 15/17] drm/xe/uapi: Separate bo_create placement from flags

[Souza, Jose]

On Wed, 2023-10-11 at 13:59 +0000, Francois Dugast wrote:
> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> Although the flags are about the creation, the memory placement
> of the BO deserves a proper dedicated field in the uapi.
> 
> Besides getting more clear, it also allows to remove the
> 'magic' shifts from the flags that was a concern during the
> uapi reviews.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 15 +++++++--------
>  include/uapi/drm/xe_drm.h  | 12 ++++++------
>  2 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 4f0cd65c8522..c2cc859b0921 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1768,19 +1768,18 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  	u32 handle;
>  	int err;
>  
> -	if (XE_IOCTL_DBG(xe, args->extensions) || XE_IOCTL_DBG(xe, args->pad) ||
> +	if (XE_IOCTL_DBG(xe, args->extensions) ||
>  	    XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
>  		return -EINVAL;
>  
> +	/* at least one valid memory placement must be specified */
> +	if (XE_IOCTL_DBG(xe, !(args->placement & xe->info.mem_region_mask)))
> +		return -EINVAL;
> +
>  	if (XE_IOCTL_DBG(xe, args->flags &
>  			 ~(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING |
>  			   DRM_XE_GEM_CREATE_FLAG_SCANOUT |
> -			   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM |
> -			   xe->info.mem_region_mask)))
> -		return -EINVAL;
> -
> -	/* at least one memory type must be specified */
> -	if (XE_IOCTL_DBG(xe, !(args->flags & xe->info.mem_region_mask)))
> +			   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)))
>  		return -EINVAL;
>  
>  	if (XE_IOCTL_DBG(xe, args->handle))
> @@ -1801,7 +1800,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  	if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT)
>  		bo_flags |= XE_BO_SCANOUT_BIT;
>  
> -	bo_flags |= args->flags << (ffs(XE_BO_CREATE_SYSTEM_BIT) - 1);
> +	bo_flags |= args->placement << (ffs(XE_BO_CREATE_SYSTEM_BIT) - 1);
>  
>  	if (args->flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM) {
>  		if (XE_IOCTL_DBG(xe, !(bo_flags & XE_BO_CREATE_VRAM_MASK)))
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index df0561fc6c37..97e5f8f4aca3 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -607,9 +607,12 @@ struct drm_xe_gem_create {
>  	 */
>  	__u64 size;
>  
> -#define DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING		(0x1 << 24)
> -#define DRM_XE_GEM_CREATE_FLAG_SCANOUT			(0x1 << 25)
> -#define DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM	(0x1 << 26)
> +	/** @placement: A mask of memory instances of where BO can be placed. */
> +	__u32 placement;
> +
> +#define DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING		(1 << 0)
> +#define DRM_XE_GEM_CREATE_FLAG_SCANOUT			(1 << 1)
> +#define DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM	(1 << 2)

#define DRM_XE_GEM_CREATE_PLACEMENT_FLAG_DEFER_BACKING...

also this 3 flags should be define above the __u32 placement; like all other flags in xe_drm.h



>  	/**
>  	 * @flags: Flags, currently a mask of memory instances of where BO can
>  	 * be placed
> @@ -633,9 +636,6 @@ struct drm_xe_gem_create {
>  	 */
>  	__u32 handle;
>  
> -	/** @pad: MBZ */
> -	__u32 pad;
> -
>  	/** @reserved: Reserved */
>  	__u64 reserved[2];
>  };