Re: [Intel-gfx] [PATCH v14 5/6] drm/i915: add query uAPI
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 LandwerlinReviewed-by: Tvrtko Ursulin Acked-by: Chris Wilson @@ -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 Lets wait for the Mesa patch review still before merging. Regards, Joonas Thanks, applied locally. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v14 5/6] drm/i915: add query uAPI
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> Reviewed-by: Tvrtko Ursulin > Acked-by: Chris Wilson > @@ -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 Lets wait for the Mesa patch review still before merging. Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v14 5/6] drm/i915: add query uAPI
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 LandwerlinReviewed-by: Tvrtko Ursulin Acked-by: Chris Wilson --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_query.c | 49 +++ drivers/gpu/drm/i915/i915_query.h | 15 include/uapi/drm/i915_drm.h | 40 5 files changed, 107 insertions(+) create mode 100644 drivers/gpu/drm/i915/i915_query.c create mode 100644 drivers/gpu/drm/i915/i915_query.h 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 ..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 + +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(, user_item_ptr, sizeof(item))) + return -EFAULT; + + if