Message ID | 20180222175359.16402-6-lionel.g.landwerlin@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Lionel Landwerlin (2018-02-22 19:53:58) > 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. > > v2: Simplify dispatcher code iteration (Tvrtko) > Tweak uapi drm_i915_query_item structure (Tvrtko) > > v3: Rename pad fields into flags (Chris) > Return error on flags field != 0 (Chris) > Only copy length back to userspace in drm_i915_query_item (Chris) > > v4: Use array of functions instead of switch (Chris) > > v5: More comments in uapi (Tvrtko) > Return query item errors in length field (All) > > v6: Tweak uapi comments style to match the coding style (Lionel) > > v7: Add i915_query.h (Joonas) > > v8: (Lionel) Change the behavior of the item iterator to report > invalid queries into the query item rather than stopping the > iteration. This enables userspace applications to query newer > items on older kernels and only have failure on the items that are > not supported. > > v9: Edit copyright headers (Joonas) > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> <SNIP> > @@ -1615,6 +1617,44 @@ struct drm_i915_perf_oa_config { > __u64 flex_regs_ptr; > }; > > +struct drm_i915_query_item { > + __u64 query_id; > + > + /* > + * When set to zero by userspace, this is filled with the size of the > + * data to be written at the data_ptr pointer. The kernel set this "sets" > + * value to a negative value to signal an error on a particular query > + * item. > + */ > + __s32 length; > + > + /* > + * Unused for now. "now. Must be cleared to zero." > + */ > + __u32 flags; > + > + /* > + * 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; > + > + /* > + * Unused for now. Ditto. > + */ > + __u32 flags; > + > + /* > + * This point to an array of num_items drm_i915_query_item structures. "points" Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Lets wait for the Mesa patch review still before merging. Regards, Joonas
On 05/03/18 14:54, Joonas Lahtinen wrote: > Quoting Lionel Landwerlin (2018-02-22 19:53:58) >> 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. >> >> v2: Simplify dispatcher code iteration (Tvrtko) >> Tweak uapi drm_i915_query_item structure (Tvrtko) >> >> v3: Rename pad fields into flags (Chris) >> Return error on flags field != 0 (Chris) >> Only copy length back to userspace in drm_i915_query_item (Chris) >> >> v4: Use array of functions instead of switch (Chris) >> >> v5: More comments in uapi (Tvrtko) >> Return query item errors in length field (All) >> >> v6: Tweak uapi comments style to match the coding style (Lionel) >> >> v7: Add i915_query.h (Joonas) >> >> v8: (Lionel) Change the behavior of the item iterator to report >> invalid queries into the query item rather than stopping the >> iteration. This enables userspace applications to query newer >> items on older kernels and only have failure on the items that are >> not supported. >> >> v9: Edit copyright headers (Joonas) >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > <SNIP> > >> @@ -1615,6 +1617,44 @@ struct drm_i915_perf_oa_config { >> __u64 flex_regs_ptr; >> }; >> >> +struct drm_i915_query_item { >> + __u64 query_id; >> + >> + /* >> + * When set to zero by userspace, this is filled with the size of the >> + * data to be written at the data_ptr pointer. The kernel set this > "sets" > >> + * value to a negative value to signal an error on a particular query >> + * item. >> + */ >> + __s32 length; >> + >> + /* >> + * Unused for now. > "now. Must be cleared to zero." > >> + */ >> + __u32 flags; >> + >> + /* >> + * 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; >> + >> + /* >> + * Unused for now. > Ditto. > >> + */ >> + __u32 flags; >> + >> + /* >> + * This point to an array of num_items drm_i915_query_item structures. > "points" > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Lets wait for the Mesa patch review still before merging. > > Regards, Joonas > Thanks, applied locally.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 881d7124c597..fa9e7fdb9fd0 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_request.o \ i915_trace_points.o \ i915_vma.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 655a072290e3..73d8c5f869e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -49,6 +49,7 @@ #include "i915_drv.h" #include "i915_trace.h" #include "i915_pmu.h" +#include "i915_query.h" #include "i915_vgpu.h" #include "intel_drv.h" #include "intel_uc.h" @@ -2832,6 +2833,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_query.c b/drivers/gpu/drm/i915/i915_query.c new file mode 100644 index 000000000000..c23bfc9be617 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_query.c @@ -0,0 +1,49 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2018 Intel Corporation + */ + +#include "i915_drv.h" +#include <uapi/drm/i915_drm.h> + +static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv, + struct drm_i915_query_item *query_item) = { +}; + +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_query *args = data; + struct drm_i915_query_item __user *user_item_ptr = + u64_to_user_ptr(args->items_ptr); + u32 i; + + if (args->flags != 0) + return -EINVAL; + + for (i = 0; i < args->num_items; i++, user_item_ptr++) { + struct drm_i915_query_item item; + u64 func_idx; + int ret; + + if (copy_from_user(&item, user_item_ptr, sizeof(item))) + return -EFAULT; + + if (item.query_id == 0) + return -EINVAL; + + func_idx = item.query_id - 1; + + if (func_idx < ARRAY_SIZE(i915_query_funcs)) + ret = i915_query_funcs[func_idx](dev_priv, &item); + else + ret = -EINVAL; + + /* Only write the length back to userspace if they differ. */ + if (ret != item.length && put_user(ret, &user_item_ptr->length)) + return -EFAULT; + } + + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_query.h b/drivers/gpu/drm/i915/i915_query.h new file mode 100644 index 000000000000..31dcef181f63 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_query.h @@ -0,0 +1,15 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2018 Intel Corporation + */ + +#ifndef _I915_QUERY_H_ +#define _I915_QUERY_H_ + +struct drm_device; +struct drm_file; + +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file); + +#endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 29fa48e4755d..1cfb5038c60d 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_IOWR(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. @@ -1615,6 +1617,44 @@ struct drm_i915_perf_oa_config { __u64 flex_regs_ptr; }; +struct drm_i915_query_item { + __u64 query_id; + + /* + * When set to zero by userspace, this is filled with the size of the + * data to be written at the data_ptr pointer. The kernel set this + * value to a negative value to signal an error on a particular query + * item. + */ + __s32 length; + + /* + * Unused for now. + */ + __u32 flags; + + /* + * 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; + + /* + * Unused for now. + */ + __u32 flags; + + /* + * This point to an array of num_items drm_i915_query_item structures. + */ + __u64 items_ptr; +}; + #if defined(__cplusplus) } #endif