[PATCH i-g-t 4/8] tests/intel/xe_drm_fdinfo: Add single engine tests

Lucas De Marchi lucas.demarchi at intel.com
Mon Jul 1 17:35:24 UTC 2024


On Sat, Jun 22, 2024 at 07:00:58AM GMT, Umesh Nerlige Ramappa wrote:
>Add simple tests that submit work to one engine and measure utilization
>per class.
>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>---
> tests/intel/xe_drm_fdinfo.c | 109 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 109 insertions(+)
>
>diff --git a/tests/intel/xe_drm_fdinfo.c b/tests/intel/xe_drm_fdinfo.c
>index 27459b7f1..852931d71 100644
>--- a/tests/intel/xe_drm_fdinfo.c
>+++ b/tests/intel/xe_drm_fdinfo.c
>@@ -25,6 +25,15 @@
>  * SUBTEST: basic
>  * Description: Check if basic fdinfo content is present
>  *
>+ * SUBTEST: drm-idle
>+ * Description: Check that engines show no load when idle
>+ *
>+ * SUBTEST: drm-busy-idle
>+ * Description: Check that engines show load when idle after busy
>+ *
>+ * SUBTEST: drm-busy-idle-isolation
>+ * Description: Check that engine load does not spill over to other drm clients
>+ *

can we split the mem-related basic check from the utilization-related
basic checks?

>  * SUBTEST: drm-total-resident
>  * Description: Create and compare total and resident memory consumption by client
>  *
>@@ -39,11 +48,18 @@ IGT_TEST_DESCRIPTION("Read and verify drm client memory consumption and engine u
>
> #define BO_SIZE (65536)
>
>+/* flag masks */
>+#define TEST_BUSY		(1 << 0)
>+#define TEST_TRAILING_IDLE	(1 << 1)
>+#define TEST_ISOLATION		(1 << 2)

shoud we document the mapping to the subtests?

>+
> struct pceu_cycles {
> 	uint64_t cycles;
> 	uint64_t total_cycles;
> };
>
>+const unsigned long batch_duration_ns = 500e6;

please use * NSEC_PER_SEC

>+
> static const char *engine_map[] = {
> 	"rcs",
> 	"bcs",
>@@ -76,6 +92,27 @@ static void read_engine_cycles(int xe, struct pceu_cycles *pceu)
> 		pceu[class].total_cycles = info.total_cycles[class];
> 	}
> }
>+
>+/*
>+ * Helper for cases where we assert on time spent sleeping (directly or
>+ * indirectly), so make it more robust by ensuring the system sleep time
>+ * is within test tolerance to start with.
>+ */
>+static unsigned int measured_usleep(unsigned int usec)
>+{
>+	struct timespec ts = { };
>+	unsigned int slept;
>+
>+	slept = igt_nsec_elapsed(&ts);
>+	igt_assert(slept == 0);
>+	do {
>+		usleep(usec - slept);

mismatch between usec and nsec here in the first sleep. I guess it's
easier to convert the arg to nsec and then always have the same unit

>+		slept = igt_nsec_elapsed(&ts) / 1000;

NSEC_PER_USEC

>+	} while (slept < usec);
>+
>+	return igt_nsec_elapsed(&ts);

wrong unit? you receive usec as arg and are supposed to return usec, but
rather return nsec.

Also, I'm not sure why exactly this is needed. usleep() from the C
library already guarantees it's going to sleep for *at least* the
specified amount.

usleep(3):
...
DESCRIPTION
        The usleep() function suspends execution of the calling thread
        for (at least) usec microseconds.  The sleep may be lengthened
        slightly by any system activity or by the time spent processing
        the call or by the granularity of system timers.

RETURN VALUE
        The usleep() function returns 0 on success.  On error, -1 is
        returned, with errno set to indicate the error.


>+}
>+
> /* Subtests */
> static void test_active(int fd, struct drm_xe_engine *engine)
> {
>@@ -451,6 +488,66 @@ xe_spin_ctx_destroy(int fd, struct xe_spin_ctx *ctx)
> 	free(ctx);
> }
>
>+static void
>+check_results(struct pceu_cycles *s1, struct pceu_cycles *s2,
>+	      int class, int width, unsigned int flags)
>+{
>+	double percent;
>+
>+	igt_debug("%s: sample 1: cycles %lu, total_cycles %lu\n",
>+		  engine_map[class], s1[class].cycles, s1[class].total_cycles);
>+	igt_debug("%s: sample 2: cycles %lu, total_cycles %lu\n",
>+		  engine_map[class], s2[class].cycles, s2[class].total_cycles);
>+
>+	percent = ((s2[class].cycles - s1[class].cycles) * 100) /
>+		  ((s2[class].total_cycles + 1) - s1[class].total_cycles);
>+
>+	/* for parallel engines scale the busyness with width */

s/parallel engines/parallel submission/ ?

>+	percent = percent / width;
>+
>+	igt_debug("%s: percent: %f\n", engine_map[class], percent);
>+
>+	if (flags & TEST_BUSY)
>+		igt_assert(percent >= 95 && percent <= 105);

percent should never be > 100. Also, for >= 95 to be true this needs to
be the only client.  Should we check for number of clients and skip if
we find others?

>+	else
>+		igt_assert(!percent);
>+}
>+
>+static void
>+single(int fd, struct drm_xe_engine_class_instance *hwe, int width, int count,
>+       unsigned int flags)
>+{
>+	struct pceu_cycles pceu1[DRM_XE_ENGINE_CLASS_COMPUTE + 1];
>+	struct pceu_cycles pceu2[DRM_XE_ENGINE_CLASS_COMPUTE + 1];
>+	struct xe_spin_ctx *ctx = NULL;
>+	int local_fd = fd;
>+	uint32_t vm;
>+
>+	if (flags & TEST_ISOLATION)
>+		local_fd = drm_reopen_driver(fd);

so you have 2 clients in the same process... Maybe add this info to the
subtest description above.

>+
>+	vm = xe_vm_create(local_fd, 0, 0);
>+	if (flags & TEST_BUSY) {
>+		ctx = xe_spin_ctx_init(local_fd, hwe, vm, width, count);
>+		xe_spin_sync_start(local_fd, ctx);
>+	}
>+
>+	read_engine_cycles(local_fd, pceu1);
>+	measured_usleep(batch_duration_ns / 1000);
>+	if (flags & TEST_TRAILING_IDLE)
>+		xe_spin_sync_end(local_fd, ctx);
>+	read_engine_cycles(local_fd, pceu2);
>+
>+	check_results(pceu1, pceu2, hwe->engine_class, width, flags);
>+
>+	xe_spin_sync_end(local_fd, ctx);
>+	xe_spin_ctx_destroy(local_fd, ctx);
>+	xe_vm_destroy(local_fd, vm);
>+
>+	if (flags & TEST_ISOLATION)
>+		close(local_fd);

humn... it doesn't seem this is actually testing any isolation? It's
just reopening the fd and testing the same thing on the duplicated fd.

what I'd expect of an isolation test is, e.g.:

/proc/X/fdinfo/Y
	drm-client: A
	drm-rcs-cycles: 0

when executing on another fd:

/proc/X/fdinfo/Z
	drm-client: B
	drm-rcs-cyles: 10


so, check that read_engine_cycles() returns !0 for fd where you
submitted the spin and 0 for where you didn't.

Lucas De Marchi

>+}
>+
> igt_main
> {
> 	struct drm_xe_engine_class_instance *hwe;
>@@ -485,6 +582,18 @@ igt_main
> 	igt_subtest("basic")
> 		basic(xe, num_classes);
>
>+	igt_subtest("drm-idle")
>+		xe_for_each_engine(xe, hwe)
>+			single(xe, hwe, 1, 1, 0);
>+
>+	igt_subtest("drm-busy-idle")
>+		xe_for_each_engine(xe, hwe)
>+			single(xe, hwe, 1, 1, TEST_BUSY | TEST_TRAILING_IDLE);
>+
>+	igt_subtest("drm-busy-idle-isolation")
>+		xe_for_each_engine(xe, hwe)
>+			single(xe, hwe, 1, 1, TEST_BUSY | TEST_TRAILING_IDLE | TEST_ISOLATION);
>+
> 	igt_describe("Create and compare total and resident memory consumption by client");
> 	igt_subtest("drm-total-resident")
> 		test_total_resident(xe);
>-- 
>2.34.1
>


More information about the igt-dev mailing list