| Message ID | 20180115144159.25913-7-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/01/2018 14:41, Lionel Landwerlin wrote: > 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) > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/i915_query.c | 134 ++++++++++++++++++++++++++++++++++++++ > include/uapi/drm/i915_drm.h | 53 +++++++++++++++ > 2 files changed, 187 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c > index 5694cfea4553..465ec18a472f 100644 > --- a/drivers/gpu/drm/i915/i915_query.c > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -25,8 +25,129 @@ > #include "i915_drv.h" > #include <uapi/drm/i915_drm.h> > > +static int query_slices_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) > +{ > + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; > + struct drm_i915_query_slices_info slices_info; > + u32 data_length, length; > + > + if (sseu->max_slices == 0) > + return -ENODEV; > + > + data_length = sizeof(sseu->slice_mask); > + length = sizeof(slices_info) + data_length; > + > + /* > + * If we ever change the internal slice mask data type, we'll need to > + * update this function. > + */ > + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); > + > + if (query_item->length == 0) { > + query_item->length = length; > + return 0; > + } > + > + if (query_item->length != length) > + return -EINVAL; > + > + memset(&slices_info, 0, sizeof(slices_info)); > + slices_info.max_slices = sseu->max_slices; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &slices_info, > + sizeof(slices_info))) > + return -EFAULT; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + > + offsetof(struct drm_i915_query_slices_info, data)), > + &sseu->slice_mask, data_length)) > + return -EFAULT; > + > + return 0; > +} > + > +static int query_subslices_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) > +{ > + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; > + struct drm_i915_query_subslices_info subslices_info; > + u32 data_length, length; > + > + if (sseu->max_slices == 0) > + return -ENODEV; > + > + memset(&subslices_info, 0, sizeof(subslices_info)); > + subslices_info.max_slices = sseu->max_slices; > + subslices_info.max_subslices = sseu->max_subslices; > + > + data_length = subslices_info.max_slices * > + DIV_ROUND_UP(subslices_info.max_subslices, > + sizeof(sseu->subslice_mask[0]) * BITS_PER_BYTE); > + length = sizeof(subslices_info) + data_length; > + > + if (query_item->length == 0) { > + query_item->length = length; > + return 0; > + } > + > + if (query_item->length != length) > + return -EINVAL; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &subslices_info, > + sizeof(subslices_info))) > + return -EFAULT; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + > + offsetof(struct drm_i915_query_subslices_info, data)), > + sseu->subslice_mask, data_length)) > + return -EFAULT; > + > + return 0; > +} > + > +static int query_eus_info(struct drm_i915_private *dev_priv, > + struct drm_i915_query_item *query_item) > +{ > + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; > + struct drm_i915_query_eus_info eus_info; > + u32 data_length, length; > + > + if (sseu->max_slices == 0) > + return -ENODEV; > + > + memset(&eus_info, 0, sizeof(eus_info)); > + eus_info.max_slices = sseu->max_slices; > + eus_info.max_subslices = sseu->max_subslices; > + eus_info.max_eus_per_subslice = sseu->max_eus_per_subslice; > + > + data_length = eus_info.max_slices * eus_info.max_subslices * > + DIV_ROUND_UP(eus_info.max_eus_per_subslice, BITS_PER_BYTE); > + length = sizeof(eus_info) + data_length; > + > + if (query_item->length == 0) { > + query_item->length = length; > + return 0; > + } > + > + if (query_item->length != length) > + return -EINVAL; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &eus_info, > + sizeof(eus_info))) > + return -EFAULT; > + > + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + > + offsetof(struct drm_i915_query_eus_info, data)), > + sseu->eu_mask, data_length)) > + return -EFAULT; I think in all three queries, once you have the length, you could extract the four if statement above into a common helper? Arguments are query_item, length, data_length, ptr_query_item, ptr_data, and offset_data, unless I am missing something. Up to you if you would find that a worthy compaction. > + > + return 0; > +} > + > 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 +155,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_ID_SLICES_INFO: > + ret = query_slices_info(dev_priv, &item); > + break; > + case DRM_I915_QUERY_ID_SUBSLICES_INFO: > + ret = query_subslices_info(dev_priv, &item); > + break; > + case DRM_I915_QUERY_ID_EUS_INFO: > + ret = query_eus_info(dev_priv, &item); > + break; > default: > return -EINVAL; > } > > + if (ret) > + return ret; > + > if (copy_to_user(user_item_ptr, &item, sizeof(item))) > return -EFAULT; > } > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 39e93f10f2cd..d25f21e3c107 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1618,6 +1618,9 @@ struct drm_i915_perf_oa_config { > > struct drm_i915_query_item { > __u64 query_id; > +#define DRM_I915_QUERY_ID_SLICES_INFO 0x01 > +#define DRM_I915_QUERY_ID_SUBSLICES_INFO 0x02 > +#define DRM_I915_QUERY_ID_EUS_INFO 0x03 Would DRM_I915_QUERY_ID_"singular"_INFO read better? In struct names as well if so? > > /* > * When set to zero by userspace, this is filled with the size of the > @@ -1644,6 +1647,56 @@ struct drm_i915_query { > __u64 items_ptr; > }; Okay.. up to here it's all pretty much fine. > > +#define DRM_I915_BIT(bit) (1 << (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_slices_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_subslices_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 + \ > + (subslice) / 8] & DRM_I915_BIT((subslice) % 8)) > + __u8 data[]; > +}; > + > +/* Data written by the kernel with query DRM_I915_QUERY_ID_EUS_INFO : > + * > + * data: Each bit indicates whether a subslice is available (1) or fused off > + * (0). Use DRM_I915_QUERY_EU_AVAILABLE() to query a given EU's > + * availability. > + */ > +struct drm_i915_query_eus_info { > + __u32 max_slices; > + __u32 max_subslices; > + __u32 max_eus_per_subslice; > + > +#define DRM_I915_QUERY_EU_AVAILABLE(info, slice, subslice, eu) \ > + !!((info)->data[(slice) * ALIGN((info)->max_eus_per_subslice, 8) / 8 * (info)->max_subslices + \ > + (subslice) * ALIGN((info)->max_eus_per_subslice, 8) / 8 + \ > + (eu) / 8] & DRM_I915_BIT((eu) % 8)) > + __u8 data[]; > +}; > + > #if defined(__cplusplus) > } > #endif > This last bit however I'm a bit unsure of. I know I suggested the accessor macros but now they look a bit unsightly. I don't have ideas on how to improve them though. And the unspecified length array is also potentially questionable. I'll take the liberty of Cc-ing Chris and Joonas to start with to see if they have comments on the above. Regards, Tvrtko
On 15/01/18 17:54, Tvrtko Ursulin wrote: > > On 15/01/2018 14:41, Lionel Landwerlin wrote: >> 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) >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> drivers/gpu/drm/i915/i915_query.c | 134 >> ++++++++++++++++++++++++++++++++++++++ >> include/uapi/drm/i915_drm.h | 53 +++++++++++++++ >> 2 files changed, 187 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_query.c >> b/drivers/gpu/drm/i915/i915_query.c >> index 5694cfea4553..465ec18a472f 100644 >> --- a/drivers/gpu/drm/i915/i915_query.c >> +++ b/drivers/gpu/drm/i915/i915_query.c >> @@ -25,8 +25,129 @@ >> #include "i915_drv.h" >> #include <uapi/drm/i915_drm.h> >> +static int query_slices_info(struct drm_i915_private *dev_priv, >> + struct drm_i915_query_item *query_item) >> +{ >> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >> + struct drm_i915_query_slices_info slices_info; >> + u32 data_length, length; >> + >> + if (sseu->max_slices == 0) >> + return -ENODEV; >> + >> + data_length = sizeof(sseu->slice_mask); >> + length = sizeof(slices_info) + data_length; >> + >> + /* >> + * If we ever change the internal slice mask data type, we'll >> need to >> + * update this function. >> + */ >> + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); >> + >> + if (query_item->length == 0) { >> + query_item->length = length; >> + return 0; >> + } >> + >> + if (query_item->length != length) >> + return -EINVAL; >> + >> + memset(&slices_info, 0, sizeof(slices_info)); >> + slices_info.max_slices = sseu->max_slices; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >> &slices_info, >> + sizeof(slices_info))) >> + return -EFAULT; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >> + offsetof(struct drm_i915_query_slices_info, >> data)), >> + &sseu->slice_mask, data_length)) >> + return -EFAULT; >> + >> + return 0; >> +} >> + >> +static int query_subslices_info(struct drm_i915_private *dev_priv, >> + struct drm_i915_query_item *query_item) >> +{ >> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >> + struct drm_i915_query_subslices_info subslices_info; >> + u32 data_length, length; >> + >> + if (sseu->max_slices == 0) >> + return -ENODEV; >> + >> + memset(&subslices_info, 0, sizeof(subslices_info)); >> + subslices_info.max_slices = sseu->max_slices; >> + subslices_info.max_subslices = sseu->max_subslices; >> + >> + data_length = subslices_info.max_slices * >> + DIV_ROUND_UP(subslices_info.max_subslices, >> + sizeof(sseu->subslice_mask[0]) * BITS_PER_BYTE); >> + length = sizeof(subslices_info) + data_length; >> + >> + if (query_item->length == 0) { >> + query_item->length = length; >> + return 0; >> + } >> + >> + if (query_item->length != length) >> + return -EINVAL; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >> &subslices_info, >> + sizeof(subslices_info))) >> + return -EFAULT; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >> + offsetof(struct drm_i915_query_subslices_info, >> data)), >> + sseu->subslice_mask, data_length)) >> + return -EFAULT; >> + >> + return 0; >> +} >> + >> +static int query_eus_info(struct drm_i915_private *dev_priv, >> + struct drm_i915_query_item *query_item) >> +{ >> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >> + struct drm_i915_query_eus_info eus_info; >> + u32 data_length, length; >> + >> + if (sseu->max_slices == 0) >> + return -ENODEV; >> + >> + memset(&eus_info, 0, sizeof(eus_info)); >> + eus_info.max_slices = sseu->max_slices; >> + eus_info.max_subslices = sseu->max_subslices; >> + eus_info.max_eus_per_subslice = sseu->max_eus_per_subslice; >> + >> + data_length = eus_info.max_slices * eus_info.max_subslices * >> + DIV_ROUND_UP(eus_info.max_eus_per_subslice, BITS_PER_BYTE); >> + length = sizeof(eus_info) + data_length; >> + >> + if (query_item->length == 0) { >> + query_item->length = length; >> + return 0; >> + } >> + >> + if (query_item->length != length) >> + return -EINVAL; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &eus_info, >> + sizeof(eus_info))) >> + return -EFAULT; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >> + offsetof(struct drm_i915_query_eus_info, data)), >> + sseu->eu_mask, data_length)) >> + return -EFAULT; > > I think in all three queries, once you have the length, you could > extract the four if statement above into a common helper? Arguments > are query_item, length, data_length, ptr_query_item, ptr_data, and > offset_data, unless I am missing something. Up to you if you would > find that a worthy compaction. I just tried that but the result doesn't really look good. You need to maintain the control flow via an indirect return value... > >> + >> + return 0; >> +} >> + >> 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 +155,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_ID_SLICES_INFO: >> + ret = query_slices_info(dev_priv, &item); >> + break; >> + case DRM_I915_QUERY_ID_SUBSLICES_INFO: >> + ret = query_subslices_info(dev_priv, &item); >> + break; >> + case DRM_I915_QUERY_ID_EUS_INFO: >> + ret = query_eus_info(dev_priv, &item); >> + break; >> default: >> return -EINVAL; >> } >> + if (ret) >> + return ret; >> + >> if (copy_to_user(user_item_ptr, &item, sizeof(item))) >> return -EFAULT; >> } >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index 39e93f10f2cd..d25f21e3c107 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -1618,6 +1618,9 @@ struct drm_i915_perf_oa_config { >> struct drm_i915_query_item { >> __u64 query_id; >> +#define DRM_I915_QUERY_ID_SLICES_INFO 0x01 >> +#define DRM_I915_QUERY_ID_SUBSLICES_INFO 0x02 >> +#define DRM_I915_QUERY_ID_EUS_INFO 0x03 > > Would DRM_I915_QUERY_ID_"singular"_INFO read better? In struct names > as well if so? Hmm... Sorry, I don't understand your comment here. What would be "singular" ? > >> /* >> * When set to zero by userspace, this is filled with the size >> of the >> @@ -1644,6 +1647,56 @@ struct drm_i915_query { >> __u64 items_ptr; >> }; > > Okay.. up to here it's all pretty much fine. > >> +#define DRM_I915_BIT(bit) (1 << (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_slices_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_subslices_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 + \ >> + (subslice) / 8] & DRM_I915_BIT((subslice) % 8)) >> + __u8 data[]; >> +}; >> + >> +/* Data written by the kernel with query DRM_I915_QUERY_ID_EUS_INFO : >> + * >> + * data: Each bit indicates whether a subslice is available (1) or >> fused off >> + * (0). Use DRM_I915_QUERY_EU_AVAILABLE() to query a given EU's >> + * availability. >> + */ >> +struct drm_i915_query_eus_info { >> + __u32 max_slices; >> + __u32 max_subslices; >> + __u32 max_eus_per_subslice; >> + >> +#define DRM_I915_QUERY_EU_AVAILABLE(info, slice, subslice, eu) \ >> + !!((info)->data[(slice) * ALIGN((info)->max_eus_per_subslice, 8) >> / 8 * (info)->max_subslices + \ >> + (subslice) * ALIGN((info)->max_eus_per_subslice, 8) / 8 + \ >> + (eu) / 8] & DRM_I915_BIT((eu) % 8)) >> + __u8 data[]; >> +}; >> + >> #if defined(__cplusplus) >> } >> #endif >> > > This last bit however I'm a bit unsure of. > > I know I suggested the accessor macros but now they look a bit > unsightly. I don't have ideas on how to improve them though. > > And the unspecified length array is also potentially questionable. > > I'll take the liberty of Cc-ing Chris and Joonas to start with to see > if they have comments on the above. > > Regards, > > Tvrtko > Thanks again for your review.
On 15/01/2018 18:23, Lionel Landwerlin wrote: > On 15/01/18 17:54, Tvrtko Ursulin wrote: >> >> On 15/01/2018 14:41, Lionel Landwerlin wrote: >>> 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) >>> >>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_query.c | 134 >>> ++++++++++++++++++++++++++++++++++++++ >>> include/uapi/drm/i915_drm.h | 53 +++++++++++++++ >>> 2 files changed, 187 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_query.c >>> b/drivers/gpu/drm/i915/i915_query.c >>> index 5694cfea4553..465ec18a472f 100644 >>> --- a/drivers/gpu/drm/i915/i915_query.c >>> +++ b/drivers/gpu/drm/i915/i915_query.c >>> @@ -25,8 +25,129 @@ >>> #include "i915_drv.h" >>> #include <uapi/drm/i915_drm.h> >>> +static int query_slices_info(struct drm_i915_private *dev_priv, >>> + struct drm_i915_query_item *query_item) >>> +{ >>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>> + struct drm_i915_query_slices_info slices_info; >>> + u32 data_length, length; >>> + >>> + if (sseu->max_slices == 0) >>> + return -ENODEV; >>> + >>> + data_length = sizeof(sseu->slice_mask); >>> + length = sizeof(slices_info) + data_length; >>> + >>> + /* >>> + * If we ever change the internal slice mask data type, we'll >>> need to >>> + * update this function. >>> + */ >>> + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); >>> + >>> + if (query_item->length == 0) { >>> + query_item->length = length; >>> + return 0; >>> + } >>> + >>> + if (query_item->length != length) >>> + return -EINVAL; >>> + >>> + memset(&slices_info, 0, sizeof(slices_info)); >>> + slices_info.max_slices = sseu->max_slices; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >>> &slices_info, >>> + sizeof(slices_info))) >>> + return -EFAULT; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>> + offsetof(struct drm_i915_query_slices_info, >>> data)), >>> + &sseu->slice_mask, data_length)) >>> + return -EFAULT; >>> + >>> + return 0; >>> +} >>> + >>> +static int query_subslices_info(struct drm_i915_private *dev_priv, >>> + struct drm_i915_query_item *query_item) >>> +{ >>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>> + struct drm_i915_query_subslices_info subslices_info; >>> + u32 data_length, length; >>> + >>> + if (sseu->max_slices == 0) >>> + return -ENODEV; >>> + >>> + memset(&subslices_info, 0, sizeof(subslices_info)); >>> + subslices_info.max_slices = sseu->max_slices; >>> + subslices_info.max_subslices = sseu->max_subslices; >>> + >>> + data_length = subslices_info.max_slices * >>> + DIV_ROUND_UP(subslices_info.max_subslices, >>> + sizeof(sseu->subslice_mask[0]) * BITS_PER_BYTE); >>> + length = sizeof(subslices_info) + data_length; >>> + >>> + if (query_item->length == 0) { >>> + query_item->length = length; >>> + return 0; >>> + } >>> + >>> + if (query_item->length != length) >>> + return -EINVAL; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >>> &subslices_info, >>> + sizeof(subslices_info))) >>> + return -EFAULT; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>> + offsetof(struct drm_i915_query_subslices_info, >>> data)), >>> + sseu->subslice_mask, data_length)) >>> + return -EFAULT; >>> + >>> + return 0; >>> +} >>> + >>> +static int query_eus_info(struct drm_i915_private *dev_priv, >>> + struct drm_i915_query_item *query_item) >>> +{ >>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>> + struct drm_i915_query_eus_info eus_info; >>> + u32 data_length, length; >>> + >>> + if (sseu->max_slices == 0) >>> + return -ENODEV; >>> + >>> + memset(&eus_info, 0, sizeof(eus_info)); >>> + eus_info.max_slices = sseu->max_slices; >>> + eus_info.max_subslices = sseu->max_subslices; >>> + eus_info.max_eus_per_subslice = sseu->max_eus_per_subslice; >>> + >>> + data_length = eus_info.max_slices * eus_info.max_subslices * >>> + DIV_ROUND_UP(eus_info.max_eus_per_subslice, BITS_PER_BYTE); >>> + length = sizeof(eus_info) + data_length; >>> + >>> + if (query_item->length == 0) { >>> + query_item->length = length; >>> + return 0; >>> + } >>> + >>> + if (query_item->length != length) >>> + return -EINVAL; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &eus_info, >>> + sizeof(eus_info))) >>> + return -EFAULT; >>> + >>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>> + offsetof(struct drm_i915_query_eus_info, data)), >>> + sseu->eu_mask, data_length)) >>> + return -EFAULT; >> >> I think in all three queries, once you have the length, you could >> extract the four if statement above into a common helper? Arguments >> are query_item, length, data_length, ptr_query_item, ptr_data, and >> offset_data, unless I am missing something. Up to you if you would >> find that a worthy compaction. > > I just tried that but the result doesn't really look good. > You need to maintain the control flow via an indirect return value... I was thinking something like this: int insert_query_helper_name(query_item, query_item_sz, length, data_length, query_item_ptr, query_data_ptr) { if (query_item->length == 0) { query_item->length = length; return 0; } if (query_item->length != length) return -EINVAL; if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), query_item_ptr, query_item_sz)) return -EFAULT; if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + query_item_sz, query_data_ptr, data_length)) return -EFAULT; return 0; } Then in the handlers just end with: return insert_query_helper_hame(...); ? >> >>> + >>> + return 0; >>> +} >>> + >>> 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 +155,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_ID_SLICES_INFO: >>> + ret = query_slices_info(dev_priv, &item); >>> + break; >>> + case DRM_I915_QUERY_ID_SUBSLICES_INFO: >>> + ret = query_subslices_info(dev_priv, &item); >>> + break; >>> + case DRM_I915_QUERY_ID_EUS_INFO: >>> + ret = query_eus_info(dev_priv, &item); >>> + break; >>> default: >>> return -EINVAL; >>> } >>> + if (ret) >>> + return ret; >>> + >>> if (copy_to_user(user_item_ptr, &item, sizeof(item))) >>> return -EFAULT; >>> } >>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >>> index 39e93f10f2cd..d25f21e3c107 100644 >>> --- a/include/uapi/drm/i915_drm.h >>> +++ b/include/uapi/drm/i915_drm.h >>> @@ -1618,6 +1618,9 @@ struct drm_i915_perf_oa_config { >>> struct drm_i915_query_item { >>> __u64 query_id; >>> +#define DRM_I915_QUERY_ID_SLICES_INFO 0x01 >>> +#define DRM_I915_QUERY_ID_SUBSLICES_INFO 0x02 >>> +#define DRM_I915_QUERY_ID_EUS_INFO 0x03 >> >> Would DRM_I915_QUERY_ID_"singular"_INFO read better? In struct names >> as well if so? > > Hmm... Sorry, I don't understand your comment here. > What would be "singular" ? Opposite of plural, like: _QUERY_ID_SLICE_INFO, _QUERY_ID_SUBSLICE_INFO, _QUERY_ID_EU_INFO. Actually now I'm thinking if _ID_ part could also be removed for no loss? Regards, Tvrtko
On 16/01/18 08:42, Tvrtko Ursulin wrote: > > On 15/01/2018 18:23, Lionel Landwerlin wrote: >> On 15/01/18 17:54, Tvrtko Ursulin wrote: >>> >>> On 15/01/2018 14:41, Lionel Landwerlin wrote: >>>> 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) >>>> >>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/i915_query.c | 134 >>>> ++++++++++++++++++++++++++++++++++++++ >>>> include/uapi/drm/i915_drm.h | 53 +++++++++++++++ >>>> 2 files changed, 187 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_query.c >>>> b/drivers/gpu/drm/i915/i915_query.c >>>> index 5694cfea4553..465ec18a472f 100644 >>>> --- a/drivers/gpu/drm/i915/i915_query.c >>>> +++ b/drivers/gpu/drm/i915/i915_query.c >>>> @@ -25,8 +25,129 @@ >>>> #include "i915_drv.h" >>>> #include <uapi/drm/i915_drm.h> >>>> +static int query_slices_info(struct drm_i915_private *dev_priv, >>>> + struct drm_i915_query_item *query_item) >>>> +{ >>>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>>> + struct drm_i915_query_slices_info slices_info; >>>> + u32 data_length, length; >>>> + >>>> + if (sseu->max_slices == 0) >>>> + return -ENODEV; >>>> + >>>> + data_length = sizeof(sseu->slice_mask); >>>> + length = sizeof(slices_info) + data_length; >>>> + >>>> + /* >>>> + * If we ever change the internal slice mask data type, we'll >>>> need to >>>> + * update this function. >>>> + */ >>>> + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); >>>> + >>>> + if (query_item->length == 0) { >>>> + query_item->length = length; >>>> + return 0; >>>> + } >>>> + >>>> + if (query_item->length != length) >>>> + return -EINVAL; >>>> + >>>> + memset(&slices_info, 0, sizeof(slices_info)); >>>> + slices_info.max_slices = sseu->max_slices; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >>>> &slices_info, >>>> + sizeof(slices_info))) >>>> + return -EFAULT; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>>> + offsetof(struct drm_i915_query_slices_info, >>>> data)), >>>> + &sseu->slice_mask, data_length)) >>>> + return -EFAULT; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int query_subslices_info(struct drm_i915_private *dev_priv, >>>> + struct drm_i915_query_item *query_item) >>>> +{ >>>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>>> + struct drm_i915_query_subslices_info subslices_info; >>>> + u32 data_length, length; >>>> + >>>> + if (sseu->max_slices == 0) >>>> + return -ENODEV; >>>> + >>>> + memset(&subslices_info, 0, sizeof(subslices_info)); >>>> + subslices_info.max_slices = sseu->max_slices; >>>> + subslices_info.max_subslices = sseu->max_subslices; >>>> + >>>> + data_length = subslices_info.max_slices * >>>> + DIV_ROUND_UP(subslices_info.max_subslices, >>>> + sizeof(sseu->subslice_mask[0]) * BITS_PER_BYTE); >>>> + length = sizeof(subslices_info) + data_length; >>>> + >>>> + if (query_item->length == 0) { >>>> + query_item->length = length; >>>> + return 0; >>>> + } >>>> + >>>> + if (query_item->length != length) >>>> + return -EINVAL; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >>>> &subslices_info, >>>> + sizeof(subslices_info))) >>>> + return -EFAULT; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>>> + offsetof(struct >>>> drm_i915_query_subslices_info, data)), >>>> + sseu->subslice_mask, data_length)) >>>> + return -EFAULT; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int query_eus_info(struct drm_i915_private *dev_priv, >>>> + struct drm_i915_query_item *query_item) >>>> +{ >>>> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; >>>> + struct drm_i915_query_eus_info eus_info; >>>> + u32 data_length, length; >>>> + >>>> + if (sseu->max_slices == 0) >>>> + return -ENODEV; >>>> + >>>> + memset(&eus_info, 0, sizeof(eus_info)); >>>> + eus_info.max_slices = sseu->max_slices; >>>> + eus_info.max_subslices = sseu->max_subslices; >>>> + eus_info.max_eus_per_subslice = sseu->max_eus_per_subslice; >>>> + >>>> + data_length = eus_info.max_slices * eus_info.max_subslices * >>>> + DIV_ROUND_UP(eus_info.max_eus_per_subslice, BITS_PER_BYTE); >>>> + length = sizeof(eus_info) + data_length; >>>> + >>>> + if (query_item->length == 0) { >>>> + query_item->length = length; >>>> + return 0; >>>> + } >>>> + >>>> + if (query_item->length != length) >>>> + return -EINVAL; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >>>> &eus_info, >>>> + sizeof(eus_info))) >>>> + return -EFAULT; >>>> + >>>> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + >>>> + offsetof(struct drm_i915_query_eus_info, data)), >>>> + sseu->eu_mask, data_length)) >>>> + return -EFAULT; >>> >>> I think in all three queries, once you have the length, you could >>> extract the four if statement above into a common helper? Arguments >>> are query_item, length, data_length, ptr_query_item, ptr_data, and >>> offset_data, unless I am missing something. Up to you if you would >>> find that a worthy compaction. >> >> I just tried that but the result doesn't really look good. >> You need to maintain the control flow via an indirect return value... > > I was thinking something like this: > > int insert_query_helper_name(query_item, query_item_sz, length, > data_length, query_item_ptr, query_data_ptr) > { > if (query_item->length == 0) { > query_item->length = length; > return 0; > } > > if (query_item->length != length) > return -EINVAL; > > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), > query_item_ptr, query_item_sz)) > return -EFAULT; > > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + > query_item_sz, > query_data_ptr, data_length)) > return -EFAULT; > > return 0; > } > > Then in the handlers just end with: > > return insert_query_helper_hame(...); Ah thanks, I stopped at copying the query item but didn't do the data... That looks better indeed. > > ? > >>> >>>> + >>>> + return 0; >>>> +} >>>> + >>>> 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 +155,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_ID_SLICES_INFO: >>>> + ret = query_slices_info(dev_priv, &item); >>>> + break; >>>> + case DRM_I915_QUERY_ID_SUBSLICES_INFO: >>>> + ret = query_subslices_info(dev_priv, &item); >>>> + break; >>>> + case DRM_I915_QUERY_ID_EUS_INFO: >>>> + ret = query_eus_info(dev_priv, &item); >>>> + break; >>>> default: >>>> return -EINVAL; >>>> } >>>> + if (ret) >>>> + return ret; >>>> + >>>> if (copy_to_user(user_item_ptr, &item, sizeof(item))) >>>> return -EFAULT; >>>> } >>>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >>>> index 39e93f10f2cd..d25f21e3c107 100644 >>>> --- a/include/uapi/drm/i915_drm.h >>>> +++ b/include/uapi/drm/i915_drm.h >>>> @@ -1618,6 +1618,9 @@ struct drm_i915_perf_oa_config { >>>> struct drm_i915_query_item { >>>> __u64 query_id; >>>> +#define DRM_I915_QUERY_ID_SLICES_INFO 0x01 >>>> +#define DRM_I915_QUERY_ID_SUBSLICES_INFO 0x02 >>>> +#define DRM_I915_QUERY_ID_EUS_INFO 0x03 >>> >>> Would DRM_I915_QUERY_ID_"singular"_INFO read better? In struct names >>> as well if so? >> >> Hmm... Sorry, I don't understand your comment here. >> What would be "singular" ? > > Opposite of plural, like: > > _QUERY_ID_SLICE_INFO, _QUERY_ID_SUBSLICE_INFO, _QUERY_ID_EU_INFO. > > Actually now I'm thinking if _ID_ part could also be removed for no loss? Yeah, sure. > > Regards, > > Tvrtko > >
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 5694cfea4553..465ec18a472f 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -25,8 +25,129 @@ #include "i915_drv.h" #include <uapi/drm/i915_drm.h> +static int query_slices_info(struct drm_i915_private *dev_priv, + struct drm_i915_query_item *query_item) +{ + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; + struct drm_i915_query_slices_info slices_info; + u32 data_length, length; + + if (sseu->max_slices == 0) + return -ENODEV; + + data_length = sizeof(sseu->slice_mask); + length = sizeof(slices_info) + data_length; + + /* + * If we ever change the internal slice mask data type, we'll need to + * update this function. + */ + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); + + if (query_item->length == 0) { + query_item->length = length; + return 0; + } + + if (query_item->length != length) + return -EINVAL; + + memset(&slices_info, 0, sizeof(slices_info)); + slices_info.max_slices = sseu->max_slices; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &slices_info, + sizeof(slices_info))) + return -EFAULT; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + + offsetof(struct drm_i915_query_slices_info, data)), + &sseu->slice_mask, data_length)) + return -EFAULT; + + return 0; +} + +static int query_subslices_info(struct drm_i915_private *dev_priv, + struct drm_i915_query_item *query_item) +{ + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; + struct drm_i915_query_subslices_info subslices_info; + u32 data_length, length; + + if (sseu->max_slices == 0) + return -ENODEV; + + memset(&subslices_info, 0, sizeof(subslices_info)); + subslices_info.max_slices = sseu->max_slices; + subslices_info.max_subslices = sseu->max_subslices; + + data_length = subslices_info.max_slices * + DIV_ROUND_UP(subslices_info.max_subslices, + sizeof(sseu->subslice_mask[0]) * BITS_PER_BYTE); + length = sizeof(subslices_info) + data_length; + + if (query_item->length == 0) { + query_item->length = length; + return 0; + } + + if (query_item->length != length) + return -EINVAL; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &subslices_info, + sizeof(subslices_info))) + return -EFAULT; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + + offsetof(struct drm_i915_query_subslices_info, data)), + sseu->subslice_mask, data_length)) + return -EFAULT; + + return 0; +} + +static int query_eus_info(struct drm_i915_private *dev_priv, + struct drm_i915_query_item *query_item) +{ + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu; + struct drm_i915_query_eus_info eus_info; + u32 data_length, length; + + if (sseu->max_slices == 0) + return -ENODEV; + + memset(&eus_info, 0, sizeof(eus_info)); + eus_info.max_slices = sseu->max_slices; + eus_info.max_subslices = sseu->max_subslices; + eus_info.max_eus_per_subslice = sseu->max_eus_per_subslice; + + data_length = eus_info.max_slices * eus_info.max_subslices * + DIV_ROUND_UP(eus_info.max_eus_per_subslice, BITS_PER_BYTE); + length = sizeof(eus_info) + data_length; + + if (query_item->length == 0) { + query_item->length = length; + return 0; + } + + if (query_item->length != length) + return -EINVAL; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), &eus_info, + sizeof(eus_info))) + return -EFAULT; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + + offsetof(struct drm_i915_query_eus_info, data)), + sseu->eu_mask, data_length)) + return -EFAULT; + + return 0; +} + 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 +155,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_ID_SLICES_INFO: + ret = query_slices_info(dev_priv, &item); + break; + case DRM_I915_QUERY_ID_SUBSLICES_INFO: + ret = query_subslices_info(dev_priv, &item); + break; + case DRM_I915_QUERY_ID_EUS_INFO: + ret = query_eus_info(dev_priv, &item); + break; default: return -EINVAL; } + if (ret) + return ret; + if (copy_to_user(user_item_ptr, &item, sizeof(item))) return -EFAULT; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 39e93f10f2cd..d25f21e3c107 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1618,6 +1618,9 @@ struct drm_i915_perf_oa_config { struct drm_i915_query_item { __u64 query_id; +#define DRM_I915_QUERY_ID_SLICES_INFO 0x01 +#define DRM_I915_QUERY_ID_SUBSLICES_INFO 0x02 +#define DRM_I915_QUERY_ID_EUS_INFO 0x03 /* * When set to zero by userspace, this is filled with the size of the @@ -1644,6 +1647,56 @@ struct drm_i915_query { __u64 items_ptr; }; +#define DRM_I915_BIT(bit) (1 << (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_slices_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_subslices_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 + \ + (subslice) / 8] & DRM_I915_BIT((subslice) % 8)) + __u8 data[]; +}; + +/* Data written by the kernel with query DRM_I915_QUERY_ID_EUS_INFO : + * + * data: Each bit indicates whether a subslice is available (1) or fused off + * (0). Use DRM_I915_QUERY_EU_AVAILABLE() to query a given EU's + * availability. + */ +struct drm_i915_query_eus_info { + __u32 max_slices; + __u32 max_subslices; + __u32 max_eus_per_subslice; + +#define DRM_I915_QUERY_EU_AVAILABLE(info, slice, subslice, eu) \ + !!((info)->data[(slice) * ALIGN((info)->max_eus_per_subslice, 8) / 8 * (info)->max_subslices + \ + (subslice) * ALIGN((info)->max_eus_per_subslice, 8) / 8 + \ + (eu) / 8] & DRM_I915_BIT((eu) % 8)) + __u8 data[]; +}; + #if defined(__cplusplus) } #endif
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) Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_query.c | 134 ++++++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 53 +++++++++++++++ 2 files changed, 187 insertions(+)