Re: [PATCH i-g-t 3/3] tests/intel/xe_pmu: Add idle engine activity test

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Wed, Feb 12, 2025 at 03:28:29PM +0530, Riana Tauro wrote:
>Add a test to validate engine is idle by reading PMU counters
>(engine-active-ticks, engine-total-ticks) when idle
>
>Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>---
> tests/intel/xe_pmu.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
>diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>index cbb825755..b47be23ac 100644
>--- a/tests/intel/xe_pmu.c
>+++ b/tests/intel/xe_pmu.c
>@@ -21,6 +21,9 @@
> #define SLEEP_DURATION 2 /* in seconds */
> const double tolerance = 0.1;
>
>+/* flag masks */
>+#define TEST_BUSY	BIT(0)

s/BUSY/ACTIVE/

>+
> #define assert_within_epsilon(x, ref, tolerance) \
> 	igt_assert_f((double)(x) <= (1.0 + (tolerance)) * (double)(ref) && \
> 		     (double)(x) >= (1.0 - (tolerance)) * (double)(ref), \
>@@ -79,11 +82,13 @@ static uint64_t get_event_config(int xe, unsigned int gt, struct drm_xe_engine_c
> }
>
> /**
>+ * SUBTEST: engine-activity-idle
>+ * Description: Test to validate engine activity shows no load when idle
>+ *
>  * SUBTEST: engine-activity
>- * Description: Test to validate engine activity stats by running a workload and
>- *              reading the active ticks and total ticks PMU counters
>+ * Description: Test to validate engine activity stats by running full load
>  */
>-static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci)
>+static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci, unsigned int flags)
> {
> 	uint64_t config, busy_ticks, total_ticks, before[2], after[2];
> 	struct xe_cork *cork = NULL;
>@@ -97,14 +102,18 @@ static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci)
> 	pmu_fd[1] = open_group(fd, config, pmu_fd[0]);
>
> 	vm = xe_vm_create(fd, 0, 0);
>-	cork = xe_cork_create_opts(fd, eci, vm, 1, 1);
>-	xe_cork_sync_start(fd, cork);
>+
>+	if (flags & TEST_BUSY) {
>+		cork = xe_cork_create_opts(fd, eci, vm, 1, 1);
>+		xe_cork_sync_start(fd, cork);
>+	}
>
> 	pmu_read_multi(pmu_fd[0], 2, before);
> 	usleep(SLEEP_DURATION * USEC_PER_SEC);

Usually, I see 3 cases for a single type of test

1) idle (no spinner is run)
2) active (spinner is run, both samples are captured while running).
3) active, then idle (spinner is run, capture second sample after ending 
spinner)

Can you also please add case (3). Rest looks good.

Thanks,
Umesh
> 	pmu_read_multi(pmu_fd[0], 2, after);
>
>-	xe_cork_sync_end(fd, cork);
>+	if (flags & TEST_BUSY)
>+		xe_cork_sync_end(fd, cork);
>
> 	busy_ticks = after[0] - before[0];
> 	total_ticks = after[1] - before[1];
>@@ -122,7 +131,10 @@ static void engine_activity(int fd, struct drm_xe_engine_class_instance *eci)
> 	close(pmu_fd[0]);
> 	close(pmu_fd[1]);
>
>-	assert_within_epsilon(busy_ticks, total_ticks, tolerance);
>+	if (flags & TEST_BUSY)
>+		assert_within_epsilon(busy_ticks, total_ticks, tolerance);
>+	else
>+		igt_assert(!busy_ticks);
> }
>
> igt_main
>@@ -134,12 +146,19 @@ igt_main
> 		fd = drm_open_driver(DRIVER_XE);
> 	}
>
>+	igt_describe("Validate there is no engine activity when idle");
>+	igt_subtest_with_dynamic("engine-activity-idle")
>+		xe_for_each_engine(fd, hwe)
>+			igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe->engine_class),
>+				      hwe->engine_instance)
>+				engine_activity(fd, hwe, 0);
>+
> 	igt_describe("Validate engine activity with workload running by reading pmu counters");
> 	igt_subtest_with_dynamic("engine-activity")
> 		xe_for_each_engine(fd, hwe)
> 			igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe->engine_class),
> 				      hwe->engine_instance)
>-				engine_activity(fd, hwe);
>+				engine_activity(fd, hwe, TEST_BUSY);
>
> 	igt_fixture {
> 		close(fd);
>-- 
>2.47.1
>