On 23/07/2026 20:43, Thadeu Lima de Souza Cascardo wrote:
In order to be reuse the same dmem tests with multiple drivers, we need to
abstract a few operations. That includes getting the region name, and
allocating and releasing VRAM. As there is some initialization also when
multiple allocations are done, also provide init and deinit functions.
The Xe implementation was based on the original operations from
xe_cgroups.c written by Thomas Hellström. However, instead of doing a
deferred backing, followed by a bind, it does a simple non-deferred GEM
object creation on the VRAM region.
Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
---
lib/igt_dmem_driver.h | 24 ++++++++
lib/meson.build | 1 +
lib/xe/xe_dmem.c | 139 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 164 insertions(+)
create mode 100644 lib/igt_dmem_driver.h
create mode 100644 lib/xe/xe_dmem.c
diff --git a/lib/igt_dmem_driver.h b/lib/igt_dmem_driver.h
new file mode 100644
index 000000000000..2f4a4673ab16
--- /dev/null
+++ b/lib/igt_dmem_driver.h
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2026 Valve Corporation
+ * Authors:
+ * Thadeu Lima de Souza Cascardo <[email protected]>
+ */
+
+#ifndef __IGT_DMEM_DRIVER_H__
+#define __IGT_DMEM_DRIVER_H__
+
+#include <stdlib.h>
+
+struct igt_dmem_driver {
+ const char *name;
+ char * (*get_region_name)(int fd);
+ int (*init)(void **ctx, int fd, int max_bo);
+ void (*deinit)(void *ctx);
+ int (*allocate_vram)(void *ctx, int n_bo, size_t len);
+ void (*free_vram)(void *ctx, int n_bo);
I expect IGT maintainers might ask for kerneldoc for these.
+};
+
+extern const struct igt_dmem_driver xe_dmem_driver;
+
+#endif
diff --git a/lib/meson.build b/lib/meson.build
index 868220408f8c..012802475de0 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -126,6 +126,7 @@ lib_sources = [
'igt_msm.c',
'igt_dsc.c',
'igt_hook.c',
+ 'xe/xe_dmem.c',
'xe/xe_gt.c',
'xe/xe_ioctl.c',
'xe/xe_legacy.c',
diff --git a/lib/xe/xe_dmem.c b/lib/xe/xe_dmem.c
new file mode 100644
index 000000000000..1dec8cf8b0a1
--- /dev/null
+++ b/lib/xe/xe_dmem.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2026 Valve Corporation
+ * Authors:
+ * Thadeu Lima de Souza Cascardo <[email protected]>
+ */
+
+#include <errno.h>
+
+#include "igt.h"
+#include "igt_cgroup.h"
+#include "igt_dmem_driver.h"
+#include "xe_drm.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+
+static char * xe_dmem_get_region_name(int fd)
+{
+ uint64_t vram_region = 0;
+ uint64_t region;
+ char *cg_region;
+
+ /* Find first VRAM region */
+ xe_for_each_mem_region(fd, all_memory_regions(fd), region) {
+ if (xe_region_class(fd, region) ==
DRM_XE_MEM_REGION_CLASS_VRAM) {
+ vram_region = region;
+ break;
+ }
+ }
FWIW it looks like that from here..
+ if (!vram_region)
+ return NULL;
+
+ cg_region = xe_cgroup_region_name(fd, vram_region);
+
+ return cg_region;
.. to here could be simplified as return xe_cgroup_region_name(...),
given the helper returns NULL for non-VRAM regions. Or even return
straight from the above loop instead of the break. Up to you which of
the three options you prefer, just thinking out loud.
+}
+
+struct xe_bo {
+ uint32_t handle;
+ size_t len;
+};
+
+struct xe_dmem_ctx {
+ int fd;
+ struct xe_bo *bos;
+ uint64_t vram_region;
+ int max_bo;
+};
+
+static int xe_dmem_init(void **ctx, int fd, int max_bo)
+{
+ struct xe_dmem_ctx *xe_ctx;
+ uint64_t region;
+
+ xe_ctx = malloc(sizeof(*xe_ctx));
+ if (!xe_ctx)
+ return -ENOMEM;
+
+ xe_ctx->bos = calloc(max_bo, sizeof(xe_ctx->bos[0]));
+ if (!xe_ctx->bos)
+ goto out;
+ memset(xe_ctx->bos, 0, max_bo * sizeof(xe_ctx->bos[0]));
calloc already zeroes the memory.
+ xe_ctx->max_bo = max_bo;
Could the need for max_bo input parameter be avoided if the arrays were
allocated in chunks and grown? More complex implementation but a simpler
API.
+
+ xe_ctx->vram_region = 0;
+ /* Find first VRAM region */
+ xe_for_each_mem_region(fd, all_memory_regions(fd), region) {
+ if (xe_region_class(fd, region) ==
DRM_XE_MEM_REGION_CLASS_VRAM) {
+ xe_ctx->vram_region = region;
+ break;
+ }
+ }
+ if (!xe_ctx->vram_region)
+ goto out;
+
+ xe_ctx->fd = fd;
+
+ *ctx = xe_ctx;
+
+ return 0;
+
+out:
+ if (xe_ctx->bos)
+ free(xe_ctx->bos);
+ free(xe_ctx);
+
+ return -ENOMEM;
+}
+
+static void xe_dmem_deinit(void *ctx)
+{
+ struct xe_dmem_ctx *xe_ctx = ctx;
+
+ free(xe_ctx->bos);
+ free(xe_ctx);
Handle cleanup is not desirable?
+}
+
+static int xe_dmem_allocate_vram(void *ctx, int n_bo, size_t len)
Is the last patch in the series the only user of this API? I am thinking
whether the n_bo input parameter is required and it wouldn't be simpler
to just return a handle (or null) and grow the internal count. Ie. is
there a use case for callers to precisely need to control "allocate/free
at this index"?
+{
+ struct xe_dmem_ctx *xe_ctx = ctx;
+ uint32_t handle;
+ int err;
+
+ if (n_bo >= xe_ctx->max_bo)
+ return -ENOMEM;
If the max_bo approach remains, I'd be tempted to igt_assert here since
it is a test writer error to hit it.
Also, return error if a valid handle is already at the requested index?
+
+ err = __xe_bo_create(xe_ctx->fd, 0, len, xe_ctx->vram_region, 0,
+ NULL, &handle);
+ if (err)
+ goto out;
+
+ xe_ctx->bos[n_bo].handle = handle;
+ xe_ctx->bos[n_bo].len = len;
+
+out:
+ return err;
+}
+
+static void xe_dmem_free_vram(void *ctx, int n_bo)
+{
+ struct xe_dmem_ctx *xe_ctx = ctx;
+ size_t len;
+ if (n_bo >= xe_ctx->max_bo)
+ return;
Same assert or not comment as above.
+ len = xe_ctx->bos[n_bo].len;
+ if (len) {
+ gem_close(xe_ctx->fd, xe_ctx->bos[n_bo].handle);
+ xe_ctx->bos[n_bo].len = 0;
+ }
It looks like the len local is superflous and handle also needs clearing.
+}
+
+const struct igt_dmem_driver xe_dmem_driver = {
+ .name = "xe",
+ .get_region_name = xe_dmem_get_region_name,
+ .init = xe_dmem_init,
+ .deinit = xe_dmem_deinit,
+ .allocate_vram = xe_dmem_allocate_vram,
+ .free_vram = xe_dmem_free_vram,
+};
Regards,
Tvrtko
