| Message ID | 20230502205744.1067094-1-jordan.l.justen@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/uapi: Add DRM_I915_QUERY_GEM_CREATE_EXTENSIONS query item | expand |
Hi Jordan,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Jordan-Justen/drm-i915-uapi-Add-DRM_I915_QUERY_GEM_CREATE_EXTENSIONS-query-item/20230503-050014
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/20230502205744.1067094-1-jordan.l.justen%40intel.com
patch subject: [Intel-gfx] [PATCH] drm/i915/uapi: Add DRM_I915_QUERY_GEM_CREATE_EXTENSIONS query item
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230503/202305030956.nFO16YoS-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/1154a573531c350e27ca98fd4b4e8da7352f849e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jordan-Justen/drm-i915-uapi-Add-DRM_I915_QUERY_GEM_CREATE_EXTENSIONS-query-item/20230503-050014
git checkout 1154a573531c350e27ca98fd4b4e8da7352f849e
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305030956.nFO16YoS-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
>> drivers/gpu/drm/i915/gem/i915_gem_create.h:17:54: error: unknown type name 'size_t'
17 | int i915_gem_create_ext_get_extensions(void *buffer, size_t buffer_size);
| ^~~~~~
drivers/gpu/drm/i915/gem/i915_gem_create.h:1:1: note: 'size_t' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?
+++ |+#include <stddef.h>
1 | /* SPDX-License-Identifier: MIT */
vim +/size_t +17 drivers/gpu/drm/i915/gem/i915_gem_create.h
12
13 int i915_gem_dumb_create(struct drm_file *file_priv,
14 struct drm_device *dev,
15 struct drm_mode_create_dumb *args);
16
> 17 int i915_gem_create_ext_get_extensions(void *buffer, size_t buffer_size);
18
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@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> > --- > 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 >
On 2023-05-02 22:41:09, Joonas Lahtinen wrote: > 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. There was a preference expressed to not do anything from the i915 side, but I didn't know that my idea had been NACKed. > 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. There's nothing specific to the set-pat extension, but I would've liked to use this query to detect I915_GEM_CREATE_EXT_SET_PAT. Therefore, I was hoping to show how simple implementation of such a query would be. It doesn't really seem like any further maintainence of the query would be required as new extensions are added. Additionally, I was hoping a similar approach could be adopted by Xe. It's not that anything is particularly difficult in the previous approach, but this seems like a pretty simple thing i915 could do to give userspace a clue about which extensions it knows about. -Jordan
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 */ /**
Cc: Fei Yang <fei.yang@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> --- 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(+)