Hi Jordan,
This approach was specifically NACKed in the PAT index thread so please
at least mark any such series as RFC if they are intended to facilitate
further dialog on the topic.
I've still not seen any explanation why this would be needed at this specific
case for the PAT index setting feature. Repeating here: You should be able
to detect missing extension by trying to use it. Just because PXP has some
issues on that front doesn't mean we'll change the practices for all other
interfaces.
We should instead spend the time considering a better solution for PXP and
see how that can be implemented for the drm/xe driver.
Regards, Joonas
Quoting Jordan Justen (2023-05-02 23:57:44)
> Cc: Fei Yang <fei.yang@xxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Jordan Justen <jordan.l.justen@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 30 ++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_create.h | 2 ++
> drivers/gpu/drm/i915/i915_query.c | 36 ++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 2 ++
> 4 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> index bfe1dbda4cb7..56342a352599 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> @@ -399,6 +399,36 @@ static const i915_user_extension_fn create_extensions[] = {
> [I915_GEM_CREATE_EXT_PROTECTED_CONTENT] = ext_set_protected,
> };
>
> +/**
> + * Fills buffer will known create_ext extensions
> + * @buffer: buffer to fill with extensions
> + * @buffer_size: size of the buffer in bytes
> + *
> + * If @buffer_size is 0, then @buffer is not accessed, and the
> + * required buffer size is returned.
> + *
> + * If @buffer_size != 0, but not large enough, then -EINVAL is
> + * returned.
> + *
> + * If @buffer_size is large enough, then @buffer will be filled as a
> + * u64 array of extension names.
> + */
> +int
> +i915_gem_create_ext_get_extensions(void *buffer, size_t buffer_size)
> +{
> + unsigned int i;
> +
> + if (buffer_size == 0)
> + return ARRAY_SIZE(create_extensions) * sizeof(u64);
> + else if (buffer_size < (ARRAY_SIZE(create_extensions) * sizeof(u64)))
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(create_extensions); i++)
> + ((u64*)buffer)[i] = i;
> +
> + return ARRAY_SIZE(create_extensions) * sizeof(u64);
> +}
> +
> /**
> * i915_gem_create_ext_ioctl - Creates a new mm object and returns a handle to it.
> * @dev: drm device pointer
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.h b/drivers/gpu/drm/i915/gem/i915_gem_create.h
> index 9536aa906001..e7ebef308038 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.h
> @@ -14,4 +14,6 @@ int i915_gem_dumb_create(struct drm_file *file_priv,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> +int i915_gem_create_ext_get_extensions(void *buffer, size_t buffer_size);
> +
> #endif /* __I915_GEM_CREATE_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index 00871ef99792..f360f76516de 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -9,6 +9,7 @@
> #include "i915_drv.h"
> #include "i915_perf.h"
> #include "i915_query.h"
> +#include "gem/i915_gem_create.h"
> #include "gt/intel_engine_user.h"
> #include <uapi/drm/i915_drm.h>
>
> @@ -551,6 +552,40 @@ static int query_hwconfig_blob(struct drm_i915_private *i915,
> return hwconfig->size;
> }
>
> +static int query_gem_create_extensions(struct drm_i915_private *i915,
> + struct drm_i915_query_item *query_item)
> +{
> + void *buffer;
> + int buffer_size, fill_size;
> +
> + buffer_size = i915_gem_create_ext_get_extensions(NULL, 0);
> +
> + if (query_item->length == 0)
> + return buffer_size;
> +
> + if (query_item->length < buffer_size)
> + return -EINVAL;
> +
> + buffer = kzalloc(buffer_size, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + fill_size = i915_gem_create_ext_get_extensions(buffer, buffer_size);
> + if (fill_size != buffer_size) {
> + kfree(buffer);
> + return -EINVAL;
> + }
> +
> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
> + buffer, buffer_size)) {
> + kfree(buffer);
> + return -EFAULT;
> + }
> +
> + kfree(buffer);
> + return buffer_size;
> +}
> +
> static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
> struct drm_i915_query_item *query_item) = {
> query_topology_info,
> @@ -559,6 +594,7 @@ static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
> query_memregion_info,
> query_hwconfig_blob,
> query_geometry_subslices,
> + query_gem_create_extensions,
> };
>
> int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index dba7c5a5b25e..46be28ee3795 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2963,6 +2963,7 @@ struct drm_i915_query_item {
> * - %DRM_I915_QUERY_MEMORY_REGIONS (see struct drm_i915_query_memory_regions)
> * - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
> * - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct drm_i915_query_topology_info)
> + * - %DRM_I915_QUERY_GEM_CREATE_EXTENSIONS (u64 array of known DRM_I915_GEM_CREATE_EXT extensions)
> */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> @@ -2971,6 +2972,7 @@ struct drm_i915_query_item {
> #define DRM_I915_QUERY_MEMORY_REGIONS 4
> #define DRM_I915_QUERY_HWCONFIG_BLOB 5
> #define DRM_I915_QUERY_GEOMETRY_SUBSLICES 6
> +#define DRM_I915_QUERY_GEM_CREATE_EXTENSIONS 7
> /* Must be kept compact -- no holes and well documented */
>
> /**
> --
> 2.39.2
>
