[Intel-xe] [PATCH] drm/xe/xe_exec_queue: Add check for access counter granularity

[Welty, Brian]

On 10/19/2023 9:50 PM, priyanka.dandamudi at intel.com wrote:
> From: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> 
> Add conditional check for access counter granularity.
> This check will return -EINVAL if granularity is beyond 64M
> which is a hardware limitation.
> 
> v2: Defined
> XE_ACC_GRANULARITY_128K 0
> XE_ACC_GRANULARITY_2M 1
> XE_ACC_GRANULARITY_16M 2
> XE_ACC_GRANULARITY_64M 3
> as part of uAPI.
> So, that user can also use it.(Oak)
> 
> Cc: Oak Zeng <oak.zeng at intel.com>
> Cc: Janga Rahul Kumar <janga.rahul.kumar at intel.com>
> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> Reviewed-by: Oak Zeng <oak.zeng at intel.com>
> ---
>   drivers/gpu/drm/xe/xe_exec_queue.c | 3 +++
>   include/uapi/drm/xe_drm.h          | 5 +++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index dd61c4267e24..4fd44a9203e4 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -393,6 +393,9 @@ static int exec_queue_set_acc_granularity(struct xe_device *xe, struct xe_exec_q
>   	if (XE_IOCTL_DBG(xe, !xe->info.supports_usm))
>   		return -EINVAL;
>   
> +	if (value > XE_ACC_GRANULARITY_64M)
> +		return -EINVAL;
> +
>   	q->usm.acc_granularity = value;
>   
>   	return 0;
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 24bf8f0f52e8..8b5e7e212f61 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -126,6 +126,11 @@ struct xe_user_extension {
>   #define DRM_IOCTL_XE_WAIT_USER_FENCE		DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_WAIT_USER_FENCE, struct drm_xe_wait_user_fence)
>   #define DRM_IOCTL_XE_VM_MADVISE			 DRM_IOW(DRM_COMMAND_BASE + DRM_XE_VM_MADVISE, struct drm_xe_vm_madvise)
>   

Maybe it's obvious, but I think nice to add a comment here before the 
defines,
/* For use with XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY */

> +#define XE_ACC_GRANULARITY_128K 0
> +#define XE_ACC_GRANULARITY_2M 1
> +#define XE_ACC_GRANULARITY_16M 2
> +#define XE_ACC_GRANULARITY_64M 3

And a bit better to place these further down below, before the 
drm_xe_exec_queue_set_property structure is defined.

-Brian


> +
>   /** struct drm_xe_engine_class_instance - instance of an engine class */
>   struct drm_xe_engine_class_instance {
>   #define DRM_XE_ENGINE_CLASS_RENDER		0