[PATCH i-g-t 1/1] lib/xe: Track valid Tile IDs
Dandamudi, Priyanka
priyanka.dandamudi at intel.com
Mon Aug 25 05:41:59 UTC 2025
Firstly, the code itself is not compiling, there is a build failure. Can you check that.
Incorporate review comments as revisions in the commit message along with cover letter, as this is a single patch cover letter is also not required.
> -----Original Message-----
> From: Sharma, Nishit <nishit.sharma at intel.com>
> Sent: 19 August 2025 11:43 AM
> To: igt-dev at lists.freedesktop.org; Dandamudi, Priyanka
> <priyanka.dandamudi at intel.com>; Brost, Matthew
> <matthew.brost at intel.com>; Sharma, Nishit <nishit.sharma at intel.com>
> Cc: Roper, Matthew D <matthew.d.roper at intel.com>
> Subject: [PATCH i-g-t 1/1] lib/xe: Track valid Tile IDs
>
> From: Nishit Sharma <nishit.sharma at intel.com>
>
> Iterating drm_xe_gt structure to get valid tile ID instead of assuming tile_id ==
> gt_id and finding sysfs path for GTS in multi-tile platforms. Initially it was
> assumed tile_id == gt_id in multi-tile platform and if gt_id = X passed, it was
> considered tileX.
>
> Signed-off-by: Nishit Sharma <nishit.sharma at intel.com>
> ---
> lib/igt_sysfs.c | 28 ++++++++++++++------
> lib/xe/xe_query.c | 66 +++++++++++++++++++++++++----------------------
> lib/xe/xe_query.h | 1 +
> 3 files changed, 56 insertions(+), 39 deletions(-)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c index 96ca57ed9..0ef1182b1 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -233,9 +233,14 @@ char *xe_sysfs_gt_path(int xe_device, int gt, char
> *path, int pathlen)
> if (igt_debug_on(fstat(xe_device, &st)) ||
> igt_debug_on(!S_ISCHR(st.st_mode)))
> return NULL;
>
> - if (IS_PONTEVECCHIO(intel_get_drm_devid(xe_device)))
> + if (IS_PONTEVECCHIO(intel_get_drm_devid(xe_device))) {
> + struct xe_device *xe_dev = xe_device_get(xe_device);
> +
> + igt_assert(xe_dev);
> +
> snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile%d/gt%d",
> - major(st.st_rdev), minor(st.st_rdev), gt, gt);
> + major(st.st_rdev), minor(st.st_rdev),
> drm_xe_get_tile(xe_dev, gt), gt);
> + }
> else
> snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile0/gt%d",
> major(st.st_rdev), minor(st.st_rdev), gt); @@ -307,7
> +312,6 @@ char * xe_sysfs_engine_path(int xe_device, int gt, int class, char
> *path, int pathlen) {
> struct stat st;
> - int tile = IS_PONTEVECCHIO(intel_get_drm_devid(xe_device)) ? gt : 0;
>
> if (xe_device < 0)
> return NULL;
> @@ -315,9 +319,16 @@ xe_sysfs_engine_path(int xe_device, int gt, int class,
> char *path, int pathlen)
> if (igt_debug_on(fstat(xe_device, &st)) ||
> igt_debug_on(!S_ISCHR(st.st_mode)))
> return NULL;
>
> - snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile%d/gt%d/engines/%s",
> - major(st.st_rdev), minor(st.st_rdev), tile, gt,
> - xe_engine_class_short_string(class));
> + if (IS_PONTEVECCHIO(intel_get_drm_devid(xe_device))) {
> + struct xe_device *xe_dev = xe_device_get(xe_device);
> +
> + igt_assert(xe_dev);
> +
> + snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile%d/gt%d/engines/%s",
> + major(st.st_rdev), minor(st.st_rdev),
> drm_xe_get_tile(xe_dev, gt), gt);
Missed one argument, xe_engine_class_short_string(class) as last argument. Have you compiled it as it should throw warning as number of arguments are not matching?
[116/1206] Compiling C object lib/libigt-igt_sysfs_c.a.p/igt_sysfs.c.o
../lib/igt_sysfs.c: In function 'xe_sysfs_engine_path':
../lib/igt_sysfs.c:327:90: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
327 | snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile%d/gt%d/engines/%s",
| ~^
| |
| char *
[123/1206] Compiling C object lib/libigt-xe_xe_query_c.a.p/xe_xe_query.c.o
> + } else
> + snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile%d/gt%d/engines/%s",
> + major(st.st_rdev), minor(st.st_rdev), 0, gt,
> +xe_engine_class_short_string(class));
In else case tile is by default tile0 no need to add additional argument as 0 as above to make it uniform.
>
> if (!access(path, F_OK))
> return path;
> @@ -506,6 +517,7 @@ void igt_drm_debug_mask_reset_exit_handler(int
> sig)
> */
> void igt_drm_debug_mask_update(unsigned int mask_to_set) {
> + unsigned int new_debug_mask;
> static bool debug_mask_read_once = true;
> char buf[20];
> int dir;
> @@ -524,8 +536,8 @@ void igt_drm_debug_mask_update(unsigned int
> mask_to_set)
> }
> }
>
> - igt_debug("Setting DRM debug mask to %d\n", mask_to_set);
> - snprintf(buf, sizeof(buf), "%d", mask_to_set);
> + igt_debug("Setting DRM debug mask to %d\n", new_debug_mask);
> + snprintf(buf, sizeof(buf), "%d", new_debug_mask);
You just declared new_debug_mask in this function , no where you are either initialising and trying print and create a buffer value.
It seems something is missing.
> igt_assert(igt_sysfs_set(dir, "debug", buf));
>
> close(dir);
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c index 3b8a682f8..d522bc8fc
> 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -160,32 +160,6 @@ static struct drm_xe_query_mem_regions
> *xe_query_mem_regions_new(int fd)
> return mem_regions;
> }
> The below function removal code seems irrelevant to this patch, check this out.
> -static struct drm_xe_query_eu_stall *xe_query_eu_stall_new(int fd) -{
> - struct drm_xe_query_eu_stall *query_eu_stall;
> - struct drm_xe_device_query query = {
> - .extensions = 0,
> - .query = DRM_XE_DEVICE_QUERY_EU_STALL,
> - .size = 0,
> - .data = 0,
> - };
> -
> - /* Support older kernels where this uapi is not yet available */
> - if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
> - return NULL;
> - igt_assert_neq(query.size, 0);
> -
> - query_eu_stall = malloc(query.size);
> - igt_assert(query_eu_stall);
> -
> - query.data = to_user_pointer(query_eu_stall);
> - igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query),
> 0);
> -
> - VG(VALGRIND_MAKE_MEM_DEFINED(query_eu_stall, query.size));
> -
> - return query_eu_stall;
> -}
> -
> static struct drm_xe_query_oa_units *xe_query_oa_units_new(int fd) {
> struct drm_xe_query_oa_units *oa_units; @@ -343,7 +317,6 @@
> static void xe_device_free(struct xe_device *xe_dev)
> free(xe_dev->engines);
> free(xe_dev->mem_regions);
> free(xe_dev->vram_size);
> - free(xe_dev->eu_stall);
> free(xe_dev);
> }
>
> @@ -379,19 +352,22 @@ struct xe_device *xe_device_get(int fd)
> for (int gt = 0; gt < xe_dev->gt_list->num_gt; gt++)
> xe_dev->gt_mask |= (1ull << xe_dev->gt_list-
> >gt_list[gt].gt_id);
>
> + /* Tile IDs may be non-consecutive; keep a mask of valid IDs */
> + for (int gt = 0; gt < xe_dev->gt_list->num_gt; gt++)
> + xe_dev->tile_mask |= (1ull << xe_dev->gt_list-
> >gt_list[gt].tile_id);
I see tile_mask being removed from xe_dev structure, there is compilation failure with this.
Check this out if this change is really required.
> +
> xe_dev->memory_regions = __memory_regions(xe_dev->gt_list);
> xe_dev->engines = xe_query_engines(fd);
> xe_dev->mem_regions = xe_query_mem_regions_new(fd);
> - xe_dev->eu_stall = xe_query_eu_stall_new(fd);
> xe_dev->oa_units = xe_query_oa_units_new(fd);
>
> /*
> * vram_size[] and visible_vram_size[] are indexed by uapi ID; ensure
> * the allocation is large enough to hold the highest GT ID
> */
> - max_gt = igt_fls(xe_dev->gt_mask) - 1;
> - xe_dev->vram_size = calloc(max_gt + 1, sizeof(*xe_dev->vram_size));
> - xe_dev->visible_vram_size = calloc(max_gt + 1, sizeof(*xe_dev-
> >visible_vram_size));
> + max_gt = ffsll(xe_dev->gt_mask) - 1;
> + xe_dev->vram_size = calloc(max_gt, sizeof(*xe_dev->vram_size));
> + xe_dev->visible_vram_size = calloc(max_gt,
> +sizeof(*xe_dev->visible_vram_size));
>
> for (int idx = 0; idx < xe_dev->gt_list->num_gt; idx++) {
> struct drm_xe_gt *gt = &xe_dev->gt_list->gt_list[idx]; @@ -
> 545,6 +521,18 @@ const struct drm_xe_gt *drm_xe_get_gt(struct xe_device
> *xe_dev, int gt_id)
> return NULL;
> }
>
> +/*
> + * Given a uapi GT ID, lookup the corresponding drm_xe_gt structure in
> +the
> + * GT list and return valid tile_id.
> + */
> +uint32_t drm_xe_get_tile(struct xe_device *xe_dev, int gt_id) {
> + for (int i = 0; i < xe_dev->gt_list->num_gt; i++)
> + if (xe_dev->gt_list->gt_list[i].gt_id == gt_id)
> + return xe_dev->gt_list->gt_list[i].tile_id;
> +
> + return -1;
> +}
>
> /**
> * vram_memory:
> @@ -726,6 +714,22 @@ xe_dev_FN(xe_number_engines, engines-
> >num_engines, unsigned int);
> */
> xe_dev_FN(xe_has_vram, has_vram, bool);
>
> +/*
> + * To check whether multi-tile supported in platform
> + * Function takes device descriptior and xe_device
> + * structure pointer
> + */
> +static int
> +is_multitile_plat(struct xe_device *xe_dev) {
> + int gt_id;
> +
> + xe_for_each_gt(xe_dev->fd, gt_id)
> + if (xe_dev->gt_list->gt_list[gt_id].tile_id > 0)
> + return true;
> + return false;
> +}
> +
> /**
> * xe_vram_size:
> * @fd: xe device fd
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h index cc54ec956..ca6249b0d
> 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -101,6 +101,7 @@ unsigned int xe_dev_max_gt(int fd); uint64_t
> all_memory_regions(int fd); uint64_t system_memory(int fd); const struct
> drm_xe_gt *drm_xe_get_gt(struct xe_device *xe_dev, int gt_id);
> +uint32_t drm_xe_get_tile(struct xe_device *xe_dev, int gt_id);
> uint64_t vram_memory(int fd, int gt);
> uint64_t vram_if_possible(int fd, int gt); struct drm_xe_engine
> *xe_engines(int fd);
> --
> 2.43.0
More information about the igt-dev
mailing list