[PATCH i-g-t v2 1/2] tests/intel/xe_pmu: Add engine activity test for all functions

Umesh Nerlige Ramappa umesh.nerlige.ramappa at intel.com
Wed Mar 19 23:11:06 UTC 2025


I missed out some comments, adding it here:

On Wed, Mar 19, 2025 at 03:22:00PM -0700, Umesh Nerlige Ramappa wrote:
>On Wed, Mar 12, 2025 at 11:54:30AM +0530, Riana Tauro wrote:
>>Add a test that runs workload on all functions simultaneously and
>>validates that all engines are equally and fully loaded for the
>>entire execution quantum of the function
>>
>>v2: add a different function for function config
>>   add function details to log
>>   move pmu_fd to struct
>>   enable and provision VF's (Umesh)
>>
>>Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>---
>>tests/intel/xe_pmu.c | 130 +++++++++++++++++++++++++++++++++++++++----
>>1 file changed, 120 insertions(+), 10 deletions(-)
>>
>>diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>>index 66edf24ad..a95ee1777 100644
>>--- a/tests/intel/xe_pmu.c
>>+++ b/tests/intel/xe_pmu.c
>>@@ -14,18 +14,22 @@
>>
>>#include "igt.h"
>>#include "igt_perf.h"
>>+#include "igt_sriov_device.h"
>>#include "igt_sysfs.h"
>>
>>#include "xe/xe_gt.h"
>>#include "xe/xe_ioctl.h"
>>#include "xe/xe_spin.h"
>>+#include "xe/xe_sriov_provisioning.h"
>>
>>#define SLEEP_DURATION 2 /* in seconds */
>>+#define TOTAL_EXEC_QUANTUM 128 /* in ms */
>>/* flag masks */
>>#define TEST_LOAD		BIT(0)
>>#define TEST_TRAILING_IDLE	BIT(1)
>>
>>const double tolerance = 0.1;
>>+char xe_device[NAME_MAX];
>
>These globals should be static.
>
>Optionally, for easier reviews, it's better to split the patch in two 
>- (1) make xe_device global (2) Add the engine activity test.
>
>With just the static change, this is
>
>Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>
>Thanks,
>Umesh
>
>>
>>#define test_each_engine(test, fd, hwe) \
>>	igt_subtest_with_dynamic(test) \
>>@@ -92,7 +96,7 @@ static unsigned long read_idle_residency(int fd, int gt)
>>	return residency;
>>}
>>
>>-static uint64_t add_format_config(const char *xe_device, const char *format, uint64_t val)
>>+static uint64_t add_format_config(const char *format, uint64_t val)
>>{
>>	int ret;
>>	uint32_t shift;
>>@@ -105,26 +109,30 @@ static uint64_t add_format_config(const char *xe_device, const char *format, uin
>>	return config;
>>}
>>
>>-static uint64_t get_event_config(int xe, unsigned int gt, struct drm_xe_engine_class_instance *eci,
>>+static uint64_t get_event_config(unsigned int gt, struct drm_xe_engine_class_instance *eci,
>>				 const char *event)
>>{
>>-	char xe_device[100];
>>	uint64_t pmu_config = 0;
>>	int ret;
>>
>>-	xe_perf_device(xe, xe_device, sizeof(xe_device));
>>	ret = perf_event_config(xe_device, event, &pmu_config);
>>	igt_assert(ret >= 0);
>>-	pmu_config |= add_format_config(xe_device, "gt", gt);
>>+	pmu_config |= add_format_config("gt", gt);
>>
>>	if (eci) {
>>-		pmu_config |= add_format_config(xe_device, "engine_class", eci->engine_class);
>>-		pmu_config |= add_format_config(xe_device, "engine_instance", eci->engine_instance);
>>+		pmu_config |= add_format_config("engine_class", eci->engine_class);
>>+		pmu_config |= add_format_config("engine_instance", eci->engine_instance);
>>	}
>>
>>	return pmu_config;
>>}
>>
>>+static uint64_t get_event_config_fn(unsigned int gt, int function,
>>+				    struct drm_xe_engine_class_instance *eci, const char *event)
>>+{
>>+	return get_event_config(gt, eci, event) | add_format_config("function", function);
>>+}
>>+
>>/**
>> * SUBTEST: engine-activity-idle
>> * Description: Test to validate engine activity shows no load when idle
>>@@ -144,10 +152,10 @@ static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci, un
>>	uint32_t vm;
>>	int pmu_fd[2];
>>
>>-	config = get_event_config(fd, eci->gt_id, eci, "engine-active-ticks");
>>+	config = get_event_config(eci->gt_id, eci, "engine-active-ticks");
>>	pmu_fd[0] = open_group(fd, config, -1);
>>
>>-	config = get_event_config(fd, eci->gt_id, eci, "engine-total-ticks");
>>+	config = get_event_config(eci->gt_id, eci, "engine-total-ticks");
>>	pmu_fd[1] = open_group(fd, config, pmu_fd[0]);
>>
>>	vm = xe_vm_create(fd, 0, 0);
>>@@ -188,6 +196,73 @@ static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci, un
>>		igt_assert(!engine_active_ticks);
>>}
>>
>>+/**
>>+ * SUBTEST: all-fn-engine-activity-load
>>+ * Description: Test to validate engine activity by running load on all functions simultaneously
>>+ */
>>+static void engine_activity_all_fn(int fd, struct drm_xe_engine_class_instance *eci, int num_fns)
>>+{
>>+	uint64_t config, engine_active_ticks, engine_total_ticks;
>>+	uint64_t after[2 * num_fns], before[2 * num_fns];
>>+	struct pmu_function {
>>+		struct xe_cork *cork;
>>+		uint32_t vm;
>>+		uint64_t pmu_fd[2];
>>+		int fd;
>>+	} fn[num_fns];
>>+	int i;
>>+
>>+	fn[0].pmu_fd[0] = -1;
>>+	for (i = 0; i < num_fns; i++) {
>>+		config = get_event_config_fn(eci->gt_id, i, eci, "engine-active-ticks");
>>+		fn[i].pmu_fd[0] = open_group(fd, config, fn[0].pmu_fd[0]);
>>+
>>+		config = get_event_config_fn(eci->gt_id, i, eci, "engine-total-ticks");
>>+		fn[i].pmu_fd[1] = open_group(fd, config, fn[0].pmu_fd[0]);
>>+
>>+		if (i > 0)
>>+			fn[i].fd = igt_sriov_open_vf_drm_device(fd, i);
>>+		else
>>+			fn[i].fd = fd;
>>+
>>+		igt_assert_fd(fn[i].fd);
>>+
>>+		fn[i].vm = xe_vm_create(fn[i].fd, 0, 0);
>>+		fn[i].cork = xe_cork_create_opts(fn[i].fd, eci, fn[i].vm, 1, 1);
>>+		xe_cork_sync_start(fn[i].fd, fn[i].cork);
>>+	}
>>+
>>+	pmu_read_multi(fn[0].pmu_fd[0], 2 * num_fns, before);
>>+	usleep(SLEEP_DURATION * USEC_PER_SEC);
>>+	pmu_read_multi(fn[0].pmu_fd[0], 2 * num_fns, after);
>>+
>>+	for (i = 0; i < num_fns; i++) {
>>+		int idx = i * 2;
>>+
>>+		xe_cork_sync_end(fn[i].fd, fn[i].cork);
>>+		engine_active_ticks = after[idx] - before[idx];
>>+		engine_total_ticks = after[idx + 1] - before[idx + 1];
>>+
>>+		igt_debug("[%d] Engine active ticks: after %ld, before %ld delta %ld\n", i,
>>+			  after[idx], before[idx], engine_active_ticks);
>>+		igt_debug("[%d] Engine total ticks: after %ld, before %ld delta %ld\n", i,
>>+			  after[idx + 1], before[idx + 1], engine_total_ticks);
>>+
>>+		if (fn[i].cork)
>>+			xe_cork_destroy(fn[i].fd, fn[i].cork);
>>+
>>+		xe_vm_destroy(fn[i].fd, fn[i].vm);
>>+
>>+		close(fn[i].pmu_fd[0]);
>>+		close(fn[i].pmu_fd[1]);
>>+
>>+		if (i > 0)
>>+			close(fn[i].fd);
>>+
>>+		assert_within_epsilon(engine_active_ticks, engine_total_ticks, tolerance);
>>+	}
>>+}
>>+
>>/**
>> * SUBTEST: gt-c6-idle
>> * Description: Basic residency test to validate idle residency
>>@@ -202,7 +277,7 @@ static void test_gt_c6_idle(int xe, unsigned int gt)
>>	uint64_t val;
>>
>>	/* Get the PMU config for the gt-c6 event */
>>-	pmu_config = get_event_config(xe, gt, NULL, "gt-c6-residency");
>>+	pmu_config = get_event_config(gt, NULL, "gt-c6-residency");
>>
>>	pmu_fd = open_pmu(xe, pmu_config);
>>
>>@@ -226,13 +301,34 @@ static void test_gt_c6_idle(int xe, unsigned int gt)
>>	close(pmu_fd);
>>}
>>
>>+static unsigned int enable_and_provision_vfs(int fd)
>>+{
>>+	unsigned int gt, num_vfs;
>>+
>>+	igt_sriov_enable_vfs(fd, 2);
>>+	num_vfs = igt_sriov_get_enabled_vfs(fd);
>>+	igt_require(num_vfs);

igt_assert(num_vfs == 2)

I am thinking maybe you could use globals instead of the define and 
initialize them here.

	pf_quanta_msec = 64;
	vf_quanta_msec = 32;
	total_quanta = pf_quanta_msec + (num_vfs * vf_quanta_msec);

>>+
>>+	/* Set 32ms for VF execution quantum and 64ms for PF execution quantum */
>>+	xe_for_each_gt(fd, gt) {
>>+		xe_sriov_set_sched_if_idle(fd, gt, 0);
>>+		for (int fn = 0; fn <= num_vfs; fn++)
>>+			xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ? TOTAL_EXEC_QUANTUM / 4 :
>>+						     TOTAL_EXEC_QUANTUM / 2);
>>+	}

maybe also cover the case for sched_if_idle = 1?

Thanks,
Umesh

>>+
>>+	return num_vfs;
>>+}
>>+
>>igt_main
>>{
>>	int fd, gt;
>>+	unsigned int num_fns;
>>	struct drm_xe_engine_class_instance *eci;
>>
>>	igt_fixture {
>>		fd = drm_open_driver(DRIVER_XE);
>>+		xe_perf_device(fd, xe_device, sizeof(xe_device));
>>	}
>>
>>	igt_describe("Validate PMU gt-c6 residency counters when idle");
>>@@ -254,6 +350,20 @@ igt_main
>>	test_each_engine("engine-activity-load", fd, eci)
>>		engine_activity(fd, eci, TEST_LOAD);
>>
>>+	igt_subtest_group {
>>+		igt_fixture {
>>+			igt_require(igt_sriov_is_pf(fd));
>>+			num_fns = enable_and_provision_vfs(fd) + 1;

How does this work if VFs are already provisioned before starting the 
test?


>>+		}
>>+
>>+		igt_describe("Validate engine activity on all functions");
>>+		test_each_engine("all-fn-engine-activity-load", fd, eci)
>>+			engine_activity_all_fn(fd, eci, num_fns);
>>+
>>+		igt_fixture
>>+			igt_sriov_disable_vfs(fd);
>>+	}
>>+
>>	igt_fixture {
>>		close(fd);
>>	}
>>-- 
>>2.47.1
>>


More information about the igt-dev mailing list