Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT
@ 2023-07-07 11:30 Himal Prasad Ghimiray
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles Himal Prasad Ghimiray
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Himal Prasad Ghimiray @ 2023-07-07 11:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Upadhyay

With seperation of GT and tiles in KMD, we will have sysfs entries
for properties associated with tile and gtX sysfs parent will be moving
under tileX. This series provides helper functions for accessing tile
associated sysfs entries and gt specific sysfs under respective tile.

series addresses gt related sysfs path change for xe_guc_pc.c test case 
and introduces new test to verify addr_range for each vram.

Cc: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Francois Dugast <francois.dugast@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Upadhyay <tejas.upadhyay@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

Himal Prasad Ghimiray (4):
  lib/igt_sysfs: Add support to query number of tiles
  lib/igt_sysfs: Handling gt related sysfs uapi changes
  tests/xe/xe_guc_pc: Change the sysfs paths
  tests/xe/xe_sysfs_tile: adds new test to verify per tile vram
    addr_range

 lib/igt_sysfs.c          | 125 ++++++++++++++++++++++++
 lib/igt_sysfs.h          |  12 +++
 tests/meson.build        |   1 +
 tests/xe/xe_guc_pc.c     | 206 +++++++++++++++++++++------------------
 tests/xe/xe_sysfs_tile.c |  55 +++++++++++
 5 files changed, 303 insertions(+), 96 deletions(-)
 create mode 100644 tests/xe/xe_sysfs_tile.c

-- 
2.25.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles
  2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
@ 2023-07-07 11:30 ` Himal Prasad Ghimiray
  2023-07-08 19:29   ` Dixit, Ashutosh
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes Himal Prasad Ghimiray
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Himal Prasad Ghimiray @ 2023-07-07 11:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Upadhyay

With tile and GT seperation in KMD, we need to know
number of tiles supported by platform.
We will need to access tile associated properties from IGT.
Hence adding iterator for all supported tiles.

v2:
- Calculate number of tiles once within iterator. (Rahul)
- Use snprintf instead of sprintf.

v3:
- Remove unrequired for_each_sysfs_tile_dirfd (Ashutosh)

v4:
- Implement tiles related functions in lib. (Ashutosh)

v5:
- Fix comments and remove not required conditional check. (Ashutosh)

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Upadhyay <tejas.upadhyay@intel.com>
Cc: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 lib/igt_sysfs.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h | 10 +++++++
 2 files changed, 81 insertions(+)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 0876f4c6b..eb35a8088 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -903,3 +903,74 @@ void igt_sysfs_engines(int xe, int engines, const char **property,
 		close(engine_fd);
 	}
 }
+
+/**
+ * xe_sysfs_tile_path:
+ * @xe_device: fd of the device
+ * @tile: tile number
+ * @path: buffer to fill with the sysfs tile path to the device
+ * @pathlen: length of @path buffer
+ *
+ * Returns:
+ * The directory path, or NULL on failure.
+ */
+char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int pathlen)
+{
+	struct stat st;
+
+	if (xe_device < 0)
+		return NULL;
+
+	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",
+		 major(st.st_rdev), minor(st.st_rdev), tile);
+
+	if (!access(path, F_OK))
+		return path;
+	return NULL;
+}
+
+/**
+ * xe_sysfs_tile_open:
+ * @xe_device: fd of the device
+ * @tile: tile number
+ *
+ * This opens the sysfs tile directory corresponding to device and tile for use
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int xe_sysfs_tile_open(int xe_device, int tile)
+{
+	char path[96];
+
+	if (!xe_sysfs_tile_path(xe_device, tile, path, sizeof(path)))
+		return -1;
+
+	return open(path, O_RDONLY);
+}
+
+/**
+ * xe_sysfs_get_num_tiles:
+ * @xe_device: fd of the device
+ *
+ * Reads number of tile sysfs entries.
+ * Asserts for at least one tile entry.
+ * (see xe_sysfs_tile_path).
+ *
+ * Returns: Number of tiles.
+ */
+int xe_sysfs_get_num_tiles(int xe_device)
+{
+	int num_tiles = 0;
+	char path[96];
+
+	while (xe_sysfs_tile_path(xe_device, num_tiles, path, sizeof(path)))
+		++num_tiles;
+
+	igt_assert_f(num_tiles > 0, "No GT sysfs entry is found.");
+
+	return num_tiles;
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 5635fc690..5d584b1c7 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -38,6 +38,11 @@
 	     (dirfd__ = igt_sysfs_gt_open(i915__, gt__)) != -1; \
 	     close(dirfd__), gt__++)
 
+#define for_each_sysfs_tile_dirfd(xe__, dirfd__, tile__) \
+	for (tile__ = 0; \
+	     (dirfd__ = xe_sysfs_tile_open(xe__, tile__)) != -1; \
+	     close(dirfd__), tile__++)
+
 #define i915_for_each_gt for_each_sysfs_gt_dirfd
 
 #define igt_sysfs_rps_write(dir, id, data, len) \
@@ -73,6 +78,8 @@
 #define igt_sysfs_rps_set_boolean(dir, id, value) \
 	igt_sysfs_set_boolean(dir, igt_sysfs_dir_id_to_name(dir, id), value)
 
+#define xe_for_each_tile for_each_sysfs_tile_dirfd
+
 enum i915_attr_id {
 	RPS_ACT_FREQ_MHZ,
 	RPS_CUR_FREQ_MHZ,
@@ -150,4 +157,7 @@ void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
 void igt_sysfs_engines(int xe, int engines, const char **property,
 		       void (*test)(int, int, const char **));
 
+char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int pathlen);
+int xe_sysfs_tile_open(int xe_device, int tile);
+int xe_sysfs_get_num_tiles(int xe_device);
 #endif /* __IGT_SYSFS_H__ */
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes
  2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles Himal Prasad Ghimiray
@ 2023-07-07 11:30 ` Himal Prasad Ghimiray
  2023-07-08 19:30   ` Dixit, Ashutosh
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths Himal Prasad Ghimiray
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Himal Prasad Ghimiray @ 2023-07-07 11:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Upadhyay

Patch https://patchwork.freedesktop.org/series/118927/
is moving gt sysfs parent under tile folder.

With the above patch path for sysfs changes:
from: /sys/class/drm/cardX/device/gtN/
to : /sys/class/drm/cardX/device/tileN/gtN

Adding xe_gt_sysfs_path function to access new path.

v2:
- Calculate number of tiles once within iterator. (Rahul)

v3:
- Drop usage of local variable for tilecount.
- Change order of tile and gt. (Ashutosh)

v4:
- Drop xe_for_each_gt_under_each_tile macro
  add xe_gt_sysfs_path to return path. (Ashutosh)
- Support older dir path along with new. (kamil)

v5:
- Dont use fixed paths. Find gt in available tiles. (Ashutosh)

v6:
- Define gt related functions for xe. (Ashutosh)

Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Upadhyay <tejas.upadhyay@intel.com>
Cc: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
Cc: Francois Dugast <francois.dugast@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 lib/igt_sysfs.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h |  2 ++
 2 files changed, 56 insertions(+)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index eb35a8088..5bed91304 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -45,6 +45,7 @@
 #include "igt_sysfs.h"
 #include "igt_device.h"
 #include "igt_io.h"
+#include "intel_chipset.h"
 
 /**
  * SECTION:igt_sysfs
@@ -904,6 +905,59 @@ void igt_sysfs_engines(int xe, int engines, const char **property,
 	}
 }
 
+/**
+ * xe_sysfs_gt_path:
+ * @xe_device: fd of the device
+ * @gt: gt number
+ * @path: buffer to fill with the sysfs gt path to the device
+ * @pathlen: length of @path buffer
+ *
+ * Returns:
+ * The directory path, or NULL on failure.
+ */
+char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int pathlen)
+{
+	struct stat st;
+
+	if (xe_device < 0)
+		return NULL;
+
+	if (igt_debug_on(fstat(xe_device, &st)) || igt_debug_on(!S_ISCHR(st.st_mode)))
+		return NULL;
+
+	if (IS_METEORLAKE(xe_device))
+		snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile0/gt%d",
+			 major(st.st_rdev), minor(st.st_rdev), gt);
+	else
+		snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile%d/gt%d",
+			 major(st.st_rdev), minor(st.st_rdev), gt, gt);
+
+	if (!access(path, F_OK))
+		return path;
+
+	return NULL;
+}
+
+/**
+ * xe_sysfs_gt_open:
+ * @xe_device: fd of the device
+ * @gt: gt number
+ *
+ * This opens the sysfs gt directory corresponding to device and tile for use
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int xe_sysfs_gt_open(int xe_device, int gt)
+{
+	char path[96];
+
+	if (!xe_sysfs_gt_path(xe_device, gt, path, sizeof(path)))
+		return -1;
+
+	return open(path, O_RDONLY);
+}
+
 /**
  * xe_sysfs_tile_path:
  * @xe_device: fd of the device
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 5d584b1c7..afcafb07c 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -157,6 +157,8 @@ void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
 void igt_sysfs_engines(int xe, int engines, const char **property,
 		       void (*test)(int, int, const char **));
 
+char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int pathlen);
+int xe_sysfs_gt_open(int xe_device, int gt);
 char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int pathlen);
 int xe_sysfs_tile_open(int xe_device, int tile);
 int xe_sysfs_get_num_tiles(int xe_device);
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths
  2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles Himal Prasad Ghimiray
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes Himal Prasad Ghimiray
@ 2023-07-07 11:30 ` Himal Prasad Ghimiray
  2023-07-08 19:30   ` Dixit, Ashutosh
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 4/4] tests/xe/xe_sysfs_tile: adds new test to verify per tile vram addr_range Himal Prasad Ghimiray
  2023-07-07 11:51 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Handle GT and tile seperation in IGT (rev7) Patchwork
  4 siblings, 1 reply; 11+ messages in thread
From: Himal Prasad Ghimiray @ 2023-07-07 11:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Upadhyay, Badal Nilawar

Changes to access sysfs entries under tile directory.
Access freq sysfs from /sys/class/drm/cardX/device/tileN/gtN
path.

v2:
- Use snprintf instead of sprintf and check error.
- Describe what changes are done. (Kamil)

v3:
- change order of gt and tile and drop passing tilecount.(Ashutosh)

v4:
- Rebase

v5:
- Drop usage of xe_for_each_gt_under_each_tile and use
  a function to return gt path. (Ashutosh)

v6:
- use gt_path instead of func call (Ashutosh)

v7:
- use new api's for gt sysfs access.

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Upadhyay <tejas.upadhyay@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Riana Tauro <riana.tauro@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 tests/xe/xe_guc_pc.c | 206 +++++++++++++++++++++++--------------------
 1 file changed, 110 insertions(+), 96 deletions(-)

diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c
index 827693eb4..e20ab5f88 100644
--- a/tests/xe/xe_guc_pc.c
+++ b/tests/xe/xe_guc_pc.c
@@ -133,25 +133,38 @@ static void exec_basic(int fd, struct drm_xe_engine_class_instance *eci,
 	xe_vm_destroy(fd, vm);
 }
 
-static int set_freq(int sysfs, int gt_id, const char *freq_name, uint32_t freq)
+static int set_freq(int fd, int gt_id, const char *freq_name, uint32_t freq)
 {
 	int ret = -EAGAIN;
-	char path[32];
+	char freq_attr[16];
+	int gt_fd;
+
+	snprintf(freq_attr, sizeof(freq_attr), "freq_%s", freq_name);
+	gt_fd = xe_sysfs_gt_open(fd, gt_id);
+	igt_assert(gt_fd != -1);
 
-	sprintf(path, "device/gt%d/freq_%s", gt_id, freq_name);
 	while (ret == -EAGAIN)
-		ret = igt_sysfs_printf(sysfs, path, "%u", freq);
+		ret = igt_sysfs_printf(gt_fd, freq_attr, "%u", freq);
+
+	close(gt_fd);
 	return ret;
 }
 
-static uint32_t get_freq(int sysfs, int gt_id, const char *freq_name)
+static uint32_t get_freq(int fd, int gt_id, const char *freq_name)
 {
 	uint32_t freq;
 	int err = -EAGAIN;
-	char path[32];
-	sprintf(path, "device/gt%d/freq_%s", gt_id, freq_name);
+	char freq_attr[16];
+	int gt_fd;
+
+	snprintf(freq_attr, sizeof(freq_attr), "freq_%s", freq_name);
+	gt_fd = xe_sysfs_gt_open(fd, gt_id);
+	igt_assert(gt_fd != -1);
+
 	while (err == -EAGAIN)
-		err = igt_sysfs_scanf(sysfs, path, "%u", &freq);
+		err = igt_sysfs_scanf(gt_fd, freq_attr, "%u", &freq);
+
+	close(gt_fd);
 	return freq;
 }
 
@@ -162,37 +175,37 @@ static uint32_t get_freq(int sysfs, int gt_id, const char *freq_name)
  * Run type: BAT
  */
 
-static void test_freq_basic_api(int sysfs, int gt_id)
+static void test_freq_basic_api(int fd, int gt_id)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
-	uint32_t rpe = get_freq(sysfs, gt_id, "rpe");
-	uint32_t rp0 = get_freq(sysfs, gt_id, "rp0");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
+	uint32_t rpe = get_freq(fd, gt_id, "rpe");
+	uint32_t rp0 = get_freq(fd, gt_id, "rp0");
 
 	/*
 	 * Negative bound tests
 	 * RPn is the floor
 	 * RP0 is the ceiling
 	 */
-	igt_assert(set_freq(sysfs, gt_id, "min", rpn - 1) < 0);
-	igt_assert(set_freq(sysfs, gt_id, "min", rp0 + 1) < 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpn - 1) < 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rp0 + 1) < 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpn - 1) < 0);
+	igt_assert(set_freq(fd, gt_id, "min", rp0 + 1) < 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpn - 1) < 0);
+	igt_assert(set_freq(fd, gt_id, "max", rp0 + 1) < 0);
 
 	/* Assert min requests are respected from rp0 to rpn */
-	igt_assert(set_freq(sysfs, gt_id, "min", rp0) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "min") == rp0);
-	igt_assert(set_freq(sysfs, gt_id, "min", rpe) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "min") == rpe);
-	igt_assert(set_freq(sysfs, gt_id, "min", rpn) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "min") == rpn);
+	igt_assert(set_freq(fd, gt_id, "min", rp0) > 0);
+	igt_assert(get_freq(fd, gt_id, "min") == rp0);
+	igt_assert(set_freq(fd, gt_id, "min", rpe) > 0);
+	igt_assert(get_freq(fd, gt_id, "min") == rpe);
+	igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
+	igt_assert(get_freq(fd, gt_id, "min") == rpn);
 
 	/* Assert max requests are respected from rpn to rp0 */
-	igt_assert(set_freq(sysfs, gt_id, "max", rpn) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "max") == rpn);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpe) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "max") == rpe);
-	igt_assert(set_freq(sysfs, gt_id, "max", rp0) > 0);
-	igt_assert(get_freq(sysfs, gt_id, "max") == rp0);
+	igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
+	igt_assert(get_freq(fd, gt_id, "max") == rpn);
+	igt_assert(set_freq(fd, gt_id, "max", rpe) > 0);
+	igt_assert(get_freq(fd, gt_id, "max") == rpe);
+	igt_assert(set_freq(fd, gt_id, "max", rp0) > 0);
+	igt_assert(get_freq(fd, gt_id, "max") == rp0);
 }
 
 /**
@@ -205,11 +218,11 @@ static void test_freq_basic_api(int sysfs, int gt_id)
  * Run type: FULL
  */
 
-static void test_freq_fixed(int sysfs, int gt_id)
+static void test_freq_fixed(int fd, int gt_id)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
-	uint32_t rpe = get_freq(sysfs, gt_id, "rpe");
-	uint32_t rp0 = get_freq(sysfs, gt_id, "rp0");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
+	uint32_t rpe = get_freq(fd, gt_id, "rpe");
+	uint32_t rp0 = get_freq(fd, gt_id, "rp0");
 
 	igt_debug("Starting testing fixed request\n");
 
@@ -218,27 +231,27 @@ static void test_freq_fixed(int sysfs, int gt_id)
 	 * Then we check if hardware is actually operating at the desired freq
 	 * And let's do this for all the 3 known Render Performance (RP) values.
 	 */
-	igt_assert(set_freq(sysfs, gt_id, "min", rpn) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
-	igt_assert(get_freq(sysfs, gt_id, "cur") == rpn);
-	igt_assert(get_freq(sysfs, gt_id, "act") == rpn);
+	igt_assert(get_freq(fd, gt_id, "cur") == rpn);
+	igt_assert(get_freq(fd, gt_id, "act") == rpn);
 
-	igt_assert(set_freq(sysfs, gt_id, "min", rpe) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpe) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpe) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpe) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
-	igt_assert(get_freq(sysfs, gt_id, "cur") == rpe);
-	igt_assert(get_freq(sysfs, gt_id, "act") == rpe);
+	igt_assert(get_freq(fd, gt_id, "cur") == rpe);
+	igt_assert(get_freq(fd, gt_id, "act") == rpe);
 
-	igt_assert(set_freq(sysfs, gt_id, "min", rp0) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rp0) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rp0) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rp0) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
 	/*
 	 * It is unlikely that PCODE will *always* respect any request above RPe
 	 * So for this level let's only check if GuC PC is doing its job
 	 * and respecting our request, by propagating it to the hardware.
 	 */
-	igt_assert(get_freq(sysfs, gt_id, "cur") == rp0);
+	igt_assert(get_freq(fd, gt_id, "cur") == rp0);
 
 	igt_debug("Finished testing fixed request\n");
 }
@@ -253,20 +266,20 @@ static void test_freq_fixed(int sysfs, int gt_id)
  * Run type: FULL
  */
 
-static void test_freq_range(int sysfs, int gt_id)
+static void test_freq_range(int fd, int gt_id)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
-	uint32_t rpe = get_freq(sysfs, gt_id, "rpe");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
+	uint32_t rpe = get_freq(fd, gt_id, "rpe");
 	uint32_t cur, act;
 
 	igt_debug("Starting testing range request\n");
 
-	igt_assert(set_freq(sysfs, gt_id, "min", rpn) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpe) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpe) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
-	cur = get_freq(sysfs, gt_id, "cur");
+	cur = get_freq(fd, gt_id, "cur");
 	igt_assert(rpn <= cur && cur <= rpe);
-	act = get_freq(sysfs, gt_id, "act");
+	act = get_freq(fd, gt_id, "act");
 	igt_assert(rpn <= act && act <= rpe);
 
 	igt_debug("Finished testing range request\n");
@@ -278,20 +291,20 @@ static void test_freq_range(int sysfs, int gt_id)
  * Run type: FULL
  */
 
-static void test_freq_low_max(int sysfs, int gt_id)
+static void test_freq_low_max(int fd, int gt_id)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
-	uint32_t rpe = get_freq(sysfs, gt_id, "rpe");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
+	uint32_t rpe = get_freq(fd, gt_id, "rpe");
 
 	/*
 	 *  When max request < min request, max is ignored and min works like
 	 * a fixed one. Let's assert this assumption
 	 */
-	igt_assert(set_freq(sysfs, gt_id, "min", rpe) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpe) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
-	igt_assert(get_freq(sysfs, gt_id, "cur") == rpe);
-	igt_assert(get_freq(sysfs, gt_id, "act") == rpe);
+	igt_assert(get_freq(fd, gt_id, "cur") == rpe);
+	igt_assert(get_freq(fd, gt_id, "act") == rpe);
 }
 
 /**
@@ -300,20 +313,20 @@ static void test_freq_low_max(int sysfs, int gt_id)
  * Run type: FULL
  */
 
-static void test_suspend(int sysfs, int gt_id)
+static void test_suspend(int fd, int gt_id)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
 
-	igt_assert(set_freq(sysfs, gt_id, "min", rpn) > 0);
-	igt_assert(set_freq(sysfs, gt_id, "max", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
+	igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
 	usleep(ACT_FREQ_LATENCY_US);
-	igt_assert(get_freq(sysfs, gt_id, "cur") == rpn);
+	igt_assert(get_freq(fd, gt_id, "cur") == rpn);
 
 	igt_system_suspend_autoresume(SUSPEND_STATE_S3,
 				      SUSPEND_TEST_NONE);
 
-	igt_assert(get_freq(sysfs, gt_id, "min") == rpn);
-	igt_assert(get_freq(sysfs, gt_id, "max") == rpn);
+	igt_assert(get_freq(fd, gt_id, "min") == rpn);
+	igt_assert(get_freq(fd, gt_id, "max") == rpn);
 }
 
 /**
@@ -326,24 +339,24 @@ static void test_suspend(int sysfs, int gt_id)
  * Run type: FULL
  */
 
-static void test_reset(int fd, int sysfs, int gt_id, int cycles)
+static void test_reset(int fd, int gt_id, int cycles)
 {
-	uint32_t rpn = get_freq(sysfs, gt_id, "rpn");
+	uint32_t rpn = get_freq(fd, gt_id, "rpn");
 
 	for (int i = 0; i < cycles; i++) {
-		igt_assert_f(set_freq(sysfs, gt_id, "min", rpn) > 0,
+		igt_assert_f(set_freq(fd, gt_id, "min", rpn) > 0,
 			     "Failed after %d good cycles\n", i);
-		igt_assert_f(set_freq(sysfs, gt_id, "max", rpn) > 0,
+		igt_assert_f(set_freq(fd, gt_id, "max", rpn) > 0,
 			     "Failed after %d good cycles\n", i);
 		usleep(ACT_FREQ_LATENCY_US);
-		igt_assert_f(get_freq(sysfs, gt_id, "cur") == rpn,
+		igt_assert_f(get_freq(fd, gt_id, "cur") == rpn,
 			     "Failed after %d good cycles\n", i);
 
 		xe_force_gt_reset(fd, gt_id);
 
-		igt_assert_f(get_freq(sysfs, gt_id, "min") == rpn,
+		igt_assert_f(get_freq(fd, gt_id, "min") == rpn,
 			     "Failed after %d good cycles\n", i);
-		igt_assert_f(get_freq(sysfs, gt_id, "max") == rpn,
+		igt_assert_f(get_freq(fd, gt_id, "max") == rpn,
 			     "Failed after %d good cycles\n", i);
 	}
 }
@@ -359,12 +372,18 @@ static void test_reset(int fd, int sysfs, int gt_id, int cycles)
  * Run type: BAT
  */
 
-static bool in_rc6(int sysfs, int gt_id)
+static bool in_rc6(int fd, int gt_id)
 {
-	char path[32];
+	int rc_stat, gt_fd;
 	char rc[8];
-	sprintf(path, "device/gt%d/rc_status", gt_id);
-	if (igt_sysfs_scanf(sysfs, path, "%s", rc) < 0)
+
+	gt_fd = xe_sysfs_gt_open(fd, gt_id);
+	igt_assert(gt_fd != -1);
+
+	rc_stat = igt_sysfs_scanf(gt_fd, "rc_status", "%s", rc);
+	close(gt_fd);
+
+	if (rc_stat < 0)
 		return false;
 	return strcmp(rc, "rc6") == 0;
 }
@@ -374,7 +393,6 @@ igt_main
 	struct drm_xe_engine_class_instance *hwe;
 	int fd;
 	int gt;
-	static int sysfs = -1;
 	int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
 	uint32_t stash_min;
 	uint32_t stash_max;
@@ -382,22 +400,19 @@ igt_main
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_XE);
 
-		sysfs = igt_sysfs_open(fd);
-		igt_assert(sysfs != -1);
-
 		/* The defaults are the same. Stashing the gt0 is enough */
-		stash_min = get_freq(sysfs, 0, "min");
-		stash_max = get_freq(sysfs, 0, "max");
+		stash_min = get_freq(fd, 0, "min");
+		stash_max = get_freq(fd, 0, "max");
 	}
 
 	igt_subtest("freq_basic_api") {
 		xe_for_each_gt(fd, gt)
-			test_freq_basic_api(sysfs, gt);
+			test_freq_basic_api(fd, gt);
 	}
 
 	igt_subtest("freq_fixed_idle") {
 		xe_for_each_gt(fd, gt) {
-			test_freq_fixed(sysfs, gt);
+			test_freq_fixed(fd, gt);
 		}
 	}
 
@@ -410,14 +425,14 @@ igt_main
 					igt_debug("Execution Finished\n");
 				}
 			/* While exec in threads above, let's check the freq */
-			test_freq_fixed(sysfs, gt);
+			test_freq_fixed(fd, gt);
 			igt_waitchildren();
 		}
 	}
 
 	igt_subtest("freq_range_idle") {
 		xe_for_each_gt(fd, gt) {
-			test_freq_range(sysfs, gt);
+			test_freq_range(fd, gt);
 		}
 	}
 
@@ -430,46 +445,46 @@ igt_main
 					igt_debug("Execution Finished\n");
 				}
 			/* While exec in threads above, let's check the freq */
-			test_freq_range(sysfs, gt);
+			test_freq_range(fd, gt);
 			igt_waitchildren();
 		}
 	}
 
 	igt_subtest("freq_low_max") {
 		xe_for_each_gt(fd, gt) {
-			test_freq_low_max(sysfs, gt);
+			test_freq_low_max(fd, gt);
 		}
 	}
 
 	igt_subtest("freq_suspend") {
 		xe_for_each_gt(fd, gt) {
-			test_suspend(sysfs, gt);
+			test_suspend(fd, gt);
 		}
 	}
 
 	igt_subtest("freq_reset") {
 		xe_for_each_gt(fd, gt) {
-			test_reset(fd, sysfs, gt, 1);
+			test_reset(fd, gt, 1);
 		}
 	}
 
 	igt_subtest("freq_reset_multiple") {
 		xe_for_each_gt(fd, gt) {
-			test_reset(fd, sysfs, gt, 50);
+			test_reset(fd, gt, 50);
 		}
 	}
 
 	igt_subtest("rc6_on_idle") {
 		igt_require(!IS_PONTEVECCHIO(xe_dev_id(fd)));
 		xe_for_each_gt(fd, gt) {
-			assert(igt_wait(in_rc6(sysfs, gt), 1000, 1));
+			assert(igt_wait(in_rc6(fd, gt), 1000, 1));
 		}
 	}
 
 	igt_subtest("rc0_on_exec") {
 		igt_require(!IS_PONTEVECCHIO(xe_dev_id(fd)));
 		xe_for_each_gt(fd, gt) {
-			assert(igt_wait(in_rc6(sysfs, gt), 1000, 1));
+			assert(igt_wait(in_rc6(fd, gt), 1000, 1));
 			xe_for_each_hw_engine(fd, hwe)
 				igt_fork(child, ncpus) {
 					igt_debug("Execution Started\n");
@@ -477,17 +492,16 @@ igt_main
 					igt_debug("Execution Finished\n");
 				}
 			/* While exec in threads above, let's check rc_status */
-			assert(igt_wait(!in_rc6(sysfs, gt), 1000, 1));
+			assert(igt_wait(!in_rc6(fd, gt), 1000, 1));
 			igt_waitchildren();
 		}
 	}
 
 	igt_fixture {
 		xe_for_each_gt(fd, gt) {
-			set_freq(sysfs, gt, "min", stash_min);
-			set_freq(sysfs, gt, "max", stash_max);
+			set_freq(fd, gt, "min", stash_min);
+			set_freq(fd, gt, "max", stash_max);
 		}
-		close(sysfs);
 		drm_close_driver(fd);
 	}
 }
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t v8 4/4] tests/xe/xe_sysfs_tile: adds new test to verify per tile vram addr_range
  2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
                   ` (2 preceding siblings ...)
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths Himal Prasad Ghimiray
@ 2023-07-07 11:30 ` Himal Prasad Ghimiray
  2023-07-07 11:51 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Handle GT and tile seperation in IGT (rev7) Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Himal Prasad Ghimiray @ 2023-07-07 11:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Upadhyay

For each tile the test reads the sysfs entry physical_vram_size_bytes
and compares the value with vram size exposed from query ioctl.

v2:
- Change sysfs entry name. (Tejas)
- Change test name to xe_sysfs_tile_prop. (Rahul)

v3:
- Rectify assertion condition.

v4:
- use igt_assert_lt_u64 instead of igt_assert_lt
for comparing u64.

v5:
- update xe_for_each_tile call.

v6:
- Rename testcase to xe_sysfs_tile
- Pass tilefd and use it in igt_sysfs_scanf. (Ashutosh)

Reviewed-by: Upadhyay <tejas.upadhyay@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Upadhyay <tejas.upadhyay@intel.com>
Cc: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 tests/meson.build        |  1 +
 tests/xe/xe_sysfs_tile.c | 55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 tests/xe/xe_sysfs_tile.c

diff --git a/tests/meson.build b/tests/meson.build
index ee066b849..0567449bf 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -268,6 +268,7 @@ xe_progs = [
 	'xe_pm',
 	'xe_prime_self_import',
 	'xe_query',
+	'xe_sysfs_tile',
 	'xe_vm',
 	'xe_waitfence',
 	'xe_spin_batch',
diff --git a/tests/xe/xe_sysfs_tile.c b/tests/xe/xe_sysfs_tile.c
new file mode 100644
index 000000000..9ac3397a8
--- /dev/null
+++ b/tests/xe/xe_sysfs_tile.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+/**
+ * TEST: Verify physical_vram_size_bytes of each tiles
+ * SUBTEST: physical_vram_size_bytes
+ * Description:
+ *             Read sysfs entry for physical_vram_size_bytes and compare with
+ *             vram size. physical_vram_size_bytes should be more than vram size.
+ */
+
+#include <string.h>
+#include <sys/time.h>
+
+#include "igt.h"
+#include "igt_sysfs.h"
+
+#include "xe_drm.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+
+static void test_vram_physical_vram_size_bytes(int tile_fd, int tile_num, u64 vram_size)
+{
+	u64 physical_vram_size_bytes;
+
+	igt_assert(igt_sysfs_scanf(tile_fd, "physical_vram_size_bytes",
+				   "%lx", &physical_vram_size_bytes) > 0);
+	igt_assert_lt_u64(vram_size, physical_vram_size_bytes);
+}
+
+igt_main
+{
+	int fd, tilefd, tile;
+	u64 vram_size;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_XE);
+		xe_device_get(fd);
+	}
+
+	igt_subtest("physical_vram_size_bytes") {
+		igt_require(xe_has_vram(fd));
+		xe_for_each_tile(fd, tilefd, tile) {
+			vram_size = xe_vram_size(fd, tile);
+			test_vram_physical_vram_size_bytes(tilefd, tile, vram_size);
+		}
+	}
+
+	igt_fixture {
+		xe_device_put(fd);
+		close(fd);
+	}
+}
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] ✗ Fi.CI.BUILD: failure for Handle GT and tile seperation in IGT (rev7)
  2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
                   ` (3 preceding siblings ...)
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 4/4] tests/xe/xe_sysfs_tile: adds new test to verify per tile vram addr_range Himal Prasad Ghimiray
@ 2023-07-07 11:51 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2023-07-07 11:51 UTC (permalink / raw)
  To: Himal Prasad Ghimiray; +Cc: igt-dev

== Series Details ==

Series: Handle GT and tile seperation in IGT (rev7)
URL   : https://patchwork.freedesktop.org/series/119801/
State : failure

== Summary ==

Applying: lib/igt_sysfs: Add support to query number of tiles
Applying: lib/igt_sysfs: Handling gt related sysfs uapi changes
Applying: tests/xe/xe_guc_pc: Change the sysfs paths
Using index info to reconstruct a base tree...
M	tests/xe/xe_guc_pc.c
Falling back to patching base and 3-way merge...
Auto-merging tests/xe/xe_guc_pc.c
CONFLICT (content): Merge conflict in tests/xe/xe_guc_pc.c
Patch failed at 0003 tests/xe/xe_guc_pc: Change the sysfs paths
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles Himal Prasad Ghimiray
@ 2023-07-08 19:29   ` Dixit, Ashutosh
  2023-07-10  4:12     ` Ghimiray, Himal Prasad
  0 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-07-08 19:29 UTC (permalink / raw)
  To: Himal Prasad Ghimiray; +Cc: igt-dev, Upadhyay

On Fri, 07 Jul 2023 04:30:52 -0700, Himal Prasad Ghimiray wrote:
>
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 5635fc690..5d584b1c7 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -38,6 +38,11 @@
>	     (dirfd__ = igt_sysfs_gt_open(i915__, gt__)) != -1; \
>	     close(dirfd__), gt__++)
>
> +#define for_each_sysfs_tile_dirfd(xe__, dirfd__, tile__) \
> +	for (tile__ = 0; \
> +	     (dirfd__ = xe_sysfs_tile_open(xe__, tile__)) != -1; \
> +	     close(dirfd__), tile__++)
> +
>  #define i915_for_each_gt for_each_sysfs_gt_dirfd
>
>  #define igt_sysfs_rps_write(dir, id, data, len) \
> @@ -73,6 +78,8 @@
>  #define igt_sysfs_rps_set_boolean(dir, id, value) \
>	igt_sysfs_set_boolean(dir, igt_sysfs_dir_id_to_name(dir, id), value)
>
> +#define xe_for_each_tile for_each_sysfs_tile_dirfd

Let's delete xe_for_each_tile and directly use for_each_sysfs_tile_dirfd in
Patch 4. Because xe_for_each_gt is written without the dirfd so it will be
confusing if xe_for_each_tile now has dirfd.

Thanks.
--
Ashutosh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes Himal Prasad Ghimiray
@ 2023-07-08 19:30   ` Dixit, Ashutosh
  2023-07-10  4:14     ` Ghimiray, Himal Prasad
  0 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-07-08 19:30 UTC (permalink / raw)
  To: Himal Prasad Ghimiray; +Cc: Upadhyay, igt-dev

On Fri, 07 Jul 2023 04:30:53 -0700, Himal Prasad Ghimiray wrote:
>
> +char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int pathlen)
> +{
> +	struct stat st;
> +
> +	if (xe_device < 0)
> +		return NULL;
> +
> +	if (igt_debug_on(fstat(xe_device, &st)) || igt_debug_on(!S_ISCHR(st.st_mode)))
> +		return NULL;
> +
> +	if (IS_METEORLAKE(xe_device))

This is wrong, it should be:

	if (IS_METEORLAKE(intel_get_drm_devid(xe_device)))

Also why not use the IS_PONTEVECCHIO() check as I suggested? Several
upcoming platforms will have the MTL tile/gt configuration so we'll need to
keep fixing this, if we use PVC we can leave this code alone for a
while. Anyway I am ok as is too.

Also maybe move these 2 functions above igt_sysfs_gt_path() in the file. I
have a reason for that, will explain later.

Apart from this, almost there I think. I can suggest a couple more changes
but will hold off so that we can get this merged first.

But the build seems to have failed so let's fix that and also make sure
xe_guc_pc IGT's are passing in pre-merge CI (just in case we've missed
anything). I'll take a look and then R-b the patches.

Thanks.
--
Ashutosh


> +		snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile0/gt%d",
> +			 major(st.st_rdev), minor(st.st_rdev), gt);
> +	else
> +		snprintf(path, pathlen, "/sys/dev/char/%d:%d/device/tile%d/gt%d",
> +			 major(st.st_rdev), minor(st.st_rdev), gt, gt);
> +
> +	if (!access(path, F_OK))
> +		return path;
> +
> +	return NULL;
> +}
> +
> +/**
> + * xe_sysfs_gt_open:
> + * @xe_device: fd of the device
> + * @gt: gt number
> + *
> + * This opens the sysfs gt directory corresponding to device and tile for use
> + *
> + * Returns:
> + * The directory fd, or -1 on failure.
> + */
> +int xe_sysfs_gt_open(int xe_device, int gt)
> +{
> +	char path[96];
> +
> +	if (!xe_sysfs_gt_path(xe_device, gt, path, sizeof(path)))
> +		return -1;
> +
> +	return open(path, O_RDONLY);
> +}
> +
>  /**
>   * xe_sysfs_tile_path:
>   * @xe_device: fd of the device
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 5d584b1c7..afcafb07c 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -157,6 +157,8 @@ void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
>  void igt_sysfs_engines(int xe, int engines, const char **property,
>		       void (*test)(int, int, const char **));
>
> +char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int pathlen);
> +int xe_sysfs_gt_open(int xe_device, int gt);
>  char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int pathlen);
>  int xe_sysfs_tile_open(int xe_device, int tile);
>  int xe_sysfs_get_num_tiles(int xe_device);
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths
  2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths Himal Prasad Ghimiray
@ 2023-07-08 19:30   ` Dixit, Ashutosh
  0 siblings, 0 replies; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-07-08 19:30 UTC (permalink / raw)
  To: Himal Prasad Ghimiray; +Cc: Upadhyay, igt-dev, Badal Nilawar

On Fri, 07 Jul 2023 04:30:54 -0700, Himal Prasad Ghimiray wrote:
> diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c
> index 827693eb4..e20ab5f88 100644
> --- a/tests/xe/xe_guc_pc.c
> +++ b/tests/xe/xe_guc_pc.c
> @@ -133,25 +133,38 @@ static void exec_basic(int fd, struct drm_xe_engine_class_instance *eci,
>	xe_vm_destroy(fd, vm);
>  }
>
> -static int set_freq(int sysfs, int gt_id, const char *freq_name, uint32_t freq)
> +static int set_freq(int fd, int gt_id, const char *freq_name, uint32_t freq)
>  {
>	int ret = -EAGAIN;
> -	char path[32];
> +	char freq_attr[16];
> +	int gt_fd;
> +
> +	snprintf(freq_attr, sizeof(freq_attr), "freq_%s", freq_name);
> +	gt_fd = xe_sysfs_gt_open(fd, gt_id);
> +	igt_assert(gt_fd != -1);

nit: probably doesn't matter and ok as is too, but I'd change this assert
to:

	igt_assert(gt_fd >= 0);

Since that's the convention followed in the code.

It's there in several places in the file.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles
  2023-07-08 19:29   ` Dixit, Ashutosh
@ 2023-07-10  4:12     ` Ghimiray, Himal Prasad
  0 siblings, 0 replies; 11+ messages in thread
From: Ghimiray, Himal Prasad @ 2023-07-10  4:12 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: igt-dev@lists.freedesktop.org, Upadhyay, Tejas



> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: 09 July 2023 01:00
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Iddamsetty, Aravind
> <aravind.iddamsetty@intel.com>; Upadhyay, Tejas
> <tejas.upadhyay@intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar@intel.com>
> Subject: Re: [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number
> of tiles
> 
> On Fri, 07 Jul 2023 04:30:52 -0700, Himal Prasad Ghimiray wrote:
> >
> > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h index
> > 5635fc690..5d584b1c7 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -38,6 +38,11 @@
> >	     (dirfd__ = igt_sysfs_gt_open(i915__, gt__)) != -1; \
> >	     close(dirfd__), gt__++)
> >
> > +#define for_each_sysfs_tile_dirfd(xe__, dirfd__, tile__) \
> > +	for (tile__ = 0; \
> > +	     (dirfd__ = xe_sysfs_tile_open(xe__, tile__)) != -1; \
> > +	     close(dirfd__), tile__++)
> > +
> >  #define i915_for_each_gt for_each_sysfs_gt_dirfd
> >
> >  #define igt_sysfs_rps_write(dir, id, data, len) \  @@ -73,6 +78,8 @@
> >  #define igt_sysfs_rps_set_boolean(dir, id, value) \
> >	igt_sysfs_set_boolean(dir, igt_sysfs_dir_id_to_name(dir, id), value)
> >
> > +#define xe_for_each_tile for_each_sysfs_tile_dirfd
> 
> Let's delete xe_for_each_tile and directly use for_each_sysfs_tile_dirfd in
> Patch 4. Because xe_for_each_gt is written without the dirfd so it will be
> confusing if xe_for_each_tile now has dirfd.

Sure.

> 
> Thanks.
> --
> Ashutosh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes
  2023-07-08 19:30   ` Dixit, Ashutosh
@ 2023-07-10  4:14     ` Ghimiray, Himal Prasad
  0 siblings, 0 replies; 11+ messages in thread
From: Ghimiray, Himal Prasad @ 2023-07-10  4:14 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: Upadhyay, Tejas, igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: 09 July 2023 01:00
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Kamil Konieczny
> <kamil.konieczny@linux.intel.com>; Iddamsetty, Aravind
> <aravind.iddamsetty@intel.com>; Upadhyay, Tejas
> <tejas.upadhyay@intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar@intel.com>; Dugast, Francois
> <francois.dugast@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: Re: [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi
> changes
> 
> On Fri, 07 Jul 2023 04:30:53 -0700, Himal Prasad Ghimiray wrote:
> >
> > +char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int
> > +pathlen) {
> > +	struct stat st;
> > +
> > +	if (xe_device < 0)
> > +		return NULL;
> > +
> > +	if (igt_debug_on(fstat(xe_device, &st)) ||
> igt_debug_on(!S_ISCHR(st.st_mode)))
> > +		return NULL;
> > +
> > +	if (IS_METEORLAKE(xe_device))
> 
> This is wrong, it should be:
> 
> 	if (IS_METEORLAKE(intel_get_drm_devid(xe_device)))

Thanks for pointing this.

> 
> Also why not use the IS_PONTEVECCHIO() check as I suggested? Several
> upcoming platforms will have the MTL tile/gt configuration so we'll need to
> keep fixing this, if we use PVC we can leave this code alone for a while.
> Anyway I am ok as is too.

Will change to IS_PONTEVECCHIO()

> 
> Also maybe move these 2 functions above igt_sysfs_gt_path() in the file. I
> have a reason for that, will explain later.
Ok.
> 
> Apart from this, almost there I think. I can suggest a couple more changes
> but will hold off so that we can get this merged first.
> 
> But the build seems to have failed so let's fix that and also make sure
> xe_guc_pc IGT's are passing in pre-merge CI (just in case we've missed
> anything). I'll take a look and then R-b the patches.

New changes merged on Friday causing conflict. Will address the conflicts and push new patch.

> 
> Thanks.
> --
> Ashutosh
> 
> 
> > +		snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile0/gt%d",
> > +			 major(st.st_rdev), minor(st.st_rdev), gt);
> > +	else
> > +		snprintf(path, pathlen,
> "/sys/dev/char/%d:%d/device/tile%d/gt%d",
> > +			 major(st.st_rdev), minor(st.st_rdev), gt, gt);
> > +
> > +	if (!access(path, F_OK))
> > +		return path;
> > +
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * xe_sysfs_gt_open:
> > + * @xe_device: fd of the device
> > + * @gt: gt number
> > + *
> > + * This opens the sysfs gt directory corresponding to device and tile
> > +for use
> > + *
> > + * Returns:
> > + * The directory fd, or -1 on failure.
> > + */
> > +int xe_sysfs_gt_open(int xe_device, int gt) {
> > +	char path[96];
> > +
> > +	if (!xe_sysfs_gt_path(xe_device, gt, path, sizeof(path)))
> > +		return -1;
> > +
> > +	return open(path, O_RDONLY);
> > +}
> > +
> >  /**
> >   * xe_sysfs_tile_path:
> >   * @xe_device: fd of the device
> > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h index
> > 5d584b1c7..afcafb07c 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -157,6 +157,8 @@ void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t
> >*rw);
> >  void igt_sysfs_engines(int xe, int engines, const char **property,
> >		       void (*test)(int, int, const char **));
> >
> > +char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int
> > +pathlen); int xe_sysfs_gt_open(int xe_device, int gt);
> >  char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int
> > pathlen);  int xe_sysfs_tile_open(int xe_device, int tile);  int
> > xe_sysfs_get_num_tiles(int xe_device);
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-07-10  4:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-07 11:30 [igt-dev] [PATCH i-g-t v8 0/4] Handle GT and tile seperation in IGT Himal Prasad Ghimiray
2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 1/4] lib/igt_sysfs: Add support to query number of tiles Himal Prasad Ghimiray
2023-07-08 19:29   ` Dixit, Ashutosh
2023-07-10  4:12     ` Ghimiray, Himal Prasad
2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes Himal Prasad Ghimiray
2023-07-08 19:30   ` Dixit, Ashutosh
2023-07-10  4:14     ` Ghimiray, Himal Prasad
2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 3/4] tests/xe/xe_guc_pc: Change the sysfs paths Himal Prasad Ghimiray
2023-07-08 19:30   ` Dixit, Ashutosh
2023-07-07 11:30 ` [igt-dev] [PATCH i-g-t v8 4/4] tests/xe/xe_sysfs_tile: adds new test to verify per tile vram addr_range Himal Prasad Ghimiray
2023-07-07 11:51 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Handle GT and tile seperation in IGT (rev7) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox