[PATCH i-g-t v2] lib/xe: Track valid Tile IDs
Kamil Konieczny
kamil.konieczny at linux.intel.com
Wed Aug 27 12:02:31 UTC 2025
Hi Nishit,
On 2025-08-25 at 06:10:54 +0000, nishit.sharma at intel.com wrote:
> 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.
Add newline here.
> Fixed compilation issue. Removed unused funtion from library.
I guess this is version changes, so imho better:
v2: Fixed compilation, removed unused funtion from library.
Btw are you referring to removing xe_query_eu_stall_new()?
If yes, please make it in separate patch, not in this one.
>
> Signed-off-by: Nishit Sharma <nishit.sharma at intel.com>
> ---
> lib/igt_sysfs.c | 28 ++++++++++++++++++--------
> lib/xe/xe_query.c | 50 ++++++++++++++++++-----------------------------
> lib/xe/xe_query.h | 4 ++++
> 3 files changed, 43 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);
Btw why do we need checks for PVC here? Cannot this be simplified
into one-liner:
snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile%d/gt%d",
major(st.st_rdev), minor(st.st_rdev), drm_xe_get_tile(xe_dev, gt), 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);
> + } 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));
This is repeating code from function xe_sysfs_gt_path() plus
engine and class, imho reuse that function here.
Less code is better.
>
> 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;
This looks like unrelated change, for a separate patch,
remove it.
> 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);
Same here, unrelated change.
> 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..ead44cda6 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;
> }
>
> -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;
> -}
> -
Same here, unrelated change.
> 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);
Same here, unrelated change.
> 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);
> +
> 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);
Same here, unrelated change.
> 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));
Same here, unrelated change.
>
> 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;
This should be unsigned? imho this should be defined as
invalid and returned instead.
Regards,
Kamil
> +}
>
> /**
> * vram_memory:
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index cc54ec956..d8da1f9eb 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -39,6 +39,9 @@ struct xe_device {
> /** @gt_mask: bitmask of GT IDs */
> uint64_t gt_mask;
>
> + /** @tile_mask: bitmask of Tile IDs */
> + uint64_t tile_mask;
> +
> /** @memory_regions: bitmask of all memory regions */
> uint64_t memory_regions;
>
> @@ -101,6 +104,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