Re: [Intel-xe] [19/21] drm/xe/uapi: Drop OA_IOCTL_VERSION

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 19/09/2023 19:10, Ashutosh Dixit wrote:
> OA version was previously used to track which OA properties were introduced
> at which version. However OA version is an outlier in that a similar
> version is not used anywhere else in the kernel.
>
> For XE, we will track addition of new properties by means of
> xe_user_extension. Userland can either maintain a mapping of OA properties
> against the kernel version, or rely on return codes (e.g. ENOTSUPP) to
> "discover" OA properties.


As a userspace driver this terrible.

You're making us write more code because we have to discover every 
single piece of implementation tweaks ever made to the kernel driver.

Yes OA version was an outlier in that it was better than the rest of 
kernel API.

By reading a single value we could make decisions about how to 
communicate with the kernel driver.


Having xe_user_extension here is not helping, we still have to fuzz our 
way around the kernel to figure out what feature level is available.


-Lionel


>
> Suggested-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_oa.c    | 5 -----
>   drivers/gpu/drm/xe/xe_oa.h    | 1 -
>   drivers/gpu/drm/xe/xe_query.c | 1 -
>   include/uapi/drm/xe_drm.h     | 3 +--
>   4 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index 077698a0c5628..7cb900fc88f58 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -2331,11 +2331,6 @@ static struct ctl_table oa_ctl_table[] = {
>   	{}
>   };
>   
> -int xe_oa_ioctl_version(struct xe_device *xe)
> -{
> -	return 1;
> -}
> -
>   int xe_oa_sysctl_register(void)
>   {
>   	sysctl_header = register_sysctl("dev/xe", oa_ctl_table);
> diff --git a/drivers/gpu/drm/xe/xe_oa.h b/drivers/gpu/drm/xe/xe_oa.h
> index cc6f64bc24ddf..c5a2745ab7cfe 100644
> --- a/drivers/gpu/drm/xe/xe_oa.h
> +++ b/drivers/gpu/drm/xe/xe_oa.h
> @@ -12,7 +12,6 @@ int xe_oa_init(struct xe_device *xe);
>   void xe_oa_fini(struct xe_device *xe);
>   void xe_oa_register(struct xe_device *xe);
>   void xe_oa_unregister(struct xe_device *xe);
> -int xe_oa_ioctl_version(struct xe_device *xe);
>   int xe_oa_sysctl_register(void);
>   void xe_oa_sysctl_unregister(void);
>   
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index ad280bac9eed4..8246ce4e24ce5 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -202,7 +202,6 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
>   		hweight_long(xe->info.mem_region_mask);
>   	config->info[XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY] =
>   		xe_exec_queue_device_get_max_priority(xe);
> -	config->info[XE_QUERY_OA_IOCTL_VERSION] = xe_oa_ioctl_version(xe);
>   
>   	if (copy_to_user(query_ptr, config, size)) {
>   		kfree(config);
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 3b106bed42ea6..b0563cfc351ee 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -259,8 +259,7 @@ struct drm_xe_query_config {
>   #define XE_QUERY_CONFIG_GT_COUNT		4
>   #define XE_QUERY_CONFIG_MEM_REGION_COUNT	5
>   #define XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY	6
> -#define XE_QUERY_OA_IOCTL_VERSION		7
> -#define XE_QUERY_CONFIG_NUM_PARAM		(XE_QUERY_OA_IOCTL_VERSION + 1)
> +#define XE_QUERY_CONFIG_NUM_PARAM		(XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY + 1)
>   	/** @info: array of elements containing the config info */
>   	__u64 info[];
>   };