Skip to contentSkip to conversation

tests/i915_pm_rps: Fix test after silent conflict

PATCH Superseded Tvrtko Ursulin 4 participants 7 messages Jul 17, 2023 – Jul 18, 2023

lore
Share & tools
lore

Tvrtko Ursulin

author
#0 lore raw

PATCH 1/1 tests/i915_pm_rps: Fix test after silent conflict

From: Tvrtko Ursulin <[email protected]>

A silent conflict sneaked in as I was merging
d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds") in a way that igt_sysfs_set_u32 has became a function returning void.

Assert is now built-in so drop it from the test.

Signed-off-by Tvrtko Ursulin <[email protected]>
Fixes d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds")

Reference: 54dc25efaf10 ("lib/igt_sysfs: add asserting helpers for read/write operations")
Cc: Rodrigo Vivi <[email protected]>
Cc: Lukasz Laguna <[email protected]>
Cc: Kamil Konieczny <[email protected]>

---
 tests/i915/i915_pm_rps.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
tests/i915/i915_pm_rps.c +6 -6
index 68bb99d62c19..6bebecb6bf59 100644
@@ -1010,8 +1010,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	igt_require(def_up && def_down);
 
 	/* Check invalid percentages are rejected */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
+	igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101);
+	igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101);
 
 	/*
 	 * Invent some random up-down thresholds, but always include 0 and 100

@@ -1034,8 +1034,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
 	for (i = 0; i < points; i++) {
 		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
+		igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]);
+		igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]);
 
 		if (flags & TEST_IDLE) {
 			gem_quiescent_gpu(i915);

@@ -1069,8 +1069,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	gem_quiescent_gpu(i915);
 
 	/* Restore defaults */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
+	igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up);
+	igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down);
 
 	free(ta);
 	free(tb);
Signature · 1 line
2.39.2
CI
#1 lore raw

[igt-dev] ✗ Fi.CI.BUILD: failure for tests/i915_pm_rps: Fix test after silent conflict

== Series Details ==

== Series Details ==

Series: tests/i915_pm_rps: Fix test after silent conflict
URL : https://patchwork.freedesktop.org/series/120859/
State : failure

== Summary ==

Applying: tests/i915_pm_rps: Fix test after silent conflict Using index info to reconstruct a base tree...
M tests/i915/i915_pm_rps.c
Falling back to patching base and 3-way merge... Auto-merging tests/i915/i915_pm_rps.c
CONFLICT (content): Merge conflict in tests/i915/i915_pm_rps.c Patch failed at 0001 tests/i915_pm_rps: Fix test after silent conflict When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".

author
#2 lore raw

PATCH v2 1/1 tests/i915_pm_rps: Fix test after silent conflict

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

A silent conflict sneaked in as I was merging
d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds") in a way that igt_sysfs_set_u32 has became a function returning void.

Assert is now built-in so drop it from the test.

v2:
* Fix invalid value test.
* Assert new values after write while at it.

Signed-off-by Tvrtko Ursulin <[email protected]>
Fixes d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds")

Reference: 54dc25efaf10 ("lib/igt_sysfs: add asserting helpers for read/write operations")
Cc: Rodrigo Vivi <[email protected]>
Cc: Lukasz Laguna <[email protected]>
Cc: Kamil Konieczny <[email protected]>

---
 tests/i915/i915_pm_rps.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)
tests/i915/i915_pm_rps.c +28 -6
index 68bb99d62c19..15c74cc703c2 100644
@@ -988,6 +988,28 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
 	return __igt_sync_spin(i915, ahnd, *ctx, &e);
 }
 
+static void sysfs_fail_set_u32(int dir, const char *attr, uint32_t set)
+{
+	u32 old, new;
+	bool ret;
+
+	old = igt_sysfs_get_u32(dir, attr);
+	ret = __igt_sysfs_set_u32(dir, attr, set);
+	igt_assert_eq(ret, false);
+	new = igt_sysfs_get_u32(dir, attr);
+	igt_assert_eq(old, new);
+}
+
+static void sysfs_set_u32(int dir, const char *attr, uint32_t set)
+{
+	u32 new;
+
+	igt_sysfs_set_u32(dir, attr, set);
+
+	new = igt_sysfs_get_u32(dir, attr);
+	igt_assert_eq(set, new);
+}
+
 #define TEST_IDLE 0x1
 #define TEST_PARK 0x2
 static void test_thresholds(int i915, unsigned int gt, unsigned int flags)

@@ -1010,8 +1032,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	igt_require(def_up && def_down);
 
 	/* Check invalid percentages are rejected */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
+	sysfs_fail_set_u32(sysfs, "rps_up_threshold_pct", 101);
+	sysfs_fail_set_u32(sysfs, "rps_down_threshold_pct", 101);
 
 	/*
 	 * Invent some random up-down thresholds, but always include 0 and 100

@@ -1034,8 +1056,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
 	for (i = 0; i < points; i++) {
 		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
+		sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]);
+		sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]);
 
 		if (flags & TEST_IDLE) {
 			gem_quiescent_gpu(i915);

@@ -1069,8 +1091,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	gem_quiescent_gpu(i915);
 
 	/* Restore defaults */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
+	sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up);
+	sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down);
 
 	free(ta);
 	free(tb);
Signature · 1 line
2.39.2
Reviewed-by on v2 1/1
#3 lore raw
Quoted 11 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

A silent conflict sneaked in as I was merging
d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds") in a way that igt_sysfs_set_u32 has became a function returning void.

Assert is now built-in so drop it from the test.

v2:
* Fix invalid value test.
* Assert new values after write while at it.

Reviewed-by Rodrigo Vivi <[email protected]>
Quoted 80 lines
Signed-off-by Tvrtko Ursulin <[email protected]>
Fixes d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds")

Reference: 54dc25efaf10 ("lib/igt_sysfs: add asserting helpers for read/write operations")
Cc: Rodrigo Vivi <[email protected]>
Cc: Lukasz Laguna <[email protected]>
Cc: Kamil Konieczny <[email protected]>

---
 tests/i915/i915_pm_rps.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)
tests/i915/i915_pm_rps.c +28 -6
index 68bb99d62c19..15c74cc703c2 100644
@@ -988,6 +988,28 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
 	return __igt_sync_spin(i915, ahnd, *ctx, &e);
 }
 
+static void sysfs_fail_set_u32(int dir, const char *attr, uint32_t set)
+{
+	u32 old, new;
+	bool ret;
+
+	old = igt_sysfs_get_u32(dir, attr);
+	ret = __igt_sysfs_set_u32(dir, attr, set);
+	igt_assert_eq(ret, false);
+	new = igt_sysfs_get_u32(dir, attr);
+	igt_assert_eq(old, new);
+}
+
+static void sysfs_set_u32(int dir, const char *attr, uint32_t set)
+{
+	u32 new;
+
+	igt_sysfs_set_u32(dir, attr, set);
+
+	new = igt_sysfs_get_u32(dir, attr);
+	igt_assert_eq(set, new);
+}
+
 #define TEST_IDLE 0x1
 #define TEST_PARK 0x2
 static void test_thresholds(int i915, unsigned int gt, unsigned int flags)

@@ -1010,8 +1032,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	igt_require(def_up && def_down);
 
 	/* Check invalid percentages are rejected */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
+	sysfs_fail_set_u32(sysfs, "rps_up_threshold_pct", 101);
+	sysfs_fail_set_u32(sysfs, "rps_down_threshold_pct", 101);
 
 	/*
 	 * Invent some random up-down thresholds, but always include 0 and 100

@@ -1034,8 +1056,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
 	for (i = 0; i < points; i++) {
 		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
-		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
+		sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]);
+		sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]);
 
 		if (flags & TEST_IDLE) {
 			gem_quiescent_gpu(i915);

@@ -1069,8 +1091,8 @@ static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
 	gem_quiescent_gpu(i915);
 
 	/* Restore defaults */
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
-	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
+	sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up);
+	sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down);
 
 	free(ta);
 	free(tb);
Signature · 1 line
2.39.2
#4 lore raw
Quoted 8 lines · Rodrigo Vivi
Quoted 6 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

A silent conflict sneaked in as I was merging
d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds") in a way that igt_sysfs_set_u32 has became a function returning void.

Quoted 7 lines
Quoted 5 lines

Assert is now built-in so drop it from the test.

v2:
* Fix invalid value test.
* Assert new values after write while at it.

Reviewed-by Rodrigo Vivi <[email protected]>

Merged to fix the IGT build.

author
#5 lore raw
Quoted 11 lines · Ashutosh
Quoted 10 lines · Rodrigo Vivi
Quoted 8 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

A silent conflict sneaked in as I was merging
d86ca7e17b58 ("tests/i915_pm_rps: Exercise sysfs thresholds") in a way that igt_sysfs_set_u32 has became a function returning void.

Assert is now built-in so drop it from the test.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

v2:
* Fix invalid value test.
* Assert new values after write while at it.

Reviewed-by Rodrigo Vivi <[email protected]>

Merged to fix the IGT build.

Ah thank you, didn't realize it was broken, I thought it was only warnings.

Regards,

Tvrtko

CI
#6 lore raw

[igt-dev] ✗ Fi.CI.BUILD: failure for tests/i915_pm_rps: Fix test after silent conflict (rev2)

In reply to Tvrtko Ursulin
== Series Details ==

== Series Details ==

Series: tests/i915_pm_rps: Fix test after silent conflict (rev2)
URL : https://patchwork.freedesktop.org/series/120859/
State : failure

== Summary ==

Applying: tests/i915_pm_rps: Fix test after silent conflict Using index info to reconstruct a base tree...
M tests/i915/i915_pm_rps.c
Falling back to patching base and 3-way merge... Auto-merging tests/i915/i915_pm_rps.c
CONFLICT (content): Merge conflict in tests/i915/i915_pm_rps.c Patch failed at 0001 tests/i915_pm_rps: Fix test after silent conflict When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".