* [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt
@ 2023-03-30 0:36 Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers Umesh Nerlige Ramappa
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
Add support for using gt specific counters in IGT and intel_gpu_top.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Riana Tauro (1):
tests/i915/perf_pmu: Use correct pmu config for multi-tile
Tvrtko Ursulin (4):
lib/debugfs: GT directory helpers
tests/i915/perf_pmu: Support multi-tile in rc6 subtest
tests/i915/perf_pmu: Two new rc6 subtests
tests/i915/perf_pmu: Support multi-tile in frequency subtest
Umesh Nerlige Ramappa (2):
tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without
spinner
tools/intel_gpu_top: Add support for gt specific counters
include/drm-uapi/i915_drm.h | 22 +-
lib/igt_debugfs.c | 60 ++++++
lib/igt_debugfs.h | 4 +
tests/i915/perf_pmu.c | 389 +++++++++++++++++++++++++++---------
tools/intel_gpu_top.c | 56 ++++--
5 files changed, 422 insertions(+), 109 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 8:49 ` Tvrtko Ursulin
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 2/7] tests/i915/perf_pmu: Support multi-tile in rc6 subtest Umesh Nerlige Ramappa
` (7 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Add two debugfs helpers to be used for opening per-gt debugfs directories.
Note: This likely needs updating once the final debugfs layout is set.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
lib/igt_debugfs.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
lib/igt_debugfs.h | 4 ++++
2 files changed, 64 insertions(+)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 05889bbe..afde2da6 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -217,6 +217,37 @@ int igt_debugfs_dir(int device)
return open(path, O_RDONLY);
}
+/**
+ * igt_debugfs_gt_dir:
+ * @device: fd of the device
+ * @gt: GT instance number
+ *
+ * This opens the debugfs directory corresponding to device for use
+ * with igt_sysfs_get() and related functions.
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_debugfs_gt_dir(int device, unsigned int gt)
+{
+ int debugfs_gt_dir_fd;
+ char path[PATH_MAX];
+ char gtpath[16];
+ int ret;
+
+ if (!igt_debugfs_path(device, path, sizeof(path)))
+ return -1;
+
+ ret = snprintf(gtpath, sizeof(gtpath), "/gt%u", gt);
+ igt_assert(ret < sizeof(gtpath));
+ strncat(path, gtpath, sizeof(path) - 1);
+
+ debugfs_gt_dir_fd = open(path, O_RDONLY);
+ igt_debug_on_f(debugfs_gt_dir_fd < 0, "path: %s\n", path);
+
+ return debugfs_gt_dir_fd;
+}
+
/**
* igt_debugfs_connector_dir:
* @device: fd of the device
@@ -313,6 +344,35 @@ bool igt_debugfs_exists(int device, const char *filename, int mode)
return false;
}
+/**
+ * igt_debugfs_gt_open:
+ * @device: open i915 drm fd
+ * @gt: gt instance number
+ * @filename: name of the debugfs node to open
+ * @mode: mode bits as used by open()
+ *
+ * This opens a debugfs file as a Unix file descriptor. The filename should be
+ * relative to the drm device's root, i.e. without "drm/$minor".
+ *
+ * Returns:
+ * The Unix file descriptor for the debugfs file or -1 if that didn't work out.
+ */
+int
+igt_debugfs_gt_open(int device, unsigned int gt, const char *filename, int mode)
+{
+ int dir, ret;
+
+ dir = igt_debugfs_gt_dir(device, gt);
+ if (dir < 0)
+ return dir;
+
+ ret = openat(dir, filename, mode);
+
+ close(dir);
+
+ return ret;
+}
+
/**
* igt_debugfs_simple_read:
* @dir: fd of the debugfs directory
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 4824344a..3e6194ad 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -45,6 +45,10 @@ void __igt_debugfs_write(int fd, const char *filename, const char *buf, int size
int igt_debugfs_simple_read(int dir, const char *filename, char *buf, int size);
bool igt_debugfs_search(int fd, const char *filename, const char *substring);
+int igt_debugfs_gt_dir(int device, unsigned int gt);
+int igt_debugfs_gt_open(int device, unsigned int gt, const char *filename,
+ int mode);
+
/**
* igt_debugfs_read:
* @filename: name of the debugfs file
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 2/7] tests/i915/perf_pmu: Support multi-tile in rc6 subtest
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 3/7] tests/i915/perf_pmu: Two new rc6 subtests Umesh Nerlige Ramappa
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Teach test how to wake up a particular tile and make it iterate all of
them using dynamic subtests.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
include/drm-uapi/i915_drm.h | 18 +++++++++++++++-
tests/i915/perf_pmu.c | 41 ++++++++++++++++++++++++++-----------
2 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index a0876ee4..3b5e3b51 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -280,7 +280,17 @@ enum drm_i915_pmu_engine_sample {
#define I915_PMU_ENGINE_SEMA(class, instance) \
__I915_PMU_ENGINE(class, instance, I915_SAMPLE_SEMA)
-#define __I915_PMU_OTHER(x) (__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x))
+/*
+ * Top 8 bits of every non-engine counter are GT id.
+ * FIXME: __I915_PMU_GT_SHIFT will be changed to 56
+ */
+#define __I915_PMU_GT_SHIFT (60)
+
+#define ___I915_PMU_OTHER(gt, x) \
+ (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
+ ((__u64)(gt) << __I915_PMU_GT_SHIFT))
+
+#define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
#define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
#define I915_PMU_REQUESTED_FREQUENCY __I915_PMU_OTHER(1)
@@ -290,6 +300,12 @@ enum drm_i915_pmu_engine_sample {
#define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
+#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
+#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
+#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
+#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
+#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
+
/* Each region is a minimum of 16k, and there are at most 255 of them.
*/
#define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 197e7cd2..72f95b98 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -1707,8 +1707,16 @@ static bool wait_for_suspended(int gem_fd)
return suspended;
}
+static int open_forcewake_handle(int fd, unsigned int gt)
+{
+ if (getenv("IGT_NO_FORCEWAKE"))
+ return -1;
+
+ return igt_debugfs_gt_open(fd, gt, "forcewake_user", O_WRONLY);
+}
+
static void
-test_rc6(int gem_fd, unsigned int flags)
+test_rc6(int gem_fd, unsigned int gt, unsigned int flags)
{
int64_t duration_ns = 2e9;
uint64_t idle, busy, prev, ts[2];
@@ -1717,7 +1725,7 @@ test_rc6(int gem_fd, unsigned int flags)
gem_quiescent_gpu(gem_fd);
- fd = open_pmu(gem_fd, I915_PMU_RC6_RESIDENCY);
+ fd = open_pmu(gem_fd, __I915_PMU_RC6_RESIDENCY(gt));
if (flags & TEST_RUNTIME_PM) {
drmModeRes *res;
@@ -1784,8 +1792,8 @@ test_rc6(int gem_fd, unsigned int flags)
assert_within_epsilon(idle - prev, ts[1] - ts[0], tolerance);
/* Wake up device and check no RC6. */
- fw = igt_open_forcewake_handle(gem_fd);
- igt_assert(fw >= 0);
+ fw = open_forcewake_handle(gem_fd, gt);
+ igt_require(fw >= 0);
usleep(1e3); /* wait for the rc6 cycle counter to stop ticking */
prev = pmu_read_single(fd);
@@ -2173,12 +2181,17 @@ static void pmu_read(int i915)
for_each_if((e)->class == I915_ENGINE_CLASS_RENDER) \
igt_dynamic_f("%s", e->name)
+#define for_each_gt(i915, gtid, tmp) \
+ for ((gtid) = 0; \
+ ((tmp) = igt_sysfs_gt_open((i915), (gtid))) != -1; \
+ close(tmp), (gtid)++)
+
igt_main
{
const struct intel_execution_engine2 *e;
unsigned int num_engines = 0;
const intel_ctx_t *ctx = NULL;
- int fd = -1;
+ int gt, tmp, fd = -1;
/**
* All PMU should be accompanied by a test.
@@ -2395,17 +2408,21 @@ igt_main
/**
* Test RC6 residency reporting.
*/
- igt_subtest("rc6")
- test_rc6(fd, 0);
+ igt_subtest_with_dynamic("rc6") {
+ for_each_gt(fd, gt, tmp) {
+ igt_dynamic_f("gt%u", gt)
+ test_rc6(fd, gt, 0);
- igt_subtest("rc6-runtime-pm")
- test_rc6(fd, TEST_RUNTIME_PM);
+ igt_dynamic_f("runtime-pm-gt%u", gt)
+ test_rc6(fd, gt, TEST_RUNTIME_PM);
- igt_subtest("rc6-runtime-pm-long")
- test_rc6(fd, TEST_RUNTIME_PM | FLAG_LONG);
+ igt_dynamic_f("runtime-pm-long-gt%u", gt)
+ test_rc6(fd, gt, TEST_RUNTIME_PM | FLAG_LONG);
+ }
+ }
igt_subtest("rc6-suspend")
- test_rc6(fd, TEST_S3);
+ test_rc6(fd, 0, TEST_S3);
/**
* Test GT wakeref tracking (similar to RC0, opposite of RC6)
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 3/7] tests/i915/perf_pmu: Two new rc6 subtests
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 2/7] tests/i915/perf_pmu: Support multi-tile in rc6 subtest Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest Umesh Nerlige Ramappa
` (5 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
1.
Keep one tile awake and check rc6 counters on all tiles.
2.
Keep all tiles awake and check rc6 counters on all.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
tests/i915/perf_pmu.c | 149 +++++++++++++++++++++++++++++++-----------
1 file changed, 111 insertions(+), 38 deletions(-)
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 72f95b98..5e1177b9 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -173,6 +173,8 @@ static unsigned int measured_usleep(unsigned int usec)
#define FLAG_LONG (16)
#define FLAG_HANG (32)
#define TEST_S3 (64)
+#define TEST_OTHER (128)
+#define TEST_ALL (256)
static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
const struct intel_execution_engine2 *e)
@@ -1676,20 +1678,23 @@ test_frequency_idle(int gem_fd)
"Actual frequency should be 0 while parked!\n");
}
-static bool wait_for_rc6(int fd, int timeout)
+static bool wait_for_rc6(int fd, int timeout, unsigned int pmus, unsigned int idx)
{
struct timespec tv = {};
+ uint64_t val[pmus];
uint64_t start, now;
/* First wait for roughly an RC6 Evaluation Interval */
usleep(160 * 1000);
/* Then poll for RC6 to start ticking */
- now = pmu_read_single(fd);
+ pmu_read_multi(fd, pmus, val);
+ now = val[idx];
do {
start = now;
usleep(5000);
- now = pmu_read_single(fd);
+ pmu_read_multi(fd, pmus, val);
+ now = val[idx];
if (now - start > 1e6)
return true;
} while (igt_seconds_elapsed(&tv) <= timeout);
@@ -1716,16 +1721,38 @@ static int open_forcewake_handle(int fd, unsigned int gt)
}
static void
-test_rc6(int gem_fd, unsigned int gt, unsigned int flags)
+test_rc6(int gem_fd, unsigned int gt, unsigned int num_gt, unsigned int flags)
{
int64_t duration_ns = 2e9;
- uint64_t idle, busy, prev, ts[2];
+ uint64_t idle[16], busy[16], prev[16], ts[2];
+ int fd[num_gt], fw[num_gt], gt_, pmus = 0, test_idx = -1;
unsigned long slept;
- int fd, fw;
+
+ igt_require(!(flags & TEST_OTHER) ||
+ ((flags & TEST_OTHER) && num_gt > 1));
+
+ igt_require(!(flags & TEST_ALL) ||
+ ((flags & TEST_ALL) && num_gt > 1));
gem_quiescent_gpu(gem_fd);
- fd = open_pmu(gem_fd, __I915_PMU_RC6_RESIDENCY(gt));
+ fd[0] = -1;
+ for (gt_ = 0; gt_ < num_gt; gt_++) {
+ if (gt_ != gt && !(flags & TEST_OTHER))
+ continue;
+
+ if (gt_ == gt) {
+ igt_assert(test_idx == -1);
+ test_idx = pmus;
+ }
+
+ fd[pmus] = perf_i915_open_group(gem_fd,
+ __I915_PMU_RC6_RESIDENCY(gt_),
+ fd[0]);
+ igt_skip_on(fd[pmus] < 0 && errno == ENODEV);
+ pmus++;
+ }
+ igt_assert(test_idx >= 0);
if (flags & TEST_RUNTIME_PM) {
drmModeRes *res;
@@ -1746,21 +1773,26 @@ test_rc6(int gem_fd, unsigned int gt, unsigned int flags)
* drifted to far in advance of real RC6.
*/
if (flags & FLAG_LONG) {
- pmu_read_single(fd);
+ pmu_read_multi(fd[0], pmus, idle);
sleep(5);
- pmu_read_single(fd);
+ pmu_read_multi(fd[0], pmus, idle);
}
}
- igt_require(wait_for_rc6(fd, 1));
+ igt_require(wait_for_rc6(fd[0], 1, pmus, test_idx));
/* While idle check full RC6. */
- prev = __pmu_read_single(fd, &ts[0]);
+ ts[0] = pmu_read_multi(fd[0], pmus, prev);
slept = measured_usleep(duration_ns / 1000);
- idle = __pmu_read_single(fd, &ts[1]);
-
- igt_debug("slept=%lu perf=%"PRIu64"\n", slept, ts[1] - ts[0]);
- assert_within_epsilon(idle - prev, ts[1] - ts[0], tolerance);
+ ts[1] = pmu_read_multi(fd[0], pmus, idle);
+
+ for (gt_ = 0; gt_ < pmus; gt_++) {
+ igt_debug("gt%u: idle rc6=%"PRIu64", slept=%lu, perf=%"PRIu64"\n",
+ gt_, idle[gt_] - prev[gt_], slept, ts[1] - ts[0]);
+ assert_within_epsilon(idle[gt_] - prev[gt_],
+ ts[1] - ts[0],
+ tolerance);
+ }
if (flags & TEST_S3) {
/*
@@ -1773,40 +1805,70 @@ test_rc6(int gem_fd, unsigned int gt, unsigned int flags)
* However, in practice it appears we are not entering rc6
* immediately after resume... A bug?
*/
- prev = __pmu_read_single(fd, &ts[0]);
+ ts[0] = pmu_read_multi(fd[0], pmus, prev);
igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
SUSPEND_TEST_NONE);
- idle = __pmu_read_single(fd, &ts[1]);
- igt_debug("suspend=%"PRIu64", rc6=%"PRIu64"\n",
- ts[1] - ts[0], idle -prev);
- //assert_within_epsilon(idle - prev, ts[1] - ts[0], tolerance);
+ ts[1] = pmu_read_multi(fd[0], pmus, idle);
+ for (gt_ = 0; gt_ < pmus; gt_++) {
+ igt_debug("gt%u: rc6=%"PRIu64", suspend=%"PRIu64"\n",
+ gt_, idle[gt_] - prev[gt_], ts[1] - ts[0]);
+ // assert_within_epsilon(idle[gt_] - prev[gt_],
+ // ts[1] - ts[0], tolerance);
+ }
}
- igt_assert(wait_for_rc6(fd, 5));
+ igt_assert(wait_for_rc6(fd[0], 5, pmus, test_idx));
- prev = __pmu_read_single(fd, &ts[0]);
+ ts[0] = pmu_read_multi(fd[0], pmus, prev);
slept = measured_usleep(duration_ns / 1000);
- idle = __pmu_read_single(fd, &ts[1]);
-
- igt_debug("slept=%lu perf=%"PRIu64"\n", slept, ts[1] - ts[0]);
- assert_within_epsilon(idle - prev, ts[1] - ts[0], tolerance);
+ ts[1] = pmu_read_multi(fd[0], pmus, idle);
+
+ for (gt_ = 0; gt_ < pmus; gt_++) {
+ igt_debug("gt%u: idle rc6=%"PRIu64", slept=%lu, perf=%"PRIu64"\n",
+ gt_, idle[gt_] - prev[gt_], slept, ts[1] - ts[0]);
+ assert_within_epsilon(idle[gt_] - prev[gt_],
+ ts[1] - ts[0],
+ tolerance);
+ }
/* Wake up device and check no RC6. */
- fw = open_forcewake_handle(gem_fd, gt);
- igt_require(fw >= 0);
+ for (gt_ = 0; gt_ < num_gt; gt_++) {
+ if (gt_ != gt && !(flags & TEST_ALL))
+ continue;
+
+ fw[gt_] = open_forcewake_handle(gem_fd, gt_);
+ igt_require(fw[gt_] >= 0);
+ }
+
usleep(1e3); /* wait for the rc6 cycle counter to stop ticking */
- prev = pmu_read_single(fd);
- usleep(duration_ns / 1000);
- busy = pmu_read_single(fd);
+ ts[0] = pmu_read_multi(fd[0], pmus, prev);
+ slept = measured_usleep(duration_ns / 1000);
+ ts[1] = pmu_read_multi(fd[0], pmus, busy);
- close(fw);
- close(fd);
+ for (gt_ = 0; gt_ < num_gt; gt_++) {
+ if (gt_ == gt || (flags & TEST_ALL))
+ close(fw[gt_]);
+ }
+
+ for (gt_ = 0; gt_ < pmus; gt_++)
+ close(fd[gt_]);
if (flags & TEST_RUNTIME_PM)
igt_restore_runtime_pm();
- assert_within_epsilon(busy - prev, 0.0, tolerance);
+ for (gt_ = 0; gt_ < pmus; gt_++) {
+ igt_debug("gt%u: busy rc6=%"PRIu64", slept=%lu, perf=%"PRIu64"\n",
+ gt_, busy[gt_] - prev[gt_], slept, ts[1] - ts[0]);
+ if (gt_ == test_idx || (flags & TEST_ALL))
+ assert_within_epsilon(busy[gt_] - prev[gt_],
+ 0.0,
+ tolerance);
+ else
+ assert_within_epsilon(busy[gt_] - prev[gt_],
+ ts[1] - ts[0],
+ tolerance);
+ }
}
static void
@@ -2192,6 +2254,7 @@ igt_main
unsigned int num_engines = 0;
const intel_ctx_t *ctx = NULL;
int gt, tmp, fd = -1;
+ int num_gt = 0;
/**
* All PMU should be accompanied by a test.
@@ -2210,6 +2273,9 @@ igt_main
for_each_ctx_engine(fd, ctx, e)
num_engines++;
igt_require(num_engines);
+
+ for_each_gt(fd, gt, tmp)
+ num_gt++;
}
igt_describe("Verify i915 pmu dir exists and read all events");
@@ -2411,18 +2477,25 @@ igt_main
igt_subtest_with_dynamic("rc6") {
for_each_gt(fd, gt, tmp) {
igt_dynamic_f("gt%u", gt)
- test_rc6(fd, gt, 0);
+ test_rc6(fd, gt, num_gt, 0);
igt_dynamic_f("runtime-pm-gt%u", gt)
- test_rc6(fd, gt, TEST_RUNTIME_PM);
+ test_rc6(fd, gt, num_gt, TEST_RUNTIME_PM);
igt_dynamic_f("runtime-pm-long-gt%u", gt)
- test_rc6(fd, gt, TEST_RUNTIME_PM | FLAG_LONG);
+ test_rc6(fd, gt, num_gt,
+ TEST_RUNTIME_PM | FLAG_LONG);
+
+ igt_dynamic_f("other-idle-gt%u", gt)
+ test_rc6(fd, gt, num_gt, TEST_OTHER);
}
}
igt_subtest("rc6-suspend")
- test_rc6(fd, 0, TEST_S3);
+ test_rc6(fd, 0, num_gt, TEST_S3);
+
+ igt_subtest("rc6-all-gts")
+ test_rc6(fd, 0, num_gt, TEST_ALL | TEST_OTHER);
/**
* Test GT wakeref tracking (similar to RC0, opposite of RC6)
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (2 preceding siblings ...)
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 3/7] tests/i915/perf_pmu: Two new rc6 subtests Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 8:54 ` Tvrtko Ursulin
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner Umesh Nerlige Ramappa
` (4 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Simple conversion to run the frequency tests per each tile, as dynamic
subtests, picking the correct engine to stimulate each.
Note: Needs fixing to support pre-multi-tile sysfs layout, once it is
known how that will look.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com> (v2)
---
tests/i915/perf_pmu.c | 195 +++++++++++++++++++++++++++++++++---------
1 file changed, 153 insertions(+), 42 deletions(-)
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 5e1177b9..1bea3e57 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -238,19 +238,6 @@ static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
return __spin_sync(fd, ahnd, ctx, e);
}
-static igt_spin_t *spin_sync_flags(int fd, uint64_t ahnd,
- const intel_ctx_t *ctx, unsigned int flags)
-{
- struct intel_execution_engine2 e = { };
-
- e.class = gem_execbuf_flags_to_engine_class(flags);
- e.instance = (flags & (I915_EXEC_BSD_MASK | I915_EXEC_RING_MASK)) ==
- (I915_EXEC_BSD | I915_EXEC_BSD_RING2) ? 1 : 0;
- e.flags = flags;
-
- return spin_sync(fd, ahnd, ctx, &e);
-}
-
static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
{
if (!spin)
@@ -1539,8 +1526,125 @@ test_interrupts_sync(int gem_fd)
igt_assert_lte(target, busy);
}
+static int
+__i915_query(int fd, struct drm_i915_query *q)
+{
+ if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
+ return -errno;
+
+ return 0;
+}
+
+static int
+__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t n_items)
+{
+ struct drm_i915_query q = {
+ .num_items = n_items,
+ .items_ptr = to_user_pointer(items),
+ };
+
+ return __i915_query(fd, &q);
+}
+
+#define i915_query_items(fd, items, n_items) \
+do { \
+ igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
+ errno = 0; \
+} while (0)
+
+static bool
+engine_in_gt(int i915, const struct i915_engine_class_instance *ci,
+ unsigned int gt)
+{
+ /* If just one gt, return true always */
+ if (!IS_METEORLAKE(intel_get_drm_devid(i915)))
+ return true;
+
+ /*
+ * FIXME: engine_to_gt map is hardcoded here until other mechanisms to
+ * query it are upstreamed.
+ */
+ switch (ci->engine_class) {
+ case I915_ENGINE_CLASS_RENDER:
+ case I915_ENGINE_CLASS_COMPUTE:
+ case I915_ENGINE_CLASS_COPY:
+ return gt == 0;
+ case I915_ENGINE_CLASS_VIDEO:
+ case I915_ENGINE_CLASS_VIDEO_ENHANCE:
+ return gt == 1;
+ default:
+ igt_assert_f(0, "Unsupported engine class %d\n", ci->engine_class);
+ return false;
+ }
+}
+
+static struct i915_engine_class_instance
+find_dword_engine(int i915, const unsigned int gt)
+{
+ struct drm_i915_query_engine_info *engines;
+ struct i915_engine_class_instance ci = { -1, -1 };
+ struct drm_i915_query_item item;
+ unsigned int i;
+
+ engines = malloc(4096);
+ igt_assert(engines);
+
+ memset(engines, 0, 4096);
+ memset(&item, 0, sizeof(item));
+ item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+ item.length = 4096;
+ item.data_ptr = to_user_pointer(engines);
+ i915_query_items(i915, &item, 1);
+ igt_assert(item.length > 0);
+
+ for (i = 0; i < engines->num_engines; i++) {
+ struct drm_i915_engine_info *e =
+ (struct drm_i915_engine_info *)&engines->engines[i];
+
+ if (!gem_class_can_store_dword(i915, e->engine.engine_class))
+ continue;
+
+ if (engine_in_gt(i915, &e->engine, gt)) {
+ ci.engine_class = e->engine.engine_class;
+ ci.engine_instance = e->engine.engine_instance;
+ }
+ }
+
+ free(engines);
+
+ return ci;
+}
+
+static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
+ const intel_ctx_t **ctx)
+{
+ struct i915_engine_class_instance ci = { -1, -1 };
+ struct intel_execution_engine2 e = { };
+
+ ci = find_dword_engine(i915, gt);
+
+ igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID);
+
+ if (gem_has_contexts(i915)) {
+ e.class = ci.engine_class;
+ e.instance = ci.engine_instance;
+ e.flags = 0;
+ *ctx = intel_ctx_create_for_engine(i915, e.class, e.instance);
+ } else {
+ igt_require(gt == 0); /* Impossible anyway. */
+ e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT);
+ e.instance = 0;
+ e.flags = I915_EXEC_DEFAULT;
+ *ctx = intel_ctx_0(i915);
+ }
+
+ igt_debug("Using engine %u:%u\n", e.class, e.instance);
+
+ return spin_sync(i915, ahnd, *ctx, &e);
+}
+
static void
-test_frequency(int gem_fd)
+test_frequency(int gem_fd, unsigned int gt)
{
uint32_t min_freq, max_freq, boost_freq;
uint64_t val[2], start[2], slept;
@@ -1548,13 +1652,14 @@ test_frequency(int gem_fd)
igt_spin_t *spin;
int fd[2], sysfs;
uint64_t ahnd = get_reloc_ahnd(gem_fd, 0);
+ const intel_ctx_t *ctx;
- sysfs = igt_sysfs_open(gem_fd);
+ sysfs = igt_sysfs_gt_open(gem_fd, gt);
igt_require(sysfs >= 0);
- min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
- max_freq = igt_sysfs_get_u32(sysfs, "gt_RP0_freq_mhz");
- boost_freq = igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz");
+ min_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
+ max_freq = igt_sysfs_get_u32(sysfs, "rps_RP0_freq_mhz");
+ boost_freq = igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz");
igt_info("Frequency: min=%u, max=%u, boost=%u MHz\n",
min_freq, max_freq, boost_freq);
igt_require(min_freq > 0 && max_freq > 0 && boost_freq > 0);
@@ -1567,15 +1672,15 @@ test_frequency(int gem_fd)
/*
* Set GPU to min frequency and read PMU counters.
*/
- igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", min_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == min_freq);
- igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", min_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == min_freq);
- igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", min_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == min_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", min_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") == min_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_max_freq_mhz", min_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz") == min_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_boost_freq_mhz", min_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz") == min_freq);
gem_quiescent_gpu(gem_fd); /* Idle to be sure the change takes effect */
- spin = spin_sync_flags(gem_fd, ahnd, 0, I915_EXEC_DEFAULT);
+ spin = spin_sync_gt(gem_fd, ahnd, gt, &ctx);
slept = pmu_read_multi(fd[0], 2, start);
measured_usleep(batch_duration_ns / 1000);
@@ -1584,6 +1689,7 @@ test_frequency(int gem_fd)
min[0] = 1e9*(val[0] - start[0]) / slept;
min[1] = 1e9*(val[1] - start[1]) / slept;
+ intel_ctx_destroy(gem_fd, ctx);
igt_spin_free(gem_fd, spin);
gem_quiescent_gpu(gem_fd); /* Don't leak busy bo into the next phase */
@@ -1592,16 +1698,16 @@ test_frequency(int gem_fd)
/*
* Set GPU to max frequency and read PMU counters.
*/
- igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", max_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == max_freq);
- igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", boost_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == boost_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_max_freq_mhz", max_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz") == max_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_boost_freq_mhz", boost_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz") == boost_freq);
- igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", max_freq));
- igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == max_freq);
+ igt_require(igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", max_freq));
+ igt_require(igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") == max_freq);
gem_quiescent_gpu(gem_fd);
- spin = spin_sync_flags(gem_fd, ahnd, 0, I915_EXEC_DEFAULT);
+ spin = spin_sync_gt(gem_fd, ahnd, gt, &ctx);
slept = pmu_read_multi(fd[0], 2, start);
measured_usleep(batch_duration_ns / 1000);
@@ -1610,16 +1716,17 @@ test_frequency(int gem_fd)
max[0] = 1e9*(val[0] - start[0]) / slept;
max[1] = 1e9*(val[1] - start[1]) / slept;
+ intel_ctx_destroy(gem_fd, ctx);
igt_spin_free(gem_fd, spin);
gem_quiescent_gpu(gem_fd);
/*
* Restore min/max.
*/
- igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", min_freq);
- if (igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") != min_freq)
+ igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", min_freq);
+ if (igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") != min_freq)
igt_warn("Unable to restore min frequency to saved value [%u MHz], now %u MHz\n",
- min_freq, igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz"));
+ min_freq, igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz"));
close(fd[0]);
close(fd[1]);
put_ahnd(ahnd);
@@ -1638,17 +1745,17 @@ test_frequency(int gem_fd)
}
static void
-test_frequency_idle(int gem_fd)
+test_frequency_idle(int gem_fd, unsigned int gt)
{
uint32_t min_freq;
uint64_t val[2], start[2], slept;
double idle[2];
int fd[2], sysfs;
- sysfs = igt_sysfs_open(gem_fd);
+ sysfs = igt_sysfs_gt_open(gem_fd, gt);
igt_require(sysfs >= 0);
- min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
+ min_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
close(sysfs);
/* While parked, our convention is to report the GPU at 0Hz */
@@ -2457,10 +2564,14 @@ igt_main
/**
* Test GPU frequency.
*/
- igt_subtest("frequency")
- test_frequency(fd);
- igt_subtest("frequency-idle")
- test_frequency_idle(fd);
+ igt_subtest_with_dynamic("frequency") {
+ for_each_gt(fd, gt, tmp) {
+ igt_dynamic_f("gt%u", gt)
+ test_frequency(fd, gt);
+ igt_dynamic_f("idle-gt%u", gt)
+ test_frequency_idle(fd, gt);
+ }
+ }
/**
* Test interrupt count reporting.
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (3 preceding siblings ...)
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 9:00 ` Tvrtko Ursulin
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 6/7] tests/i915/perf_pmu: Use correct pmu config for multi-tile Umesh Nerlige Ramappa
` (3 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
The assumption in some tests is that the engines are not busy if no
spinners are being run. This is not true in some cases where we see
that the render is busy at the start of the test. Quiesce GPU to wait
for such work to complete before checking for idle busyness.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
tests/i915/perf_pmu.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 1bea3e57..5185cd9a 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -281,10 +281,12 @@ single(int gem_fd, const intel_ctx_t *ctx,
fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
- if (flags & TEST_BUSY)
+ if (flags & TEST_BUSY) {
spin = spin_sync(gem_fd, ahnd, ctx, e);
- else
+ } else {
spin = NULL;
+ gem_quiescent_gpu(gem_fd);
+ }
val = pmu_read_single(fd);
slept = measured_usleep(batch_duration_ns / 1000);
@@ -644,10 +646,12 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
fd[1] = open_group(gem_fd, I915_PMU_ENGINE_WAIT(e->class, e->instance),
fd[0]);
- if (flags & TEST_BUSY)
+ if (flags & TEST_BUSY) {
spin = spin_sync(gem_fd, ahnd, ctx, e);
- else
+ } else {
spin = NULL;
+ gem_quiescent_gpu(gem_fd);
+ }
pmu_read_multi(fd[0], 2, val[0]);
measured_usleep(batch_duration_ns / 1000);
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 6/7] tests/i915/perf_pmu: Use correct pmu config for multi-tile
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (4 preceding siblings ...)
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters Umesh Nerlige Ramappa
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
From: Riana Tauro <riana.tauro@intel.com>
Use the correct perf_pmu config for actual and requested frequency in
multi-tile frequency test.
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
tests/i915/perf_pmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 5185cd9a..af6de762 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -1670,8 +1670,8 @@ test_frequency(int gem_fd, unsigned int gt)
igt_require(max_freq > min_freq);
igt_require(boost_freq > min_freq);
- fd[0] = open_group(gem_fd, I915_PMU_REQUESTED_FREQUENCY, -1);
- fd[1] = open_group(gem_fd, I915_PMU_ACTUAL_FREQUENCY, fd[0]);
+ fd[0] = open_group(gem_fd, __I915_PMU_REQUESTED_FREQUENCY(gt), -1);
+ fd[1] = open_group(gem_fd, __I915_PMU_ACTUAL_FREQUENCY(gt), fd[0]);
/*
* Set GPU to min frequency and read PMU counters.
@@ -1764,8 +1764,8 @@ test_frequency_idle(int gem_fd, unsigned int gt)
/* While parked, our convention is to report the GPU at 0Hz */
- fd[0] = open_group(gem_fd, I915_PMU_REQUESTED_FREQUENCY, -1);
- fd[1] = open_group(gem_fd, I915_PMU_ACTUAL_FREQUENCY, fd[0]);
+ fd[0] = open_group(gem_fd, __I915_PMU_REQUESTED_FREQUENCY(gt), -1);
+ fd[1] = open_group(gem_fd, __I915_PMU_ACTUAL_FREQUENCY(gt), fd[0]);
gem_quiescent_gpu(gem_fd); /* Be idle! */
measured_usleep(2000); /* Wait for timers to cease */
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (5 preceding siblings ...)
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 6/7] tests/i915/perf_pmu: Use correct pmu config for multi-tile Umesh Nerlige Ramappa
@ 2023-03-30 0:36 ` Umesh Nerlige Ramappa
2023-03-30 0:45 ` Umesh Nerlige Ramappa
2023-03-30 9:25 ` Tvrtko Ursulin
2023-03-30 1:23 ` [igt-dev] ✓ Fi.CI.BAT: success for Add MTL PMU support for multi-gt Patchwork
2023-03-30 18:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
8 siblings, 2 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:36 UTC (permalink / raw)
To: igt-dev; +Cc: badal.nilawar, arjun.melkaveri
With MTL frequency and rc6 counters are gt specific. Add support for
intel_gpu_top to show these counters separately.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
include/drm-uapi/i915_drm.h | 14 ++++++----
tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
2 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index 3b5e3b51..1a0c43e7 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
(((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
((__u64)(gt) << __I915_PMU_GT_SHIFT))
+/* Aggregate from all gts */
#define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
#define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
@@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
#define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
-#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
-#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
-#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
-#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
-#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
+/* GT specific counters */
+#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
+
+#define __I915_PMU_ACTUAL_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 0)
+#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
+#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
+#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
+#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ____I915_PMU_OTHER(gt, 4)
/* Each region is a minimum of 16k, and there are at most 255 of them.
*/
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index a4302aa3..9fc8b996 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -87,6 +87,7 @@ struct engine_class {
unsigned int num_engines;
};
+#define MAX_GTS 4
struct engines {
unsigned int num_engines;
unsigned int num_classes;
@@ -105,14 +106,16 @@ struct engines {
struct pmu_counter imc_writes;
unsigned int num_imc;
- struct pmu_counter freq_req;
- struct pmu_counter freq_act;
+ struct pmu_counter freq_req[MAX_GTS];
+ struct pmu_counter freq_act[MAX_GTS];
struct pmu_counter irq;
- struct pmu_counter rc6;
+ struct pmu_counter rc6[MAX_GTS];
bool discrete;
char *device;
+ int num_gts;
+
/* Do not edit below this line.
* This structure is reallocated every time a new engine is
* found and size is increased by sizeof (engine).
@@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
static int pmu_init(struct engines *engines)
{
unsigned int i;
- int fd;
+ int fd, ret;
uint64_t type = igt_perf_type_id(engines->device);
engines->fd = -1;
@@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
if (fd < 0)
return -1;
- engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
- _open_pmu(type, engines->num_counters, &engines->freq_req, engines->fd);
+ engines->num_gts = 1;
+ for (i = 0; i < MAX_GTS; i++) {
+ engines->freq_req[i].config = __I915_PMU_REQUESTED_FREQUENCY(i);
- engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
- _open_pmu(type, engines->num_counters, &engines->freq_act, engines->fd);
+ errno = 0;
+ ret = _open_pmu(type, engines->num_counters, &engines->freq_req[i], engines->fd);
+ if (ret >= 0)
+ continue;
- engines->rc6.config = I915_PMU_RC6_RESIDENCY;
- _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
+ if (errno != ENOENT)
+ return ret;
+
+ engines->num_gts = i;
+ errno = 0;
+ break;
+ }
+
+ for (i = 0; i < engines->num_gts; i++) {
+ engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
+ _open_pmu(type, engines->num_counters, &engines->freq_act[i], engines->fd);
+
+ engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
+ _open_pmu(type, engines->num_counters, &engines->rc6[i], engines->fd);
+ }
for (i = 0; i < engines->num_engines; i++) {
struct engine *engine = engine_ptr(engines, i);
@@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
engines->ts.prev = engines->ts.cur;
engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
- update_sample(&engines->freq_req, val);
- update_sample(&engines->freq_act, val);
+ for (i = 0; i < engines->num_gts; i++) {
+ update_sample(&engines->freq_req[i], val);
+ update_sample(&engines->freq_act[i], val);
+ update_sample(&engines->rc6[i], val);
+ }
update_sample(&engines->irq, val);
- update_sample(&engines->rc6, val);
for (i = 0; i < engines->num_engines; i++) {
struct engine *engine = engine_ptr(engines, i);
@@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
.items = period_items,
};
struct cnt_item freq_items[] = {
- { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
- { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
+ { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0", "req-gt0" },
+ { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0", "act-gt0" },
+ { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1", "req-gt1" },
+ { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1", "act-gt1" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
{ },
};
@@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
.items = irq_items,
};
struct cnt_item rc6_items[] = {
- { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
+ { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
+ { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
{ NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
{ },
};
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters Umesh Nerlige Ramappa
@ 2023-03-30 0:45 ` Umesh Nerlige Ramappa
2023-03-30 9:25 ` Tvrtko Ursulin
1 sibling, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 0:45 UTC (permalink / raw)
To: igt-dev; +Cc: arjun.melkaveri, badal.nilawar
On Wed, Mar 29, 2023 at 05:36:56PM -0700, Umesh Nerlige Ramappa wrote:
>With MTL frequency and rc6 counters are gt specific. Add support for
>intel_gpu_top to show these counters separately.
>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
> include/drm-uapi/i915_drm.h | 14 ++++++----
> tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
> 2 files changed, 49 insertions(+), 21 deletions(-)
>
>diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
>index 3b5e3b51..1a0c43e7 100644
>--- a/include/drm-uapi/i915_drm.h
>+++ b/include/drm-uapi/i915_drm.h
>@@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
> (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
> ((__u64)(gt) << __I915_PMU_GT_SHIFT))
>
>+/* Aggregate from all gts */
> #define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
>
> #define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
>@@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
>
> #define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
>
>-#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
>-#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
>-#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
>-#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
>-#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
>+/* GT specific counters */
>+#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
>+
>+#define __I915_PMU_ACTUAL_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 0)
>+#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
>+#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
>+#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
>+#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ____I915_PMU_OTHER(gt, 4)
The above changes were specifically made to enable legacy events and
don't belong to this patch. I will separate it out in the next revision.
Umesh
>
> /* Each region is a minimum of 16k, and there are at most 255 of them.
> */
>diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>index a4302aa3..9fc8b996 100644
>--- a/tools/intel_gpu_top.c
>+++ b/tools/intel_gpu_top.c
>@@ -87,6 +87,7 @@ struct engine_class {
> unsigned int num_engines;
> };
>
>+#define MAX_GTS 4
> struct engines {
> unsigned int num_engines;
> unsigned int num_classes;
>@@ -105,14 +106,16 @@ struct engines {
> struct pmu_counter imc_writes;
> unsigned int num_imc;
>
>- struct pmu_counter freq_req;
>- struct pmu_counter freq_act;
>+ struct pmu_counter freq_req[MAX_GTS];
>+ struct pmu_counter freq_act[MAX_GTS];
> struct pmu_counter irq;
>- struct pmu_counter rc6;
>+ struct pmu_counter rc6[MAX_GTS];
>
> bool discrete;
> char *device;
>
>+ int num_gts;
>+
> /* Do not edit below this line.
> * This structure is reallocated every time a new engine is
> * found and size is increased by sizeof (engine).
>@@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
> static int pmu_init(struct engines *engines)
> {
> unsigned int i;
>- int fd;
>+ int fd, ret;
> uint64_t type = igt_perf_type_id(engines->device);
>
> engines->fd = -1;
>@@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
> if (fd < 0)
> return -1;
>
>- engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
>- _open_pmu(type, engines->num_counters, &engines->freq_req, engines->fd);
>+ engines->num_gts = 1;
>+ for (i = 0; i < MAX_GTS; i++) {
>+ engines->freq_req[i].config = __I915_PMU_REQUESTED_FREQUENCY(i);
>
>- engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
>- _open_pmu(type, engines->num_counters, &engines->freq_act, engines->fd);
>+ errno = 0;
>+ ret = _open_pmu(type, engines->num_counters, &engines->freq_req[i], engines->fd);
>+ if (ret >= 0)
>+ continue;
>
>- engines->rc6.config = I915_PMU_RC6_RESIDENCY;
>- _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
>+ if (errno != ENOENT)
>+ return ret;
>+
>+ engines->num_gts = i;
>+ errno = 0;
>+ break;
>+ }
>+
>+ for (i = 0; i < engines->num_gts; i++) {
>+ engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
>+ _open_pmu(type, engines->num_counters, &engines->freq_act[i], engines->fd);
>+
>+ engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
>+ _open_pmu(type, engines->num_counters, &engines->rc6[i], engines->fd);
>+ }
>
> for (i = 0; i < engines->num_engines; i++) {
> struct engine *engine = engine_ptr(engines, i);
>@@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
> engines->ts.prev = engines->ts.cur;
> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>
>- update_sample(&engines->freq_req, val);
>- update_sample(&engines->freq_act, val);
>+ for (i = 0; i < engines->num_gts; i++) {
>+ update_sample(&engines->freq_req[i], val);
>+ update_sample(&engines->freq_act[i], val);
>+ update_sample(&engines->rc6[i], val);
>+ }
> update_sample(&engines->irq, val);
>- update_sample(&engines->rc6, val);
>
> for (i = 0; i < engines->num_engines; i++) {
> struct engine *engine = engine_ptr(engines, i);
>@@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
> .items = period_items,
> };
> struct cnt_item freq_items[] = {
>- { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
>- { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
>+ { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0", "req-gt0" },
>+ { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0", "act-gt0" },
>+ { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1", "req-gt1" },
>+ { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1", "act-gt1" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
> { },
> };
>@@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
> .items = irq_items,
> };
> struct cnt_item rc6_items[] = {
>- { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
>+ { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
>+ { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
> { },
> };
>--
>2.36.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for Add MTL PMU support for multi-gt
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (6 preceding siblings ...)
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters Umesh Nerlige Ramappa
@ 2023-03-30 1:23 ` Patchwork
2023-03-30 18:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2023-03-30 1:23 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 4412 bytes --]
== Series Details ==
Series: Add MTL PMU support for multi-gt
URL : https://patchwork.freedesktop.org/series/115835/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12937 -> IGTPW_8716
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/index.html
Participating hosts (36 -> 35)
------------------------------
Additional (1): fi-pnv-d510
Missing (2): fi-kbl-soraka fi-snb-2520m
Known issues
------------
Here are the changes found in IGTPW_8716 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][1] -> [DMESG-FAIL][2] ([i915#5334])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- bat-adlp-9: [PASS][3] -> [DMESG-FAIL][4] ([i915#7699])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/bat-adlp-9/igt@i915_selftest@live@migrate.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/bat-adlp-9/igt@i915_selftest@live@migrate.html
* igt@i915_selftest@live@requests:
- bat-rpls-1: [PASS][5] -> [ABORT][6] ([i915#7911] / [i915#7982])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/bat-rpls-1/igt@i915_selftest@live@requests.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/bat-rpls-1/igt@i915_selftest@live@requests.html
* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-dg2-11: NOTRUN -> [SKIP][7] ([i915#7828])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/bat-dg2-11/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
* igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
- bat-dg2-8: [PASS][8] -> [FAIL][9] ([i915#7932])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510: NOTRUN -> [SKIP][10] ([fdo#109271]) +38 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-glk-j4005: [DMESG-FAIL][11] ([i915#5334]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- bat-dg2-11: [ABORT][13] ([i915#7913]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_7226 -> IGTPW_8716
CI-20190529: 20190529
CI_DRM_12937: 6848d3613c0a63382d00ff550c41394902bda903 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8716: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/index.html
IGT_7226: 41be8b4ab86f9e11388c10366dfd71e5032589c1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Testlist changes
----------------
+igt@perf_pmu@rc6-all-gts
-igt@perf_pmu@frequency-idle
-igt@perf_pmu@rc6-runtime-pm
-igt@perf_pmu@rc6-runtime-pm-long
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/index.html
[-- Attachment #2: Type: text/html, Size: 5288 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers Umesh Nerlige Ramappa
@ 2023-03-30 8:49 ` Tvrtko Ursulin
0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2023-03-30 8:49 UTC (permalink / raw)
To: Umesh Nerlige Ramappa, igt-dev; +Cc: Andi Shyti, badal.nilawar, arjun.melkaveri
On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Add two debugfs helpers to be used for opening per-gt debugfs directories.
>
> Note: This likely needs updating once the final debugfs layout is set.
Andi - final debugfs layout is set now, right? If so probably remove
this note from the commit message.
Regards,
Tvrtko
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> lib/igt_debugfs.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
> lib/igt_debugfs.h | 4 ++++
> 2 files changed, 64 insertions(+)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 05889bbe..afde2da6 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -217,6 +217,37 @@ int igt_debugfs_dir(int device)
> return open(path, O_RDONLY);
> }
>
> +/**
> + * igt_debugfs_gt_dir:
> + * @device: fd of the device
> + * @gt: GT instance number
> + *
> + * This opens the debugfs directory corresponding to device for use
> + * with igt_sysfs_get() and related functions.
> + *
> + * Returns:
> + * The directory fd, or -1 on failure.
> + */
> +int igt_debugfs_gt_dir(int device, unsigned int gt)
> +{
> + int debugfs_gt_dir_fd;
> + char path[PATH_MAX];
> + char gtpath[16];
> + int ret;
> +
> + if (!igt_debugfs_path(device, path, sizeof(path)))
> + return -1;
> +
> + ret = snprintf(gtpath, sizeof(gtpath), "/gt%u", gt);
> + igt_assert(ret < sizeof(gtpath));
> + strncat(path, gtpath, sizeof(path) - 1);
> +
> + debugfs_gt_dir_fd = open(path, O_RDONLY);
> + igt_debug_on_f(debugfs_gt_dir_fd < 0, "path: %s\n", path);
> +
> + return debugfs_gt_dir_fd;
> +}
> +
> /**
> * igt_debugfs_connector_dir:
> * @device: fd of the device
> @@ -313,6 +344,35 @@ bool igt_debugfs_exists(int device, const char *filename, int mode)
> return false;
> }
>
> +/**
> + * igt_debugfs_gt_open:
> + * @device: open i915 drm fd
> + * @gt: gt instance number
> + * @filename: name of the debugfs node to open
> + * @mode: mode bits as used by open()
> + *
> + * This opens a debugfs file as a Unix file descriptor. The filename should be
> + * relative to the drm device's root, i.e. without "drm/$minor".
> + *
> + * Returns:
> + * The Unix file descriptor for the debugfs file or -1 if that didn't work out.
> + */
> +int
> +igt_debugfs_gt_open(int device, unsigned int gt, const char *filename, int mode)
> +{
> + int dir, ret;
> +
> + dir = igt_debugfs_gt_dir(device, gt);
> + if (dir < 0)
> + return dir;
> +
> + ret = openat(dir, filename, mode);
> +
> + close(dir);
> +
> + return ret;
> +}
> +
> /**
> * igt_debugfs_simple_read:
> * @dir: fd of the debugfs directory
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 4824344a..3e6194ad 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -45,6 +45,10 @@ void __igt_debugfs_write(int fd, const char *filename, const char *buf, int size
> int igt_debugfs_simple_read(int dir, const char *filename, char *buf, int size);
> bool igt_debugfs_search(int fd, const char *filename, const char *substring);
>
> +int igt_debugfs_gt_dir(int device, unsigned int gt);
> +int igt_debugfs_gt_open(int device, unsigned int gt, const char *filename,
> + int mode);
> +
> /**
> * igt_debugfs_read:
> * @filename: name of the debugfs file
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest Umesh Nerlige Ramappa
@ 2023-03-30 8:54 ` Tvrtko Ursulin
0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2023-03-30 8:54 UTC (permalink / raw)
To: Umesh Nerlige Ramappa, igt-dev; +Cc: Andi Shyti, badal.nilawar, arjun.melkaveri
On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Simple conversion to run the frequency tests per each tile, as dynamic
> subtests, picking the correct engine to stimulate each.
>
> Note: Needs fixing to support pre-multi-tile sysfs layout, once it is
> known how that will look.
Same question as for debugfs in patch 1.
Regards,
Tvrtko
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com> (v2)
> ---
> tests/i915/perf_pmu.c | 195 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 153 insertions(+), 42 deletions(-)
>
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index 5e1177b9..1bea3e57 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -238,19 +238,6 @@ static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> return __spin_sync(fd, ahnd, ctx, e);
> }
>
> -static igt_spin_t *spin_sync_flags(int fd, uint64_t ahnd,
> - const intel_ctx_t *ctx, unsigned int flags)
> -{
> - struct intel_execution_engine2 e = { };
> -
> - e.class = gem_execbuf_flags_to_engine_class(flags);
> - e.instance = (flags & (I915_EXEC_BSD_MASK | I915_EXEC_RING_MASK)) ==
> - (I915_EXEC_BSD | I915_EXEC_BSD_RING2) ? 1 : 0;
> - e.flags = flags;
> -
> - return spin_sync(fd, ahnd, ctx, &e);
> -}
> -
> static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
> {
> if (!spin)
> @@ -1539,8 +1526,125 @@ test_interrupts_sync(int gem_fd)
> igt_assert_lte(target, busy);
> }
>
> +static int
> +__i915_query(int fd, struct drm_i915_query *q)
> +{
> + if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
> + return -errno;
> +
> + return 0;
> +}
> +
> +static int
> +__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t n_items)
> +{
> + struct drm_i915_query q = {
> + .num_items = n_items,
> + .items_ptr = to_user_pointer(items),
> + };
> +
> + return __i915_query(fd, &q);
> +}
> +
> +#define i915_query_items(fd, items, n_items) \
> +do { \
> + igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
> + errno = 0; \
> +} while (0)
> +
> +static bool
> +engine_in_gt(int i915, const struct i915_engine_class_instance *ci,
> + unsigned int gt)
> +{
> + /* If just one gt, return true always */
> + if (!IS_METEORLAKE(intel_get_drm_devid(i915)))
> + return true;
> +
> + /*
> + * FIXME: engine_to_gt map is hardcoded here until other mechanisms to
> + * query it are upstreamed.
> + */
> + switch (ci->engine_class) {
> + case I915_ENGINE_CLASS_RENDER:
> + case I915_ENGINE_CLASS_COMPUTE:
> + case I915_ENGINE_CLASS_COPY:
> + return gt == 0;
> + case I915_ENGINE_CLASS_VIDEO:
> + case I915_ENGINE_CLASS_VIDEO_ENHANCE:
> + return gt == 1;
> + default:
> + igt_assert_f(0, "Unsupported engine class %d\n", ci->engine_class);
> + return false;
> + }
> +}
> +
> +static struct i915_engine_class_instance
> +find_dword_engine(int i915, const unsigned int gt)
> +{
> + struct drm_i915_query_engine_info *engines;
> + struct i915_engine_class_instance ci = { -1, -1 };
> + struct drm_i915_query_item item;
> + unsigned int i;
> +
> + engines = malloc(4096);
> + igt_assert(engines);
> +
> + memset(engines, 0, 4096);
> + memset(&item, 0, sizeof(item));
> + item.query_id = DRM_I915_QUERY_ENGINE_INFO;
> + item.length = 4096;
> + item.data_ptr = to_user_pointer(engines);
> + i915_query_items(i915, &item, 1);
> + igt_assert(item.length > 0);
> +
> + for (i = 0; i < engines->num_engines; i++) {
> + struct drm_i915_engine_info *e =
> + (struct drm_i915_engine_info *)&engines->engines[i];
> +
> + if (!gem_class_can_store_dword(i915, e->engine.engine_class))
> + continue;
> +
> + if (engine_in_gt(i915, &e->engine, gt)) {
> + ci.engine_class = e->engine.engine_class;
> + ci.engine_instance = e->engine.engine_instance;
> + }
> + }
> +
> + free(engines);
> +
> + return ci;
> +}
> +
> +static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
> + const intel_ctx_t **ctx)
> +{
> + struct i915_engine_class_instance ci = { -1, -1 };
> + struct intel_execution_engine2 e = { };
> +
> + ci = find_dword_engine(i915, gt);
> +
> + igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID);
> +
> + if (gem_has_contexts(i915)) {
> + e.class = ci.engine_class;
> + e.instance = ci.engine_instance;
> + e.flags = 0;
> + *ctx = intel_ctx_create_for_engine(i915, e.class, e.instance);
> + } else {
> + igt_require(gt == 0); /* Impossible anyway. */
> + e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT);
> + e.instance = 0;
> + e.flags = I915_EXEC_DEFAULT;
> + *ctx = intel_ctx_0(i915);
> + }
> +
> + igt_debug("Using engine %u:%u\n", e.class, e.instance);
> +
> + return spin_sync(i915, ahnd, *ctx, &e);
> +}
> +
> static void
> -test_frequency(int gem_fd)
> +test_frequency(int gem_fd, unsigned int gt)
> {
> uint32_t min_freq, max_freq, boost_freq;
> uint64_t val[2], start[2], slept;
> @@ -1548,13 +1652,14 @@ test_frequency(int gem_fd)
> igt_spin_t *spin;
> int fd[2], sysfs;
> uint64_t ahnd = get_reloc_ahnd(gem_fd, 0);
> + const intel_ctx_t *ctx;
>
> - sysfs = igt_sysfs_open(gem_fd);
> + sysfs = igt_sysfs_gt_open(gem_fd, gt);
> igt_require(sysfs >= 0);
>
> - min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
> - max_freq = igt_sysfs_get_u32(sysfs, "gt_RP0_freq_mhz");
> - boost_freq = igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz");
> + min_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
> + max_freq = igt_sysfs_get_u32(sysfs, "rps_RP0_freq_mhz");
> + boost_freq = igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz");
> igt_info("Frequency: min=%u, max=%u, boost=%u MHz\n",
> min_freq, max_freq, boost_freq);
> igt_require(min_freq > 0 && max_freq > 0 && boost_freq > 0);
> @@ -1567,15 +1672,15 @@ test_frequency(int gem_fd)
> /*
> * Set GPU to min frequency and read PMU counters.
> */
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", min_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == min_freq);
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", min_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == min_freq);
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", min_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == min_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", min_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") == min_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_max_freq_mhz", min_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz") == min_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_boost_freq_mhz", min_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz") == min_freq);
>
> gem_quiescent_gpu(gem_fd); /* Idle to be sure the change takes effect */
> - spin = spin_sync_flags(gem_fd, ahnd, 0, I915_EXEC_DEFAULT);
> + spin = spin_sync_gt(gem_fd, ahnd, gt, &ctx);
>
> slept = pmu_read_multi(fd[0], 2, start);
> measured_usleep(batch_duration_ns / 1000);
> @@ -1584,6 +1689,7 @@ test_frequency(int gem_fd)
> min[0] = 1e9*(val[0] - start[0]) / slept;
> min[1] = 1e9*(val[1] - start[1]) / slept;
>
> + intel_ctx_destroy(gem_fd, ctx);
> igt_spin_free(gem_fd, spin);
> gem_quiescent_gpu(gem_fd); /* Don't leak busy bo into the next phase */
>
> @@ -1592,16 +1698,16 @@ test_frequency(int gem_fd)
> /*
> * Set GPU to max frequency and read PMU counters.
> */
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_max_freq_mhz", max_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_max_freq_mhz") == max_freq);
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_boost_freq_mhz", boost_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == boost_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_max_freq_mhz", max_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz") == max_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_boost_freq_mhz", boost_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz") == boost_freq);
>
> - igt_require(igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", max_freq));
> - igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == max_freq);
> + igt_require(igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", max_freq));
> + igt_require(igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") == max_freq);
>
> gem_quiescent_gpu(gem_fd);
> - spin = spin_sync_flags(gem_fd, ahnd, 0, I915_EXEC_DEFAULT);
> + spin = spin_sync_gt(gem_fd, ahnd, gt, &ctx);
>
> slept = pmu_read_multi(fd[0], 2, start);
> measured_usleep(batch_duration_ns / 1000);
> @@ -1610,16 +1716,17 @@ test_frequency(int gem_fd)
> max[0] = 1e9*(val[0] - start[0]) / slept;
> max[1] = 1e9*(val[1] - start[1]) / slept;
>
> + intel_ctx_destroy(gem_fd, ctx);
> igt_spin_free(gem_fd, spin);
> gem_quiescent_gpu(gem_fd);
>
> /*
> * Restore min/max.
> */
> - igt_sysfs_set_u32(sysfs, "gt_min_freq_mhz", min_freq);
> - if (igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") != min_freq)
> + igt_sysfs_set_u32(sysfs, "rps_min_freq_mhz", min_freq);
> + if (igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz") != min_freq)
> igt_warn("Unable to restore min frequency to saved value [%u MHz], now %u MHz\n",
> - min_freq, igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz"));
> + min_freq, igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz"));
> close(fd[0]);
> close(fd[1]);
> put_ahnd(ahnd);
> @@ -1638,17 +1745,17 @@ test_frequency(int gem_fd)
> }
>
> static void
> -test_frequency_idle(int gem_fd)
> +test_frequency_idle(int gem_fd, unsigned int gt)
> {
> uint32_t min_freq;
> uint64_t val[2], start[2], slept;
> double idle[2];
> int fd[2], sysfs;
>
> - sysfs = igt_sysfs_open(gem_fd);
> + sysfs = igt_sysfs_gt_open(gem_fd, gt);
> igt_require(sysfs >= 0);
>
> - min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
> + min_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
> close(sysfs);
>
> /* While parked, our convention is to report the GPU at 0Hz */
> @@ -2457,10 +2564,14 @@ igt_main
> /**
> * Test GPU frequency.
> */
> - igt_subtest("frequency")
> - test_frequency(fd);
> - igt_subtest("frequency-idle")
> - test_frequency_idle(fd);
> + igt_subtest_with_dynamic("frequency") {
> + for_each_gt(fd, gt, tmp) {
> + igt_dynamic_f("gt%u", gt)
> + test_frequency(fd, gt);
> + igt_dynamic_f("idle-gt%u", gt)
> + test_frequency_idle(fd, gt);
> + }
> + }
>
> /**
> * Test interrupt count reporting.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner Umesh Nerlige Ramappa
@ 2023-03-30 9:00 ` Tvrtko Ursulin
2023-03-30 18:35 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 19+ messages in thread
From: Tvrtko Ursulin @ 2023-03-30 9:00 UTC (permalink / raw)
To: Umesh Nerlige Ramappa, igt-dev; +Cc: arjun.melkaveri, badal.nilawar
On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
> The assumption in some tests is that the engines are not busy if no
> spinners are being run. This is not true in some cases where we see
> that the render is busy at the start of the test. Quiesce GPU to wait
> for such work to complete before checking for idle busyness.
Interesting.. hit in CI?
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
> tests/i915/perf_pmu.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index 1bea3e57..5185cd9a 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -281,10 +281,12 @@ single(int gem_fd, const intel_ctx_t *ctx,
>
> fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>
> - if (flags & TEST_BUSY)
> + if (flags & TEST_BUSY) {
> spin = spin_sync(gem_fd, ahnd, ctx, e);
> - else
> + } else {
> spin = NULL;
> + gem_quiescent_gpu(gem_fd);
> + }
But I'd probably just do it unconditionally at the start of tests, given
that if we are testing 100% busy we want it to be from our own spinner,
and not some potential leftover, which will then context switch to our
spinner at a random point.
Regards,
Tvrtko
>
> val = pmu_read_single(fd);
> slept = measured_usleep(batch_duration_ns / 1000);
> @@ -644,10 +646,12 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
> fd[1] = open_group(gem_fd, I915_PMU_ENGINE_WAIT(e->class, e->instance),
> fd[0]);
>
> - if (flags & TEST_BUSY)
> + if (flags & TEST_BUSY) {
> spin = spin_sync(gem_fd, ahnd, ctx, e);
> - else
> + } else {
> spin = NULL;
> + gem_quiescent_gpu(gem_fd);
> + }
>
> pmu_read_multi(fd[0], 2, val[0]);
> measured_usleep(batch_duration_ns / 1000);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters Umesh Nerlige Ramappa
2023-03-30 0:45 ` Umesh Nerlige Ramappa
@ 2023-03-30 9:25 ` Tvrtko Ursulin
2023-03-30 18:43 ` Umesh Nerlige Ramappa
2023-04-26 21:13 ` Umesh Nerlige Ramappa
1 sibling, 2 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2023-03-30 9:25 UTC (permalink / raw)
To: Umesh Nerlige Ramappa, igt-dev; +Cc: arjun.melkaveri, badal.nilawar
On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
> With MTL frequency and rc6 counters are gt specific. Add support for
> intel_gpu_top to show these counters separately.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
> include/drm-uapi/i915_drm.h | 14 ++++++----
> tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
> 2 files changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index 3b5e3b51..1a0c43e7 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
> (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
> ((__u64)(gt) << __I915_PMU_GT_SHIFT))
>
> +/* Aggregate from all gts */
> #define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
>
> #define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
> @@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
>
> #define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
>
> -#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
> -#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
> -#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
> -#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
> -#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
> +/* GT specific counters */
> +#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
> +
> +#define __I915_PMU_ACTUAL_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 0)
> +#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
> +#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
> +#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
> +#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ____I915_PMU_OTHER(gt, 4)
>
> /* Each region is a minimum of 16k, and there are at most 255 of them.
> */
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index a4302aa3..9fc8b996 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -87,6 +87,7 @@ struct engine_class {
> unsigned int num_engines;
> };
>
> +#define MAX_GTS 4
> struct engines {
> unsigned int num_engines;
> unsigned int num_classes;
> @@ -105,14 +106,16 @@ struct engines {
> struct pmu_counter imc_writes;
> unsigned int num_imc;
>
> - struct pmu_counter freq_req;
> - struct pmu_counter freq_act;
> + struct pmu_counter freq_req[MAX_GTS];
> + struct pmu_counter freq_act[MAX_GTS];
> struct pmu_counter irq;
> - struct pmu_counter rc6;
> + struct pmu_counter rc6[MAX_GTS];
>
> bool discrete;
> char *device;
>
> + int num_gts;
> +
> /* Do not edit below this line.
> * This structure is reallocated every time a new engine is
> * found and size is increased by sizeof (engine).
> @@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
> static int pmu_init(struct engines *engines)
> {
> unsigned int i;
> - int fd;
> + int fd, ret;
> uint64_t type = igt_perf_type_id(engines->device);
>
> engines->fd = -1;
> @@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
> if (fd < 0)
> return -1;
>
> - engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
> - _open_pmu(type, engines->num_counters, &engines->freq_req, engines->fd);
> + engines->num_gts = 1;
> + for (i = 0; i < MAX_GTS; i++) {
> + engines->freq_req[i].config = __I915_PMU_REQUESTED_FREQUENCY(i);
>
> - engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
> - _open_pmu(type, engines->num_counters, &engines->freq_act, engines->fd);
> + errno = 0;
> + ret = _open_pmu(type, engines->num_counters, &engines->freq_req[i], engines->fd);
> + if (ret >= 0)
> + continue;
>
> - engines->rc6.config = I915_PMU_RC6_RESIDENCY;
> - _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
> + if (errno != ENOENT)
> + return ret;
> +
> + engines->num_gts = i;
> + errno = 0;
> + break;
> + }
> +
> + for (i = 0; i < engines->num_gts; i++) {
> + engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
> + _open_pmu(type, engines->num_counters, &engines->freq_act[i], engines->fd);
> +
> + engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
> + _open_pmu(type, engines->num_counters, &engines->rc6[i], engines->fd);
> + }
>
> for (i = 0; i < engines->num_engines; i++) {
> struct engine *engine = engine_ptr(engines, i);
> @@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
> engines->ts.prev = engines->ts.cur;
> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>
> - update_sample(&engines->freq_req, val);
> - update_sample(&engines->freq_act, val);
> + for (i = 0; i < engines->num_gts; i++) {
> + update_sample(&engines->freq_req[i], val);
> + update_sample(&engines->freq_act[i], val);
> + update_sample(&engines->rc6[i], val);
> + }
> update_sample(&engines->irq, val);
> - update_sample(&engines->rc6, val);
>
> for (i = 0; i < engines->num_engines; i++) {
> struct engine *engine = engine_ptr(engines, i);
> @@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
> .items = period_items,
> };
> struct cnt_item freq_items[] = {
> - { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
> - { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
> + { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0", "req-gt0" },
> + { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0", "act-gt0" },
> + { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1", "req-gt1" },
> + { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1", "act-gt1" },
Why is width going to 8? 9999 MHz is not enough? ;)
[Comes back later..]
Ah for the header label.. hm.. maybe we should try putting the GT information into the parent. It would looks nicer, be more logical, even for JSON output we now have:
Terminal:
Freq MHz ...
req act ...
JSON:
"rc6": {
"value": 29.309568,
"unit": "%"
},
You propose something like:
Freq MHz Freq MHz ...
req-gt0 act-gt0 req-gt0 act-gt0 ...
"rc6": {
"value-gt0": 29.309568,
"value-gt1": 29.309568,
"unit": "%"
},
Which is not very nice UI wise. How about something like:
Freq GT0 MHz Freq GT1 MHz ...
req act req act ...
JSON should potentially be an array:
"rc6": [{
"gt": 0,
"value": 29.309568,
"unit": "%"
},
"gt": 0,
"value": 29.309568,
"unit": "%"
}],
Or at least:
"rc6": {
"value": 29.309568,
"unit": "%"
},
"rc6-gt1": {
"value": 29.309568,
"unit": "%"
},
Which also brings the point if maybe we shouldn't change the output for pre-MTL platforms. The approach is not IMHO even consistent with the proposed kernel change to have the aggregated counters, which I don't think I agree with at all.
Let me mull it all over.
Regards,
Tvrtko
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
> { },
> };
> @@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
> .items = irq_items,
> };
> struct cnt_item rc6_items[] = {
> - { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
> + { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
> + { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
> { },
> };
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner
2023-03-30 9:00 ` Tvrtko Ursulin
@ 2023-03-30 18:35 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 18:35 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, badal.nilawar, arjun.melkaveri
On Thu, Mar 30, 2023 at 10:00:28AM +0100, Tvrtko Ursulin wrote:
>
>On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
>>The assumption in some tests is that the engines are not busy if no
>>spinners are being run. This is not true in some cases where we see
>>that the render is busy at the start of the test. Quiesce GPU to wait
>>for such work to complete before checking for idle busyness.
>
>Interesting.. hit in CI?
Was failing almost always on MTL in local runs. I think I saw one bug
report as well.
>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>> tests/i915/perf_pmu.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>>diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
>>index 1bea3e57..5185cd9a 100644
>>--- a/tests/i915/perf_pmu.c
>>+++ b/tests/i915/perf_pmu.c
>>@@ -281,10 +281,12 @@ single(int gem_fd, const intel_ctx_t *ctx,
>> fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>>- if (flags & TEST_BUSY)
>>+ if (flags & TEST_BUSY) {
>> spin = spin_sync(gem_fd, ahnd, ctx, e);
>>- else
>>+ } else {
>> spin = NULL;
>>+ gem_quiescent_gpu(gem_fd);
>>+ }
>
>But I'd probably just do it unconditionally at the start of tests,
>given that if we are testing 100% busy we want it to be from our own
>spinner, and not some potential leftover, which will then context
>switch to our spinner at a random point.
Sure, will do.
Thanks,
Umesh
>
>Regards,
>
>Tvrtko
>
>> val = pmu_read_single(fd);
>> slept = measured_usleep(batch_duration_ns / 1000);
>>@@ -644,10 +646,12 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
>> fd[1] = open_group(gem_fd, I915_PMU_ENGINE_WAIT(e->class, e->instance),
>> fd[0]);
>>- if (flags & TEST_BUSY)
>>+ if (flags & TEST_BUSY) {
>> spin = spin_sync(gem_fd, ahnd, ctx, e);
>>- else
>>+ } else {
>> spin = NULL;
>>+ gem_quiescent_gpu(gem_fd);
>>+ }
>> pmu_read_multi(fd[0], 2, val[0]);
>> measured_usleep(batch_duration_ns / 1000);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-03-30 9:25 ` Tvrtko Ursulin
@ 2023-03-30 18:43 ` Umesh Nerlige Ramappa
2023-04-26 21:13 ` Umesh Nerlige Ramappa
1 sibling, 0 replies; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-03-30 18:43 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, badal.nilawar, arjun.melkaveri
On Thu, Mar 30, 2023 at 10:25:47AM +0100, Tvrtko Ursulin wrote:
>
>On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
>>With MTL frequency and rc6 counters are gt specific. Add support for
>>intel_gpu_top to show these counters separately.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>> include/drm-uapi/i915_drm.h | 14 ++++++----
>> tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
>> 2 files changed, 49 insertions(+), 21 deletions(-)
>>
>>diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
>>index 3b5e3b51..1a0c43e7 100644
>>--- a/include/drm-uapi/i915_drm.h
>>+++ b/include/drm-uapi/i915_drm.h
>>@@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
>> (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
>> ((__u64)(gt) << __I915_PMU_GT_SHIFT))
>>+/* Aggregate from all gts */
>> #define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
>> #define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
>>@@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
>> #define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
>>-#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
>>-#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
>>-#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
>>-#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
>>-#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
>>+/* GT specific counters */
>>+#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
>>+
>>+#define __I915_PMU_ACTUAL_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 0)
>>+#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
>>+#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
>>+#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
>>+#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ____I915_PMU_OTHER(gt, 4)
>> /* Each region is a minimum of 16k, and there are at most 255 of them.
>> */
>>diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>index a4302aa3..9fc8b996 100644
>>--- a/tools/intel_gpu_top.c
>>+++ b/tools/intel_gpu_top.c
>>@@ -87,6 +87,7 @@ struct engine_class {
>> unsigned int num_engines;
>> };
>>+#define MAX_GTS 4
>> struct engines {
>> unsigned int num_engines;
>> unsigned int num_classes;
>>@@ -105,14 +106,16 @@ struct engines {
>> struct pmu_counter imc_writes;
>> unsigned int num_imc;
>>- struct pmu_counter freq_req;
>>- struct pmu_counter freq_act;
>>+ struct pmu_counter freq_req[MAX_GTS];
>>+ struct pmu_counter freq_act[MAX_GTS];
>> struct pmu_counter irq;
>>- struct pmu_counter rc6;
>>+ struct pmu_counter rc6[MAX_GTS];
>> bool discrete;
>> char *device;
>>+ int num_gts;
>>+
>> /* Do not edit below this line.
>> * This structure is reallocated every time a new engine is
>> * found and size is increased by sizeof (engine).
>>@@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
>> static int pmu_init(struct engines *engines)
>> {
>> unsigned int i;
>>- int fd;
>>+ int fd, ret;
>> uint64_t type = igt_perf_type_id(engines->device);
>> engines->fd = -1;
>>@@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
>> if (fd < 0)
>> return -1;
>>- engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
>>- _open_pmu(type, engines->num_counters, &engines->freq_req, engines->fd);
>>+ engines->num_gts = 1;
>>+ for (i = 0; i < MAX_GTS; i++) {
>>+ engines->freq_req[i].config = __I915_PMU_REQUESTED_FREQUENCY(i);
>>- engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
>>- _open_pmu(type, engines->num_counters, &engines->freq_act, engines->fd);
>>+ errno = 0;
>>+ ret = _open_pmu(type, engines->num_counters, &engines->freq_req[i], engines->fd);
>>+ if (ret >= 0)
>>+ continue;
>>- engines->rc6.config = I915_PMU_RC6_RESIDENCY;
>>- _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
>>+ if (errno != ENOENT)
>>+ return ret;
>>+
>>+ engines->num_gts = i;
>>+ errno = 0;
>>+ break;
>>+ }
>>+
>>+ for (i = 0; i < engines->num_gts; i++) {
>>+ engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
>>+ _open_pmu(type, engines->num_counters, &engines->freq_act[i], engines->fd);
>>+
>>+ engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
>>+ _open_pmu(type, engines->num_counters, &engines->rc6[i], engines->fd);
>>+ }
>> for (i = 0; i < engines->num_engines; i++) {
>> struct engine *engine = engine_ptr(engines, i);
>>@@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
>> engines->ts.prev = engines->ts.cur;
>> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>>- update_sample(&engines->freq_req, val);
>>- update_sample(&engines->freq_act, val);
>>+ for (i = 0; i < engines->num_gts; i++) {
>>+ update_sample(&engines->freq_req[i], val);
>>+ update_sample(&engines->freq_act[i], val);
>>+ update_sample(&engines->rc6[i], val);
>>+ }
>> update_sample(&engines->irq, val);
>>- update_sample(&engines->rc6, val);
>> for (i = 0; i < engines->num_engines; i++) {
>> struct engine *engine = engine_ptr(engines, i);
>>@@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
>> .items = period_items,
>> };
>> struct cnt_item freq_items[] = {
>>- { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
>>- { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
>>+ { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0", "req-gt0" },
>>+ { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0", "act-gt0" },
>>+ { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1", "req-gt1" },
>>+ { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1", "act-gt1" },
>
>Why is width going to 8? 9999 MHz is not enough? ;)
>
>[Comes back later..]
>
>Ah for the header label.. hm.. maybe we should try putting the GT information into the parent. It would looks nicer, be more logical, even for JSON output we now have:
>
>Terminal:
>
> Freq MHz ...
> req act ...
>
>JSON:
>
> "rc6": {
> "value": 29.309568,
> "unit": "%"
> },
>You propose something like:
>
> Freq MHz Freq MHz ...
> req-gt0 act-gt0 req-gt0 act-gt0 ...
>
> "rc6": {
> "value-gt0": 29.309568,
> "value-gt1": 29.309568,
> "unit": "%"
> },
>
>Which is not very nice UI wise. How about something like:
>
> Freq GT0 MHz Freq GT1 MHz ...
> req act req act ...
I can try to do that^
>
>JSON should potentially be an array:
>
> "rc6": [{
> "gt": 0,
> "value": 29.309568,
> "unit": "%"
> },
> "gt": 0,
> "value": 29.309568,
> "unit": "%"
> }],
>
>Or at least:
>
> "rc6": {
> "value": 29.309568,
> "unit": "%"
> },
> "rc6-gt1": {
> "value": 29.309568,
> "unit": "%"
> },
>
>Which also brings the point if maybe we shouldn't change the output for pre-MTL platforms. The approach is not IMHO even consistent with the proposed kernel change to have the aggregated counters, which I don't think I agree with at all.
I thought the gt0 is the default way to go even for older platforms, so
showing that. Old intel_gpu_top binaries should still work with new
kernel since the intent is to still support the legacy events.
Maybe we have a switch that will show individual gt values only if users
demands to see it. If not, the UI is as before.
Thanks,
Umesh
>
>Let me mull it all over.
>
>Regards,
>
>Tvrtko
>
>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
>> { },
>> };
>>@@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
>> .items = irq_items,
>> };
>> struct cnt_item rc6_items[] = {
>>- { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
>>+ { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
>>+ { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
>> { },
>> };
^ permalink raw reply [flat|nested] 19+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for Add MTL PMU support for multi-gt
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
` (7 preceding siblings ...)
2023-03-30 1:23 ` [igt-dev] ✓ Fi.CI.BAT: success for Add MTL PMU support for multi-gt Patchwork
@ 2023-03-30 18:58 ` Patchwork
8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2023-03-30 18:58 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 16360 bytes --]
== Series Details ==
Series: Add MTL PMU support for multi-gt
URL : https://patchwork.freedesktop.org/series/115835/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12937_full -> IGTPW_8716_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_8716_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_8716_full, 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_8716/index.html
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_8716_full:
### IGT changes ###
#### Possible regressions ####
* {igt@perf_pmu@frequency@idle-gt0} (NEW):
- shard-snb: NOTRUN -> [FAIL][1] +2 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-snb5/igt@perf_pmu@frequency@idle-gt0.html
- {shard-dg1}: NOTRUN -> [FAIL][2] +4 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-dg1-18/igt@perf_pmu@frequency@idle-gt0.html
* {igt@perf_pmu@rc6-all-gts} (NEW):
- {shard-dg1}: NOTRUN -> [SKIP][3] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-dg1-18/igt@perf_pmu@rc6-all-gts.html
- {shard-tglu}: NOTRUN -> [SKIP][4] +1 similar issue
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-5/igt@perf_pmu@rc6-all-gts.html
* igt@perf_pmu@rc6-suspend:
- shard-snb: [PASS][5] -> [FAIL][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-snb4/igt@perf_pmu@rc6-suspend.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-snb7/igt@perf_pmu@rc6-suspend.html
- shard-apl: [PASS][7] -> [FAIL][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-apl1/igt@perf_pmu@rc6-suspend.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-apl6/igt@perf_pmu@rc6-suspend.html
- shard-glk: [PASS][9] -> [FAIL][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-glk9/igt@perf_pmu@rc6-suspend.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk3/igt@perf_pmu@rc6-suspend.html
* {igt@perf_pmu@rc6@runtime-pm-gt0} (NEW):
- shard-apl: NOTRUN -> [FAIL][11] +4 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-apl2/igt@perf_pmu@rc6@runtime-pm-gt0.html
- {shard-tglu}: NOTRUN -> [FAIL][12] +4 similar issues
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-6/igt@perf_pmu@rc6@runtime-pm-gt0.html
- shard-glk: NOTRUN -> [FAIL][13] +4 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk2/igt@perf_pmu@rc6@runtime-pm-gt0.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@perf_pmu@rc6-suspend:
- {shard-tglu}: [PASS][14] -> [FAIL][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-tglu-7/igt@perf_pmu@rc6-suspend.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-3/igt@perf_pmu@rc6-suspend.html
New tests
---------
New tests have been introduced between CI_DRM_12937_full and IGTPW_8716_full:
### New IGT tests (7) ###
* igt@perf_pmu@frequency@gt0:
- Statuses : 5 fail(s)
- Exec time: [0.0] s
* igt@perf_pmu@frequency@idle-gt0:
- Statuses : 5 fail(s)
- Exec time: [0.0] s
* igt@perf_pmu@rc6-all-gts:
- Statuses : 5 skip(s)
- Exec time: [0.0] s
* igt@perf_pmu@rc6@gt0:
- Statuses : 5 fail(s)
- Exec time: [0.0] s
* igt@perf_pmu@rc6@other-idle-gt0:
- Statuses : 5 skip(s)
- Exec time: [0.0] s
* igt@perf_pmu@rc6@runtime-pm-gt0:
- Statuses : 4 fail(s) 1 skip(s)
- Exec time: [0.0] s
* igt@perf_pmu@rc6@runtime-pm-long-gt0:
- Statuses : 4 fail(s) 1 skip(s)
- Exec time: [0.0] s
Known issues
------------
Here are the changes found in IGTPW_8716_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@thriceslice:
- shard-snb: NOTRUN -> [SKIP][16] ([fdo#109271]) +61 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-snb5/igt@gem_exec_schedule@thriceslice.html
* igt@gen9_exec_parse@allowed-single:
- shard-apl: [PASS][17] -> [ABORT][18] ([i915#5566])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-apl1/igt@gen9_exec_parse@allowed-single.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-apl2/igt@gen9_exec_parse@allowed-single.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-glk: [PASS][19] -> [FAIL][20] ([i915#2346]) +1 similar issue
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-glk2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2:
- shard-glk: [PASS][21] -> [FAIL][22] ([i915#79])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-glk1/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html
* igt@kms_flip@flip-vs-suspend@a-hdmi-a2:
- shard-glk: [PASS][23] -> [INCOMPLETE][24] ([i915#4839])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-glk8/igt@kms_flip@flip-vs-suspend@a-hdmi-a2.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk6/igt@kms_flip@flip-vs-suspend@a-hdmi-a2.html
* {igt@perf_pmu@rc6-all-gts} (NEW):
- shard-apl: NOTRUN -> [SKIP][25] ([fdo#109271]) +1 similar issue
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-apl4/igt@perf_pmu@rc6-all-gts.html
- shard-glk: NOTRUN -> [SKIP][26] ([fdo#109271]) +1 similar issue
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-glk4/igt@perf_pmu@rc6-all-gts.html
#### Possible fixes ####
* igt@gem_exec_endless@dispatch@rcs0:
- {shard-tglu}: [TIMEOUT][27] ([i915#3778]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-tglu-5/igt@gem_exec_endless@dispatch@rcs0.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-8/igt@gem_exec_endless@dispatch@rcs0.html
* igt@gem_exec_whisper@basic-fds-forked-all:
- {shard-tglu}: [INCOMPLETE][29] ([i915#6755] / [i915#7663]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-tglu-10/igt@gem_exec_whisper@basic-fds-forked-all.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-3/igt@gem_exec_whisper@basic-fds-forked-all.html
* igt@i915_pm_rc6_residency@rc6-idle@rcs0:
- {shard-tglu}: [WARN][31] ([i915#2681]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-tglu-2/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-tglu-5/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
* igt@i915_pm_rps@reset:
- shard-snb: [DMESG-FAIL][33] ([i915#8319]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-snb4/igt@i915_pm_rps@reset.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-snb2/igt@i915_pm_rps@reset.html
* igt@perf_pmu@all-busy-idle-check-all:
- {shard-dg1}: [FAIL][35] ([i915#5234]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-dg1-17/igt@perf_pmu@all-busy-idle-check-all.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-dg1-18/igt@perf_pmu@all-busy-idle-check-all.html
* igt@sysfs_heartbeat_interval@precise@rcs0:
- {shard-dg1}: [FAIL][37] ([i915#1755]) -> [PASS][38] +3 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12937/shard-dg1-18/igt@sysfs_heartbeat_interval@precise@rcs0.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/shard-dg1-17/igt@sysfs_heartbeat_interval@precise@rcs0.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#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
[fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
[i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
[i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
[i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3318]: https://gitlab.freedesktop.org/drm/intel/issues/3318
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
[i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
[i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
[i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
[i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
[i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
[i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
[i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
[i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
[i915#4818]: https://gitlab.freedesktop.org/drm/intel/issues/4818
[i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
[i915#4839]: https://gitlab.freedesktop.org/drm/intel/issues/4839
[i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
[i915#4854]: https://gitlab.freedesktop.org/drm/intel/issues/4854
[i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
[i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
[i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
[i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
[i915#5234]: https://gitlab.freedesktop.org/drm/intel/issues/5234
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6755]: https://gitlab.freedesktop.org/drm/intel/issues/6755
[i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7663]: https://gitlab.freedesktop.org/drm/intel/issues/7663
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#8319]: https://gitlab.freedesktop.org/drm/intel/issues/8319
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_7226 -> IGTPW_8716
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_12937: 6848d3613c0a63382d00ff550c41394902bda903 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8716: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/index.html
IGT_7226: 41be8b4ab86f9e11388c10366dfd71e5032589c1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8716/index.html
[-- Attachment #2: Type: text/html, Size: 11825 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-03-30 9:25 ` Tvrtko Ursulin
2023-03-30 18:43 ` Umesh Nerlige Ramappa
@ 2023-04-26 21:13 ` Umesh Nerlige Ramappa
2023-04-27 14:52 ` Tvrtko Ursulin
1 sibling, 1 reply; 19+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-26 21:13 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, badal.nilawar, arjun.melkaveri
On Thu, Mar 30, 2023 at 10:25:47AM +0100, Tvrtko Ursulin wrote:
>
>On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
>>With MTL frequency and rc6 counters are gt specific. Add support for
>>intel_gpu_top to show these counters separately.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>> include/drm-uapi/i915_drm.h | 14 ++++++----
>> tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
>> 2 files changed, 49 insertions(+), 21 deletions(-)
>>
>>diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
>>index 3b5e3b51..1a0c43e7 100644
>>--- a/include/drm-uapi/i915_drm.h
>>+++ b/include/drm-uapi/i915_drm.h
>>@@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
>> (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
>> ((__u64)(gt) << __I915_PMU_GT_SHIFT))
>>+/* Aggregate from all gts */
>> #define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
>> #define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
>>@@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
>> #define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
>>-#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
>>-#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
>>-#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
>>-#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
>>-#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ___I915_PMU_OTHER(gt, 4)
>>+/* GT specific counters */
>>+#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
>>+
>>+#define __I915_PMU_ACTUAL_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 0)
>>+#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
>>+#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
>>+#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
>>+#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt) ____I915_PMU_OTHER(gt, 4)
>> /* Each region is a minimum of 16k, and there are at most 255 of them.
>> */
>>diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>index a4302aa3..9fc8b996 100644
>>--- a/tools/intel_gpu_top.c
>>+++ b/tools/intel_gpu_top.c
>>@@ -87,6 +87,7 @@ struct engine_class {
>> unsigned int num_engines;
>> };
>>+#define MAX_GTS 4
>> struct engines {
>> unsigned int num_engines;
>> unsigned int num_classes;
>>@@ -105,14 +106,16 @@ struct engines {
>> struct pmu_counter imc_writes;
>> unsigned int num_imc;
>>- struct pmu_counter freq_req;
>>- struct pmu_counter freq_act;
>>+ struct pmu_counter freq_req[MAX_GTS];
>>+ struct pmu_counter freq_act[MAX_GTS];
>> struct pmu_counter irq;
>>- struct pmu_counter rc6;
>>+ struct pmu_counter rc6[MAX_GTS];
>> bool discrete;
>> char *device;
>>+ int num_gts;
>>+
>> /* Do not edit below this line.
>> * This structure is reallocated every time a new engine is
>> * found and size is increased by sizeof (engine).
>>@@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter *pmu, struct engines *engines)
>> static int pmu_init(struct engines *engines)
>> {
>> unsigned int i;
>>- int fd;
>>+ int fd, ret;
>> uint64_t type = igt_perf_type_id(engines->device);
>> engines->fd = -1;
>>@@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
>> if (fd < 0)
>> return -1;
>>- engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
>>- _open_pmu(type, engines->num_counters, &engines->freq_req, engines->fd);
>>+ engines->num_gts = 1;
>>+ for (i = 0; i < MAX_GTS; i++) {
>>+ engines->freq_req[i].config = __I915_PMU_REQUESTED_FREQUENCY(i);
>>- engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
>>- _open_pmu(type, engines->num_counters, &engines->freq_act, engines->fd);
>>+ errno = 0;
>>+ ret = _open_pmu(type, engines->num_counters, &engines->freq_req[i], engines->fd);
>>+ if (ret >= 0)
>>+ continue;
>>- engines->rc6.config = I915_PMU_RC6_RESIDENCY;
>>- _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
>>+ if (errno != ENOENT)
>>+ return ret;
>>+
>>+ engines->num_gts = i;
>>+ errno = 0;
>>+ break;
>>+ }
>>+
>>+ for (i = 0; i < engines->num_gts; i++) {
>>+ engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
>>+ _open_pmu(type, engines->num_counters, &engines->freq_act[i], engines->fd);
>>+
>>+ engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
>>+ _open_pmu(type, engines->num_counters, &engines->rc6[i], engines->fd);
>>+ }
>> for (i = 0; i < engines->num_engines; i++) {
>> struct engine *engine = engine_ptr(engines, i);
>>@@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
>> engines->ts.prev = engines->ts.cur;
>> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>>- update_sample(&engines->freq_req, val);
>>- update_sample(&engines->freq_act, val);
>>+ for (i = 0; i < engines->num_gts; i++) {
>>+ update_sample(&engines->freq_req[i], val);
>>+ update_sample(&engines->freq_act[i], val);
>>+ update_sample(&engines->rc6[i], val);
>>+ }
>> update_sample(&engines->irq, val);
>>- update_sample(&engines->rc6, val);
>> for (i = 0; i < engines->num_engines; i++) {
>> struct engine *engine = engine_ptr(engines, i);
>>@@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
>> .items = period_items,
>> };
>> struct cnt_item freq_items[] = {
>>- { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
>>- { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
>>+ { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0", "req-gt0" },
>>+ { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0", "act-gt0" },
>>+ { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1", "req-gt1" },
>>+ { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1", "act-gt1" },
>
>Why is width going to 8? 9999 MHz is not enough? ;)
>
>[Comes back later..]
>
>Ah for the header label.. hm.. maybe we should try putting the GT information into the parent. It would looks nicer, be more logical, even for JSON output we now have:
>
>Terminal:
>
> Freq MHz ...
> req act ...
>
>JSON:
>
> "rc6": {
> "value": 29.309568,
> "unit": "%"
> },
>You propose something like:
>
> Freq MHz Freq MHz ...
> req-gt0 act-gt0 req-gt0 act-gt0 ...
>
> "rc6": {
> "value-gt0": 29.309568,
> "value-gt1": 29.309568,
> "unit": "%"
> },
>
>Which is not very nice UI wise. How about something like:
>
> Freq GT0 MHz Freq GT1 MHz ...
> req act req act ...
>
>JSON should potentially be an array:
>
> "rc6": [{
> "gt": 0,
> "value": 29.309568,
> "unit": "%"
> },
> "gt": 0,
> "value": 29.309568,
> "unit": "%"
> }],
>
>Or at least:
>
> "rc6": {
> "value": 29.309568,
> "unit": "%"
> },
> "rc6-gt1": {
> "value": 29.309568,
> "unit": "%"
> },
>
>Which also brings the point if maybe we shouldn't change the output for pre-MTL platforms. The approach is not IMHO even consistent with the proposed kernel change to have the aggregated counters, which I don't think I agree with at all.
>
>Let me mull it all over.
Any further thoughts?
Thanks,
Umesh
>
>Regards,
>
>Tvrtko
>
>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
>> { },
>> };
>>@@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
>> .items = irq_items,
>> };
>> struct cnt_item rc6_items[] = {
>>- { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
>>+ { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
>>+ { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
>> { },
>> };
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters
2023-04-26 21:13 ` Umesh Nerlige Ramappa
@ 2023-04-27 14:52 ` Tvrtko Ursulin
0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2023-04-27 14:52 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: igt-dev, badal.nilawar, arjun.melkaveri
On 26/04/2023 22:13, Umesh Nerlige Ramappa wrote:
> On Thu, Mar 30, 2023 at 10:25:47AM +0100, Tvrtko Ursulin wrote:
>>
>> On 30/03/2023 01:36, Umesh Nerlige Ramappa wrote:
>>> With MTL frequency and rc6 counters are gt specific. Add support for
>>> intel_gpu_top to show these counters separately.
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>> ---
>>> include/drm-uapi/i915_drm.h | 14 ++++++----
>>> tools/intel_gpu_top.c | 56 ++++++++++++++++++++++++++-----------
>>> 2 files changed, 49 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
>>> index 3b5e3b51..1a0c43e7 100644
>>> --- a/include/drm-uapi/i915_drm.h
>>> +++ b/include/drm-uapi/i915_drm.h
>>> @@ -290,6 +290,7 @@ enum drm_i915_pmu_engine_sample {
>>> (((__u64)__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x)) | \
>>> ((__u64)(gt) << __I915_PMU_GT_SHIFT))
>>> +/* Aggregate from all gts */
>>> #define __I915_PMU_OTHER(x) ___I915_PMU_OTHER(0, x)
>>> #define I915_PMU_ACTUAL_FREQUENCY __I915_PMU_OTHER(0)
>>> @@ -300,11 +301,14 @@ enum drm_i915_pmu_engine_sample {
>>> #define I915_PMU_LAST /* Deprecated - do not use */
>>> I915_PMU_RC6_RESIDENCY
>>> -#define __I915_PMU_ACTUAL_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 0)
>>> -#define __I915_PMU_REQUESTED_FREQUENCY(gt) ___I915_PMU_OTHER(gt, 1)
>>> -#define __I915_PMU_INTERRUPTS(gt) ___I915_PMU_OTHER(gt, 2)
>>> -#define __I915_PMU_RC6_RESIDENCY(gt) ___I915_PMU_OTHER(gt, 3)
>>> -#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt)
>>> ___I915_PMU_OTHER(gt, 4)
>>> +/* GT specific counters */
>>> +#define ____I915_PMU_OTHER(gt, x) ___I915_PMU_OTHER(((gt) + 1), x)
>>> +
>>> +#define __I915_PMU_ACTUAL_FREQUENCY(gt)
>>> ____I915_PMU_OTHER(gt, 0)
>>> +#define __I915_PMU_REQUESTED_FREQUENCY(gt) ____I915_PMU_OTHER(gt, 1)
>>> +#define __I915_PMU_INTERRUPTS(gt) ____I915_PMU_OTHER(gt, 2)
>>> +#define __I915_PMU_RC6_RESIDENCY(gt) ____I915_PMU_OTHER(gt, 3)
>>> +#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt)
>>> ____I915_PMU_OTHER(gt, 4)
>>> /* Each region is a minimum of 16k, and there are at most 255 of them.
>>> */
>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>> index a4302aa3..9fc8b996 100644
>>> --- a/tools/intel_gpu_top.c
>>> +++ b/tools/intel_gpu_top.c
>>> @@ -87,6 +87,7 @@ struct engine_class {
>>> unsigned int num_engines;
>>> };
>>> +#define MAX_GTS 4
>>> struct engines {
>>> unsigned int num_engines;
>>> unsigned int num_classes;
>>> @@ -105,14 +106,16 @@ struct engines {
>>> struct pmu_counter imc_writes;
>>> unsigned int num_imc;
>>> - struct pmu_counter freq_req;
>>> - struct pmu_counter freq_act;
>>> + struct pmu_counter freq_req[MAX_GTS];
>>> + struct pmu_counter freq_act[MAX_GTS];
>>> struct pmu_counter irq;
>>> - struct pmu_counter rc6;
>>> + struct pmu_counter rc6[MAX_GTS];
>>> bool discrete;
>>> char *device;
>>> + int num_gts;
>>> +
>>> /* Do not edit below this line.
>>> * This structure is reallocated every time a new engine is
>>> * found and size is increased by sizeof (engine).
>>> @@ -532,7 +535,7 @@ static void imc_reads_open(struct pmu_counter
>>> *pmu, struct engines *engines)
>>> static int pmu_init(struct engines *engines)
>>> {
>>> unsigned int i;
>>> - int fd;
>>> + int fd, ret;
>>> uint64_t type = igt_perf_type_id(engines->device);
>>> engines->fd = -1;
>>> @@ -543,14 +546,30 @@ static int pmu_init(struct engines *engines)
>>> if (fd < 0)
>>> return -1;
>>> - engines->freq_req.config = I915_PMU_REQUESTED_FREQUENCY;
>>> - _open_pmu(type, engines->num_counters, &engines->freq_req,
>>> engines->fd);
>>> + engines->num_gts = 1;
>>> + for (i = 0; i < MAX_GTS; i++) {
>>> + engines->freq_req[i].config =
>>> __I915_PMU_REQUESTED_FREQUENCY(i);
>>> - engines->freq_act.config = I915_PMU_ACTUAL_FREQUENCY;
>>> - _open_pmu(type, engines->num_counters, &engines->freq_act,
>>> engines->fd);
>>> + errno = 0;
>>> + ret = _open_pmu(type, engines->num_counters,
>>> &engines->freq_req[i], engines->fd);
>>> + if (ret >= 0)
>>> + continue;
>>> - engines->rc6.config = I915_PMU_RC6_RESIDENCY;
>>> - _open_pmu(type, engines->num_counters, &engines->rc6, engines->fd);
>>> + if (errno != ENOENT)
>>> + return ret;
>>> +
>>> + engines->num_gts = i;
>>> + errno = 0;
>>> + break;
>>> + }
>>> +
>>> + for (i = 0; i < engines->num_gts; i++) {
>>> + engines->freq_act[i].config = __I915_PMU_ACTUAL_FREQUENCY(i);
>>> + _open_pmu(type, engines->num_counters,
>>> &engines->freq_act[i], engines->fd);
>>> +
>>> + engines->rc6[i].config = __I915_PMU_RC6_RESIDENCY(i);
>>> + _open_pmu(type, engines->num_counters, &engines->rc6[i],
>>> engines->fd);
>>> + }
>>> for (i = 0; i < engines->num_engines; i++) {
>>> struct engine *engine = engine_ptr(engines, i);
>>> @@ -653,10 +672,12 @@ static void pmu_sample(struct engines *engines)
>>> engines->ts.prev = engines->ts.cur;
>>> engines->ts.cur = pmu_read_multi(engines->fd, num_val, val);
>>> - update_sample(&engines->freq_req, val);
>>> - update_sample(&engines->freq_act, val);
>>> + for (i = 0; i < engines->num_gts; i++) {
>>> + update_sample(&engines->freq_req[i], val);
>>> + update_sample(&engines->freq_act[i], val);
>>> + update_sample(&engines->rc6[i], val);
>>> + }
>>> update_sample(&engines->irq, val);
>>> - update_sample(&engines->rc6, val);
>>> for (i = 0; i < engines->num_engines; i++) {
>>> struct engine *engine = engine_ptr(engines, i);
>>> @@ -1727,8 +1748,10 @@ print_header(const struct igt_device_card *card,
>>> .items = period_items,
>>> };
>>> struct cnt_item freq_items[] = {
>>> - { &engines->freq_req, 4, 0, 1.0, t, 1, "requested", "req" },
>>> - { &engines->freq_act, 4, 0, 1.0, t, 1, "actual", "act" },
>>> + { &engines->freq_req[0], 8, 0, 1.0, t, 1, "requested-gt0",
>>> "req-gt0" },
>>> + { &engines->freq_act[0], 8, 0, 1.0, t, 1, "actual-gt0",
>>> "act-gt0" },
>>> + { &engines->freq_req[1], 8, 0, 1.0, t, 1, "requested-gt1",
>>> "req-gt1" },
>>> + { &engines->freq_act[1], 8, 0, 1.0, t, 1, "actual-gt1",
>>> "act-gt1" },
>>
>> Why is width going to 8? 9999 MHz is not enough? ;)
>>
>> [Comes back later..]
>>
>> Ah for the header label.. hm.. maybe we should try putting the GT
>> information into the parent. It would looks nicer, be more logical,
>> even for JSON output we now have:
>>
>> Terminal:
>>
>> Freq MHz ...
>> req act ...
>>
>> JSON:
>>
>> "rc6": {
>> "value": 29.309568,
>> "unit": "%"
>> },
>> You propose something like:
>>
>> Freq MHz Freq MHz ...
>> req-gt0 act-gt0 req-gt0 act-gt0 ...
>>
>> "rc6": {
>> "value-gt0": 29.309568,
>> "value-gt1": 29.309568,
>> "unit": "%"
>> },
>>
>> Which is not very nice UI wise. How about something like:
>>
>> Freq GT0 MHz Freq GT1 MHz ...
>> req act req act ...
>>
>> JSON should potentially be an array:
>>
>> "rc6": [{
>> "gt": 0,
>> "value": 29.309568,
>> "unit": "%"
>> },
>> "gt": 0,
>> "value": 29.309568,
>> "unit": "%"
>> }],
>>
>> Or at least:
>>
>> "rc6": {
>> "value": 29.309568,
>> "unit": "%"
>> },
>> "rc6-gt1": {
>> "value": 29.309568,
>> "unit": "%"
>> },
>>
>> Which also brings the point if maybe we shouldn't change the output
>> for pre-MTL platforms. The approach is not IMHO even consistent with
>> the proposed kernel change to have the aggregated counters, which I
>> don't think I agree with at all.
>>
>> Let me mull it all over.
>
> Any further thoughts?
This completely slipped from my radar.
What I definitely think is that as proposed in this patch is a bit
unsightly and that we really need something nicer.
Maybe we should add tile aggregation to intel_gpu_top, unless user has
run it with -p, in which case we expand to per tile view (and per engine
obviously, since that is what we already have)?
That would kind of look nicer on screen, probably, although open would
be how to aggregate. Probably for frequency and RC6 just normalize by
number of tiles?
And it would partly solve the problem of the JSON format. In aggregated
mode we could stick with:
"rc6": {
"value": 29.309568,
"unit": "%"
},
And if "-p" was specified either emit an array or rc6-$tile. Latter is I
guess more backward compatible but I am not sure if that matters much.
There could easily be no users of -p in scripts.
If you will be attempting all this probably see if it can be split into
multiple patches so its easier to review.
I don't have a good idea on how to approach this right now, would need
to spend a little bit of time to try some things out. Here I am thinking
if there will be easy ways to toggle aggregation at runtime, by maybe
some similar tricks as I have for engines, where an aggregated fake list
of engines is created at the presentation time only, while everything
internally works of the physical view.
Regards,
Tvrtko
>
> Thanks,
> Umesh
>
>>
>> Regards,
>>
>> Tvrtko
>>
>>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "MHz" },
>>> { },
>>> };
>>> @@ -1748,7 +1771,8 @@ print_header(const struct igt_device_card *card,
>>> .items = irq_items,
>>> };
>>> struct cnt_item rc6_items[] = {
>>> - { &engines->rc6, 3, 0, 1e9, t, 100, "value", "%" },
>>> + { &engines->rc6[0], 6, 0, 1e9, t, 100, "value-gt0", "%-gt0" },
>>> + { &engines->rc6[1], 6, 0, 1e9, t, 100, "value-gt1", "%-gt1" },
>>> { NULL, 0, 0, 0.0, 0.0, 0.0, "unit", "%" },
>>> { },
>>> };
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-04-27 14:52 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-30 0:36 [igt-dev] [PATCH i-g-t 0/7] Add MTL PMU support for multi-gt Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 1/7] lib/debugfs: GT directory helpers Umesh Nerlige Ramappa
2023-03-30 8:49 ` Tvrtko Ursulin
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 2/7] tests/i915/perf_pmu: Support multi-tile in rc6 subtest Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 3/7] tests/i915/perf_pmu: Two new rc6 subtests Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 4/7] tests/i915/perf_pmu: Support multi-tile in frequency subtest Umesh Nerlige Ramappa
2023-03-30 8:54 ` Tvrtko Ursulin
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 5/7] tests/i915/perf_pmu: Quiesce GPU if measuring idle busyness without spinner Umesh Nerlige Ramappa
2023-03-30 9:00 ` Tvrtko Ursulin
2023-03-30 18:35 ` Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 6/7] tests/i915/perf_pmu: Use correct pmu config for multi-tile Umesh Nerlige Ramappa
2023-03-30 0:36 ` [igt-dev] [PATCH i-g-t 7/7] tools/intel_gpu_top: Add support for gt specific counters Umesh Nerlige Ramappa
2023-03-30 0:45 ` Umesh Nerlige Ramappa
2023-03-30 9:25 ` Tvrtko Ursulin
2023-03-30 18:43 ` Umesh Nerlige Ramappa
2023-04-26 21:13 ` Umesh Nerlige Ramappa
2023-04-27 14:52 ` Tvrtko Ursulin
2023-03-30 1:23 ` [igt-dev] ✓ Fi.CI.BAT: success for Add MTL PMU support for multi-gt Patchwork
2023-03-30 18:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.