Quoting Lionel Landwerlin (2018-01-16 13:40:10) > With the introduction of asymmetric slices in CNL, we cannot rely on > the previous SUBSLICE_MASK getparam to tell userspace what subslices > are available. Here we introduce a more detailed way of querying the > Gen's GPU topology that doesn't aggregate numbers. > > This is essential for monitoring parts of the GPU with the OA unit, > because counters need to be normalized to the number of > EUs/subslices/slices. The current aggregated numbers like EU_TOTAL do > not gives us sufficient information. > > As a bonus we can draw representations of the GPU : > > https://imgur.com/a/vuqpa > > v2: Rename uapi struct s/_mask/_info/ (Tvrtko) > Report max_slice/subslice/eus_per_subslice rather than strides (Tvrtko) > Add uapi macros to read data from *_info structs (Tvrtko) > > v3: Use !!(v & DRM_I915_BIT()) for uapi macros instead of custom shifts (Tvrtko) > > v4: factorize query item writting (Tvrtko) > tweak uapi struct/define names (Tvrtko) > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_query.c | 107 ++++++++++++++++++++++++++++++++++++++ > include/uapi/drm/i915_drm.h | 53 +++++++++++++++++++ > 2 files changed, 160 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c > index 5694cfea4553..4d18fbd07cbd 100644 > --- a/drivers/gpu/drm/i915/i915_query.c > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -25,8 +25,102 @@ > #include "i915_drv.h" > #include <uapi/drm/i915_drm.h> > > +static int copy_query_data(struct drm_i915_query_item *query_item, > + const void *item_ptr, u32 item_length, > + const void *data_ptr, u32 data_length) > +{ > + u32 total_length = item_length + data_length; > + > + if (query_item->length == 0) { > + query_item->length = total_length; > + return 0; > + } > + > + if (query_item->length != total_length) > + return -EINVAL; Let the user pass in a preallocated buffer of a certain size, and only have to resort to reallocating if too small. i.e. if (query_item->length < total_length) return -EINVAL; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), > + item_ptr, item_length)) > + return -EFAULT; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + item_length), > + data_ptr, data_length)) > + return -EFAULT; > + > + return 0; > +} > + > +static int query_slice_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) > +static int query_subslice_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) > +static int query_eu_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) Couldn't spot any stray leaks. > 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); > @@ -34,15 +128,28 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > > for (i = 0; i < args->num_items; i++, user_item_ptr++) { > struct drm_i915_query_item item; > + int ret; > > if (copy_from_user(&item, user_item_ptr, sizeof(item))) > return -EFAULT; > > switch (item.query_id) { > + case DRM_I915_QUERY_SLICE_INFO: > + ret = query_slice_info(dev_priv, &item); > + break; > + case DRM_I915_QUERY_SUBSLICE_INFO: > + ret = query_subslice_info(dev_priv, &item); > + break; > + case DRM_I915_QUERY_EU_INFO: > + ret = query_eu_info(dev_priv, &item); > + break; > default: > return -EINVAL; > } > > + if (ret) > + return ret; I would make item const and return the copied length: if (ret < 0) return 0; if (ret != item.length && put_user(ret, &user_item_ptr->length)) return -EFAULT; > +#define DRM_I915_BIT(bit) (1 << (bit)) ((__u32)1 << (bit)) Might as well prepare for that 32nd bit. > + > +/* Data written by the kernel with query DRM_I915_QUERY_ID_SLICES_INFO : > + * > + * data: each bit indicates whether a slice is available (1) or fused off (0). > + * Use DRM_I915_QUERY_SLICE_AVAILABLE() to query a given slice's > + * availability. > + */ > +struct drm_i915_query_slice_info { > + __u32 max_slices; > + > +#define DRM_I915_QUERY_SLICE_AVAILABLE(info, slice) \ > + !!((info)->data[(slice) / 8] & DRM_I915_BIT((slice) % 8)) > + __u8 data[]; > +}; > + > +/* Data written by the kernel with query DRM_I915_QUERY_ID_SUBSLICES_INFO : > + * > + * data: each bit indicates whether a subslice is available (1) or fused off > + * (0). Use DRM_I915_QUERY_SUBSLICE_AVAILABLE() to query a given > + * subslice's availability. > + */ > +struct drm_i915_query_subslice_info { > + __u32 max_slices; > + __u32 max_subslices; > + > +#define DRM_I915_QUERY_SUBSLICE_AVAILABLE(info, slice, subslice) \ > + !!((info)->data[(slice) * ALIGN((info)->max_subslices, 8) / 8 + \ Where did we pull ALIGN() in from? If it's from the kernel headers, conflicts will ensue. If not, we have no definition for it. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
