[Intel-xe] [RFC] drm/xe/uapi: Kill exec_queue_set_property
Souza, Jose
jose.souza at intel.com
Wed Nov 29 20:10:00 UTC 2023
On Wed, 2023-11-29 at 11:41 -0500, Rodrigo Vivi wrote:
> All the properties should be immutable and set upon exec_queue creation
> using the existent extension. So, let's kill this useless and dangerous
> uapi.
Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
>
> Cc: Francois Dugast <francois.dugast at intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Matthew Brost <matthew.brost at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>
> Mesa is the only user space component using this IOCTL, but along
> with the exec_queue_create so it should be easy adoption since
> it just needs to use the extension and follow the IGT examples.
>
> A while ago on i915 Mesa team started a clean up on i915 uapi
> making sure we had immutable context and buffers with all the
> properties getting set at creation time. This API here now
> goes against all the good clean-up work that Faith Ekstrand had
> done in i915 and IGT.
>
> So, I hope we can entirely avoid this right now.
> Although only Mesa is using this IOCTL, that might impact other UMDs
> since it changes the ioctl numbering.
>
> drivers/gpu/drm/xe/xe_device.c | 2 --
> drivers/gpu/drm/xe/xe_exec_queue.c | 38 ------------------------
> drivers/gpu/drm/xe/xe_exec_queue.h | 2 --
> include/uapi/drm/xe_drm.h | 46 ++++++++----------------------
> 4 files changed, 12 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 54202623e255..8a96551ec836 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -121,8 +121,6 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(XE_EXEC_QUEUE_DESTROY, xe_exec_queue_destroy_ioctl,
> DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(XE_EXEC_QUEUE_SET_PROPERTY, xe_exec_queue_set_property_ioctl,
> - DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(XE_EXEC_QUEUE_GET_PROPERTY, xe_exec_queue_get_property_ioctl,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(XE_WAIT_USER_FENCE, xe_wait_user_fence_ioctl,
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index d932c31f9fa4..cfa632d60c62 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -883,44 +883,6 @@ int xe_exec_queue_destroy_ioctl(struct drm_device *dev, void *data,
> return 0;
> }
>
> -int xe_exec_queue_set_property_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *file)
> -{
> - struct xe_device *xe = to_xe_device(dev);
> - struct xe_file *xef = to_xe_file(file);
> - struct drm_xe_exec_queue_set_property *args = data;
> - struct xe_exec_queue *q;
> - int ret;
> - u32 idx;
> -
> - if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> - return -EINVAL;
> -
> - q = xe_exec_queue_lookup(xef, args->exec_queue_id);
> - if (XE_IOCTL_DBG(xe, !q))
> - return -ENOENT;
> -
> - if (XE_IOCTL_DBG(xe, args->property >=
> - ARRAY_SIZE(exec_queue_set_property_funcs))) {
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - idx = array_index_nospec(args->property,
> - ARRAY_SIZE(exec_queue_set_property_funcs));
> - ret = exec_queue_set_property_funcs[idx](xe, q, args->value, false);
> - if (XE_IOCTL_DBG(xe, ret))
> - goto out;
> -
> - if (args->extensions)
> - ret = exec_queue_user_extensions(xe, q, args->extensions, 0,
> - false);
> -out:
> - xe_exec_queue_put(q);
> -
> - return ret;
> -}
> -
> static void xe_exec_queue_last_fence_lockdep_assert(struct xe_exec_queue *q,
> struct xe_vm *vm)
> {
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.h b/drivers/gpu/drm/xe/xe_exec_queue.h
> index 59a54bfb9a8c..8b587d1b2c2c 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.h
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.h
> @@ -55,8 +55,6 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> int xe_exec_queue_destroy_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> -int xe_exec_queue_set_property_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *file);
> int xe_exec_queue_get_property_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> enum drm_sched_priority xe_exec_queue_device_get_max_priority(struct xe_device *xe);
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 2c44845c4ba4..73085232821e 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -106,9 +106,8 @@ struct xe_user_extension {
> #define DRM_XE_EXEC 0x06
> #define DRM_XE_EXEC_QUEUE_CREATE 0x07
> #define DRM_XE_EXEC_QUEUE_DESTROY 0x08
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY 0x09
> -#define DRM_XE_EXEC_QUEUE_GET_PROPERTY 0x0a
> -#define DRM_XE_WAIT_USER_FENCE 0x0b
> +#define DRM_XE_EXEC_QUEUE_GET_PROPERTY 0x09
> +#define DRM_XE_WAIT_USER_FENCE 0x0a
> /* Must be kept compact -- no holes */
>
> #define DRM_IOCTL_XE_DEVICE_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_DEVICE_QUERY, struct drm_xe_device_query)
> @@ -741,38 +740,17 @@ struct drm_xe_vm_bind {
> /* Monitor 64MB contiguous region with 2M sub-granularity */
> #define DRM_XE_ACC_GRANULARITY_64M 3
>
> -/**
> - * struct drm_xe_exec_queue_set_property - exec queue set property
> - *
> - * Same namespace for extensions as drm_xe_exec_queue_create
> - */
> -struct drm_xe_exec_queue_set_property {
> - /** @extensions: Pointer to the first extension struct, if any */
> - __u64 extensions;
> -
> - /** @exec_queue_id: Exec queue ID */
> - __u32 exec_queue_id;
> -
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY 0
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE 1
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PREEMPTION_TIMEOUT 2
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE 3
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT 4
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER 5
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY 6
> -#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY 7
> - /** @property: property to set */
> - __u32 property;
> -
> - /** @value: property value */
> - __u64 value;
> -
> - /** @reserved: Reserved */
> - __u64 reserved[2];
> -};
> -
> struct drm_xe_exec_queue_create {
> -#define DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY 0
> +#define DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY 0
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY 0
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE 1
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PREEMPTION_TIMEOUT 2
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE 3
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT 4
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER 5
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY 6
> +#define DRM_XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY 7
> +
> /** @extensions: Pointer to the first extension struct, if any */
> __u64 extensions;
>
More information about the Intel-xe
mailing list