| Message ID | 20171218153520.14181-6-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/12/2017 15:35, Lionel Landwerlin wrote: > There are a number of information that are readable from hardware > registers and that we would like to make accessible to userspace. One > particular example is the topology of the execution units (how are > execution units grouped in subslices and slices and also which ones > have been fused off for die recovery). > > At the moment the GET_PARAM ioctl covers some basic needs, but > generally is only able to return a single value for each defined > parameter. This is a bit problematic with topology descriptions which > are array/maps of available units. > > This change introduces a new ioctl that can deal with requests to fill > structures of potentially variable lengths. The user is expected fill > a query with length fields set at 0 on the first call, the kernel then > sets the length fields to the their expected values. A second call to > the kernel with length fields at their expected values will trigger a > copy of the data to the pointed memory locations. > > The scope of this uAPI is only to provide information to userspace, > not to allow configuration of the device. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/i915_query.c | 52 +++++++++++++++++++++++++++++++++++++++ > include/uapi/drm/i915_drm.h | 31 +++++++++++++++++++++++ > 5 files changed, 88 insertions(+) > create mode 100644 drivers/gpu/drm/i915/i915_query.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 091aef281963..9627e7e309dc 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -69,6 +69,7 @@ i915-y += i915_cmd_parser.o \ > i915_gem_timeline.o \ > i915_gem_userptr.o \ > i915_gemfs.o \ > + i915_query.o \ > i915_trace_points.o \ > i915_vma.o \ > intel_breadcrumbs.o \ > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 8b99e415c345..a90ed9f2b759 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2814,6 +2814,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), > }; > > static struct drm_driver driver = { > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 82fc59078c6a..3415a3d2399c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -4084,6 +4084,9 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv); > extern void i915_perf_register(struct drm_i915_private *dev_priv); > extern void i915_perf_unregister(struct drm_i915_private *dev_priv); > > +/* i915_query.c */ > +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file); > + > /* i915_suspend.c */ > extern int i915_save_state(struct drm_i915_private *dev_priv); > extern int i915_restore_state(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c > new file mode 100644 > index 000000000000..227a28978190 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -0,0 +1,52 @@ > +/* > + * Copyright © 2017 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + */ > + > +#include "i915_drv.h" > +#include <uapi/drm/i915_drm.h> > + > +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_i915_query *args = data; An alternative I think could be: struct drm_i915_query_item __user *user_item_ptr = u64_to_user_ptr(args->items_ptr); > + u32 i; > + > + for (i = 0; i < args->num_items; i++) { i++, user_item_ptr++ > + struct drm_i915_query_item item; > + u64 item_user_ptr = args->items_ptr + sizeof(item) * i; So you can drop this line. > + > + if (copy_from_user(&item, u64_to_user_ptr(item_user_ptr), > + sizeof(item))) > + return -EFAULT; And drop u64_to_user_ptr from here... > + > + switch (item.query_id) { > + default: > + return -EINVAL; > + } > + > + if (copy_to_user(u64_to_user_ptr(item_user_ptr), &item, ... and here. > + sizeof(item))) > + return -EFAULT; > + } > + > + return 0; > +} > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 536ee4febd74..87dd8b15548c 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -318,6 +318,7 @@ typedef struct _drm_i915_sarea { > #define DRM_I915_PERF_OPEN 0x36 > #define DRM_I915_PERF_ADD_CONFIG 0x37 > #define DRM_I915_PERF_REMOVE_CONFIG 0x38 > +#define DRM_I915_QUERY 0x39 > > #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) > #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) > @@ -375,6 +376,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) > #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) > #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) > +#define DRM_IOCTL_I915_QUERY DRM_IOW(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query) DRM_IOWR I think. > > /* Allow drivers to submit batchbuffers directly to hardware, relying > * on the security mechanisms provided by hardware. > @@ -1613,6 +1615,35 @@ struct drm_i915_perf_oa_config { > __u64 flex_regs_ptr; > }; > > + > +struct drm_i915_query_item { > + __u32 query_id; Maybe make it 64-bit, just in case we get a need to reserve blocks of bits so we have some more space then. > + > + /* > + * When left to 0 by userspace, this is filled with the size the data s/left to 0/set to zero/? s/the data/of the data/ > + * to be written at the query_data pointer. s/query_data/data_ptr/ ? Or rename the field to query_data - I don't have a case for one versus the other. > + */ > + __u32 length; > + > + /* > + * Data will be written at the location pointed by data_ptr when the > + * value of length matches the length of the data to be written by the > + * kernel. > + */ > + __u64 data_ptr; > +}; > + > +struct drm_i915_query { > + __u32 num_items; > + __u32 _pad; > + > + /* > + * This points an array of struct drm_i915_query_item of length "This point to an array of num_items drm_i915_query_item structures" ? > + * num_items. > + */ > + __u64 items_ptr; > +}; > + > #if defined(__cplusplus) > } > #endif > But in general this works for me. Can't remember who last time raised a complaint about dispatcher, within a dispatcher, within.., but we can't have it all. It's either nested dispatchers, or multiple single purpose ioctls, or sysfs. Executive decision time. :) Again, this approach looks quite easy to extend, and with low boilerplate code requirements so it looks good to me. Regards, Tvrtko
Applied all of your comments here. On 11/01/18 12:19, Tvrtko Ursulin wrote: > > On 18/12/2017 15:35, Lionel Landwerlin wrote: >> There are a number of information that are readable from hardware >> registers and that we would like to make accessible to userspace. One >> particular example is the topology of the execution units (how are >> execution units grouped in subslices and slices and also which ones >> have been fused off for die recovery). >> >> At the moment the GET_PARAM ioctl covers some basic needs, but >> generally is only able to return a single value for each defined >> parameter. This is a bit problematic with topology descriptions which >> are array/maps of available units. >> >> This change introduces a new ioctl that can deal with requests to fill >> structures of potentially variable lengths. The user is expected fill >> a query with length fields set at 0 on the first call, the kernel then >> sets the length fields to the their expected values. A second call to >> the kernel with length fields at their expected values will trigger a >> copy of the data to the pointed memory locations. >> >> The scope of this uAPI is only to provide information to userspace, >> not to allow configuration of the device. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> drivers/gpu/drm/i915/Makefile | 1 + >> drivers/gpu/drm/i915/i915_drv.c | 1 + >> drivers/gpu/drm/i915/i915_drv.h | 3 +++ >> drivers/gpu/drm/i915/i915_query.c | 52 >> +++++++++++++++++++++++++++++++++++++++ >> include/uapi/drm/i915_drm.h | 31 +++++++++++++++++++++++ >> 5 files changed, 88 insertions(+) >> create mode 100644 drivers/gpu/drm/i915/i915_query.c >> >> diff --git a/drivers/gpu/drm/i915/Makefile >> b/drivers/gpu/drm/i915/Makefile >> index 091aef281963..9627e7e309dc 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -69,6 +69,7 @@ i915-y += i915_cmd_parser.o \ >> i915_gem_timeline.o \ >> i915_gem_userptr.o \ >> i915_gemfs.o \ >> + i915_query.o \ >> i915_trace_points.o \ >> i915_vma.o \ >> intel_breadcrumbs.o \ >> diff --git a/drivers/gpu/drm/i915/i915_drv.c >> b/drivers/gpu/drm/i915/i915_drv.c >> index 8b99e415c345..a90ed9f2b759 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -2814,6 +2814,7 @@ static const struct drm_ioctl_desc >> i915_ioctls[] = { >> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, >> DRM_RENDER_ALLOW), >> DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, >> i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), >> DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, >> i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), >> + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, >> DRM_UNLOCKED|DRM_RENDER_ALLOW), >> }; >> static struct drm_driver driver = { >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 82fc59078c6a..3415a3d2399c 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -4084,6 +4084,9 @@ extern void i915_perf_fini(struct >> drm_i915_private *dev_priv); >> extern void i915_perf_register(struct drm_i915_private *dev_priv); >> extern void i915_perf_unregister(struct drm_i915_private *dev_priv); >> +/* i915_query.c */ >> +int i915_query_ioctl(struct drm_device *dev, void *data, struct >> drm_file *file); >> + >> /* i915_suspend.c */ >> extern int i915_save_state(struct drm_i915_private *dev_priv); >> extern int i915_restore_state(struct drm_i915_private *dev_priv); >> diff --git a/drivers/gpu/drm/i915/i915_query.c >> b/drivers/gpu/drm/i915/i915_query.c >> new file mode 100644 >> index 000000000000..227a28978190 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/i915_query.c >> @@ -0,0 +1,52 @@ >> +/* >> + * Copyright © 2017 Intel Corporation >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom >> the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice (including >> the next >> + * paragraph) shall be included in all copies or substantial >> portions of the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >> EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES >> OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> ARISING >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> OTHER DEALINGS >> + * IN THE SOFTWARE. >> + * >> + */ >> + >> +#include "i915_drv.h" >> +#include <uapi/drm/i915_drm.h> >> + >> +int i915_query_ioctl(struct drm_device *dev, void *data, struct >> drm_file *file) >> +{ >> + struct drm_i915_query *args = data; > > An alternative I think could be: > > struct drm_i915_query_item __user *user_item_ptr = > u64_to_user_ptr(args->items_ptr); > >> + u32 i; >> + >> + for (i = 0; i < args->num_items; i++) { > > i++, user_item_ptr++ > >> + struct drm_i915_query_item item; >> + u64 item_user_ptr = args->items_ptr + sizeof(item) * i; > So you can drop this line. > >> + >> + if (copy_from_user(&item, u64_to_user_ptr(item_user_ptr), >> + sizeof(item))) >> + return -EFAULT; > > And drop u64_to_user_ptr from here... > >> + >> + switch (item.query_id) { >> + default: >> + return -EINVAL; >> + } >> + >> + if (copy_to_user(u64_to_user_ptr(item_user_ptr), &item, > > ... and here. > >> + sizeof(item))) >> + return -EFAULT; >> + } >> + >> + return 0; >> +} >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index 536ee4febd74..87dd8b15548c 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -318,6 +318,7 @@ typedef struct _drm_i915_sarea { >> #define DRM_I915_PERF_OPEN 0x36 >> #define DRM_I915_PERF_ADD_CONFIG 0x37 >> #define DRM_I915_PERF_REMOVE_CONFIG 0x38 >> +#define DRM_I915_QUERY 0x39 >> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + >> DRM_I915_INIT, drm_i915_init_t) >> #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + >> DRM_I915_FLUSH) >> @@ -375,6 +376,7 @@ typedef struct _drm_i915_sarea { >> #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + >> DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) >> #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + >> DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) >> #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE >> + DRM_I915_PERF_REMOVE_CONFIG, __u64) >> +#define DRM_IOCTL_I915_QUERY DRM_IOW(DRM_COMMAND_BASE + >> DRM_I915_QUERY, struct drm_i915_query) > > DRM_IOWR I think. > >> /* Allow drivers to submit batchbuffers directly to hardware, >> relying >> * on the security mechanisms provided by hardware. >> @@ -1613,6 +1615,35 @@ struct drm_i915_perf_oa_config { >> __u64 flex_regs_ptr; >> }; >> + >> +struct drm_i915_query_item { >> + __u32 query_id; > > Maybe make it 64-bit, just in case we get a need to reserve blocks of > bits so we have some more space then. > >> + >> + /* >> + * When left to 0 by userspace, this is filled with the size the >> data > > s/left to 0/set to zero/? > s/the data/of the data/ > >> + * to be written at the query_data pointer. > > s/query_data/data_ptr/ ? Or rename the field to query_data - I don't > have a case for one versus the other. > >> + */ >> + __u32 length; >> + >> + /* >> + * Data will be written at the location pointed by data_ptr when >> the >> + * value of length matches the length of the data to be written >> by the >> + * kernel. >> + */ >> + __u64 data_ptr; > +}; >> + >> +struct drm_i915_query { >> + __u32 num_items; >> + __u32 _pad; >> + >> + /* >> + * This points an array of struct drm_i915_query_item of length > > "This point to an array of num_items drm_i915_query_item structures" ? > >> + * num_items. >> + */ >> + __u64 items_ptr; >> +}; >> + >> #if defined(__cplusplus) >> } >> #endif >> > > But in general this works for me. > > Can't remember who last time raised a complaint about dispatcher, > within a dispatcher, within.., but we can't have it all. It's either > nested dispatchers, or multiple single purpose ioctls, or sysfs. > Executive decision time. :) > > Again, this approach looks quite easy to extend, and with low > boilerplate code requirements so it looks good to me. > > Regards, > > Tvrtko >
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 091aef281963..9627e7e309dc 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -69,6 +69,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_timeline.o \ i915_gem_userptr.o \ i915_gemfs.o \ + i915_query.o \ i915_trace_points.o \ i915_vma.o \ intel_breadcrumbs.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8b99e415c345..a90ed9f2b759 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2814,6 +2814,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 82fc59078c6a..3415a3d2399c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -4084,6 +4084,9 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv); extern void i915_perf_register(struct drm_i915_private *dev_priv); extern void i915_perf_unregister(struct drm_i915_private *dev_priv); +/* i915_query.c */ +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file); + /* i915_suspend.c */ extern int i915_save_state(struct drm_i915_private *dev_priv); extern int i915_restore_state(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c new file mode 100644 index 000000000000..227a28978190 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_query.c @@ -0,0 +1,52 @@ +/* + * Copyright © 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "i915_drv.h" +#include <uapi/drm/i915_drm.h> + +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_query *args = data; + u32 i; + + for (i = 0; i < args->num_items; i++) { + struct drm_i915_query_item item; + u64 item_user_ptr = args->items_ptr + sizeof(item) * i; + + if (copy_from_user(&item, u64_to_user_ptr(item_user_ptr), + sizeof(item))) + return -EFAULT; + + switch (item.query_id) { + default: + return -EINVAL; + } + + if (copy_to_user(u64_to_user_ptr(item_user_ptr), &item, + sizeof(item))) + return -EFAULT; + } + + return 0; +} diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 536ee4febd74..87dd8b15548c 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -318,6 +318,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_PERF_OPEN 0x36 #define DRM_I915_PERF_ADD_CONFIG 0x37 #define DRM_I915_PERF_REMOVE_CONFIG 0x38 +#define DRM_I915_QUERY 0x39 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -375,6 +376,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) +#define DRM_IOCTL_I915_QUERY DRM_IOW(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1613,6 +1615,35 @@ struct drm_i915_perf_oa_config { __u64 flex_regs_ptr; }; + +struct drm_i915_query_item { + __u32 query_id; + + /* + * When left to 0 by userspace, this is filled with the size the data + * to be written at the query_data pointer. + */ + __u32 length; + + /* + * Data will be written at the location pointed by data_ptr when the + * value of length matches the length of the data to be written by the + * kernel. + */ + __u64 data_ptr; +}; + +struct drm_i915_query { + __u32 num_items; + __u32 _pad; + + /* + * This points an array of struct drm_i915_query_item of length + * num_items. + */ + __u64 items_ptr; +}; + #if defined(__cplusplus) } #endif
There are a number of information that are readable from hardware registers and that we would like to make accessible to userspace. One particular example is the topology of the execution units (how are execution units grouped in subslices and slices and also which ones have been fused off for die recovery). At the moment the GET_PARAM ioctl covers some basic needs, but generally is only able to return a single value for each defined parameter. This is a bit problematic with topology descriptions which are array/maps of available units. This change introduces a new ioctl that can deal with requests to fill structures of potentially variable lengths. The user is expected fill a query with length fields set at 0 on the first call, the kernel then sets the length fields to the their expected values. A second call to the kernel with length fields at their expected values will trigger a copy of the data to the pointed memory locations. The scope of this uAPI is only to provide information to userspace, not to allow configuration of the device. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_query.c | 52 +++++++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 31 +++++++++++++++++++++++ 5 files changed, 88 insertions(+) create mode 100644 drivers/gpu/drm/i915/i915_query.c