[Intel-xe] [PATCH v1 7/8] drm/xe/uapi: Differentiate WAIT_OP from WAIT_MASK

[Matt Roper]

On Tue, Nov 14, 2023 at 01:34:33PM +0000, Francois Dugast wrote:
> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> On one hand the WAIT_OP represents the operation use for waiting such
> as ==, !=, > and so on. On the other hand, the mask is applied to the
> value used for comparision. Split those two to bring clarity to the uapi.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Francois Dugast <francois.dugast at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_wait_user_fence.c | 14 +++++++-------
>  include/uapi/drm/xe_drm.h               | 21 +++++++++++----------
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_wait_user_fence.c b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> index 13562db6c07f..4d5c2555ce41 100644
> --- a/drivers/gpu/drm/xe/xe_wait_user_fence.c
> +++ b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> @@ -25,22 +25,22 @@ static int do_compare(u64 addr, u64 value, u64 mask, u16 op)
>  		return -EFAULT;
>  
>  	switch (op) {
> -	case DRM_XE_UFENCE_WAIT_EQ:
> +	case DRM_XE_UFENCE_WAIT_OP_EQ:
>  		passed = (rvalue & mask) == (value & mask);
>  		break;
> -	case DRM_XE_UFENCE_WAIT_NEQ:
> +	case DRM_XE_UFENCE_WAIT_OP_NEQ:
>  		passed = (rvalue & mask) != (value & mask);
>  		break;
> -	case DRM_XE_UFENCE_WAIT_GT:
> +	case DRM_XE_UFENCE_WAIT_OP_GT:
>  		passed = (rvalue & mask) > (value & mask);
>  		break;
> -	case DRM_XE_UFENCE_WAIT_GTE:
> +	case DRM_XE_UFENCE_WAIT_OP_GTE:
>  		passed = (rvalue & mask) >= (value & mask);
>  		break;
> -	case DRM_XE_UFENCE_WAIT_LT:
> +	case DRM_XE_UFENCE_WAIT_OP_LT:
>  		passed = (rvalue & mask) < (value & mask);
>  		break;
> -	case DRM_XE_UFENCE_WAIT_LTE:
> +	case DRM_XE_UFENCE_WAIT_OP_LTE:
>  		passed = (rvalue & mask) <= (value & mask);
>  		break;
>  	default:
> @@ -81,7 +81,7 @@ static int check_hw_engines(struct xe_device *xe,
>  
>  #define VALID_FLAGS	(DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP | \
>  			 DRM_XE_UFENCE_WAIT_FLAG_ABSTIME)
> -#define MAX_OP		DRM_XE_UFENCE_WAIT_LTE
> +#define MAX_OP		DRM_XE_UFENCE_WAIT_OP_LTE
>  
>  static long to_jiffies_timeout(struct xe_device *xe,
>  			       struct drm_xe_wait_user_fence *args)
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index b3df99dd1023..675194886851 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -914,12 +914,12 @@ struct drm_xe_wait_user_fence {
>  	 */
>  	__u64 addr;
>  
> -#define DRM_XE_UFENCE_WAIT_EQ	0
> -#define DRM_XE_UFENCE_WAIT_NEQ	1
> -#define DRM_XE_UFENCE_WAIT_GT	2
> -#define DRM_XE_UFENCE_WAIT_GTE	3
> -#define DRM_XE_UFENCE_WAIT_LT	4
> -#define DRM_XE_UFENCE_WAIT_LTE	5
> +#define DRM_XE_UFENCE_WAIT_OP_EQ	0x0
> +#define DRM_XE_UFENCE_WAIT_OP_NEQ	0x1
> +#define DRM_XE_UFENCE_WAIT_OP_GT	0x2
> +#define DRM_XE_UFENCE_WAIT_OP_GTE	0x3
> +#define DRM_XE_UFENCE_WAIT_OP_LT	0x4
> +#define DRM_XE_UFENCE_WAIT_OP_LTE	0x5
>  	/** @op: wait operation (type of comparison) */
>  	__u16 op;
>  
> @@ -934,12 +934,13 @@ struct drm_xe_wait_user_fence {
>  	/** @value: compare value */
>  	__u64 value;
>  
> -#define DRM_XE_UFENCE_WAIT_U8		0xffu
> -#define DRM_XE_UFENCE_WAIT_U16		0xffffu
> -#define DRM_XE_UFENCE_WAIT_U32		0xffffffffu
> -#define DRM_XE_UFENCE_WAIT_U64		0xffffffffffffffffu
> +#define DRM_XE_UFENCE_WAIT_MASK_U8	0xffu
> +#define DRM_XE_UFENCE_WAIT_MASK_U16	0xffffu
> +#define DRM_XE_UFENCE_WAIT_MASK_U32	0xffffffffu
> +#define DRM_XE_UFENCE_WAIT_MASK_U64	0xffffffffffffffffu

Do we even really need these mask defines in the uapi?  It's not that
these are the only mask values accepted by the ioctl, and it should be
easy enough for userspace to figure out how to build an appropriate mask
without these defines.

Regardless, the renames look fine so

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


Matt

>  	/** @mask: comparison mask */
>  	__u64 mask;
> +
>  	/**
>  	 * @timeout: how long to wait before bailing, value in nanoseconds.
>  	 * Without DRM_XE_UFENCE_WAIT_FLAG_ABSTIME flag set (relative timeout)
> -- 
> 2.34.1
> 

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