Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
@ 2023-11-08  6:43 Sujaritha Sundaresan
  2023-11-08  7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sujaritha Sundaresan @ 2023-11-08  6:43 UTC (permalink / raw)
  To: igt-dev

Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy tests.

Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
---
 tests/intel/i915_pm_rc6_residency.c | 134 ++++++++++++++++++----------
 1 file changed, 85 insertions(+), 49 deletions(-)

diff --git a/tests/intel/i915_pm_rc6_residency.c b/tests/intel/i915_pm_rc6_residency.c
index b266680ac..f865fbe94 100644
--- a/tests/intel/i915_pm_rc6_residency.c
+++ b/tests/intel/i915_pm_rc6_residency.c
@@ -36,6 +36,7 @@
 #include "i915/gem.h"
 #include "i915/gem_create.h"
 #include "igt.h"
+#include "igt_debugfs.h"
 #include "igt_perf.h"
 #include "igt_power.h"
 #include "igt_sysfs.h"
@@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
 	return enabled;
 }
 
-static bool has_rc6_residency(const char *name)
+static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
 {
 	unsigned long residency;
-	char path[128];
 
-	sprintf(path, "power/%s_residency_ms", name);
-	return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
+	return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
 }
 
-static unsigned long read_rc6_residency(const char *name)
+static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id id)
 {
 	unsigned long residency;
-	char path[128];
 
 	residency = 0;
-	sprintf(path, "power/%s_residency_ms", name);
-	igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1);
+	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1);
 	return residency;
 }
 
@@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int diff,
 			       const char *name_of_rc6_residency)
 {
 	double ratio;
+	int gt = 0, gt_current;
 
 	ratio = (double)diff / duration;
 
-	igt_info("Residency in %s or deeper state: %u ms (sleep duration %u ms) (%.1f%% of expected duration)\n",
-		 name_of_rc6_residency, diff, duration, 100*ratio);
+	gt_current = igt_sysfs_get_u32(gt, "id");
+
+	igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep duration %u ms) (%.1f%% of expected duration)\n",
+		 gt_current, name_of_rc6_residency, diff, duration, 100*ratio);
 	igt_assert_f(ratio > 0.9 && ratio < 1.05,
 		     "Sysfs RC6 residency counter is inaccurate.\n");
 }
@@ -125,28 +125,28 @@ static unsigned long gettime_ms(void)
 	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
 }
 
-static void read_residencies(int devid, unsigned int mask,
+static void read_residencies(int devid, int dirfd, unsigned int mask,
 			     struct residencies *res)
 {
 	res->duration = gettime_ms();
 
 	if (mask & RC6_ENABLED)
-		res->rc6 = read_rc6_residency("rc6");
+		res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
 
 	if ((mask & RC6_ENABLED) &&
 	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
-		res->media_rc6 = read_rc6_residency("media_rc6");
+		res->media_rc6 = read_rc6_residency(dirfd, MEDIA_RC6_RESIDENCY_MS);
 
 	if (mask & RC6P_ENABLED)
-		res->rc6p = read_rc6_residency("rc6p");
+		res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
 
 	if (mask & RC6PP_ENABLED)
-		res->rc6pp = read_rc6_residency("rc6pp");
+		res->rc6pp = read_rc6_residency(dirfd, RC6PP_RESIDENCY_MS);
 
 	res->duration += (gettime_ms() - res->duration) / 2;
 }
 
-static void measure_residencies(int devid, unsigned int mask,
+static void measure_residencies(int devid, int dirfd, unsigned int mask,
 				struct residencies *res)
 {
 	struct residencies start = { };
@@ -158,13 +158,13 @@ static void measure_residencies(int devid, unsigned int mask,
 	 * measurement, since the valid counter range is different on
 	 * different platforms and so fixing it up would be non-trivial.
 	 */
-	read_residencies(devid, mask, &end);
+	read_residencies(devid, dirfd, mask, &end);
 	igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
 		  end.duration, end.rc6, end.media_rc6, end.rc6p, end.rc6pp);
 	for (retry = 0; retry < 2; retry++) {
 		start = end;
 		sleep(SLEEP_DURATION);
-		read_residencies(devid, mask, &end);
+		read_residencies(devid, dirfd, mask, &end);
 
 		igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
 			  end.duration,
@@ -196,7 +196,7 @@ static void measure_residencies(int devid, unsigned int mask,
 	res->rc6 += res->rc6p;
 }
 
-static bool wait_for_rc6(void)
+static bool wait_for_rc6(int dirfd)
 {
 	struct timespec tv = {};
 	unsigned long start, now;
@@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
 	usleep(160 * 1000);
 
 	/* Then poll for RC6 to start ticking */
-	now = read_rc6_residency("rc6");
+	now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
 	do {
 		start = now;
 		usleep(5000);
-		now = read_rc6_residency("rc6");
+		now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
 		if (now - start > 1)
 			return true;
 	} while (!igt_seconds_elapsed(&tv));
@@ -234,14 +234,27 @@ static uint64_t pmu_read_single(int fd)
 	return __pmu_read_single(fd, NULL);
 }
 
-#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
-	igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
-		     (x) >= (ref) * (1.0 - (tol_down)/100.), \
-		     "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of %.3g)\n",\
-		     #x, #ref, (double)(x), (tol_up), (tol_down), (double)(ref))
+#define __assert_within_epsilon(x, ref, tol_up, tol_down, debug_data) \
+	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
+		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
+		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of %f)\n %s\n",\
+		     #x, #ref, (double)(x), \
+		     (tol_up) * 100.0, (tol_down) * 100.0, \
+		     (double)(ref), debug_data)
+
+#define assert_within_epsilon(x, ref, tolerance, debug_data) \
+	__assert_within_epsilon(x, ref, tolerance, tolerance, debug_data)
 
-#define assert_within_epsilon(x, ref, tolerance) \
-	__assert_within_epsilon(x, ref, tolerance, tolerance)
+char *drpc;
+
+static char *get_drpc(int i915, int gt_id)
+{
+	int gt_dir;
+
+	gt_dir = igt_debugfs_gt_dir(i915, gt_id);
+	igt_assert(gt_dir != -1);
+	return igt_sysfs_get(gt_dir, "drpc");
+}
 
 static bool __pmu_wait_for_rc6(int fd)
 {
@@ -376,7 +389,7 @@ static void kill_children(int sig)
 	signal(sig, old);
 }
 
-static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
+static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags, unsigned int gt)
 {
 	const int64_t duration_ns = SLEEP_DURATION * (int64_t)NSEC_PER_SEC;
 	const int tolerance = 20; /* Some RC6 is better than none! */
@@ -397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
 	struct igt_power gpu;
 	int fd;
 
-	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
+	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
 	igt_drop_caches_set(i915, DROP_IDLE);
 	igt_require(__pmu_wait_for_rc6(fd));
 	igt_power_open(i915, &gpu, "gpu");
@@ -418,7 +431,10 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
 			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
 			idle, (idle * 1e9) / slept);
 	}
-	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
+
+	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
+
+	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
 
 	done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 
@@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
 		igt_assert(cycles >= SLEEP_DURATION);
 
 		/* While very nearly idle, expect full RC6 */
-		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
+		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
+
+		free(drpc);
+		drpc = NULL;
 	}
 
 	munmap(done, 4096);
@@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
 	}
 }
 
-static void rc6_fence(int i915, const intel_ctx_t *ctx)
+static void rc6_fence(int i915, unsigned int gt)
 {
 	const int64_t duration_ns = SLEEP_DURATION * (int64_t)NSEC_PER_SEC;
 	const int tolerance = 20; /* Some RC6 is better than none! */
 	const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
 	const struct intel_execution_engine2 *e;
+	const intel_ctx_t *ctx;
 	struct power_sample sample[2];
 	unsigned long slept;
 	uint64_t rc6, ts[2], ahnd;
@@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
 
 	igt_require_sw_sync();
 
-	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
+	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
 	igt_drop_caches_set(i915, DROP_IDLE);
 	igt_require(__pmu_wait_for_rc6(fd));
 	igt_power_open(i915, &gpu, "gpu");
@@ -506,9 +526,12 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
 			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
 			idle, (idle * 1e9) / slept);
 	}
-	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
+	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
+
+	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
 
 	/* Submit but delay execution, we should be idle and conserving power */
+	ctx = intel_ctx_create_for_gt(i915, gt);
 	ahnd = get_reloc_ahnd(i915, ctx->id);
 	for_each_ctx_engine(i915, ctx, e) {
 		igt_spin_t *spin;
@@ -546,10 +569,14 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
 
 		close(timeline);
 
-		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
+		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
 		gem_quiescent_gpu(i915);
+
+		free(drpc);
+		drpc = NULL;
 	}
 	put_ahnd(ahnd);
+	intel_ctx_destroy(i915,ctx);
 
 	igt_power_close(&gpu);
 	close(fd);
@@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
 igt_main
 {
 	int i915 = -1;
+	unsigned int dirfd, gt;
 	const intel_ctx_t *ctx;
 
 	/* Use drm_open_driver to verify device existence */
@@ -572,19 +600,25 @@ igt_main
 		igt_require_gem(i915);
 		gem_quiescent_gpu(i915);
 
-		for_each_ctx_engine(i915, ctx, e) {
-			if (e->instance == 0) {
-				igt_dynamic_f("%s", e->name)
-					rc6_idle(i915, ctx->id, e->flags);
+		i915_for_each_gt(i915, dirfd, gt) {
+			ctx = intel_ctx_create_for_gt(i915, gt);
+			for_each_ctx_engine(i915, ctx, e) {
+				if (e->instance == 0) {
+					igt_dynamic_f("gt%u-%s", gt, e->name)
+						rc6_idle(i915, ctx->id, e->flags, gt);
+				}
 			}
+			intel_ctx_destroy(i915, ctx);
 		}
 	}
 
-	igt_subtest("rc6-fence") {
+	igt_subtest_with_dynamic("rc6-fence") {
 		igt_require_gem(i915);
 		gem_quiescent_gpu(i915);
 
-		rc6_fence(i915, ctx);
+		i915_for_each_gt(i915, dirfd, gt)
+			igt_dynamic_f("gt%u", gt)
+				rc6_fence(i915, gt);
 	}
 
 	igt_subtest_group {
@@ -595,21 +629,23 @@ igt_main
 			devid = intel_get_drm_devid(i915);
 			sysfs = igt_sysfs_open(i915);
 
-			igt_require(has_rc6_residency("rc6"));
+			igt_require(has_rc6_residency(dirfd, RC6_RESIDENCY_MS));
 
 			/* Make sure rc6 counters are running */
 			igt_drop_caches_set(i915, DROP_IDLE);
-			igt_require(wait_for_rc6());
+			igt_require(wait_for_rc6(dirfd));
 
 			rc6_enabled = get_rc6_enabled_mask();
 			igt_require(rc6_enabled & RC6_ENABLED);
 		}
 
 		igt_subtest("rc6-accuracy") {
-			struct residencies res;
+			for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
+				struct residencies res;
 
-			measure_residencies(devid, rc6_enabled, &res);
-			residency_accuracy(res.rc6, res.duration, "rc6");
+				measure_residencies(devid, dirfd, rc6_enabled, &res);
+				residency_accuracy(res.rc6, res.duration, "rc6");
+			}
 		}
 
 		igt_subtest("media-rc6-accuracy") {
@@ -617,7 +653,7 @@ igt_main
 
 			igt_require(IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid));
 
-			measure_residencies(devid, rc6_enabled, &res);
+			measure_residencies(devid, sysfs, rc6_enabled, &res);
 			residency_accuracy(res.media_rc6, res.duration, "media_rc6");
 		}
 
@@ -626,7 +662,7 @@ igt_main
 	}
 
 	igt_fixture {
-		intel_ctx_destroy(i915, ctx);
+		free(drpc);
 		drm_close_driver(i915);
 	}
 }
-- 
2.25.1

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

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  6:43 [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test Sujaritha Sundaresan
@ 2023-11-08  7:27 ` Patchwork
  2023-11-08  7:28 ` [igt-dev] ✓ CI.xeBAT: success " Patchwork
  2023-11-08  8:52 ` [igt-dev] [PATCH] " Gupta, Anshuman
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-11-08  7:27 UTC (permalink / raw)
  To: Sujaritha Sundaresan; +Cc: igt-dev

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

== Series Details ==

Series: tests/intel: Add multi gt support for rc6_residency test
URL   : https://patchwork.freedesktop.org/series/126114/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13848 -> IGTPW_10134
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10134 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10134, please notify your bug team (lgci.bug.filing@intel.com) 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_10134/index.html

Participating hosts (33 -> 33)
------------------------------

  Additional (2): fi-hsw-4770 fi-tgl-1115g4 
  Missing    (2): bat-dg2-8 bat-dg2-9 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5:
    - bat-adlp-11:        [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13848/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-d-dp-5.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#4613]) +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        NOTRUN -> [INCOMPLETE][6] ([i915#9527])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-jsl-3:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13848/bat-jsl-3/igt@i915_suspend@basic-s2idle-without-i915.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/bat-jsl-3/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-tgl-1115g4:      NOTRUN -> [INCOMPLETE][9] ([i915#7443])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#5190])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][11] ([i915#4103]) +1 other test skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_dsc@dsc-basic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][12] ([i915#3555] / [i915#3840])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][13] ([fdo#109285])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-vga-1:
    - fi-hsw-4770:        NOTRUN -> [SKIP][14] ([fdo#109271]) +12 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-hsw-4770/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-a-vga-1.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5:
    - bat-adlp-11:        [PASS][15] -> [DMESG-FAIL][16] ([i915#6868])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13848/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-5.html

  * igt@kms_psr@cursor_plane_move:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][17] ([fdo#110189]) +3 other tests skip
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@kms_psr@cursor_plane_move.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-hsw-4770:        NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#1072]) +3 other tests skip
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][19] ([i915#3555])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/fi-tgl-1115g4/igt@kms_setmode@basic-clone-single-crtc.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#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
  [i915#7443]: https://gitlab.freedesktop.org/drm/intel/issues/7443
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9527]: https://gitlab.freedesktop.org/drm/intel/issues/9527


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7575 -> IGTPW_10134

  CI-20190529: 20190529
  CI_DRM_13848: 1f1d9b88e2250757516fe83bbf851f198d292d31 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10134: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/index.html
  IGT_7575: 6edf8b6808de2bde968415926d2b55817f7ea1de @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

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

* [igt-dev] ✓ CI.xeBAT: success for tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  6:43 [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test Sujaritha Sundaresan
  2023-11-08  7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2023-11-08  7:28 ` Patchwork
  2023-11-08  8:52 ` [igt-dev] [PATCH] " Gupta, Anshuman
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-11-08  7:28 UTC (permalink / raw)
  To: Sujaritha Sundaresan; +Cc: igt-dev

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

== Series Details ==

Series: tests/intel: Add multi gt support for rc6_residency test
URL   : https://patchwork.freedesktop.org/series/126114/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7575_BAT -> XEIGTPW_10134_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

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

  [Intel XE#854]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/854
  [Intel XE#862]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/862


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

  * IGT: IGT_7575 -> IGTPW_10134
  * Linux: xe-472-5a3a6fdda31fc8fa60db6d2fa30bd4db2509eb8b -> xe-478-2319a9c72748dfb1a65cfbd89d0dab6254c7cbe2

  IGTPW_10134: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10134/index.html
  IGT_7575: 6edf8b6808de2bde968415926d2b55817f7ea1de @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-472-5a3a6fdda31fc8fa60db6d2fa30bd4db2509eb8b: 5a3a6fdda31fc8fa60db6d2fa30bd4db2509eb8b
  xe-478-2319a9c72748dfb1a65cfbd89d0dab6254c7cbe2: 2319a9c72748dfb1a65cfbd89d0dab6254c7cbe2

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10134/index.html

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

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

* Re: [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  6:43 [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test Sujaritha Sundaresan
  2023-11-08  7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
  2023-11-08  7:28 ` [igt-dev] ✓ CI.xeBAT: success " Patchwork
@ 2023-11-08  8:52 ` Gupta, Anshuman
  2023-11-08  8:54   ` Sundaresan, Sujaritha
  2 siblings, 1 reply; 8+ messages in thread
From: Gupta, Anshuman @ 2023-11-08  8:52 UTC (permalink / raw)
  To: Sundaresan, Sujaritha, igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
> Sent: Wednesday, November 8, 2023 12:13 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Sundaresan, Sujaritha
> <sujaritha.sundaresan@intel.com>
> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency test
> 
> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy tests.
Please break this patch like below.
1) If any common  refactoring required, not applicable if no such refactoring.
2) Add multi gt support for rc6_idle.
3) Add multi gt support for rc6_fence.
4) Add multi gt support for rc6_accuracy test.
5) Add drpc debug support.
> 
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> ---
>  tests/intel/i915_pm_rc6_residency.c | 134 ++++++++++++++++++----------
>  1 file changed, 85 insertions(+), 49 deletions(-)
> 
> diff --git a/tests/intel/i915_pm_rc6_residency.c
> b/tests/intel/i915_pm_rc6_residency.c
> index b266680ac..f865fbe94 100644
> --- a/tests/intel/i915_pm_rc6_residency.c
> +++ b/tests/intel/i915_pm_rc6_residency.c
> @@ -36,6 +36,7 @@
>  #include "i915/gem.h"
>  #include "i915/gem_create.h"
>  #include "igt.h"
> +#include "igt_debugfs.h"
>  #include "igt_perf.h"
>  #include "igt_power.h"
>  #include "igt_sysfs.h"
> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
>  	return enabled;
>  }
> 
> -static bool has_rc6_residency(const char *name)
> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
>  {
>  	unsigned long residency;
> -	char path[128];
> 
> -	sprintf(path, "power/%s_residency_ms", name);
> -	return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
> +	return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
>  }
> 
> -static unsigned long read_rc6_residency(const char *name)
> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
> +id)
>  {
>  	unsigned long residency;
> -	char path[128];
> 
>  	residency = 0;
> -	sprintf(path, "power/%s_residency_ms", name);
> -	igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1);
> +	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1);
>  	return residency;
>  }
> 
> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int diff,
>  			       const char *name_of_rc6_residency)  {
>  	double ratio;
> +	int gt = 0, gt_current;
> 
>  	ratio = (double)diff / duration;
> 
> -	igt_info("Residency in %s or deeper state: %u ms (sleep duration %u
> ms) (%.1f%% of expected duration)\n",
> -		 name_of_rc6_residency, diff, duration, 100*ratio);
> +	gt_current = igt_sysfs_get_u32(gt, "id");
> +
> +	igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
> duration %u ms) (%.1f%% of expected duration)\n",
> +		 gt_current, name_of_rc6_residency, diff, duration,
> 100*ratio);
>  	igt_assert_f(ratio > 0.9 && ratio < 1.05,
>  		     "Sysfs RC6 residency counter is inaccurate.\n");  } @@ -
Print gt id in igt_assert_f failed message .
Thanks,
Anshuman Gupta.
> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
>  	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;  }
> 
> -static void read_residencies(int devid, unsigned int mask,
> +static void read_residencies(int devid, int dirfd, unsigned int mask,
>  			     struct residencies *res)
>  {
>  	res->duration = gettime_ms();
> 
>  	if (mask & RC6_ENABLED)
> -		res->rc6 = read_rc6_residency("rc6");
> +		res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
> 
>  	if ((mask & RC6_ENABLED) &&
>  	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
> -		res->media_rc6 = read_rc6_residency("media_rc6");
> +		res->media_rc6 = read_rc6_residency(dirfd,
> MEDIA_RC6_RESIDENCY_MS);
> 
>  	if (mask & RC6P_ENABLED)
> -		res->rc6p = read_rc6_residency("rc6p");
> +		res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
> 
>  	if (mask & RC6PP_ENABLED)
> -		res->rc6pp = read_rc6_residency("rc6pp");
> +		res->rc6pp = read_rc6_residency(dirfd,
> RC6PP_RESIDENCY_MS);
> 
>  	res->duration += (gettime_ms() - res->duration) / 2;  }
> 
> -static void measure_residencies(int devid, unsigned int mask,
> +static void measure_residencies(int devid, int dirfd, unsigned int
> +mask,
>  				struct residencies *res)
>  {
>  	struct residencies start = { };
> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
> unsigned int mask,
>  	 * measurement, since the valid counter range is different on
>  	 * different platforms and so fixing it up would be non-trivial.
>  	 */
> -	read_residencies(devid, mask, &end);
> +	read_residencies(devid, dirfd, mask, &end);
>  	igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>  		  end.duration, end.rc6, end.media_rc6, end.rc6p, end.rc6pp);
>  	for (retry = 0; retry < 2; retry++) {
>  		start = end;
>  		sleep(SLEEP_DURATION);
> -		read_residencies(devid, mask, &end);
> +		read_residencies(devid, dirfd, mask, &end);
> 
>  		igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>  			  end.duration,
> @@ -196,7 +196,7 @@ static void measure_residencies(int devid, unsigned
> int mask,
>  	res->rc6 += res->rc6p;
>  }
> 
> -static bool wait_for_rc6(void)
> +static bool wait_for_rc6(int dirfd)
>  {
>  	struct timespec tv = {};
>  	unsigned long start, now;
> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
>  	usleep(160 * 1000);
> 
>  	/* Then poll for RC6 to start ticking */
> -	now = read_rc6_residency("rc6");
> +	now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>  	do {
>  		start = now;
>  		usleep(5000);
> -		now = read_rc6_residency("rc6");
> +		now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>  		if (now - start > 1)
>  			return true;
>  	} while (!igt_seconds_elapsed(&tv));
> @@ -234,14 +234,27 @@ static uint64_t pmu_read_single(int fd)
>  	return __pmu_read_single(fd, NULL);
>  }
> 
> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
> -	igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
> -		     (x) >= (ref) * (1.0 - (tol_down)/100.), \
> -		     "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
> %.3g)\n",\
> -		     #x, #ref, (double)(x), (tol_up), (tol_down), (double)(ref))
> +#define __assert_within_epsilon(x, ref, tol_up, tol_down, debug_data) \
> +	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
> +		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
> +		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of
> %f)\n %s\n",\
> +		     #x, #ref, (double)(x), \
> +		     (tol_up) * 100.0, (tol_down) * 100.0, \
> +		     (double)(ref), debug_data)
> +
> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
> +	__assert_within_epsilon(x, ref, tolerance, tolerance, debug_data)
> 
> -#define assert_within_epsilon(x, ref, tolerance) \
> -	__assert_within_epsilon(x, ref, tolerance, tolerance)
> +char *drpc;
> +
> +static char *get_drpc(int i915, int gt_id) {
> +	int gt_dir;
> +
> +	gt_dir = igt_debugfs_gt_dir(i915, gt_id);
> +	igt_assert(gt_dir != -1);
> +	return igt_sysfs_get(gt_dir, "drpc");
> +}
> 
>  static bool __pmu_wait_for_rc6(int fd)
>  {
> @@ -376,7 +389,7 @@ static void kill_children(int sig)
>  	signal(sig, old);
>  }
> 
> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
> +unsigned int gt)
>  {
>  	const int64_t duration_ns = SLEEP_DURATION *
> (int64_t)NSEC_PER_SEC;
>  	const int tolerance = 20; /* Some RC6 is better than none! */ @@ -
> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>  	struct igt_power gpu;
>  	int fd;
> 
> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>  	igt_drop_caches_set(i915, DROP_IDLE);
>  	igt_require(__pmu_wait_for_rc6(fd));
>  	igt_power_open(i915, &gpu, "gpu");
> @@ -418,7 +431,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
> uint64_t flags)
>  			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>  			idle, (idle * 1e9) / slept);
>  	}
> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
> +
> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
> +
> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
> 
>  	done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
> 1, 0);
> 
> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
> uint64_t flags)
>  		igt_assert(cycles >= SLEEP_DURATION);
> 
>  		/* While very nearly idle, expect full RC6 */
> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
> +
> +		free(drpc);
> +		drpc = NULL;
>  	}
> 
>  	munmap(done, 4096);
> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
> uint64_t flags)
>  	}
>  }
> 
> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
> +static void rc6_fence(int i915, unsigned int gt)
>  {
>  	const int64_t duration_ns = SLEEP_DURATION *
> (int64_t)NSEC_PER_SEC;
>  	const int tolerance = 20; /* Some RC6 is better than none! */
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>  	const struct intel_execution_engine2 *e;
> +	const intel_ctx_t *ctx;
>  	struct power_sample sample[2];
>  	unsigned long slept;
>  	uint64_t rc6, ts[2], ahnd;
> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
> 
>  	igt_require_sw_sync();
> 
> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>  	igt_drop_caches_set(i915, DROP_IDLE);
>  	igt_require(__pmu_wait_for_rc6(fd));
>  	igt_power_open(i915, &gpu, "gpu");
> @@ -506,9 +526,12 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
>  			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>  			idle, (idle * 1e9) / slept);
>  	}
> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
> +
> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
> 
>  	/* Submit but delay execution, we should be idle and conserving
> power */
> +	ctx = intel_ctx_create_for_gt(i915, gt);
>  	ahnd = get_reloc_ahnd(i915, ctx->id);
>  	for_each_ctx_engine(i915, ctx, e) {
>  		igt_spin_t *spin;
> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const intel_ctx_t
> *ctx)
> 
>  		close(timeline);
> 
> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>  		gem_quiescent_gpu(i915);
> +
> +		free(drpc);
> +		drpc = NULL;
>  	}
>  	put_ahnd(ahnd);
> +	intel_ctx_destroy(i915,ctx);
> 
>  	igt_power_close(&gpu);
>  	close(fd);
> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
> igt_main  {
>  	int i915 = -1;
> +	unsigned int dirfd, gt;
>  	const intel_ctx_t *ctx;
> 
>  	/* Use drm_open_driver to verify device existence */ @@ -572,19
> +600,25 @@ igt_main
>  		igt_require_gem(i915);
>  		gem_quiescent_gpu(i915);
> 
> -		for_each_ctx_engine(i915, ctx, e) {
> -			if (e->instance == 0) {
> -				igt_dynamic_f("%s", e->name)
> -					rc6_idle(i915, ctx->id, e->flags);
> +		i915_for_each_gt(i915, dirfd, gt) {
> +			ctx = intel_ctx_create_for_gt(i915, gt);
> +			for_each_ctx_engine(i915, ctx, e) {
> +				if (e->instance == 0) {
> +					igt_dynamic_f("gt%u-%s", gt, e-
> >name)
> +						rc6_idle(i915, ctx->id, e-
> >flags, gt);
> +				}
>  			}
> +			intel_ctx_destroy(i915, ctx);
>  		}
>  	}
> 
> -	igt_subtest("rc6-fence") {
> +	igt_subtest_with_dynamic("rc6-fence") {
>  		igt_require_gem(i915);
>  		gem_quiescent_gpu(i915);
> 
> -		rc6_fence(i915, ctx);
> +		i915_for_each_gt(i915, dirfd, gt)
> +			igt_dynamic_f("gt%u", gt)
> +				rc6_fence(i915, gt);
>  	}
> 
>  	igt_subtest_group {
> @@ -595,21 +629,23 @@ igt_main
>  			devid = intel_get_drm_devid(i915);
>  			sysfs = igt_sysfs_open(i915);
> 
> -			igt_require(has_rc6_residency("rc6"));
> +			igt_require(has_rc6_residency(dirfd,
> RC6_RESIDENCY_MS));
> 
>  			/* Make sure rc6 counters are running */
>  			igt_drop_caches_set(i915, DROP_IDLE);
> -			igt_require(wait_for_rc6());
> +			igt_require(wait_for_rc6(dirfd));
> 
>  			rc6_enabled = get_rc6_enabled_mask();
>  			igt_require(rc6_enabled & RC6_ENABLED);
>  		}
> 
>  		igt_subtest("rc6-accuracy") {
> -			struct residencies res;
> +			for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
> +				struct residencies res;
> 
> -			measure_residencies(devid, rc6_enabled, &res);
> -			residency_accuracy(res.rc6, res.duration, "rc6");
> +				measure_residencies(devid, dirfd,
> rc6_enabled, &res);
> +				residency_accuracy(res.rc6, res.duration,
> "rc6");
> +			}
>  		}
> 
>  		igt_subtest("media-rc6-accuracy") {
> @@ -617,7 +653,7 @@ igt_main
> 
>  			igt_require(IS_VALLEYVIEW(devid) ||
> IS_CHERRYVIEW(devid));
> 
> -			measure_residencies(devid, rc6_enabled, &res);
> +			measure_residencies(devid, sysfs, rc6_enabled, &res);
>  			residency_accuracy(res.media_rc6, res.duration,
> "media_rc6");
>  		}
> 
> @@ -626,7 +662,7 @@ igt_main
>  	}
> 
>  	igt_fixture {
> -		intel_ctx_destroy(i915, ctx);
> +		free(drpc);
>  		drm_close_driver(i915);
>  	}
>  }
> --
> 2.25.1

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

* Re: [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  8:52 ` [igt-dev] [PATCH] " Gupta, Anshuman
@ 2023-11-08  8:54   ` Sundaresan, Sujaritha
  2023-11-08  9:04     ` Gupta, Anshuman
  0 siblings, 1 reply; 8+ messages in thread
From: Sundaresan, Sujaritha @ 2023-11-08  8:54 UTC (permalink / raw)
  To: Gupta, Anshuman, igt-dev@lists.freedesktop.org


On 11/8/2023 2:22 PM, Gupta, Anshuman wrote:
>
>> -----Original Message-----
>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>> Sent: Wednesday, November 8, 2023 12:13 PM
>> To: igt-dev@lists.freedesktop.org
>> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Sundaresan, Sujaritha
>> <sujaritha.sundaresan@intel.com>
>> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency test
>>
>> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy tests.
> Please break this patch like below.
> 1) If any common  refactoring required, not applicable if no such refactoring.
> 2) Add multi gt support for rc6_idle.
> 3) Add multi gt support for rc6_fence.
> 4) Add multi gt support for rc6_accuracy test.
> 5) Add drpc debug support.
Okay I will make this a 4 patch series then.
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> ---
>>   tests/intel/i915_pm_rc6_residency.c | 134 ++++++++++++++++++----------
>>   1 file changed, 85 insertions(+), 49 deletions(-)
>>
>> diff --git a/tests/intel/i915_pm_rc6_residency.c
>> b/tests/intel/i915_pm_rc6_residency.c
>> index b266680ac..f865fbe94 100644
>> --- a/tests/intel/i915_pm_rc6_residency.c
>> +++ b/tests/intel/i915_pm_rc6_residency.c
>> @@ -36,6 +36,7 @@
>>   #include "i915/gem.h"
>>   #include "i915/gem_create.h"
>>   #include "igt.h"
>> +#include "igt_debugfs.h"
>>   #include "igt_perf.h"
>>   #include "igt_power.h"
>>   #include "igt_sysfs.h"
>> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
>>   	return enabled;
>>   }
>>
>> -static bool has_rc6_residency(const char *name)
>> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
>>   {
>>   	unsigned long residency;
>> -	char path[128];
>>
>> -	sprintf(path, "power/%s_residency_ms", name);
>> -	return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
>> +	return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
>>   }
>>
>> -static unsigned long read_rc6_residency(const char *name)
>> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
>> +id)
>>   {
>>   	unsigned long residency;
>> -	char path[128];
>>
>>   	residency = 0;
>> -	sprintf(path, "power/%s_residency_ms", name);
>> -	igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1);
>> +	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1);
>>   	return residency;
>>   }
>>
>> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int diff,
>>   			       const char *name_of_rc6_residency)  {
>>   	double ratio;
>> +	int gt = 0, gt_current;
>>
>>   	ratio = (double)diff / duration;
>>
>> -	igt_info("Residency in %s or deeper state: %u ms (sleep duration %u
>> ms) (%.1f%% of expected duration)\n",
>> -		 name_of_rc6_residency, diff, duration, 100*ratio);
>> +	gt_current = igt_sysfs_get_u32(gt, "id");
>> +
>> +	igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
>> duration %u ms) (%.1f%% of expected duration)\n",
>> +		 gt_current, name_of_rc6_residency, diff, duration,
>> 100*ratio);
>>   	igt_assert_f(ratio > 0.9 && ratio < 1.05,
>>   		     "Sysfs RC6 residency counter is inaccurate.\n");  } @@ -
> Print gt id in igt_assert_f failed message .
> Thanks,
> Anshuman Gupta.

I thought maybe giving out the gt id as part of info would be useful, 
but sure I'll move it to the assert.

Will be better fro debug.

Thanks for the review.

Suja

>> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
>>   	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;  }
>>
>> -static void read_residencies(int devid, unsigned int mask,
>> +static void read_residencies(int devid, int dirfd, unsigned int mask,
>>   			     struct residencies *res)
>>   {
>>   	res->duration = gettime_ms();
>>
>>   	if (mask & RC6_ENABLED)
>> -		res->rc6 = read_rc6_residency("rc6");
>> +		res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>
>>   	if ((mask & RC6_ENABLED) &&
>>   	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
>> -		res->media_rc6 = read_rc6_residency("media_rc6");
>> +		res->media_rc6 = read_rc6_residency(dirfd,
>> MEDIA_RC6_RESIDENCY_MS);
>>
>>   	if (mask & RC6P_ENABLED)
>> -		res->rc6p = read_rc6_residency("rc6p");
>> +		res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
>>
>>   	if (mask & RC6PP_ENABLED)
>> -		res->rc6pp = read_rc6_residency("rc6pp");
>> +		res->rc6pp = read_rc6_residency(dirfd,
>> RC6PP_RESIDENCY_MS);
>>
>>   	res->duration += (gettime_ms() - res->duration) / 2;  }
>>
>> -static void measure_residencies(int devid, unsigned int mask,
>> +static void measure_residencies(int devid, int dirfd, unsigned int
>> +mask,
>>   				struct residencies *res)
>>   {
>>   	struct residencies start = { };
>> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
>> unsigned int mask,
>>   	 * measurement, since the valid counter range is different on
>>   	 * different platforms and so fixing it up would be non-trivial.
>>   	 */
>> -	read_residencies(devid, mask, &end);
>> +	read_residencies(devid, dirfd, mask, &end);
>>   	igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>   		  end.duration, end.rc6, end.media_rc6, end.rc6p, end.rc6pp);
>>   	for (retry = 0; retry < 2; retry++) {
>>   		start = end;
>>   		sleep(SLEEP_DURATION);
>> -		read_residencies(devid, mask, &end);
>> +		read_residencies(devid, dirfd, mask, &end);
>>
>>   		igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>   			  end.duration,
>> @@ -196,7 +196,7 @@ static void measure_residencies(int devid, unsigned
>> int mask,
>>   	res->rc6 += res->rc6p;
>>   }
>>
>> -static bool wait_for_rc6(void)
>> +static bool wait_for_rc6(int dirfd)
>>   {
>>   	struct timespec tv = {};
>>   	unsigned long start, now;
>> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
>>   	usleep(160 * 1000);
>>
>>   	/* Then poll for RC6 to start ticking */
>> -	now = read_rc6_residency("rc6");
>> +	now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>   	do {
>>   		start = now;
>>   		usleep(5000);
>> -		now = read_rc6_residency("rc6");
>> +		now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>   		if (now - start > 1)
>>   			return true;
>>   	} while (!igt_seconds_elapsed(&tv));
>> @@ -234,14 +234,27 @@ static uint64_t pmu_read_single(int fd)
>>   	return __pmu_read_single(fd, NULL);
>>   }
>>
>> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
>> -	igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
>> -		     (x) >= (ref) * (1.0 - (tol_down)/100.), \
>> -		     "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
>> %.3g)\n",\
>> -		     #x, #ref, (double)(x), (tol_up), (tol_down), (double)(ref))
>> +#define __assert_within_epsilon(x, ref, tol_up, tol_down, debug_data) \
>> +	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
>> +		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
>> +		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of
>> %f)\n %s\n",\
>> +		     #x, #ref, (double)(x), \
>> +		     (tol_up) * 100.0, (tol_down) * 100.0, \
>> +		     (double)(ref), debug_data)
>> +
>> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
>> +	__assert_within_epsilon(x, ref, tolerance, tolerance, debug_data)
>>
>> -#define assert_within_epsilon(x, ref, tolerance) \
>> -	__assert_within_epsilon(x, ref, tolerance, tolerance)
>> +char *drpc;
>> +
>> +static char *get_drpc(int i915, int gt_id) {
>> +	int gt_dir;
>> +
>> +	gt_dir = igt_debugfs_gt_dir(i915, gt_id);
>> +	igt_assert(gt_dir != -1);
>> +	return igt_sysfs_get(gt_dir, "drpc");
>> +}
>>
>>   static bool __pmu_wait_for_rc6(int fd)
>>   {
>> @@ -376,7 +389,7 @@ static void kill_children(int sig)
>>   	signal(sig, old);
>>   }
>>
>> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
>> +unsigned int gt)
>>   {
>>   	const int64_t duration_ns = SLEEP_DURATION *
>> (int64_t)NSEC_PER_SEC;
>>   	const int tolerance = 20; /* Some RC6 is better than none! */ @@ -
>> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>   	struct igt_power gpu;
>>   	int fd;
>>
>> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>   	igt_drop_caches_set(i915, DROP_IDLE);
>>   	igt_require(__pmu_wait_for_rc6(fd));
>>   	igt_power_open(i915, &gpu, "gpu");
>> @@ -418,7 +431,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>> uint64_t flags)
>>   			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>   			idle, (idle * 1e9) / slept);
>>   	}
>> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>> +
>> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>> +
>> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>
>>   	done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
>> 1, 0);
>>
>> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>> uint64_t flags)
>>   		igt_assert(cycles >= SLEEP_DURATION);
>>
>>   		/* While very nearly idle, expect full RC6 */
>> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>> +
>> +		free(drpc);
>> +		drpc = NULL;
>>   	}
>>
>>   	munmap(done, 4096);
>> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>> uint64_t flags)
>>   	}
>>   }
>>
>> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
>> +static void rc6_fence(int i915, unsigned int gt)
>>   {
>>   	const int64_t duration_ns = SLEEP_DURATION *
>> (int64_t)NSEC_PER_SEC;
>>   	const int tolerance = 20; /* Some RC6 is better than none! */
>>   	const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>>   	const struct intel_execution_engine2 *e;
>> +	const intel_ctx_t *ctx;
>>   	struct power_sample sample[2];
>>   	unsigned long slept;
>>   	uint64_t rc6, ts[2], ahnd;
>> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
>>
>>   	igt_require_sw_sync();
>>
>> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>   	igt_drop_caches_set(i915, DROP_IDLE);
>>   	igt_require(__pmu_wait_for_rc6(fd));
>>   	igt_power_open(i915, &gpu, "gpu");
>> @@ -506,9 +526,12 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
>>   			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>   			idle, (idle * 1e9) / slept);
>>   	}
>> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>> +
>> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>
>>   	/* Submit but delay execution, we should be idle and conserving
>> power */
>> +	ctx = intel_ctx_create_for_gt(i915, gt);
>>   	ahnd = get_reloc_ahnd(i915, ctx->id);
>>   	for_each_ctx_engine(i915, ctx, e) {
>>   		igt_spin_t *spin;
>> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const intel_ctx_t
>> *ctx)
>>
>>   		close(timeline);
>>
>> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>   		gem_quiescent_gpu(i915);
>> +
>> +		free(drpc);
>> +		drpc = NULL;
>>   	}
>>   	put_ahnd(ahnd);
>> +	intel_ctx_destroy(i915,ctx);
>>
>>   	igt_power_close(&gpu);
>>   	close(fd);
>> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t *ctx)
>> igt_main  {
>>   	int i915 = -1;
>> +	unsigned int dirfd, gt;
>>   	const intel_ctx_t *ctx;
>>
>>   	/* Use drm_open_driver to verify device existence */ @@ -572,19
>> +600,25 @@ igt_main
>>   		igt_require_gem(i915);
>>   		gem_quiescent_gpu(i915);
>>
>> -		for_each_ctx_engine(i915, ctx, e) {
>> -			if (e->instance == 0) {
>> -				igt_dynamic_f("%s", e->name)
>> -					rc6_idle(i915, ctx->id, e->flags);
>> +		i915_for_each_gt(i915, dirfd, gt) {
>> +			ctx = intel_ctx_create_for_gt(i915, gt);
>> +			for_each_ctx_engine(i915, ctx, e) {
>> +				if (e->instance == 0) {
>> +					igt_dynamic_f("gt%u-%s", gt, e-
>>> name)
>> +						rc6_idle(i915, ctx->id, e-
>>> flags, gt);
>> +				}
>>   			}
>> +			intel_ctx_destroy(i915, ctx);
>>   		}
>>   	}
>>
>> -	igt_subtest("rc6-fence") {
>> +	igt_subtest_with_dynamic("rc6-fence") {
>>   		igt_require_gem(i915);
>>   		gem_quiescent_gpu(i915);
>>
>> -		rc6_fence(i915, ctx);
>> +		i915_for_each_gt(i915, dirfd, gt)
>> +			igt_dynamic_f("gt%u", gt)
>> +				rc6_fence(i915, gt);
>>   	}
>>
>>   	igt_subtest_group {
>> @@ -595,21 +629,23 @@ igt_main
>>   			devid = intel_get_drm_devid(i915);
>>   			sysfs = igt_sysfs_open(i915);
>>
>> -			igt_require(has_rc6_residency("rc6"));
>> +			igt_require(has_rc6_residency(dirfd,
>> RC6_RESIDENCY_MS));
>>
>>   			/* Make sure rc6 counters are running */
>>   			igt_drop_caches_set(i915, DROP_IDLE);
>> -			igt_require(wait_for_rc6());
>> +			igt_require(wait_for_rc6(dirfd));
>>
>>   			rc6_enabled = get_rc6_enabled_mask();
>>   			igt_require(rc6_enabled & RC6_ENABLED);
>>   		}
>>
>>   		igt_subtest("rc6-accuracy") {
>> -			struct residencies res;
>> +			for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>> +				struct residencies res;
>>
>> -			measure_residencies(devid, rc6_enabled, &res);
>> -			residency_accuracy(res.rc6, res.duration, "rc6");
>> +				measure_residencies(devid, dirfd,
>> rc6_enabled, &res);
>> +				residency_accuracy(res.rc6, res.duration,
>> "rc6");
>> +			}
>>   		}
>>
>>   		igt_subtest("media-rc6-accuracy") {
>> @@ -617,7 +653,7 @@ igt_main
>>
>>   			igt_require(IS_VALLEYVIEW(devid) ||
>> IS_CHERRYVIEW(devid));
>>
>> -			measure_residencies(devid, rc6_enabled, &res);
>> +			measure_residencies(devid, sysfs, rc6_enabled, &res);
>>   			residency_accuracy(res.media_rc6, res.duration,
>> "media_rc6");
>>   		}
>>
>> @@ -626,7 +662,7 @@ igt_main
>>   	}
>>
>>   	igt_fixture {
>> -		intel_ctx_destroy(i915, ctx);
>> +		free(drpc);
>>   		drm_close_driver(i915);
>>   	}
>>   }
>> --
>> 2.25.1

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

* Re: [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  8:54   ` Sundaresan, Sujaritha
@ 2023-11-08  9:04     ` Gupta, Anshuman
  2023-11-08  9:05       ` Sundaresan, Sujaritha
  0 siblings, 1 reply; 8+ messages in thread
From: Gupta, Anshuman @ 2023-11-08  9:04 UTC (permalink / raw)
  To: Sundaresan, Sujaritha, igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
> Sent: Wednesday, November 8, 2023 2:25 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>; igt-
> dev@lists.freedesktop.org
> Subject: Re: [PATCH] tests/intel: Add multi gt support for rc6_residency test
> 
> 
> On 11/8/2023 2:22 PM, Gupta, Anshuman wrote:
> >
> >> -----Original Message-----
> >> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
> >> Sent: Wednesday, November 8, 2023 12:13 PM
> >> To: igt-dev@lists.freedesktop.org
> >> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Sundaresan,
> Sujaritha
> >> <sujaritha.sundaresan@intel.com>
> >> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency
> >> test
> >>
> >> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy tests.
> > Please break this patch like below.
> > 1) If any common  refactoring required, not applicable if no such refactoring.
> > 2) Add multi gt support for rc6_idle.
> > 3) Add multi gt support for rc6_fence.
> > 4) Add multi gt support for rc6_accuracy test.
> > 5) Add drpc debug support.
> Okay I will make this a 4 patch series then.
> >> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> >> ---
> >>   tests/intel/i915_pm_rc6_residency.c | 134 ++++++++++++++++++--------
> --
> >>   1 file changed, 85 insertions(+), 49 deletions(-)
> >>
> >> diff --git a/tests/intel/i915_pm_rc6_residency.c
> >> b/tests/intel/i915_pm_rc6_residency.c
> >> index b266680ac..f865fbe94 100644
> >> --- a/tests/intel/i915_pm_rc6_residency.c
> >> +++ b/tests/intel/i915_pm_rc6_residency.c
> >> @@ -36,6 +36,7 @@
> >>   #include "i915/gem.h"
> >>   #include "i915/gem_create.h"
> >>   #include "igt.h"
> >> +#include "igt_debugfs.h"
> >>   #include "igt_perf.h"
> >>   #include "igt_power.h"
> >>   #include "igt_sysfs.h"
> >> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
> >>   	return enabled;
> >>   }
> >>
> >> -static bool has_rc6_residency(const char *name)
> >> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
> >>   {
> >>   	unsigned long residency;
> >> -	char path[128];
> >>
> >> -	sprintf(path, "power/%s_residency_ms", name);
> >> -	return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
> >> +	return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
> >>   }
> >>
> >> -static unsigned long read_rc6_residency(const char *name)
> >> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
> >> +id)
> >>   {
> >>   	unsigned long residency;
> >> -	char path[128];
> >>
> >>   	residency = 0;
> >> -	sprintf(path, "power/%s_residency_ms", name);
> >> -	igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1);
> >> +	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1);
> >>   	return residency;
> >>   }
> >>
> >> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int diff,
> >>   			       const char *name_of_rc6_residency)  {
> >>   	double ratio;
> >> +	int gt = 0, gt_current;
> >>
> >>   	ratio = (double)diff / duration;
> >>
> >> -	igt_info("Residency in %s or deeper state: %u ms (sleep duration %u
> >> ms) (%.1f%% of expected duration)\n",
> >> -		 name_of_rc6_residency, diff, duration, 100*ratio);
> >> +	gt_current = igt_sysfs_get_u32(gt, "id");
> >> +
> >> +	igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
> >> duration %u ms) (%.1f%% of expected duration)\n",
> >> +		 gt_current, name_of_rc6_residency, diff, duration,
> >> 100*ratio);
> >>   	igt_assert_f(ratio > 0.9 && ratio < 1.05,
> >>   		     "Sysfs RC6 residency counter is inaccurate.\n");  } @@ -
> > Print gt id in igt_assert_f failed message .
> > Thanks,
> > Anshuman Gupta.
> 
> I thought maybe giving out the gt id as part of info would be useful, but sure
> I'll move it to the assert.
IMO it will be noticeable as part of failure message, but if you can think it is more readable
in igt_info message. Please keep it in at igt_info.
Thanks,
Anshuman Gupta.
> 
> Will be better fro debug.
> 
> Thanks for the review.
> 
> Suja
> 
> >> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
> >>   	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;  }
> >>
> >> -static void read_residencies(int devid, unsigned int mask,
> >> +static void read_residencies(int devid, int dirfd, unsigned int
> >> +mask,
> >>   			     struct residencies *res)
> >>   {
> >>   	res->duration = gettime_ms();
> >>
> >>   	if (mask & RC6_ENABLED)
> >> -		res->rc6 = read_rc6_residency("rc6");
> >> +		res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
> >>
> >>   	if ((mask & RC6_ENABLED) &&
> >>   	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
> >> -		res->media_rc6 = read_rc6_residency("media_rc6");
> >> +		res->media_rc6 = read_rc6_residency(dirfd,
> >> MEDIA_RC6_RESIDENCY_MS);
> >>
> >>   	if (mask & RC6P_ENABLED)
> >> -		res->rc6p = read_rc6_residency("rc6p");
> >> +		res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
> >>
> >>   	if (mask & RC6PP_ENABLED)
> >> -		res->rc6pp = read_rc6_residency("rc6pp");
> >> +		res->rc6pp = read_rc6_residency(dirfd,
> >> RC6PP_RESIDENCY_MS);
> >>
> >>   	res->duration += (gettime_ms() - res->duration) / 2;  }
> >>
> >> -static void measure_residencies(int devid, unsigned int mask,
> >> +static void measure_residencies(int devid, int dirfd, unsigned int
> >> +mask,
> >>   				struct residencies *res)
> >>   {
> >>   	struct residencies start = { };
> >> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
> >> unsigned int mask,
> >>   	 * measurement, since the valid counter range is different on
> >>   	 * different platforms and so fixing it up would be non-trivial.
> >>   	 */
> >> -	read_residencies(devid, mask, &end);
> >> +	read_residencies(devid, dirfd, mask, &end);
> >>   	igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
> >>   		  end.duration, end.rc6, end.media_rc6, end.rc6p, end.rc6pp);
> >>   	for (retry = 0; retry < 2; retry++) {
> >>   		start = end;
> >>   		sleep(SLEEP_DURATION);
> >> -		read_residencies(devid, mask, &end);
> >> +		read_residencies(devid, dirfd, mask, &end);
> >>
> >>   		igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
> >>   			  end.duration,
> >> @@ -196,7 +196,7 @@ static void measure_residencies(int devid,
> >> unsigned int mask,
> >>   	res->rc6 += res->rc6p;
> >>   }
> >>
> >> -static bool wait_for_rc6(void)
> >> +static bool wait_for_rc6(int dirfd)
> >>   {
> >>   	struct timespec tv = {};
> >>   	unsigned long start, now;
> >> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
> >>   	usleep(160 * 1000);
> >>
> >>   	/* Then poll for RC6 to start ticking */
> >> -	now = read_rc6_residency("rc6");
> >> +	now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
> >>   	do {
> >>   		start = now;
> >>   		usleep(5000);
> >> -		now = read_rc6_residency("rc6");
> >> +		now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
> >>   		if (now - start > 1)
> >>   			return true;
> >>   	} while (!igt_seconds_elapsed(&tv)); @@ -234,14 +234,27 @@ static
> >> uint64_t pmu_read_single(int fd)
> >>   	return __pmu_read_single(fd, NULL);
> >>   }
> >>
> >> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
> >> -	igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
> >> -		     (x) >= (ref) * (1.0 - (tol_down)/100.), \
> >> -		     "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
> >> %.3g)\n",\
> >> -		     #x, #ref, (double)(x), (tol_up), (tol_down), (double)(ref))
> >> +#define __assert_within_epsilon(x, ref, tol_up, tol_down, debug_data) \
> >> +	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
> >> +		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
> >> +		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of
> >> %f)\n %s\n",\
> >> +		     #x, #ref, (double)(x), \
> >> +		     (tol_up) * 100.0, (tol_down) * 100.0, \
> >> +		     (double)(ref), debug_data)
> >> +
> >> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
> >> +	__assert_within_epsilon(x, ref, tolerance, tolerance, debug_data)
> >>
> >> -#define assert_within_epsilon(x, ref, tolerance) \
> >> -	__assert_within_epsilon(x, ref, tolerance, tolerance)
> >> +char *drpc;
> >> +
> >> +static char *get_drpc(int i915, int gt_id) {
> >> +	int gt_dir;
> >> +
> >> +	gt_dir = igt_debugfs_gt_dir(i915, gt_id);
> >> +	igt_assert(gt_dir != -1);
> >> +	return igt_sysfs_get(gt_dir, "drpc"); }
> >>
> >>   static bool __pmu_wait_for_rc6(int fd)
> >>   {
> >> @@ -376,7 +389,7 @@ static void kill_children(int sig)
> >>   	signal(sig, old);
> >>   }
> >>
> >> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
> >> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
> >> +unsigned int gt)
> >>   {
> >>   	const int64_t duration_ns = SLEEP_DURATION *
> >> (int64_t)NSEC_PER_SEC;
> >>   	const int tolerance = 20; /* Some RC6 is better than none! */ @@ -
> >> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t
> flags)
> >>   	struct igt_power gpu;
> >>   	int fd;
> >>
> >> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
> >> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
> >>   	igt_drop_caches_set(i915, DROP_IDLE);
> >>   	igt_require(__pmu_wait_for_rc6(fd));
> >>   	igt_power_open(i915, &gpu, "gpu"); @@ -418,7 +431,10 @@ static
> >> void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
> >>   			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
> >>   			idle, (idle * 1e9) / slept);
> >>   	}
> >> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
> >> +
> >> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
> >> +
> >> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
> >>
> >>   	done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
> 1, 0);
> >>
> >> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
> >> uint64_t flags)
> >>   		igt_assert(cycles >= SLEEP_DURATION);
> >>
> >>   		/* While very nearly idle, expect full RC6 */
> >> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
> >> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
> >> +
> >> +		free(drpc);
> >> +		drpc = NULL;
> >>   	}
> >>
> >>   	munmap(done, 4096);
> >> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
> >> uint64_t flags)
> >>   	}
> >>   }
> >>
> >> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
> >> +static void rc6_fence(int i915, unsigned int gt)
> >>   {
> >>   	const int64_t duration_ns = SLEEP_DURATION *
> >> (int64_t)NSEC_PER_SEC;
> >>   	const int tolerance = 20; /* Some RC6 is better than none! */
> >>   	const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
> >>   	const struct intel_execution_engine2 *e;
> >> +	const intel_ctx_t *ctx;
> >>   	struct power_sample sample[2];
> >>   	unsigned long slept;
> >>   	uint64_t rc6, ts[2], ahnd;
> >> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t
> >> *ctx)
> >>
> >>   	igt_require_sw_sync();
> >>
> >> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
> >> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
> >>   	igt_drop_caches_set(i915, DROP_IDLE);
> >>   	igt_require(__pmu_wait_for_rc6(fd));
> >>   	igt_power_open(i915, &gpu, "gpu"); @@ -506,9 +526,12 @@ static
> >> void rc6_fence(int i915, const intel_ctx_t *ctx)
> >>   			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
> >>   			idle, (idle * 1e9) / slept);
> >>   	}
> >> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
> >> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
> >> +
> >> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
> >>
> >>   	/* Submit but delay execution, we should be idle and conserving
> >> power */
> >> +	ctx = intel_ctx_create_for_gt(i915, gt);
> >>   	ahnd = get_reloc_ahnd(i915, ctx->id);
> >>   	for_each_ctx_engine(i915, ctx, e) {
> >>   		igt_spin_t *spin;
> >> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const
> >> intel_ctx_t
> >> *ctx)
> >>
> >>   		close(timeline);
> >>
> >> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
> >> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
> >>   		gem_quiescent_gpu(i915);
> >> +
> >> +		free(drpc);
> >> +		drpc = NULL;
> >>   	}
> >>   	put_ahnd(ahnd);
> >> +	intel_ctx_destroy(i915,ctx);
> >>
> >>   	igt_power_close(&gpu);
> >>   	close(fd);
> >> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t
> >> *ctx) igt_main  {
> >>   	int i915 = -1;
> >> +	unsigned int dirfd, gt;
> >>   	const intel_ctx_t *ctx;
> >>
> >>   	/* Use drm_open_driver to verify device existence */ @@ -572,19
> >> +600,25 @@ igt_main
> >>   		igt_require_gem(i915);
> >>   		gem_quiescent_gpu(i915);
> >>
> >> -		for_each_ctx_engine(i915, ctx, e) {
> >> -			if (e->instance == 0) {
> >> -				igt_dynamic_f("%s", e->name)
> >> -					rc6_idle(i915, ctx->id, e->flags);
> >> +		i915_for_each_gt(i915, dirfd, gt) {
> >> +			ctx = intel_ctx_create_for_gt(i915, gt);
> >> +			for_each_ctx_engine(i915, ctx, e) {
> >> +				if (e->instance == 0) {
> >> +					igt_dynamic_f("gt%u-%s", gt, e-
> >>> name)
> >> +						rc6_idle(i915, ctx->id, e-
> >>> flags, gt);
> >> +				}
> >>   			}
> >> +			intel_ctx_destroy(i915, ctx);
> >>   		}
> >>   	}
> >>
> >> -	igt_subtest("rc6-fence") {
> >> +	igt_subtest_with_dynamic("rc6-fence") {
> >>   		igt_require_gem(i915);
> >>   		gem_quiescent_gpu(i915);
> >>
> >> -		rc6_fence(i915, ctx);
> >> +		i915_for_each_gt(i915, dirfd, gt)
> >> +			igt_dynamic_f("gt%u", gt)
> >> +				rc6_fence(i915, gt);
> >>   	}
> >>
> >>   	igt_subtest_group {
> >> @@ -595,21 +629,23 @@ igt_main
> >>   			devid = intel_get_drm_devid(i915);
> >>   			sysfs = igt_sysfs_open(i915);
> >>
> >> -			igt_require(has_rc6_residency("rc6"));
> >> +			igt_require(has_rc6_residency(dirfd,
> >> RC6_RESIDENCY_MS));
> >>
> >>   			/* Make sure rc6 counters are running */
> >>   			igt_drop_caches_set(i915, DROP_IDLE);
> >> -			igt_require(wait_for_rc6());
> >> +			igt_require(wait_for_rc6(dirfd));
> >>
> >>   			rc6_enabled = get_rc6_enabled_mask();
> >>   			igt_require(rc6_enabled & RC6_ENABLED);
> >>   		}
> >>
> >>   		igt_subtest("rc6-accuracy") {
> >> -			struct residencies res;
> >> +			for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
> >> +				struct residencies res;
> >>
> >> -			measure_residencies(devid, rc6_enabled, &res);
> >> -			residency_accuracy(res.rc6, res.duration, "rc6");
> >> +				measure_residencies(devid, dirfd,
> >> rc6_enabled, &res);
> >> +				residency_accuracy(res.rc6, res.duration,
> >> "rc6");
> >> +			}
> >>   		}
> >>
> >>   		igt_subtest("media-rc6-accuracy") { @@ -617,7 +653,7 @@
> igt_main
> >>
> >>   			igt_require(IS_VALLEYVIEW(devid) ||
> IS_CHERRYVIEW(devid));
> >>
> >> -			measure_residencies(devid, rc6_enabled, &res);
> >> +			measure_residencies(devid, sysfs, rc6_enabled, &res);
> >>   			residency_accuracy(res.media_rc6, res.duration,
> "media_rc6");
> >>   		}
> >>
> >> @@ -626,7 +662,7 @@ igt_main
> >>   	}
> >>
> >>   	igt_fixture {
> >> -		intel_ctx_destroy(i915, ctx);
> >> +		free(drpc);
> >>   		drm_close_driver(i915);
> >>   	}
> >>   }
> >> --
> >> 2.25.1

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

* Re: [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  9:04     ` Gupta, Anshuman
@ 2023-11-08  9:05       ` Sundaresan, Sujaritha
  2023-11-09  5:04         ` Sundaresan, Sujaritha
  0 siblings, 1 reply; 8+ messages in thread
From: Sundaresan, Sujaritha @ 2023-11-08  9:05 UTC (permalink / raw)
  To: Gupta, Anshuman, igt-dev@lists.freedesktop.org


On 11/8/2023 2:34 PM, Gupta, Anshuman wrote:
>
>> -----Original Message-----
>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>> Sent: Wednesday, November 8, 2023 2:25 PM
>> To: Gupta, Anshuman <anshuman.gupta@intel.com>; igt-
>> dev@lists.freedesktop.org
>> Subject: Re: [PATCH] tests/intel: Add multi gt support for rc6_residency test
>>
>>
>> On 11/8/2023 2:22 PM, Gupta, Anshuman wrote:
>>>> -----Original Message-----
>>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>>>> Sent: Wednesday, November 8, 2023 12:13 PM
>>>> To: igt-dev@lists.freedesktop.org
>>>> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Sundaresan,
>> Sujaritha
>>>> <sujaritha.sundaresan@intel.com>
>>>> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency
>>>> test
>>>>
>>>> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy tests.
>>> Please break this patch like below.
>>> 1) If any common  refactoring required, not applicable if no such refactoring.
>>> 2) Add multi gt support for rc6_idle.
>>> 3) Add multi gt support for rc6_fence.
>>> 4) Add multi gt support for rc6_accuracy test.
>>> 5) Add drpc debug support.
>> Okay I will make this a 4 patch series then.
>>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>>> ---
>>>>    tests/intel/i915_pm_rc6_residency.c | 134 ++++++++++++++++++--------
>> --
>>>>    1 file changed, 85 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/tests/intel/i915_pm_rc6_residency.c
>>>> b/tests/intel/i915_pm_rc6_residency.c
>>>> index b266680ac..f865fbe94 100644
>>>> --- a/tests/intel/i915_pm_rc6_residency.c
>>>> +++ b/tests/intel/i915_pm_rc6_residency.c
>>>> @@ -36,6 +36,7 @@
>>>>    #include "i915/gem.h"
>>>>    #include "i915/gem_create.h"
>>>>    #include "igt.h"
>>>> +#include "igt_debugfs.h"
>>>>    #include "igt_perf.h"
>>>>    #include "igt_power.h"
>>>>    #include "igt_sysfs.h"
>>>> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
>>>>    	return enabled;
>>>>    }
>>>>
>>>> -static bool has_rc6_residency(const char *name)
>>>> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
>>>>    {
>>>>    	unsigned long residency;
>>>> -	char path[128];
>>>>
>>>> -	sprintf(path, "power/%s_residency_ms", name);
>>>> -	return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
>>>> +	return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
>>>>    }
>>>>
>>>> -static unsigned long read_rc6_residency(const char *name)
>>>> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
>>>> +id)
>>>>    {
>>>>    	unsigned long residency;
>>>> -	char path[128];
>>>>
>>>>    	residency = 0;
>>>> -	sprintf(path, "power/%s_residency_ms", name);
>>>> -	igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1);
>>>> +	igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1);
>>>>    	return residency;
>>>>    }
>>>>
>>>> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int diff,
>>>>    			       const char *name_of_rc6_residency)  {
>>>>    	double ratio;
>>>> +	int gt = 0, gt_current;
>>>>
>>>>    	ratio = (double)diff / duration;
>>>>
>>>> -	igt_info("Residency in %s or deeper state: %u ms (sleep duration %u
>>>> ms) (%.1f%% of expected duration)\n",
>>>> -		 name_of_rc6_residency, diff, duration, 100*ratio);
>>>> +	gt_current = igt_sysfs_get_u32(gt, "id");
>>>> +
>>>> +	igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
>>>> duration %u ms) (%.1f%% of expected duration)\n",
>>>> +		 gt_current, name_of_rc6_residency, diff, duration,
>>>> 100*ratio);
>>>>    	igt_assert_f(ratio > 0.9 && ratio < 1.05,
>>>>    		     "Sysfs RC6 residency counter is inaccurate.\n");  } @@ -
>>> Print gt id in igt_assert_f failed message .
>>> Thanks,
>>> Anshuman Gupta.
>> I thought maybe giving out the gt id as part of info would be useful, but sure
>> I'll move it to the assert.
> IMO it will be noticeable as part of failure message, but if you can think it is more readable
> in igt_info message. Please keep it in at igt_info.
> Thanks,
> Anshuman Gupta.

Sure I will take a call as I am splitting out the series.

Thanks,

Suja

>> Will be better fro debug.
>>
>> Thanks for the review.
>>
>> Suja
>>
>>>> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
>>>>    	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;  }
>>>>
>>>> -static void read_residencies(int devid, unsigned int mask,
>>>> +static void read_residencies(int devid, int dirfd, unsigned int
>>>> +mask,
>>>>    			     struct residencies *res)
>>>>    {
>>>>    	res->duration = gettime_ms();
>>>>
>>>>    	if (mask & RC6_ENABLED)
>>>> -		res->rc6 = read_rc6_residency("rc6");
>>>> +		res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>
>>>>    	if ((mask & RC6_ENABLED) &&
>>>>    	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
>>>> -		res->media_rc6 = read_rc6_residency("media_rc6");
>>>> +		res->media_rc6 = read_rc6_residency(dirfd,
>>>> MEDIA_RC6_RESIDENCY_MS);
>>>>
>>>>    	if (mask & RC6P_ENABLED)
>>>> -		res->rc6p = read_rc6_residency("rc6p");
>>>> +		res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
>>>>
>>>>    	if (mask & RC6PP_ENABLED)
>>>> -		res->rc6pp = read_rc6_residency("rc6pp");
>>>> +		res->rc6pp = read_rc6_residency(dirfd,
>>>> RC6PP_RESIDENCY_MS);
>>>>
>>>>    	res->duration += (gettime_ms() - res->duration) / 2;  }
>>>>
>>>> -static void measure_residencies(int devid, unsigned int mask,
>>>> +static void measure_residencies(int devid, int dirfd, unsigned int
>>>> +mask,
>>>>    				struct residencies *res)
>>>>    {
>>>>    	struct residencies start = { };
>>>> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
>>>> unsigned int mask,
>>>>    	 * measurement, since the valid counter range is different on
>>>>    	 * different platforms and so fixing it up would be non-trivial.
>>>>    	 */
>>>> -	read_residencies(devid, mask, &end);
>>>> +	read_residencies(devid, dirfd, mask, &end);
>>>>    	igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>    		  end.duration, end.rc6, end.media_rc6, end.rc6p, end.rc6pp);
>>>>    	for (retry = 0; retry < 2; retry++) {
>>>>    		start = end;
>>>>    		sleep(SLEEP_DURATION);
>>>> -		read_residencies(devid, mask, &end);
>>>> +		read_residencies(devid, dirfd, mask, &end);
>>>>
>>>>    		igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>    			  end.duration,
>>>> @@ -196,7 +196,7 @@ static void measure_residencies(int devid,
>>>> unsigned int mask,
>>>>    	res->rc6 += res->rc6p;
>>>>    }
>>>>
>>>> -static bool wait_for_rc6(void)
>>>> +static bool wait_for_rc6(int dirfd)
>>>>    {
>>>>    	struct timespec tv = {};
>>>>    	unsigned long start, now;
>>>> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
>>>>    	usleep(160 * 1000);
>>>>
>>>>    	/* Then poll for RC6 to start ticking */
>>>> -	now = read_rc6_residency("rc6");
>>>> +	now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>    	do {
>>>>    		start = now;
>>>>    		usleep(5000);
>>>> -		now = read_rc6_residency("rc6");
>>>> +		now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>    		if (now - start > 1)
>>>>    			return true;
>>>>    	} while (!igt_seconds_elapsed(&tv)); @@ -234,14 +234,27 @@ static
>>>> uint64_t pmu_read_single(int fd)
>>>>    	return __pmu_read_single(fd, NULL);
>>>>    }
>>>>
>>>> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
>>>> -	igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
>>>> -		     (x) >= (ref) * (1.0 - (tol_down)/100.), \
>>>> -		     "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
>>>> %.3g)\n",\
>>>> -		     #x, #ref, (double)(x), (tol_up), (tol_down), (double)(ref))
>>>> +#define __assert_within_epsilon(x, ref, tol_up, tol_down, debug_data) \
>>>> +	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
>>>> +		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
>>>> +		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of
>>>> %f)\n %s\n",\
>>>> +		     #x, #ref, (double)(x), \
>>>> +		     (tol_up) * 100.0, (tol_down) * 100.0, \
>>>> +		     (double)(ref), debug_data)
>>>> +
>>>> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
>>>> +	__assert_within_epsilon(x, ref, tolerance, tolerance, debug_data)
>>>>
>>>> -#define assert_within_epsilon(x, ref, tolerance) \
>>>> -	__assert_within_epsilon(x, ref, tolerance, tolerance)
>>>> +char *drpc;
>>>> +
>>>> +static char *get_drpc(int i915, int gt_id) {
>>>> +	int gt_dir;
>>>> +
>>>> +	gt_dir = igt_debugfs_gt_dir(i915, gt_id);
>>>> +	igt_assert(gt_dir != -1);
>>>> +	return igt_sysfs_get(gt_dir, "drpc"); }
>>>>
>>>>    static bool __pmu_wait_for_rc6(int fd)
>>>>    {
>>>> @@ -376,7 +389,7 @@ static void kill_children(int sig)
>>>>    	signal(sig, old);
>>>>    }
>>>>
>>>> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
>>>> +unsigned int gt)
>>>>    {
>>>>    	const int64_t duration_ns = SLEEP_DURATION *
>>>> (int64_t)NSEC_PER_SEC;
>>>>    	const int tolerance = 20; /* Some RC6 is better than none! */ @@ -
>>>> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, uint64_t
>> flags)
>>>>    	struct igt_power gpu;
>>>>    	int fd;
>>>>
>>>> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>    	igt_drop_caches_set(i915, DROP_IDLE);
>>>>    	igt_require(__pmu_wait_for_rc6(fd));
>>>>    	igt_power_open(i915, &gpu, "gpu"); @@ -418,7 +431,10 @@ static
>>>> void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>>    			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>    			idle, (idle * 1e9) / slept);
>>>>    	}
>>>> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>> +
>>>> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>> +
>>>> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>
>>>>    	done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
>> 1, 0);
>>>> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>> uint64_t flags)
>>>>    		igt_assert(cycles >= SLEEP_DURATION);
>>>>
>>>>    		/* While very nearly idle, expect full RC6 */
>>>> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>> +
>>>> +		free(drpc);
>>>> +		drpc = NULL;
>>>>    	}
>>>>
>>>>    	munmap(done, 4096);
>>>> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>> uint64_t flags)
>>>>    	}
>>>>    }
>>>>
>>>> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>> +static void rc6_fence(int i915, unsigned int gt)
>>>>    {
>>>>    	const int64_t duration_ns = SLEEP_DURATION *
>>>> (int64_t)NSEC_PER_SEC;
>>>>    	const int tolerance = 20; /* Some RC6 is better than none! */
>>>>    	const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>>>>    	const struct intel_execution_engine2 *e;
>>>> +	const intel_ctx_t *ctx;
>>>>    	struct power_sample sample[2];
>>>>    	unsigned long slept;
>>>>    	uint64_t rc6, ts[2], ahnd;
>>>> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>> *ctx)
>>>>
>>>>    	igt_require_sw_sync();
>>>>
>>>> -	fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>> +	fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>    	igt_drop_caches_set(i915, DROP_IDLE);
>>>>    	igt_require(__pmu_wait_for_rc6(fd));
>>>>    	igt_power_open(i915, &gpu, "gpu"); @@ -506,9 +526,12 @@ static
>>>> void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>>    			"Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>    			idle, (idle * 1e9) / slept);
>>>>    	}
>>>> -	assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>> +	drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>> +
>>>> +	assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>
>>>>    	/* Submit but delay execution, we should be idle and conserving
>>>> power */
>>>> +	ctx = intel_ctx_create_for_gt(i915, gt);
>>>>    	ahnd = get_reloc_ahnd(i915, ctx->id);
>>>>    	for_each_ctx_engine(i915, ctx, e) {
>>>>    		igt_spin_t *spin;
>>>> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const
>>>> intel_ctx_t
>>>> *ctx)
>>>>
>>>>    		close(timeline);
>>>>
>>>> -		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>> +		assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>>    		gem_quiescent_gpu(i915);
>>>> +
>>>> +		free(drpc);
>>>> +		drpc = NULL;
>>>>    	}
>>>>    	put_ahnd(ahnd);
>>>> +	intel_ctx_destroy(i915,ctx);
>>>>
>>>>    	igt_power_close(&gpu);
>>>>    	close(fd);
>>>> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>> *ctx) igt_main  {
>>>>    	int i915 = -1;
>>>> +	unsigned int dirfd, gt;
>>>>    	const intel_ctx_t *ctx;
>>>>
>>>>    	/* Use drm_open_driver to verify device existence */ @@ -572,19
>>>> +600,25 @@ igt_main
>>>>    		igt_require_gem(i915);
>>>>    		gem_quiescent_gpu(i915);
>>>>
>>>> -		for_each_ctx_engine(i915, ctx, e) {
>>>> -			if (e->instance == 0) {
>>>> -				igt_dynamic_f("%s", e->name)
>>>> -					rc6_idle(i915, ctx->id, e->flags);
>>>> +		i915_for_each_gt(i915, dirfd, gt) {
>>>> +			ctx = intel_ctx_create_for_gt(i915, gt);
>>>> +			for_each_ctx_engine(i915, ctx, e) {
>>>> +				if (e->instance == 0) {
>>>> +					igt_dynamic_f("gt%u-%s", gt, e-
>>>>> name)
>>>> +						rc6_idle(i915, ctx->id, e-
>>>>> flags, gt);
>>>> +				}
>>>>    			}
>>>> +			intel_ctx_destroy(i915, ctx);
>>>>    		}
>>>>    	}
>>>>
>>>> -	igt_subtest("rc6-fence") {
>>>> +	igt_subtest_with_dynamic("rc6-fence") {
>>>>    		igt_require_gem(i915);
>>>>    		gem_quiescent_gpu(i915);
>>>>
>>>> -		rc6_fence(i915, ctx);
>>>> +		i915_for_each_gt(i915, dirfd, gt)
>>>> +			igt_dynamic_f("gt%u", gt)
>>>> +				rc6_fence(i915, gt);
>>>>    	}
>>>>
>>>>    	igt_subtest_group {
>>>> @@ -595,21 +629,23 @@ igt_main
>>>>    			devid = intel_get_drm_devid(i915);
>>>>    			sysfs = igt_sysfs_open(i915);
>>>>
>>>> -			igt_require(has_rc6_residency("rc6"));
>>>> +			igt_require(has_rc6_residency(dirfd,
>>>> RC6_RESIDENCY_MS));
>>>>
>>>>    			/* Make sure rc6 counters are running */
>>>>    			igt_drop_caches_set(i915, DROP_IDLE);
>>>> -			igt_require(wait_for_rc6());
>>>> +			igt_require(wait_for_rc6(dirfd));
>>>>
>>>>    			rc6_enabled = get_rc6_enabled_mask();
>>>>    			igt_require(rc6_enabled & RC6_ENABLED);
>>>>    		}
>>>>
>>>>    		igt_subtest("rc6-accuracy") {
>>>> -			struct residencies res;
>>>> +			for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>>>> +				struct residencies res;
>>>>
>>>> -			measure_residencies(devid, rc6_enabled, &res);
>>>> -			residency_accuracy(res.rc6, res.duration, "rc6");
>>>> +				measure_residencies(devid, dirfd,
>>>> rc6_enabled, &res);
>>>> +				residency_accuracy(res.rc6, res.duration,
>>>> "rc6");
>>>> +			}
>>>>    		}
>>>>
>>>>    		igt_subtest("media-rc6-accuracy") { @@ -617,7 +653,7 @@
>> igt_main
>>>>    			igt_require(IS_VALLEYVIEW(devid) ||
>> IS_CHERRYVIEW(devid));
>>>> -			measure_residencies(devid, rc6_enabled, &res);
>>>> +			measure_residencies(devid, sysfs, rc6_enabled, &res);
>>>>    			residency_accuracy(res.media_rc6, res.duration,
>> "media_rc6");
>>>>    		}
>>>>
>>>> @@ -626,7 +662,7 @@ igt_main
>>>>    	}
>>>>
>>>>    	igt_fixture {
>>>> -		intel_ctx_destroy(i915, ctx);
>>>> +		free(drpc);
>>>>    		drm_close_driver(i915);
>>>>    	}
>>>>    }
>>>> --
>>>> 2.25.1

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

* Re: [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test
  2023-11-08  9:05       ` Sundaresan, Sujaritha
@ 2023-11-09  5:04         ` Sundaresan, Sujaritha
  0 siblings, 0 replies; 8+ messages in thread
From: Sundaresan, Sujaritha @ 2023-11-09  5:04 UTC (permalink / raw)
  To: Gupta, Anshuman, igt-dev@lists.freedesktop.org


On 11/8/2023 2:35 PM, Sundaresan, Sujaritha wrote:
>
> On 11/8/2023 2:34 PM, Gupta, Anshuman wrote:
>>
>>> -----Original Message-----
>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>>> Sent: Wednesday, November 8, 2023 2:25 PM
>>> To: Gupta, Anshuman <anshuman.gupta@intel.com>; igt-
>>> dev@lists.freedesktop.org
>>> Subject: Re: [PATCH] tests/intel: Add multi gt support for 
>>> rc6_residency test
>>>
>>>
>>> On 11/8/2023 2:22 PM, Gupta, Anshuman wrote:
>>>>> -----Original Message-----
>>>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>>>>> Sent: Wednesday, November 8, 2023 12:13 PM
>>>>> To: igt-dev@lists.freedesktop.org
>>>>> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Sundaresan,
>>> Sujaritha
>>>>> <sujaritha.sundaresan@intel.com>
>>>>> Subject: [PATCH] tests/intel: Add multi gt support for rc6_residency
>>>>> test
>>>>>
>>>>> Adding multi-gt support for rc6_idle, rc6_fence and rc6_accuracy 
>>>>> tests.
>>>> Please break this patch like below.
>>>> 1) If any common  refactoring required, not applicable if no such 
>>>> refactoring.
>>>> 2) Add multi gt support for rc6_idle.
>>>> 3) Add multi gt support for rc6_fence.
>>>> 4) Add multi gt support for rc6_accuracy test.
>>>> 5) Add drpc debug support.
>>> Okay I will make this a 4 patch series then.
>>>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>>>> ---
>>>>>    tests/intel/i915_pm_rc6_residency.c | 134 
>>>>> ++++++++++++++++++--------
>>> -- 
>>>>>    1 file changed, 85 insertions(+), 49 deletions(-)
>>>>>
>>>>> diff --git a/tests/intel/i915_pm_rc6_residency.c
>>>>> b/tests/intel/i915_pm_rc6_residency.c
>>>>> index b266680ac..f865fbe94 100644
>>>>> --- a/tests/intel/i915_pm_rc6_residency.c
>>>>> +++ b/tests/intel/i915_pm_rc6_residency.c
>>>>> @@ -36,6 +36,7 @@
>>>>>    #include "i915/gem.h"
>>>>>    #include "i915/gem_create.h"
>>>>>    #include "igt.h"
>>>>> +#include "igt_debugfs.h"
>>>>>    #include "igt_perf.h"
>>>>>    #include "igt_power.h"
>>>>>    #include "igt_sysfs.h"
>>>>> @@ -82,23 +83,19 @@ static unsigned long get_rc6_enabled_mask(void)
>>>>>        return enabled;
>>>>>    }
>>>>>
>>>>> -static bool has_rc6_residency(const char *name)
>>>>> +static bool has_rc6_residency(int dirfd, enum i915_attr_id id)
>>>>>    {
>>>>>        unsigned long residency;
>>>>> -    char path[128];
>>>>>
>>>>> -    sprintf(path, "power/%s_residency_ms", name);
>>>>> -    return igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 1;
>>>>> +    return igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) == 1;
>>>>>    }
>>>>>
>>>>> -static unsigned long read_rc6_residency(const char *name)
>>>>> +static unsigned long read_rc6_residency(int dirfd, enum i915_attr_id
>>>>> +id)
>>>>>    {
>>>>>        unsigned long residency;
>>>>> -    char path[128];
>>>>>
>>>>>        residency = 0;
>>>>> -    sprintf(path, "power/%s_residency_ms", name);
>>>>> -    igt_assert(igt_sysfs_scanf(sysfs, path, "%lu", &residency) == 
>>>>> 1);
>>>>> +    igt_assert(igt_sysfs_rps_scanf(dirfd, id, "%lu", &residency) 
>>>>> == 1);
>>>>>        return residency;
>>>>>    }
>>>>>
>>>>> @@ -107,11 +104,14 @@ static void residency_accuracy(unsigned int 
>>>>> diff,
>>>>>                       const char *name_of_rc6_residency) {
>>>>>        double ratio;
>>>>> +    int gt = 0, gt_current;
>>>>>
>>>>>        ratio = (double)diff / duration;
>>>>>
>>>>> -    igt_info("Residency in %s or deeper state: %u ms (sleep 
>>>>> duration %u
>>>>> ms) (%.1f%% of expected duration)\n",
>>>>> -         name_of_rc6_residency, diff, duration, 100*ratio);
>>>>> +    gt_current = igt_sysfs_get_u32(gt, "id");
>>>>> +
>>>>> +    igt_info("GT[%d]: Residency in %s or deeper state: %u ms (sleep
>>>>> duration %u ms) (%.1f%% of expected duration)\n",
>>>>> +         gt_current, name_of_rc6_residency, diff, duration,
>>>>> 100*ratio);
>>>>>        igt_assert_f(ratio > 0.9 && ratio < 1.05,
>>>>>                 "Sysfs RC6 residency counter is inaccurate.\n");  
>>>>> } @@ -
>>>> Print gt id in igt_assert_f failed message .
>>>> Thanks,
>>>> Anshuman Gupta.
>>> I thought maybe giving out the gt id as part of info would be 
>>> useful, but sure
>>> I'll move it to the assert.
>> IMO it will be noticeable as part of failure message, but if you can 
>> think it is more readable
>> in igt_info message. Please keep it in at igt_info.
>> Thanks,
>> Anshuman Gupta.
>
> Sure I will take a call as I am splitting out the series.
>
> Thanks,
>
> Suja

Hi Anshuman,

The gt id looks more cleaner and more visible in the gt info message. So 
I have left it there for the the next series.

Thanks,

Suja

>
>>> Will be better fro debug.
>>>
>>> Thanks for the review.
>>>
>>> Suja
>>>
>>>>> 125,28 +125,28 @@ static unsigned long gettime_ms(void)
>>>>>        return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;  }
>>>>>
>>>>> -static void read_residencies(int devid, unsigned int mask,
>>>>> +static void read_residencies(int devid, int dirfd, unsigned int
>>>>> +mask,
>>>>>                     struct residencies *res)
>>>>>    {
>>>>>        res->duration = gettime_ms();
>>>>>
>>>>>        if (mask & RC6_ENABLED)
>>>>> -        res->rc6 = read_rc6_residency("rc6");
>>>>> +        res->rc6 = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>>
>>>>>        if ((mask & RC6_ENABLED) &&
>>>>>            (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
>>>>> -        res->media_rc6 = read_rc6_residency("media_rc6");
>>>>> +        res->media_rc6 = read_rc6_residency(dirfd,
>>>>> MEDIA_RC6_RESIDENCY_MS);
>>>>>
>>>>>        if (mask & RC6P_ENABLED)
>>>>> -        res->rc6p = read_rc6_residency("rc6p");
>>>>> +        res->rc6p = read_rc6_residency(dirfd, RC6P_RESIDENCY_MS);
>>>>>
>>>>>        if (mask & RC6PP_ENABLED)
>>>>> -        res->rc6pp = read_rc6_residency("rc6pp");
>>>>> +        res->rc6pp = read_rc6_residency(dirfd,
>>>>> RC6PP_RESIDENCY_MS);
>>>>>
>>>>>        res->duration += (gettime_ms() - res->duration) / 2;  }
>>>>>
>>>>> -static void measure_residencies(int devid, unsigned int mask,
>>>>> +static void measure_residencies(int devid, int dirfd, unsigned int
>>>>> +mask,
>>>>>                    struct residencies *res)
>>>>>    {
>>>>>        struct residencies start = { };
>>>>> @@ -158,13 +158,13 @@ static void measure_residencies(int devid,
>>>>> unsigned int mask,
>>>>>         * measurement, since the valid counter range is different on
>>>>>         * different platforms and so fixing it up would be 
>>>>> non-trivial.
>>>>>         */
>>>>> -    read_residencies(devid, mask, &end);
>>>>> +    read_residencies(devid, dirfd, mask, &end);
>>>>>        igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>>              end.duration, end.rc6, end.media_rc6, end.rc6p, 
>>>>> end.rc6pp);
>>>>>        for (retry = 0; retry < 2; retry++) {
>>>>>            start = end;
>>>>>            sleep(SLEEP_DURATION);
>>>>> -        read_residencies(devid, mask, &end);
>>>>> +        read_residencies(devid, dirfd, mask, &end);
>>>>>
>>>>>            igt_debug("time=%d: rc6=(%d, %d), rc6p=%d, rc6pp=%d\n",
>>>>>                  end.duration,
>>>>> @@ -196,7 +196,7 @@ static void measure_residencies(int devid,
>>>>> unsigned int mask,
>>>>>        res->rc6 += res->rc6p;
>>>>>    }
>>>>>
>>>>> -static bool wait_for_rc6(void)
>>>>> +static bool wait_for_rc6(int dirfd)
>>>>>    {
>>>>>        struct timespec tv = {};
>>>>>        unsigned long start, now;
>>>>> @@ -205,11 +205,11 @@ static bool wait_for_rc6(void)
>>>>>        usleep(160 * 1000);
>>>>>
>>>>>        /* Then poll for RC6 to start ticking */
>>>>> -    now = read_rc6_residency("rc6");
>>>>> +    now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>>        do {
>>>>>            start = now;
>>>>>            usleep(5000);
>>>>> -        now = read_rc6_residency("rc6");
>>>>> +        now = read_rc6_residency(dirfd, RC6_RESIDENCY_MS);
>>>>>            if (now - start > 1)
>>>>>                return true;
>>>>>        } while (!igt_seconds_elapsed(&tv)); @@ -234,14 +234,27 @@ 
>>>>> static
>>>>> uint64_t pmu_read_single(int fd)
>>>>>        return __pmu_read_single(fd, NULL);
>>>>>    }
>>>>>
>>>>> -#define __assert_within_epsilon(x, ref, tol_up, tol_down) \
>>>>> -    igt_assert_f((x) <= (ref) * (1.0 + (tol_up)/100.) && \
>>>>> -             (x) >= (ref) * (1.0 - (tol_down)/100.), \
>>>>> -             "'%s' != '%s' (%.3g not within +%d%%/-%d%% tolerance of
>>>>> %.3g)\n",\
>>>>> -             #x, #ref, (double)(x), (tol_up), (tol_down), 
>>>>> (double)(ref))
>>>>> +#define __assert_within_epsilon(x, ref, tol_up, tol_down, 
>>>>> debug_data) \
>>>>> +    igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) 
>>>>> && \
>>>>> +             (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
>>>>> +             "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% 
>>>>> tolerance of
>>>>> %f)\n %s\n",\
>>>>> +             #x, #ref, (double)(x), \
>>>>> +             (tol_up) * 100.0, (tol_down) * 100.0, \
>>>>> +             (double)(ref), debug_data)
>>>>> +
>>>>> +#define assert_within_epsilon(x, ref, tolerance, debug_data) \
>>>>> +    __assert_within_epsilon(x, ref, tolerance, tolerance, 
>>>>> debug_data)
>>>>>
>>>>> -#define assert_within_epsilon(x, ref, tolerance) \
>>>>> -    __assert_within_epsilon(x, ref, tolerance, tolerance)
>>>>> +char *drpc;
>>>>> +
>>>>> +static char *get_drpc(int i915, int gt_id) {
>>>>> +    int gt_dir;
>>>>> +
>>>>> +    gt_dir = igt_debugfs_gt_dir(i915, gt_id);
>>>>> +    igt_assert(gt_dir != -1);
>>>>> +    return igt_sysfs_get(gt_dir, "drpc"); }
>>>>>
>>>>>    static bool __pmu_wait_for_rc6(int fd)
>>>>>    {
>>>>> @@ -376,7 +389,7 @@ static void kill_children(int sig)
>>>>>        signal(sig, old);
>>>>>    }
>>>>>
>>>>> -static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>>> +static void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags,
>>>>> +unsigned int gt)
>>>>>    {
>>>>>        const int64_t duration_ns = SLEEP_DURATION *
>>>>> (int64_t)NSEC_PER_SEC;
>>>>>        const int tolerance = 20; /* Some RC6 is better than none! 
>>>>> */ @@ -
>>>>> 397,7 +410,7 @@ static void rc6_idle(int i915, uint32_t ctx_id, 
>>>>> uint64_t
>>> flags)
>>>>>        struct igt_power gpu;
>>>>>        int fd;
>>>>>
>>>>> -    fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>>> +    fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>>        igt_drop_caches_set(i915, DROP_IDLE);
>>>>>        igt_require(__pmu_wait_for_rc6(fd));
>>>>>        igt_power_open(i915, &gpu, "gpu"); @@ -418,7 +431,10 @@ static
>>>>> void rc6_idle(int i915, uint32_t ctx_id, uint64_t flags)
>>>>>                "Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>>                idle, (idle * 1e9) / slept);
>>>>>        }
>>>>> -    assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>>> +
>>>>> +    drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>>> +
>>>>> +    assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>>
>>>>>        done = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -
>>> 1, 0);
>>>>> @@ -454,7 +470,10 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>>> uint64_t flags)
>>>>>            igt_assert(cycles >= SLEEP_DURATION);
>>>>>
>>>>>            /* While very nearly idle, expect full RC6 */
>>>>> -        assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>>> +        assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>>> +
>>>>> +        free(drpc);
>>>>> +        drpc = NULL;
>>>>>        }
>>>>>
>>>>>        munmap(done, 4096);
>>>>> @@ -471,12 +490,13 @@ static void rc6_idle(int i915, uint32_t ctx_id,
>>>>> uint64_t flags)
>>>>>        }
>>>>>    }
>>>>>
>>>>> -static void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>>> +static void rc6_fence(int i915, unsigned int gt)
>>>>>    {
>>>>>        const int64_t duration_ns = SLEEP_DURATION *
>>>>> (int64_t)NSEC_PER_SEC;
>>>>>        const int tolerance = 20; /* Some RC6 is better than none! */
>>>>>        const unsigned int gen = intel_gen(intel_get_drm_devid(i915));
>>>>>        const struct intel_execution_engine2 *e;
>>>>> +    const intel_ctx_t *ctx;
>>>>>        struct power_sample sample[2];
>>>>>        unsigned long slept;
>>>>>        uint64_t rc6, ts[2], ahnd;
>>>>> @@ -485,7 +505,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>>> *ctx)
>>>>>
>>>>>        igt_require_sw_sync();
>>>>>
>>>>> -    fd = open_pmu(i915, I915_PMU_RC6_RESIDENCY);
>>>>> +    fd = open_pmu(i915, __I915_PMU_RC6_RESIDENCY(gt));
>>>>>        igt_drop_caches_set(i915, DROP_IDLE);
>>>>>        igt_require(__pmu_wait_for_rc6(fd));
>>>>>        igt_power_open(i915, &gpu, "gpu"); @@ -506,9 +526,12 @@ static
>>>>> void rc6_fence(int i915, const intel_ctx_t *ctx)
>>>>>                "Total energy used while idle: %.1fmJ (%.1fmW)\n",
>>>>>                idle, (idle * 1e9) / slept);
>>>>>        }
>>>>> -    assert_within_epsilon(rc6, ts[1] - ts[0], 5);
>>>>> +    drpc = get_drpc(i915, igt_sysfs_get_u32(gt, "id"));
>>>>> +
>>>>> +    assert_within_epsilon(rc6, ts[1] - ts[0], 5, drpc);
>>>>>
>>>>>        /* Submit but delay execution, we should be idle and 
>>>>> conserving
>>>>> power */
>>>>> +    ctx = intel_ctx_create_for_gt(i915, gt);
>>>>>        ahnd = get_reloc_ahnd(i915, ctx->id);
>>>>>        for_each_ctx_engine(i915, ctx, e) {
>>>>>            igt_spin_t *spin;
>>>>> @@ -546,10 +569,14 @@ static void rc6_fence(int i915, const
>>>>> intel_ctx_t
>>>>> *ctx)
>>>>>
>>>>>            close(timeline);
>>>>>
>>>>> -        assert_within_epsilon(rc6, ts[1] - ts[0], tolerance);
>>>>> +        assert_within_epsilon(rc6, ts[1] - ts[0], tolerance, drpc);
>>>>>            gem_quiescent_gpu(i915);
>>>>> +
>>>>> +        free(drpc);
>>>>> +        drpc = NULL;
>>>>>        }
>>>>>        put_ahnd(ahnd);
>>>>> +    intel_ctx_destroy(i915,ctx);
>>>>>
>>>>>        igt_power_close(&gpu);
>>>>>        close(fd);
>>>>> @@ -558,6 +585,7 @@ static void rc6_fence(int i915, const intel_ctx_t
>>>>> *ctx) igt_main  {
>>>>>        int i915 = -1;
>>>>> +    unsigned int dirfd, gt;
>>>>>        const intel_ctx_t *ctx;
>>>>>
>>>>>        /* Use drm_open_driver to verify device existence */ @@ 
>>>>> -572,19
>>>>> +600,25 @@ igt_main
>>>>>            igt_require_gem(i915);
>>>>>            gem_quiescent_gpu(i915);
>>>>>
>>>>> -        for_each_ctx_engine(i915, ctx, e) {
>>>>> -            if (e->instance == 0) {
>>>>> -                igt_dynamic_f("%s", e->name)
>>>>> -                    rc6_idle(i915, ctx->id, e->flags);
>>>>> +        i915_for_each_gt(i915, dirfd, gt) {
>>>>> +            ctx = intel_ctx_create_for_gt(i915, gt);
>>>>> +            for_each_ctx_engine(i915, ctx, e) {
>>>>> +                if (e->instance == 0) {
>>>>> +                    igt_dynamic_f("gt%u-%s", gt, e-
>>>>>> name)
>>>>> +                        rc6_idle(i915, ctx->id, e-
>>>>>> flags, gt);
>>>>> +                }
>>>>>                }
>>>>> +            intel_ctx_destroy(i915, ctx);
>>>>>            }
>>>>>        }
>>>>>
>>>>> -    igt_subtest("rc6-fence") {
>>>>> +    igt_subtest_with_dynamic("rc6-fence") {
>>>>>            igt_require_gem(i915);
>>>>>            gem_quiescent_gpu(i915);
>>>>>
>>>>> -        rc6_fence(i915, ctx);
>>>>> +        i915_for_each_gt(i915, dirfd, gt)
>>>>> +            igt_dynamic_f("gt%u", gt)
>>>>> +                rc6_fence(i915, gt);
>>>>>        }
>>>>>
>>>>>        igt_subtest_group {
>>>>> @@ -595,21 +629,23 @@ igt_main
>>>>>                devid = intel_get_drm_devid(i915);
>>>>>                sysfs = igt_sysfs_open(i915);
>>>>>
>>>>> -            igt_require(has_rc6_residency("rc6"));
>>>>> +            igt_require(has_rc6_residency(dirfd,
>>>>> RC6_RESIDENCY_MS));
>>>>>
>>>>>                /* Make sure rc6 counters are running */
>>>>>                igt_drop_caches_set(i915, DROP_IDLE);
>>>>> -            igt_require(wait_for_rc6());
>>>>> +            igt_require(wait_for_rc6(dirfd));
>>>>>
>>>>>                rc6_enabled = get_rc6_enabled_mask();
>>>>>                igt_require(rc6_enabled & RC6_ENABLED);
>>>>>            }
>>>>>
>>>>>            igt_subtest("rc6-accuracy") {
>>>>> -            struct residencies res;
>>>>> +            for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>>>>> +                struct residencies res;
>>>>>
>>>>> -            measure_residencies(devid, rc6_enabled, &res);
>>>>> -            residency_accuracy(res.rc6, res.duration, "rc6");
>>>>> +                measure_residencies(devid, dirfd,
>>>>> rc6_enabled, &res);
>>>>> +                residency_accuracy(res.rc6, res.duration,
>>>>> "rc6");
>>>>> +            }
>>>>>            }
>>>>>
>>>>>            igt_subtest("media-rc6-accuracy") { @@ -617,7 +653,7 @@
>>> igt_main
>>>>> igt_require(IS_VALLEYVIEW(devid) ||
>>> IS_CHERRYVIEW(devid));
>>>>> - measure_residencies(devid, rc6_enabled, &res);
>>>>> +            measure_residencies(devid, sysfs, rc6_enabled, &res);
>>>>>                residency_accuracy(res.media_rc6, res.duration,
>>> "media_rc6");
>>>>>            }
>>>>>
>>>>> @@ -626,7 +662,7 @@ igt_main
>>>>>        }
>>>>>
>>>>>        igt_fixture {
>>>>> -        intel_ctx_destroy(i915, ctx);
>>>>> +        free(drpc);
>>>>>            drm_close_driver(i915);
>>>>>        }
>>>>>    }
>>>>> -- 
>>>>> 2.25.1

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

end of thread, other threads:[~2023-11-09  5:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-08  6:43 [igt-dev] [PATCH] tests/intel: Add multi gt support for rc6_residency test Sujaritha Sundaresan
2023-11-08  7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2023-11-08  7:28 ` [igt-dev] ✓ CI.xeBAT: success " Patchwork
2023-11-08  8:52 ` [igt-dev] [PATCH] " Gupta, Anshuman
2023-11-08  8:54   ` Sundaresan, Sujaritha
2023-11-08  9:04     ` Gupta, Anshuman
2023-11-08  9:05       ` Sundaresan, Sujaritha
2023-11-09  5:04         ` Sundaresan, Sujaritha

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