Re: [Intel-xe] [PATCH 11/17] drm/xe/uapi: Use common drm_xe_ext_set_property extension

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

On Tue, Sep 19, 2023 at 10:24:58AM -0400, Rodrigo Vivi wrote:
> From: Ashutosh Dixit <ashutosh.dixit@intel.com>
> 
> There really is no difference between 'struct drm_xe_ext_vm_set_property'
> and 'struct drm_xe_ext_exec_queue_set_property', they are extensions which
> specify a <property, value> pair. Replace the two extensions with a single
> common 'struct drm_xe_ext_set_property' extension. The rationale is that
> rather than have each XE module (including future modules) invent their own
> property/value extensions, all XE modules use a common set_property
> extension when possible.
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c |  2 +-
>  drivers/gpu/drm/xe/xe_vm.c         |  2 +-
>  include/uapi/drm/xe_drm.h          | 21 +++------------------
>  3 files changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index f70332986c73..411ba38cb2b8 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -418,7 +418,7 @@ static int exec_queue_user_ext_set_property(struct xe_device *xe,
>  					    bool create)
>  {
>  	u64 __user *address = u64_to_user_ptr(extension);
> -	struct drm_xe_ext_exec_queue_set_property ext;
> +	struct drm_xe_ext_set_property ext;
>  	int err;
>  	u32 idx;
>  
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 2af331cf2d35..33a02aca8971 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1935,7 +1935,7 @@ static int vm_user_ext_set_property(struct xe_device *xe, struct xe_vm *vm,
>  				    u64 extension)
>  {
>  	u64 __user *address = u64_to_user_ptr(extension);
> -	struct drm_xe_ext_vm_set_property ext;
> +	struct drm_xe_ext_set_property ext;
>  	int err;
>  
>  	err = __copy_from_user(&ext, address, sizeof(ext));
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index fb7c3a8ac572..8be3b25928bd 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -572,12 +572,11 @@ struct drm_xe_vm_bind_op_error_capture {
>  	__u64 size;
>  };
>  
> -/** struct drm_xe_ext_vm_set_property - VM set property extension */
> -struct drm_xe_ext_vm_set_property {
> +/** struct drm_xe_ext_set_property - XE set property extension */
> +struct drm_xe_ext_set_property {
>  	/** @base: base user extension */
>  	struct xe_user_extension base;
>  
> -#define XE_VM_PROPERTY_BIND_OP_ERROR_CAPTURE_ADDRESS		0
>  	/** @property: property to set */
>  	__u32 property;
>  
> @@ -593,6 +592,7 @@ struct drm_xe_ext_vm_set_property {
>  
>  struct drm_xe_vm_create {
>  #define XE_VM_EXTENSION_SET_PROPERTY	0
> +#define XE_VM_PROPERTY_BIND_OP_ERROR_CAPTURE_ADDRESS		0
>  	/** @extensions: Pointer to the first extension struct, if any */
>  	__u64 extensions;
>  
> @@ -757,21 +757,6 @@ struct drm_xe_vm_bind {
>  	__u64 reserved[2];
>  };
>  
> -/** struct drm_xe_ext_exec_queue_set_property - exec queue set property extension */
> -struct drm_xe_ext_exec_queue_set_property {
> -	/** @base: base user extension */
> -	struct xe_user_extension base;
> -
> -	/** @property: property to set */
> -	__u32 property;
> -
> -	/** @pad: MBZ */
> -	__u32 pad;
> -
> -	/** @value: property value */
> -	__u64 value;
> -};
> -
>  /**
>   * struct drm_xe_exec_queue_set_property - exec queue set property
>   *
> -- 
> 2.41.0
>