[PATCH i-g-t v10 3/5] tools/gputop.src/xe_gputop: Use direct ioctls with card_fd cleanup

[Soham Purkait <soham.purkait@intel.com>]

During xe engine initialization, card_fd is needed to query engines
and obtain PMU busyness configuration, but keeping it open makes the
process appear as a DRM client, which conflicts with KMS test
requirements. To avoid this, close card_fd once engine discovery is
complete and replace the lib/xe/* dependency with direct ioctl calls,
simplifying the implementation and reducing IGT test library dependencies.

v1:
 - Initialize pmu_device_obj to null.
v2:
 - Remove dependency on lib/xe/* libraries. (Kamil)
v3:
 - Refactor ioctl failure check. (Vinay)
 - Check q.size only once. (Vinay)
 - Add num_bytes for asprintf return value. (Vinay)
v4:
 - Replace malloc/memset with calloc. (Vinay)
 - Refactor the commit message to make it concise. (Kamil)

Fixes: c8106465683f ("tools/gputop/xe_gputop: Add gputop support for xe specific devices")
Signed-off-by: Soham Purkait <soham.purkait@intel.com>

Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 tools/gputop.src/xe_gputop.c | 150 ++++++++++++++++++++++++++---------
 tools/gputop.src/xe_gputop.h |   8 +-
 2 files changed, 115 insertions(+), 43 deletions(-)

diff --git a/tools/gputop.src/xe_gputop.c b/tools/gputop.src/xe_gputop.c
index bb2caa6ea..30f9c593f 100644
--- a/tools/gputop.src/xe_gputop.c
+++ b/tools/gputop.src/xe_gputop.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: MIT
 /*
- * Copyright © 2025 Intel Corporation
+ * Copyright © 2025-2026 Intel Corporation
  */
 
+#include <sys/ioctl.h>
+
+#include "igt_perf.h"
 #include "xe_gputop.h"
 
 #define engine_ptr(pmu_device, n) (&(pmu_device)->engine + (n))
@@ -65,7 +68,7 @@ void xe_clean_up(void *obj, int len)
 			}
 			if (gputop_dev[i].pmu_device_obj->device)
 				free(gputop_dev[i].pmu_device_obj->device);
-			free(gputop_dev->pmu_device_obj);
+			free(gputop_dev[i].pmu_device_obj);
 		}
 	}
 }
@@ -92,6 +95,7 @@ void xe_gputop_init(void *ptr, int index,
 
 	obj = ((struct xe_gputop *)ptr) + index;
 	obj->card = card;
+	obj->pmu_device_obj = NULL;
 }
 
 static int pmu_format_shift(int xe, const char *name)
@@ -121,20 +125,50 @@ static int engine_cmp(const void *__a, const void *__b)
 		return a->drm_xe_engine.engine_instance - b->drm_xe_engine.engine_instance;
 }
 
+static int engine_query(int fd, struct drm_xe_query_engines **engine_q)
+{
+	struct drm_xe_device_query q;
+	int ret = 0, num_eng;
+
+	memset(&q, 0, sizeof(q));
+	q.query = DRM_XE_DEVICE_QUERY_ENGINES;
+
+	ret = ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &q);
+	if (ret != 0 || q.size == 0)
+		return 0;
+
+	num_eng = (int)((q.size - sizeof(struct drm_xe_query_engines)) /
+			sizeof(struct drm_xe_engine_class_instance));
+
+	*engine_q = (struct drm_xe_query_engines *)calloc(1, q.size);
+	if (!*engine_q)
+		return -1;
+
+	q.data = (uintptr_t)(*engine_q);
+
+	ret = ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &q);
+	if (ret != 0) {
+		free(*engine_q);
+		return -1;
+	}
+
+	return num_eng;
+}
+
 void *xe_populate_engines(const void *obj, int index)
 {
 	struct xe_gputop *ptr = ((struct xe_gputop *)obj) + index;
 	struct igt_device_card *card = ptr->card;
 	uint64_t engine_active_config, engine_total_config;
 	uint64_t engine_class, engine_instance, gt_shift;
-	struct drm_xe_engine_class_instance *hwe;
-	struct xe_pmu_device *engines;
+	struct drm_xe_query_engines *engine_q = NULL;
+	struct xe_pmu_device *engines = NULL;
 	char device[30];
-	int ret = 0;
-	int card_fd;
+	int ret = 0, num_eng;
+	int card_fd = -1;
 
-	if (!card || !strlen(card->card) || !strlen(card->render))
-		return NULL;
+	if (!card || (!strlen(card->card) && !strlen(card->render)))
+		goto err;
 
 	if (strlen(card->card)) {
 		card_fd = igt_open_card(card);
@@ -142,39 +176,53 @@ void *xe_populate_engines(const void *obj, int index)
 		card_fd = igt_open_render(card);
 	} else {
 		fprintf(stderr, "Failed to detect device!\n");
-		return NULL;
+		goto err;
+	}
+
+	num_eng = engine_query(card_fd, &engine_q);
+	if (num_eng <= 0) {
+		fprintf(stderr, "Engine query failed!\n");
+		goto err;
 	}
-	xe_device_get(card_fd);
-	engines = malloc(sizeof(struct xe_pmu_device) +
-			 xe_number_engines(card_fd) * sizeof(struct xe_engine));
-	if (!engines)
-		return NULL;
 
-	memset(engines, 0, sizeof(struct xe_pmu_device) +
-	       xe_number_engines(card_fd) * sizeof(struct xe_engine));
+	engines = calloc(1, sizeof(struct xe_pmu_device) +
+			 num_eng * sizeof(struct xe_engine));
+	if (!engines)
+		goto err;
 
 	engines->num_engines = 0;
 	gt_shift = pmu_format_shift(card_fd, "gt");
 	engine_class = pmu_format_shift(card_fd, "engine_class");
 	engine_instance = pmu_format_shift(card_fd, "engine_instance");
 	xe_perf_device(card_fd, device, sizeof(device));
+	close(card_fd);
+	card_fd = -1;
+
 	engines->device = strdup(device);
+	if (!engines->device) {
+		errno = ENOMEM;
+		goto err;
+	}
 	ret = perf_event_config(device, "engine-active-ticks", &engine_active_config);
 	if (ret < 0)
-		return NULL;
+		goto err;
 
 	ret = perf_event_config(device, "engine-total-ticks", &engine_total_config);
 	if (ret < 0)
-		return NULL;
+		goto err;
 
-	xe_for_each_engine(card_fd, hwe) {
+	while (engines->num_engines < engine_q->num_engines) {
+		int num_bytes = 0;
 		uint64_t  param_config;
 		struct xe_engine *engine;
+		struct drm_xe_engine_class_instance hwe;
+
+		hwe = engine_q->engines[engines->num_engines].instance;
 
 		engine = engine_ptr(engines, engines->num_engines);
-		param_config = (uint64_t)hwe->gt_id << gt_shift | hwe->engine_class << engine_class
-			| hwe->engine_instance << engine_instance;
-		engine->drm_xe_engine = *hwe;
+		param_config = (uint64_t)hwe.gt_id << gt_shift | hwe.engine_class << engine_class
+			| hwe.engine_instance << engine_instance;
+		engine->drm_xe_engine = hwe;
 		engine->engine_active_ticks.config = engine_active_config | param_config;
 		engine->engine_total_ticks.config = engine_total_config | param_config;
 
@@ -184,12 +232,12 @@ void *xe_populate_engines(const void *obj, int index)
 			break;
 		}
 
-		ret = asprintf(&engine->display_name, "GT:%u %s/%u",
-			       hwe->gt_id,
-			       class_display_name(engine->drm_xe_engine.engine_class),
-			       engine->drm_xe_engine.engine_instance);
+		num_bytes = asprintf(&engine->display_name, "GT:%u %s/%u",
+				     hwe.gt_id,
+				     class_display_name(engine->drm_xe_engine.engine_class),
+				     engine->drm_xe_engine.engine_instance);
 
-		if (ret <= 0) {
+		if (num_bytes <= 0) {
 			ret = errno;
 			break;
 		}
@@ -197,9 +245,12 @@ void *xe_populate_engines(const void *obj, int index)
 		engines->num_engines++;
 	}
 
-	if (!ret) {
+	free(engine_q);
+	engine_q = NULL;
+
+	if (ret) {
 		errno = ret;
-		return NULL;
+		goto err;
 	}
 
 	qsort(engine_ptr(engines, 0), engines->num_engines,
@@ -208,9 +259,26 @@ void *xe_populate_engines(const void *obj, int index)
 	ptr->pmu_device_obj = engines;
 
 	return engines;
+
+err:
+	if (card_fd >= 0)
+		close(card_fd);
+	if (engines) {
+		for (unsigned int i = 0; i < engines->num_engines; i++) {
+			struct xe_engine *engine = engine_ptr(engines, i);
+
+			free(engine->display_name);
+		}
+		if (engines->device)
+			free(engines->device);
+		free(engines);
+	}
+	if (engine_q)
+		free(engine_q);
+	return NULL;
 }
 
-static uint64_t pmu_read_multi(int fd, unsigned int num, uint64_t *val)
+static int pmu_read_multi(int fd, unsigned int num, uint64_t *val)
 {
 	uint64_t buf[2 + num];
 	unsigned int i;
@@ -219,15 +287,16 @@ static uint64_t pmu_read_multi(int fd, unsigned int num, uint64_t *val)
 	memset(buf, 0, sizeof(buf));
 
 	len = read(fd, buf, sizeof(buf));
-	assert(len == sizeof(buf));
+	if (len != sizeof(buf))
+		return -1;
 
 	for (i = 0; i < num; i++)
 		val[i] = buf[2 + i];
 
-	return buf[1];
+	return 0;
 }
 
-void xe_pmu_sample(const void *obj, int index)
+int xe_pmu_sample(const void *obj, int index)
 {
 	struct xe_gputop *ptr = ((struct xe_gputop *)obj) + index;
 	struct xe_pmu_device *engines = ptr->pmu_device_obj;
@@ -235,7 +304,8 @@ void xe_pmu_sample(const void *obj, int index)
 	uint64_t val[2 + num_val];
 	unsigned int i;
 
-	pmu_read_multi(engines->fd, num_val, val);
+	if (pmu_read_multi(engines->fd, num_val, val) < 0)
+		return -1;
 
 	for (i = 0; i < engines->num_engines; i++) {
 		struct xe_engine *engine = engine_ptr(engines, i);
@@ -243,6 +313,8 @@ void xe_pmu_sample(const void *obj, int index)
 		update_sample(&engine->engine_active_ticks, val);
 		update_sample(&engine->engine_total_ticks, val);
 	}
+
+	return 0;
 }
 
 int xe_pmu_init(const void *obj, int index)
@@ -263,11 +335,11 @@ int xe_pmu_init(const void *obj, int index)
 		fd = _open_pmu(type, &engines->num_counters, &engine->engine_active_ticks,
 			       &engines->fd);
 		if (fd < 0)
-			return -1;
+			return fd;
 		fd = _open_pmu(type, &engines->num_counters, &engine->engine_total_ticks,
 			       &engines->fd);
 		if (fd < 0)
-			return -1;
+			return fd;
 	}
 	return 0;
 }
@@ -280,7 +352,10 @@ static double pmu_active_percentage(struct xe_engine *engine)
 				 engine->engine_total_ticks.val.prev;
 	double percentage;
 
-	percentage = (pmu_active_ticks * 100) / pmu_total_ticks;
+	if (pmu_total_ticks)
+		percentage = (pmu_active_ticks * 100) / pmu_total_ticks;
+	else
+		percentage = 0.0;
 
 	return clamp(percentage, 0., 100.);
 }
@@ -359,4 +434,3 @@ int xe_print_engines(const void *obj, int index, int lines, int w, int h)
 
 	return lines;
 }
-
diff --git a/tools/gputop.src/xe_gputop.h b/tools/gputop.src/xe_gputop.h
index 1e3856071..d89db400c 100644
--- a/tools/gputop.src/xe_gputop.h
+++ b/tools/gputop.src/xe_gputop.h
@@ -1,17 +1,15 @@
 /* SPDX-License-Identifier: MIT */
 /*
- * Copyright © 2025 Intel Corporation
+ * Copyright © 2025-2026 Intel Corporation
  */
 
 #ifndef __XE_GPUTOP_H__
 #define __XE_GPUTOP_H__
 
 #include <dirent.h>
+#include <xe_drm.h>
 
-#include "igt_device_scan.h"
-#include "igt_perf.h"
 #include "utils.h"
-#include "xe/xe_query.h"
 
 struct xe_pmu_pair {
 	uint64_t cur;
@@ -52,7 +50,7 @@ struct xe_gputop {
 void xe_gputop_init(void *ptr, int index, struct igt_device_card *card);
 void *xe_populate_engines(const void *obj, int index);
 int xe_pmu_init(const void *obj, int index);
-void xe_pmu_sample(const void *obj, int index);
+int xe_pmu_sample(const void *obj, int index);
 int xe_print_engines(const void *obj, int index, int lines, int w, int h);
 void xe_clean_up(void *obj, int len);
 
-- 
2.34.1