[igt-dev] [PATCH i-g-t v6 0/4] Handle GT and tile seperation in IGT

[igt-dev] [PATCH i-g-t v6 0/4] Handle GT and tile seperation in IGT

[Himal Prasad Ghimiray]

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_prop: adds new test to verify per tile vram
    addr_range

 lib/igt_sysfs.c               | 70 +++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h               |  8 ++++
 tests/meson.build             |  1 +
 tests/xe/xe_guc_pc.c          | 14 ++++++-
 tests/xe/xe_sysfs_tile_prop.c | 63 +++++++++++++++++++++++++++++++
 5 files changed, 154 insertions(+), 2 deletions(-)
 create mode 100644 tests/xe/xe_sysfs_tile_prop.c

-- 
2.25.1

[igt-dev] [PATCH i-g-t v6 1/4] lib/igt_sysfs: Add support to query number of tiles

[Himal Prasad Ghimiray]

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)

Reviewed-by: Upadhyay <tejas.upadhyay@intel.com>
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 | 30 ++++++++++++++++++++++++++++++
 lib/igt_sysfs.h |  7 +++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 0876f4c6b..2ea9eb1b9 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -287,6 +287,36 @@ int igt_sysfs_get_num_gt(int device)
 	return num_gts;
 }
 
+/**
+ * igt_sysfs_get_num_tiles:
+ * @device: fd of the device
+ *
+ * Reads number of tiles sysfs entries.
+ * Asserts for at least one tile entry.
+ *
+ * Returns: Number of tiles.
+ */
+int igt_sysfs_get_num_tiles(int device)
+{
+	int num_tiles;
+	char sysfs[48];
+	char tiledir[96];
+
+	num_tiles = 0;
+	if (!igt_sysfs_path(device, sysfs, sizeof(sysfs)))
+		return -1;
+
+	do {
+		igt_assert(snprintf(tiledir, sizeof(tiledir), "%s/device/tile%d",
+				    sysfs, num_tiles) < sizeof(tiledir));
+		num_tiles++;
+	} while (!access(tiledir, F_OK));
+
+	num_tiles--;
+	igt_assert_f(num_tiles > 0, "No tiles sysfs entry is found.");
+	return num_tiles;
+}
+
 /**
  * igt_sysfs_write:
  * @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 5635fc690..c31765542 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -73,6 +73,12 @@
 #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(xe__, tile__, tile_cnt__) \
+	for (tile__ = 0, tile_cnt__ = igt_sysfs_get_num_tiles(xe__); \
+	     tile__ < tile_cnt__; \
+	     ++tile__)
+
+
 enum i915_attr_id {
 	RPS_ACT_FREQ_MHZ,
 	RPS_CUR_FREQ_MHZ,
@@ -97,6 +103,7 @@ int igt_sysfs_open(int device);
 char *igt_sysfs_gt_path(int device, int gt, char *path, int pathlen);
 int igt_sysfs_gt_open(int device, int gt);
 int igt_sysfs_get_num_gt(int device);
+int igt_sysfs_get_num_tiles(int device);
 bool igt_sysfs_has_attr(int dir, const char *attr);
 const char *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id);
 const char *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id);
-- 
2.25.1

[igt-dev] [PATCH i-g-t v6 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes

[Himal Prasad Ghimiray]

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:
- Initialize the variable and use for loop
instead of while.

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 | 40 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 2ea9eb1b9..32d78cfb6 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -46,6 +46,7 @@
 #include "igt_device.h"
 #include "igt_io.h"
 
+#define XE_MAX_GT_SYSFS_PATH 3
 /**
  * SECTION:igt_sysfs
  * @short_description: Support code for sysfs features
@@ -933,3 +934,42 @@ void igt_sysfs_engines(int xe, int engines, const char **property,
 		close(engine_fd);
 	}
 }
+
+/**
+ * xe_gt_sysfs_path::
+ * @sysfs: fd of sysfs
+ * @gt_id: gt id
+ *
+ * This function returns the path for gtX directory
+ */
+char *xe_gt_sysfs_path(int sysfs, int gt_id)
+{
+	char path[XE_MAX_GT_SYSFS_PATH][32];
+	struct stat gtpathstat;
+	char *gt_path;
+	int i;
+
+	/* Path for multitile platforms with single gt on each tile
+	 * Tile id will be same as gt id.
+	 */
+	snprintf(path[0], sizeof(path[0]), "device/tile%d/gt%d/", gt_id, gt_id);
+
+	/* Path for single tile platforms with multiple gt's
+	 * all gt's will be under tile0 eg: MTL.
+	 */
+	snprintf(path[1], sizeof(path[1]), "device/tile0/gt%d/", gt_id);
+
+	/* Path not under tile.
+	 * To support older dir structure.
+	 */
+	snprintf(path[2], sizeof(path[2]), "device/gt%d/", gt_id);
+
+	for (i = 0; i < XE_MAX_GT_SYSFS_PATH; i++) {
+		if (!fstatat(sysfs, path[i], &gtpathstat, 0) && S_ISDIR(gtpathstat.st_mode)) {
+			gt_path = malloc(sizeof(path[i]));
+			strcpy(gt_path, path[i]);
+			return gt_path;
+		}
+	}
+	igt_assert_f(false, "No gt%d dir found in sysfs", gt_id);
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index c31765542..a937ee7b5 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -153,6 +153,7 @@ typedef struct igt_sysfs_rw_attr {
 } igt_sysfs_rw_attr_t;
 
 void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
+char *xe_gt_sysfs_path(int sysfs, int gt_id);
 
 void igt_sysfs_engines(int xe, int engines, const char **property,
 		       void (*test)(int, int, const char **));
-- 
2.25.1

[igt-dev] [PATCH i-g-t v6 3/4] tests/xe/xe_guc_pc: Change the sysfs paths

[Himal Prasad Ghimiray]

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)

Reviewed-by: Upadhyay <tejas.upadhyay@intel.com>
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 | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c
index 827693eb4..046791d51 100644
--- a/tests/xe/xe_guc_pc.c
+++ b/tests/xe/xe_guc_pc.c
@@ -137,8 +137,13 @@ static int set_freq(int sysfs, int gt_id, const char *freq_name, uint32_t freq)
 {
 	int ret = -EAGAIN;
 	char path[32];
+	char *gt_path;
+
+	gt_path = xe_gt_sysfs_path(sysfs, gt_id);
+	igt_assert(snprintf(path, sizeof(path), "%s/freq_%s",
+			    xe_gt_sysfs_path(sysfs, gt_id), freq_name) < sizeof(path));
+	free(gt_path);
 
-	sprintf(path, "device/gt%d/freq_%s", gt_id, freq_name);
 	while (ret == -EAGAIN)
 		ret = igt_sysfs_printf(sysfs, path, "%u", freq);
 	return ret;
@@ -149,7 +154,12 @@ static uint32_t get_freq(int sysfs, 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 *gt_path;
+
+	gt_path = xe_gt_sysfs_path(sysfs, gt_id);
+	igt_assert(snprintf(path, sizeof(path), "%s/freq_%s",
+			    gt_path, freq_name) < sizeof(path));
+	free(gt_path);
 	while (err == -EAGAIN)
 		err = igt_sysfs_scanf(sysfs, path, "%u", &freq);
 	return freq;
-- 
2.25.1

[igt-dev] [PATCH i-g-t v6 4/4] tests/xe/xe_sysfs_tile_prop: adds new test to verify per tile vram addr_range

[Himal Prasad Ghimiray]

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.

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_prop.c | 63 +++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 tests/xe/xe_sysfs_tile_prop.c

diff --git a/tests/meson.build b/tests/meson.build
index ee066b849..fac9123b5 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_prop',
 	'xe_vm',
 	'xe_waitfence',
 	'xe_spin_batch',
diff --git a/tests/xe/xe_sysfs_tile_prop.c b/tests/xe/xe_sysfs_tile_prop.c
new file mode 100644
index 000000000..293506c46
--- /dev/null
+++ b/tests/xe/xe_sysfs_tile_prop.c
@@ -0,0 +1,63 @@
+// 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 sysfs, int tile_num, u64 vram_size)
+{
+	u64 physical_vram_size_bytes;
+	char path[40];
+
+	igt_assert(snprintf(path, sizeof(path), "device/tile%d/physical_vram_size_bytes",
+			    tile_num) < sizeof(path));
+	igt_assert(igt_sysfs_scanf(sysfs, path, "%lx", &physical_vram_size_bytes) > 0);
+	igt_assert_lt_u64(vram_size, physical_vram_size_bytes);
+}
+
+igt_main
+{
+	int fd;
+	int tile, total_tiles;
+	static int sysfs = -1;
+	u64 vram_size;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_XE);
+		xe_device_get(fd);
+
+		sysfs = igt_sysfs_open(fd);
+		igt_assert(sysfs != -1);
+	}
+
+	igt_subtest("physical_vram_size_bytes") {
+		igt_require(xe_has_vram(fd));
+		xe_for_each_tile(fd, tile, total_tiles) {
+			vram_size = xe_vram_size(fd, tile);
+			test_vram_physical_vram_size_bytes(sysfs, tile, vram_size);
+		}
+	}
+
+	igt_fixture {
+		close(sysfs);
+		xe_device_put(fd);
+		close(fd);
+	}
+}
-- 
2.25.1

[igt-dev] ✗ GitLab.Pipeline: warning for Handle GT and tile seperation in IGT (rev5)

[Patchwork]

== Series Details ==

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

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/927340 for the overview.

build-containers:build-debian has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/44957835):
      "DockerVersion": "",
      "Labels": {},
      "Architecture": "amd64",
      "Os": "linux",
      "Layers": [
          "sha256:46a290212d18d6f820439b9a479e0894356e5f5b3409e37adeb7cd0d480fc368"
      ]
  }
  Skipping, already built
  Getting image source signatures
  Copying blob sha256:46a290212d18d6f820439b9a479e0894356e5f5b3409e37adeb7cd0d480fc368
  Copying config sha256:ca01fc804bb92e5df42a202dd7e0470610c6711c66a808525defcb8bbb774078
  Writing manifest to image destination
  time="2023-07-05T13:37:46Z" level=fatal msg="Error writing manifest: Error uploading manifest commit-16f34045c6fa4c608b462a81a7c6f8d504004de4 to registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian: received unexpected HTTP status: 500 Internal Server Error" 
  section_end:1688564266:step_script
  section_start:1688564266:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1688564267:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/927340

[igt-dev] ✗ Fi.CI.BAT: failure for Handle GT and tile seperation in IGT (rev5)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 13840 bytes --]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from IGT_7371 -> IGTPW_9339
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_9339 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9339, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/index.html

Participating hosts (40 -> 40)
------------------------------

  Additional (2): fi-kbl-soraka bat-atsm-1 
  Missing    (2): bat-rpls-2 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_9339:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_addfb_basic@bad-pitch-0:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-kbl-soraka/igt@kms_addfb_basic@bad-pitch-0.html

  
Known issues
------------

  Here are the changes found in IGTPW_9339 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-adlp-11:        NOTRUN -> [SKIP][2] ([i915#7456])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@gem_mmap@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][5] ([i915#4083])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][6] ([i915#4079]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][7] ([i915#4077]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-adlp-11:        NOTRUN -> [SKIP][8] ([i915#3282])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - bat-mtlp-8:         [PASS][9] -> [ABORT][10] ([i915#7077] / [i915#7977] / [i915#8668])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rps@basic-api:
    - bat-atsm-1:         NOTRUN -> [SKIP][11] ([i915#6621])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][12] ([i915#1886] / [i915#7913])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-atsm-1:         NOTRUN -> [SKIP][13] ([i915#6645])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@size-max:
    - bat-atsm-1:         NOTRUN -> [SKIP][14] ([i915#6077]) +36 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_addfb_basic@size-max.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - bat-adlp-11:        NOTRUN -> [SKIP][15] ([i915#7828]) +7 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - fi-glk-j4005:       NOTRUN -> [SKIP][16] ([fdo#109271])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-glk-j4005/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
    - fi-bsw-n3050:       NOTRUN -> [SKIP][17] ([fdo#109271])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-bsw-n3050/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][18] ([fdo#109271]) +15 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-adlp-11:        NOTRUN -> [SKIP][19] ([i915#4103]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - bat-atsm-1:         NOTRUN -> [SKIP][20] ([i915#6078]) +19 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - bat-atsm-1:         NOTRUN -> [SKIP][21] ([i915#6166]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_flip@basic-flip-vs-wf_vblank.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-atsm-1:         NOTRUN -> [SKIP][22] ([i915#6093]) +3 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-adlp-11:        NOTRUN -> [SKIP][23] ([i915#4093]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][24] ([i915#1845] / [i915#5354]) +3 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
    - bat-atsm-1:         NOTRUN -> [SKIP][25] ([i915#1836]) +6 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html

  * igt@kms_prop_blob@basic:
    - bat-atsm-1:         NOTRUN -> [SKIP][26] ([i915#7357])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_prop_blob@basic.html

  * igt@kms_psr@primary_mmap_gtt:
    - bat-rplp-1:         NOTRUN -> [SKIP][27] ([i915#1072]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-atsm-1:         NOTRUN -> [SKIP][28] ([i915#1072]) +3 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rplp-1:         NOTRUN -> [ABORT][29] ([i915#8260] / [i915#8668])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-rplp-1/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-atsm-1:         NOTRUN -> [SKIP][30] ([i915#6094])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-atsm-1:         NOTRUN -> [SKIP][31] ([fdo#109295] / [i915#6078])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-gtt:
    - bat-atsm-1:         NOTRUN -> [SKIP][32] ([fdo#109295] / [i915#4077]) +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-write:
    - bat-atsm-1:         NOTRUN -> [SKIP][33] ([fdo#109295]) +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-atsm-1/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests@dma_fence:
    - fi-glk-j4005:       [DMESG-FAIL][34] ([i915#8189]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/fi-glk-j4005/igt@dmabuf@all-tests@dma_fence.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-glk-j4005/igt@dmabuf@all-tests@dma_fence.html

  * igt@dmabuf@all-tests@sanitycheck:
    - fi-glk-j4005:       [ABORT][36] ([i915#8143] / [i915#8144] / [i915#8218]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/fi-glk-j4005/igt@dmabuf@all-tests@sanitycheck.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-glk-j4005/igt@dmabuf@all-tests@sanitycheck.html

  * igt@i915_module_load@load:
    - bat-adlp-11:        [ABORT][38] ([i915#4423]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/bat-adlp-11/igt@i915_module_load@load.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-adlp-11/igt@i915_module_load@load.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-n3050:       [ABORT][40] ([i915#7913]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/fi-bsw-n3050/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@guc:
    - bat-rpls-1:         [DMESG-WARN][42] ([i915#7852]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/bat-rpls-1/igt@i915_selftest@live@guc.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-rpls-1/igt@i915_selftest@live@guc.html

  
#### Warnings ####

  * igt@kms_psr@cursor_plane_move:
    - bat-rplp-1:         [ABORT][44] ([i915#8434] / [i915#8668]) -> [SKIP][45] ([i915#1072])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7371/bat-rplp-1/igt@kms_psr@cursor_plane_move.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/bat-rplp-1/igt@kms_psr@cursor_plane_move.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
  [i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
  [i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
  [i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
  [i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7077]: https://gitlab.freedesktop.org/drm/intel/issues/7077
  [i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7852]: https://gitlab.freedesktop.org/drm/intel/issues/7852
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7977]: https://gitlab.freedesktop.org/drm/intel/issues/7977
  [i915#8143]: https://gitlab.freedesktop.org/drm/intel/issues/8143
  [i915#8144]: https://gitlab.freedesktop.org/drm/intel/issues/8144
  [i915#8189]: https://gitlab.freedesktop.org/drm/intel/issues/8189
  [i915#8218]: https://gitlab.freedesktop.org/drm/intel/issues/8218
  [i915#8260]: https://gitlab.freedesktop.org/drm/intel/issues/8260
  [i915#8434]: https://gitlab.freedesktop.org/drm/intel/issues/8434
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7371 -> IGTPW_9339

  CI-20190529: 20190529
  CI_DRM_13346: c5442b2363bf5ad916805d105ff03ce5805070e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9339: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/index.html
  IGT_7371: f8d05fd574fad1526cbf5e20672910df87b8839b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@xe_sysfs_tile_prop@physical_vram_size_bytes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9339/index.html

[-- Attachment #2: Type: text/html, Size: 16257 bytes --]

Re: [igt-dev] [PATCH i-g-t v6 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes

[Dixit, Ashutosh]

On Wed, 05 Jul 2023 06:18:51 -0700, Himal Prasad Ghimiray wrote:
>
> 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:
> - Initialize the variable and use for loop
> instead of while.
>
> 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 | 40 ++++++++++++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h |  1 +
>  2 files changed, 41 insertions(+)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 2ea9eb1b9..32d78cfb6 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -46,6 +46,7 @@
>  #include "igt_device.h"
>  #include "igt_io.h"
>
> +#define XE_MAX_GT_SYSFS_PATH 3
>  /**
>   * SECTION:igt_sysfs
>   * @short_description: Support code for sysfs features
> @@ -933,3 +934,42 @@ void igt_sysfs_engines(int xe, int engines, const char **property,
>		close(engine_fd);
>	}
>  }
> +
> +/**
> + * xe_gt_sysfs_path::
> + * @sysfs: fd of sysfs
> + * @gt_id: gt id
> + *
> + * This function returns the path for gtX directory
> + */
> +char *xe_gt_sysfs_path(int sysfs, int gt_id)
> +{
> +	char path[XE_MAX_GT_SYSFS_PATH][32];
> +	struct stat gtpathstat;
> +	char *gt_path;
> +	int i;
> +
> +	/* Path for multitile platforms with single gt on each tile
> +	 * Tile id will be same as gt id.
> +	 */
> +	snprintf(path[0], sizeof(path[0]), "device/tile%d/gt%d/", gt_id, gt_id);
> +
> +	/* Path for single tile platforms with multiple gt's
> +	 * all gt's will be under tile0 eg: MTL.
> +	 */
> +	snprintf(path[1], sizeof(path[1]), "device/tile0/gt%d/", gt_id);
> +
> +	/* Path not under tile.
> +	 * To support older dir structure.
> +	 */
> +	snprintf(path[2], sizeof(path[2]), "device/gt%d/", gt_id);

Is this needed now? If not I'd say get rid of it.

> +
> +	for (i = 0; i < XE_MAX_GT_SYSFS_PATH; i++) {
> +		if (!fstatat(sysfs, path[i], &gtpathstat, 0) && S_ISDIR(gtpathstat.st_mode)) {
> +			gt_path = malloc(sizeof(path[i]));
> +			strcpy(gt_path, path[i]);
> +			return gt_path;
> +		}
> +	}
> +	igt_assert_f(false, "No gt%d dir found in sysfs", gt_id);
> +}

I prefer this: start with tile0, look for all gt%d under under tile0 and
see if gtN is there, repeat for next tile, till we have gone over all
tiles. This will work always whereas the code above breaks as soon as we
have a platform which has say: tile0/gt0, tile0/gt1, tile1/gt2.

Since we are doing the work, with just a little extra work, do a good job
of it.

Thanks.
--
Ashutosh





> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index c31765542..a937ee7b5 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -153,6 +153,7 @@ typedef struct igt_sysfs_rw_attr {
>  } igt_sysfs_rw_attr_t;
>
>  void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
> +char *xe_gt_sysfs_path(int sysfs, int gt_id);
>
>  void igt_sysfs_engines(int xe, int engines, const char **property,
>		       void (*test)(int, int, const char **));
> --
> 2.25.1
>

Re: [igt-dev] [PATCH i-g-t v6 1/4] lib/igt_sysfs: Add support to query number of tiles

[Dixit, Ashutosh]

On Wed, 05 Jul 2023 06:18:50 -0700, Himal Prasad Ghimiray wrote:
>

Hi Himal,

> 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)
>
> Reviewed-by: Upadhyay <tejas.upadhyay@intel.com>
> 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 | 30 ++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h |  7 +++++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 0876f4c6b..2ea9eb1b9 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -287,6 +287,36 @@ int igt_sysfs_get_num_gt(int device)
>	return num_gts;
>  }
>
> +/**
> + * igt_sysfs_get_num_tiles:
> + * @device: fd of the device
> + *
> + * Reads number of tiles sysfs entries.
> + * Asserts for at least one tile entry.
> + *
> + * Returns: Number of tiles.
> + */
> +int igt_sysfs_get_num_tiles(int device)
> +{
> +	int num_tiles;
> +	char sysfs[48];
> +	char tiledir[96];
> +
> +	num_tiles = 0;
> +	if (!igt_sysfs_path(device, sysfs, sizeof(sysfs)))
> +		return -1;
> +
> +	do {
> +		igt_assert(snprintf(tiledir, sizeof(tiledir), "%s/device/tile%d",
> +				    sysfs, num_tiles) < sizeof(tiledir));
> +		num_tiles++;
> +	} while (!access(tiledir, F_OK));
> +
> +	num_tiles--;

This decrement is ugly. Can we get rid of it?

I seriously think we should model these functions similar to
igt_sysfs_gt_open, igt_sysfs_get_num_gt, igt_sysfs_gt_path,
for_each_sysfs_gt_dirfd. So we could have xe_sysfs_tile_open,
xe_sysfs_get_num_tile, xe_sysfs_tile_path, xe_for_each_sysfs_tile_dirfd
etc. This will cleanup patch 4 too.

Thanks.
--
Ashutosh

> +	igt_assert_f(num_tiles > 0, "No tiles sysfs entry is found.");
> +	return num_tiles;
> +}
> +
>  /**
>   * igt_sysfs_write:
>   * @dir: directory for the device from igt_sysfs_open()
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 5635fc690..c31765542 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -73,6 +73,12 @@
>  #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(xe__, tile__, tile_cnt__) \
> +	for (tile__ = 0, tile_cnt__ = igt_sysfs_get_num_tiles(xe__); \
> +	     tile__ < tile_cnt__; \
> +	     ++tile__)
> +
> +
>  enum i915_attr_id {
>	RPS_ACT_FREQ_MHZ,
>	RPS_CUR_FREQ_MHZ,
> @@ -97,6 +103,7 @@ int igt_sysfs_open(int device);
>  char *igt_sysfs_gt_path(int device, int gt, char *path, int pathlen);
>  int igt_sysfs_gt_open(int device, int gt);
>  int igt_sysfs_get_num_gt(int device);
> +int igt_sysfs_get_num_tiles(int device);
>  bool igt_sysfs_has_attr(int dir, const char *attr);
>  const char *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id);
>  const char *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id);
> --
> 2.25.1
>

Re: [igt-dev] [PATCH i-g-t v6 1/4] lib/igt_sysfs: Add support to query number of tiles

[Ghimiray, Himal Prasad]

> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: 06 July 2023 07:33
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Upadhyay, Tejas
> <tejas.upadhyay@intel.com>; Iddamsetty, Aravind
> <aravind.iddamsetty@intel.com>; Kumar, Janga Rahul
> <janga.rahul.kumar@intel.com>
> Subject: Re: [PATCH i-g-t v6 1/4] lib/igt_sysfs: Add support to query number
> of tiles
> 
> On Wed, 05 Jul 2023 06:18:50 -0700, Himal Prasad Ghimiray wrote:
> >
> 
> Hi Himal,
> 
> > 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)
> >
> > Reviewed-by: Upadhyay <tejas.upadhyay@intel.com>
> > 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 | 30 ++++++++++++++++++++++++++++++  lib/igt_sysfs.h
> > |  7 +++++++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c index
> > 0876f4c6b..2ea9eb1b9 100644
> > --- a/lib/igt_sysfs.c
> > +++ b/lib/igt_sysfs.c
> > @@ -287,6 +287,36 @@ int igt_sysfs_get_num_gt(int device)
> >	return num_gts;
> >  }
> >
> > +/**
> > + * igt_sysfs_get_num_tiles:
> > + * @device: fd of the device
> > + *
> > + * Reads number of tiles sysfs entries.
> > + * Asserts for at least one tile entry.
> > + *
> > + * Returns: Number of tiles.
> > + */
> > +int igt_sysfs_get_num_tiles(int device) {
> > +	int num_tiles;
> > +	char sysfs[48];
> > +	char tiledir[96];
> > +
> > +	num_tiles = 0;
> > +	if (!igt_sysfs_path(device, sysfs, sizeof(sysfs)))
> > +		return -1;
> > +
> > +	do {
> > +		igt_assert(snprintf(tiledir, sizeof(tiledir), "%s/device/tile%d",
> > +				    sysfs, num_tiles) < sizeof(tiledir));
> > +		num_tiles++;
> > +	} while (!access(tiledir, F_OK));
> > +
> > +	num_tiles--;
> 
> This decrement is ugly. Can we get rid of it?
Will address this.
> 
> I seriously think we should model these functions similar to
> igt_sysfs_gt_open, igt_sysfs_get_num_gt, igt_sysfs_gt_path,
> for_each_sysfs_gt_dirfd. So we could have xe_sysfs_tile_open,
> xe_sysfs_get_num_tile, xe_sysfs_tile_path, xe_for_each_sysfs_tile_dirfd etc.
> This will cleanup patch 4 too.

Let me look into it.
> 
> Thanks.
> --
> Ashutosh
> 
> > +	igt_assert_f(num_tiles > 0, "No tiles sysfs entry is found.");
> > +	return num_tiles;
> > +}
> > +
> >  /**
> >   * igt_sysfs_write:
> >   * @dir: directory for the device from igt_sysfs_open() diff --git
> > a/lib/igt_sysfs.h b/lib/igt_sysfs.h index 5635fc690..c31765542 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -73,6 +73,12 @@
> >  #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(xe__, tile__, tile_cnt__) \
> > +	for (tile__ = 0, tile_cnt__ = igt_sysfs_get_num_tiles(xe__); \
> > +	     tile__ < tile_cnt__; \
> > +	     ++tile__)
> > +
> > +
> >  enum i915_attr_id {
> >	RPS_ACT_FREQ_MHZ,
> >	RPS_CUR_FREQ_MHZ,
> > @@ -97,6 +103,7 @@ int igt_sysfs_open(int device);
> >  char *igt_sysfs_gt_path(int device, int gt, char *path, int pathlen);
> >  int igt_sysfs_gt_open(int device, int gt);
> >  int igt_sysfs_get_num_gt(int device);
> > +int igt_sysfs_get_num_tiles(int device);
> >  bool igt_sysfs_has_attr(int dir, const char *attr);  const char
> > *igt_sysfs_dir_id_to_name(int dir, enum i915_attr_id id);  const char
> > *igt_sysfs_path_id_to_name(const char *path, enum i915_attr_id id);
> > --
> > 2.25.1
> >

Re: [igt-dev] [PATCH i-g-t v6 2/4] lib/igt_sysfs: Handling gt related sysfs uapi changes

[Ghimiray, Himal Prasad]

> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: 06 July 2023 05:47
> 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 v6 2/4] lib/igt_sysfs: Handling gt related sysfs uapi
> changes
> 
> On Wed, 05 Jul 2023 06:18:51 -0700, Himal Prasad Ghimiray wrote:
> >
> > 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:
> > - Initialize the variable and use for loop instead of while.
> >
> > 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 | 40 ++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_sysfs.h |  1 +
> >  2 files changed, 41 insertions(+)
> >
> > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c index
> > 2ea9eb1b9..32d78cfb6 100644
> > --- a/lib/igt_sysfs.c
> > +++ b/lib/igt_sysfs.c
> > @@ -46,6 +46,7 @@
> >  #include "igt_device.h"
> >  #include "igt_io.h"
> >
> > +#define XE_MAX_GT_SYSFS_PATH 3
> >  /**
> >   * SECTION:igt_sysfs
> >   * @short_description: Support code for sysfs features  @@ -933,3
> >+934,42 @@ void igt_sysfs_engines(int xe, int engines, const char
> **property,
> >		close(engine_fd);
> >	}
> >  }
> > +
> > +/**
> > + * xe_gt_sysfs_path::
> > + * @sysfs: fd of sysfs
> > + * @gt_id: gt id
> > + *
> > + * This function returns the path for gtX directory  */ char
> > +*xe_gt_sysfs_path(int sysfs, int gt_id) {
> > +	char path[XE_MAX_GT_SYSFS_PATH][32];
> > +	struct stat gtpathstat;
> > +	char *gt_path;
> > +	int i;
> > +
> > +	/* Path for multitile platforms with single gt on each tile
> > +	 * Tile id will be same as gt id.
> > +	 */
> > +	snprintf(path[0], sizeof(path[0]), "device/tile%d/gt%d/", gt_id,
> > +gt_id);
> > +
> > +	/* Path for single tile platforms with multiple gt's
> > +	 * all gt's will be under tile0 eg: MTL.
> > +	 */
> > +	snprintf(path[1], sizeof(path[1]), "device/tile0/gt%d/", gt_id);
> > +
> > +	/* Path not under tile.
> > +	 * To support older dir structure.
> > +	 */
> > +	snprintf(path[2], sizeof(path[2]), "device/gt%d/", gt_id);
> 
> Is this needed now? If not I'd say get rid of it.
@Kamil Konieczny to comment on this. He recommended to support old dir structure.
> 
> > +
> > +	for (i = 0; i < XE_MAX_GT_SYSFS_PATH; i++) {
> > +		if (!fstatat(sysfs, path[i], &gtpathstat, 0) &&
> S_ISDIR(gtpathstat.st_mode)) {
> > +			gt_path = malloc(sizeof(path[i]));
> > +			strcpy(gt_path, path[i]);
> > +			return gt_path;
> > +		}
> > +	}
> > +	igt_assert_f(false, "No gt%d dir found in sysfs", gt_id); }
> 
> I prefer this: start with tile0, look for all gt%d under under tile0 and see if gtN
> is there, repeat for next tile, till we have gone over all tiles. This will work
> always whereas the code above breaks as soon as we have a platform which
> has say: tile0/gt0, tile0/gt1, tile1/gt2.
Sure. Will address.
> 
> Since we are doing the work, with just a little extra work, do a good job of it.
> 
> Thanks.
> --
> Ashutosh
> 
> 
> 
> 
> 
> > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h index
> > c31765542..a937ee7b5 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -153,6 +153,7 @@ typedef struct igt_sysfs_rw_attr {  }
> > igt_sysfs_rw_attr_t;
> >
> >  void igt_sysfs_rw_attr_verify(igt_sysfs_rw_attr_t *rw);
> > +char *xe_gt_sysfs_path(int sysfs, int gt_id);
> >
> >  void igt_sysfs_engines(int xe, int engines, const char **property,
> >		       void (*test)(int, int, const char **));
> > --
> > 2.25.1
> >