[PATCH 14/17] drm/xe/oa/uapi: Query OA unit properties
Umesh Nerlige Ramappa
umesh.nerlige.ramappa at intel.com
Sat Dec 23 00:40:47 UTC 2023
On Thu, Dec 07, 2023 at 10:43:26PM -0800, Ashutosh Dixit wrote:
>Implement query for properties of OA units present on a device.
>
>Signed-off-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
>---
> drivers/gpu/drm/xe/xe_oa.h | 2 +
> drivers/gpu/drm/xe/xe_query.c | 81 +++++++++++++++++++++++++++++++++++
> include/uapi/drm/xe_drm.h | 64 +++++++++++++++++++++++++++
> 3 files changed, 147 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_oa.h b/drivers/gpu/drm/xe/xe_oa.h
>index a0f9a876ea6b4..b88914693cdb3 100644
>--- a/drivers/gpu/drm/xe/xe_oa.h
>+++ b/drivers/gpu/drm/xe/xe_oa.h
>@@ -25,5 +25,7 @@ int xe_oa_add_config_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> int xe_oa_remove_config_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
>+u32 xe_oa_timestamp_frequency(struct xe_gt *gt);
>+u16 xe_oa_unit_id(struct xe_hw_engine *hwe);
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>index 56d61bf596b2b..abe2ea088e2ec 100644
>--- a/drivers/gpu/drm/xe/xe_query.c
>+++ b/drivers/gpu/drm/xe/xe_query.c
>@@ -501,6 +501,86 @@ static int query_gt_topology(struct xe_device *xe,
> return 0;
> }
>
>+static size_t calc_oa_unit_query_size(struct xe_device *xe)
>+{
>+ size_t size = sizeof(struct drm_xe_query_oa_units);
>+ struct xe_gt *gt;
>+ int i, id;
>+
>+ for_each_gt(gt, xe, id) {
>+ for (i = 0; i < gt->oa.num_oa_units; i++) {
>+ size += sizeof(struct drm_xe_oa_unit);
>+ size += gt->oa.oa_unit[i].num_engines *
>+ sizeof(struct drm_xe_engine_class_instance);
>+ }
>+ }
>+
>+ return size;
>+}
>+
>+static int query_oa_units(struct xe_device *xe,
>+ struct drm_xe_device_query *query)
>+{
>+ void __user *query_ptr = u64_to_user_ptr(query->data);
>+ size_t size = calc_oa_unit_query_size(xe);
>+ struct drm_xe_query_oa_units *qoa;
>+ enum xe_hw_engine_id hwe_id;
>+ struct drm_xe_oa_unit *du;
>+ struct xe_hw_engine *hwe;
>+ struct xe_oa_unit *u;
>+ int gt_id, i, j, ret;
>+ struct xe_gt *gt;
>+ u8 *pdu;
>+
>+ if (query->size == 0) {
>+ query->size = size;
>+ return 0;
>+ } else if (XE_IOCTL_DBG(xe, query->size != size)) {
>+ return -EINVAL;
>+ }
>+
>+ qoa = kzalloc(size, GFP_KERNEL);
>+ if (!qoa)
>+ return -ENOMEM;
>+
>+ pdu = (u8 *)&qoa->oa_units[0];
>+ for_each_gt(gt, xe, gt_id) {
>+ for (i = 0; i < gt->oa.num_oa_units; i++) {
>+ u = >->oa.oa_unit[i];
>+ du = (struct drm_xe_oa_unit *)pdu;
>+
>+ du->oa_unit_id = u->oa_unit_id;
>+ du->oa_unit_type = u->type;
>+ du->gt_id = gt->info.id;
>+ du->open_stream = !!u->exclusive_stream;
>+ du->oa_timestamp_freq = xe_oa_timestamp_frequency(gt);
>+ du->oa_buf_size = XE_OA_BUFFER_SIZE;
>+ du->num_engines = u->num_engines;
>+
>+ for (j = 1; j < DRM_XE_OA_PROPERTY_MAX; j++)
>+ du->capabilities |= BIT(j);
>+
>+ j = 0;
>+ for_each_hw_engine(hwe, gt, hwe_id) {
>+ if (xe_oa_unit_id(hwe) == u->oa_unit_id) {
>+ du->eci[j].engine_class =
>+ xe_to_user_engine_class[hwe->class];
>+ du->eci[j].engine_instance = hwe->logical_instance;
>+ du->eci[j].gt_id = gt->info.id;
>+ j++;
>+ }
>+ }
>+ pdu += sizeof(*du) + j * sizeof(du->eci[0]);
>+ qoa->num_oa_units++;
>+ }
>+ }
>+
>+ ret = copy_to_user(query_ptr, qoa, size);
>+ kfree(qoa);
>+
>+ return ret ? -EFAULT : 0;
>+}
>+
> static int (* const xe_query_funcs[])(struct xe_device *xe,
> struct drm_xe_device_query *query) = {
> query_engines,
>@@ -510,6 +590,7 @@ static int (* const xe_query_funcs[])(struct xe_device *xe,
> query_hwconfig,
> query_gt_topology,
> query_engine_cycles,
>+ query_oa_units,
> };
>
> int xe_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>index 8156301df7315..5f41c5bfe5e0e 100644
>--- a/include/uapi/drm/xe_drm.h
>+++ b/include/uapi/drm/xe_drm.h
>@@ -517,6 +517,7 @@ struct drm_xe_device_query {
> #define DRM_XE_DEVICE_QUERY_HWCONFIG 4
> #define DRM_XE_DEVICE_QUERY_GT_TOPOLOGY 5
> #define DRM_XE_DEVICE_QUERY_ENGINE_CYCLES 6
>+#define DRM_XE_DEVICE_QUERY_OA_UNITS 7
> /** @query: The type of data to query */
> __u32 query;
>
>@@ -1182,6 +1183,69 @@ enum drm_xe_oa_unit_type {
> DRM_XE_OA_UNIT_TYPE_OAM,
> };
>
>+/**
>+ * struct drm_xe_query_oa_units - describe OA units
>+ *
>+ * If a query is made with a struct drm_xe_device_query where .query
>+ * is equal to DRM_XE_DEVICE_QUERY_OA_UNITS, then the reply uses struct
>+ * drm_xe_query_oa_units in .data.
>+ *
>+ * When there is an @open_stream, the query returns properties specific to
>+ * that @open_stream. Else default properties are returned.
>+ */
>+struct drm_xe_query_oa_units {
>+ /** @extensions: Pointer to the first extension struct, if any */
>+ __u64 extensions;
>+
>+ /** @num_oa_units: number of OA units returned in oau[] */
>+ __u32 num_oa_units;
>+
>+ /** @pad: MBZ */
>+ __u32 pad;
>+
>+ /** @reserved: MBZ */
>+ __u64 reserved[4];
For some reason I have assumed reserved fields are added only at the end
of the uApi struct, not sure though.
>+
>+ /** @oa_units: OA units returned for this device */
>+ struct drm_xe_oa_unit {
>+ /** @oa_unit_id: OA unit ID */
>+ __u16 oa_unit_id;
>+
>+ /** @oa_unit_type: OA unit type of @drm_xe_oa_unit_type */
>+ __u16 oa_unit_type;
>+
>+ /** @gt_id: GT ID for this OA unit */
>+ __u16 gt_id;
>+
>+ /** @open_stream: True if a stream is open on the OA unit */
>+ __u16 open_stream;
>+
>+ /** @internal_events: True if internal events are available */
>+ __u16 internal_events;
>+
>+ /** @pad: MBZ */
>+ __u16 pad;
__u16 pad[3] for 64bit alignment
>+
>+ /** @capabilities: OA capabilities bit-mask */
>+ __u64 capabilities;
>+
>+ /** @oa_timestamp_freq: OA timestamp freq */
>+ __u64 oa_timestamp_freq;
>+
>+ /** @oa_buf_size: OA buffer size */
>+ __u64 oa_buf_size;
>+
>+ /** @reserved: MBZ */
>+ __u64 reserved[4];
>+
>+ /** @num_engines: number of engines in @eci array */
>+ __u64 num_engines;
>+
>+ /** @eci: engines attached to this OA unit */
>+ struct drm_xe_engine_class_instance eci[];
>+ } oa_units[];
nesting of flexible arrays; not sure about that. i think some compilers
may throw an error/warning. Sending an old message from Joonas offline.
In general, I feel the pad and reserved fields sprinkled into the
structure. If we can avoid that in a way that they are all located at
the end of the struct, I think that would look good. Not sure about the
technical aspect though. I always assumed they were meant to be at the
end (but then structs are nested anyways, so really not sure).
Thanks,
Umesh
>+};
>+
> /** enum drm_xe_oa_format_type - OA format types */
> enum drm_xe_oa_format_type {
> DRM_XE_OA_FMT_TYPE_OAG,
>--
>2.41.0
>
More information about the Intel-xe
mailing list