[igt-dev] [PATCH i-g-t 0/3] Test cleanup and dynamic conversion

[igt-dev] [PATCH i-g-t 0/3] Test cleanup and dynamic conversion

[Swati Sharma]

Subtests are converted to dynamic subtests. Also, testplan
documentation is updated.
Review coomments addressed from
https://patchwork.freedesktop.org/series/109170/

Swati Sharma (3):
  tests/kms_vblank: cosmetic changes
  tests/kms_vblank: convert test to dynamic
  tests/kms_vblank: update testplan doc

 tests/kms_vblank.c | 268 +++++++++++++++------------------------------
 1 file changed, 87 insertions(+), 181 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/3] tests/kms_vblank: cosmetic changes

[Swati Sharma]

Fix igt_describe() documentation. Remove redundant documentation.

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 tests/kms_vblank.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 33e933c22..bcfd8d012 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -21,11 +21,6 @@
  * IN THE SOFTWARE.
  */
 
-/** @file kms_vblank.c
- *
- * This is a test of performance of drmWaitVblank.
- */
-
 #include "igt.h"
 #include <stdlib.h>
 #include <stdio.h>
@@ -46,7 +41,7 @@
  * Description: Test speed of WaitVblank.
  *
  * SUBTEST: crtc-id
- * Description: check the Vblank and flip events works with given crtc id
+ * Description: Check the vblank and flip events works with given crtc id
  * Driver requirement: i915, xe
  * Functionality: vblank
  * Mega feature: General Display Features
@@ -623,7 +618,7 @@ static void run_subtests_for_pipe(data_t *data)
 			if (m->flags & ~(f->valid | NOHANG))
 				continue;
 
-			igt_describe("Check if test run while hanging by introducing NOHANG flag");
+			igt_describe("Check if test run while hanging by introducing NOHANG flag.");
 			igt_subtest_f("pipe-%s-%s-%s",
 				      kmstest_pipe_name(data->pipe),
 				      f->name, m->name) {
@@ -645,7 +640,7 @@ static void run_subtests_for_pipe(data_t *data)
 			if (f->valid & NOHANG || m->flags & NOHANG)
 				continue;
 
-			igt_describe("check if injected hang is working properly");
+			igt_describe("Check if injected hang is working properly.");
 			igt_subtest_f("pipe-%s-%s-%s-hang",
 				      kmstest_pipe_name(data->pipe),
 				      f->name, m->name) {
@@ -742,11 +737,11 @@ igt_main
 		igt_display_require_output(&data.display);
 	}
 
-	igt_describe("Negative test for vblank request");
+	igt_describe("Negative test for vblank request.");
 	igt_subtest("invalid")
 		invalid_subtest(&data, fd);
 
-	igt_describe("check the Vblank and flip events works with given crtc id");
+	igt_describe("Check the vblank and flip events works with given crtc id.");
 	igt_subtest("crtc-id")
 		crtc_id_subtest(&data, fd);
 
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/3] tests/kms_vblank: convert test to dynamic

[Swati Sharma]

Convert existing subtests to dynamic subtests at pipe/output level.

v2: -Remove redundant debug prints
    -Convert all subtests into dynamic
v3: -Use pipe & output from struct data_t
v4: -Directly use data->pipe instead of using p
    -Use for_each_pipe_with_single_output()

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 tests/kms_vblank.c | 172 ++++++++++++++++++---------------------------
 1 file changed, 69 insertions(+), 103 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index bcfd8d012..507421382 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -270,10 +270,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (data->flags & RPM)
 		igt_require(igt_setup_runtime_pm(fd));
 
-	igt_info("Beginning %s on pipe %s, connector %s\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe),
-		 igt_output_name(output));
-
 	if (!(data->flags & NOHANG)) {
 		ahnd = is_i915_device(fd) ?
 			get_reloc_ahnd(fd, 0) :
@@ -314,9 +310,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (!(data->flags & NOHANG))
 		igt_post_hang_ring(fd, hang);
 
-	igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
-
 	put_ahnd(ahnd);
 
 	/* cleanup what prepare_crtc() has done */
@@ -342,63 +335,50 @@ pipe_output_combo_valid(igt_display_t *display,
 static void crtc_id_subtest(data_t *data, int fd)
 {
 	igt_display_t *display = &data->display;
-	igt_output_t *output;
-	enum pipe p;
-
-	for_each_pipe_with_valid_output(display, p, output) {
-		struct drm_event_vblank buf;
-		const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
-		unsigned crtc_id, expected_crtc_id;
-		uint64_t val;
-		union drm_wait_vblank vbl;
+	enum pipe p = data->pipe;
+	igt_output_t *output = data->output;
+	struct drm_event_vblank buf;
+	const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
+	unsigned crtc_id, expected_crtc_id;
+	uint64_t val;
+	union drm_wait_vblank vbl;
 
-		igt_display_reset(display);
+	crtc_id = display->pipes[p].crtc_id;
+	if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
+		expected_crtc_id = crtc_id;
+	else
+		expected_crtc_id = 0;
 
-		igt_output_set_pipe(output, p);
-		if (!i915_pipe_output_combo_valid(display))
-			continue;
+	prepare_crtc(data, fd, output);
 
-		igt_info("Using (pipe %s + %s) to run the subtest.\n",
-			 kmstest_pipe_name(p), igt_output_name(output));
+	memset(&vbl, 0, sizeof(vbl));
+	vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+	vbl.request.type |= pipe_id_flag;
+	vbl.request.sequence = 1;
+	igt_assert_eq(wait_vblank(fd, &vbl), 0);
 
-		crtc_id = display->pipes[p].crtc_id;
-		if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
-			expected_crtc_id = crtc_id;
-		else
-			expected_crtc_id = 0;
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		data->pipe = p;
-		prepare_crtc(data, fd, output);
+	do_or_die(drmModePageFlip(fd, crtc_id,
+				  data->primary_fb.fb_id,
+				  DRM_MODE_PAGE_FLIP_EVENT, NULL));
 
-		memset(&vbl, 0, sizeof(vbl));
-		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
-		vbl.request.type |= pipe_id_flag;
-		vbl.request.sequence = 1;
-		igt_assert_eq(wait_vblank(fd, &vbl), 0);
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-		igt_assert_eq(buf.crtc_id, expected_crtc_id);
+	if (display->is_atomic) {
+		igt_plane_t *primary = igt_output_get_plane(output, 0);
 
-		do_or_die(drmModePageFlip(fd, crtc_id,
-					  data->primary_fb.fb_id,
-					  DRM_MODE_PAGE_FLIP_EVENT, NULL));
+		igt_plane_set_fb(primary, &data->primary_fb);
+		igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
 
 		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
 		igt_assert_eq(buf.crtc_id, expected_crtc_id);
-
-		if (display->is_atomic) {
-			igt_plane_t *primary = igt_output_get_plane(output, 0);
-
-			igt_plane_set_fb(primary, &data->primary_fb);
-			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
-
-			igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-			igt_assert_eq(buf.crtc_id, expected_crtc_id);
-		}
-
-		cleanup_crtc(data, fd, output);
-		return;
 	}
+
+	cleanup_crtc(data, fd, output);
+	return;
 }
 
 static void accuracy(data_t *data, int fd, int nchildren)
@@ -576,7 +556,7 @@ static void vblank_ts_cont(data_t *data, int fd, int nchildren)
 			estimated_vblanks, seq2, seq1 + estimated_vblanks);
 }
 
-static void run_subtests_for_pipe(data_t *data)
+static void run_subtests(data_t *data)
 {
 	const struct {
 		const char *name;
@@ -610,30 +590,22 @@ static void run_subtests_for_pipe(data_t *data)
 		{ }
 	}, *m;
 
-	igt_fixture
-		igt_display_require_output_on_pipe(&data->display, data->pipe);
-
 	for (f = funcs; f->name; f++) {
 		for (m = modes; m->name; m++) {
 			if (m->flags & ~(f->valid | NOHANG))
 				continue;
 
 			igt_describe("Check if test run while hanging by introducing NOHANG flag.");
-			igt_subtest_f("pipe-%s-%s-%s",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
-				int found = 0;
-
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
-					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
+			igt_subtest_with_dynamic_f("%s-%s", f->name, m->name) {
+				for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
+					if (!pipe_output_combo_valid(&data->display, data->output, data->pipe))
 						continue;
 
-					data->flags = m->flags | NOHANG;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags | NOHANG;
+						run_test(data, f->func);
+					}
 				}
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 
 			/* Skip the -hang version if NOHANG flag is set */
@@ -641,24 +613,20 @@ static void run_subtests_for_pipe(data_t *data)
 				continue;
 
 			igt_describe("Check if injected hang is working properly.");
-			igt_subtest_f("pipe-%s-%s-%s-hang",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
+			igt_subtest_with_dynamic_f("%s-%s-hang", f->name, m->name) {
 				igt_hang_t hang;
-				int found = 0;
 
 				hang = igt_allow_hang(data->display.drm_fd, 0, 0);
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
-					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
+				for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
+					if (!pipe_output_combo_valid(&data->display, data->output, data->pipe))
 						continue;
 
-					data->flags = m->flags;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags;
+						run_test(data, f->func);
+					}
 				}
 				igt_disallow_hang(data->display.drm_fd, hang);
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 		}
 	}
@@ -668,23 +636,7 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
-	igt_display_t* display = &data->display;
-	enum pipe pipe = 0;
-	igt_output_t *output;
-
-	igt_display_reset(display);
-
-	output = igt_get_single_output_for_pipe(display, pipe);
-	igt_require(output);
-
-	data->pipe = pipe;
-	data->output = output;
-
-	igt_output_set_pipe(output, pipe);
-	igt_require(i915_pipe_output_combo_valid(display));
-
-	igt_info("Using (pipe %s + %s) to run the subtest.\n",
-		 kmstest_pipe_name(pipe), igt_output_name(output));
+	igt_output_t *output = data->output;
 
 	prepare_crtc(data, fd, output);
 
@@ -738,18 +690,32 @@ igt_main
 	}
 
 	igt_describe("Negative test for vblank request.");
-	igt_subtest("invalid")
-		invalid_subtest(&data, fd);
+	igt_subtest_with_dynamic("invalid") {
+		for_each_pipe_with_single_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.output, data.pipe))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				invalid_subtest(&data, fd);
+			break;
+		}
+	}
 
 	igt_describe("Check the vblank and flip events works with given crtc id.");
-	igt_subtest("crtc-id")
-		crtc_id_subtest(&data, fd);
+	igt_subtest_with_dynamic("crtc-id") {
+		for_each_pipe_with_single_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.output, data.pipe))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				crtc_id_subtest(&data, fd);
+		}
+	}
 
-	for_each_pipe_static(data.pipe)
-		igt_subtest_group
-			run_subtests_for_pipe(&data);
+	run_subtests(&data);
 
 	igt_fixture {
+		igt_display_fini(&data.display);
 		drm_close_driver(fd);
 	}
 }
-- 
2.25.1

[igt-dev] [PATCH i-g-t 3/3] tests/kms_vblank: update testplan doc

[Swati Sharma]

After introducing dynamic subtests, testplan documentation needs
to be updated.

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 tests/kms_vblank.c | 81 ++++++++--------------------------------------
 1 file changed, 13 insertions(+), 68 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 507421382..0433e7d46 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -56,107 +56,63 @@
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-dpms-rpm
+ * SUBTEST: ts-continuation-dpms-rpm
  * Description: Test TS continuty with DPMS & RPM while hanging by introducing
- *              NOHANG flag on %arg[1]
+ *              NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: dpms, hang, rpm, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-dpms-suspend
+ * SUBTEST: ts-continuation-dpms-suspend
  * Description: Test TS continuty with DPMS & Suspend while hanging by introducing
- *              NOHANG flag on %arg[1]
+ *              NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: dpms, hang, suspend, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-suspend
+ * SUBTEST: ts-continuation-suspend
  * Description: Test TS continuty with Suspend while hanging by introducing NOHANG
- *              flag on %arg[1]
+ *              flag
  * Driver requirement: i915, xe
  * Functionality: hang, suspend, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
  * SUBTEST: pipe-%s-ts-continuation-modeset-rpm
  * Description: Test TS continuty during Modeset with Suspend while hanging by
- *              introducing NOHANG flag on %arg[1]
+ *              introducing NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: hang, rpm, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-accuracy-idle
+ * SUBTEST: accuracy-idle
  * Description: Test Accuracy of vblank events while hanging by introducing NOHANG
- *              flag on %arg[1]
+ *              flag
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
  *
- * SUBTEST: pipe-%s-%s
- * Description: Test %arg[2] while hanging by introducing NOHANG flag on %arg[1]
+ * SUBTEST: %s
+ * Description: Test %arg[1] while hanging by introducing NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
  *
- * SUBTEST: pipe-%s-%s-hang
- * Description: Test %arg[2] with injected hang is working properly on %arg[1]
+ * SUBTEST: %s-hang
+ * Description: Test %arg[1] with injected hang is working properly
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
@@ -164,17 +120,6 @@
  *
  * arg[1]:
  *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
- *
- * arg[2]:
- *
  * @query-idle:              Time taken to Query vblank counters
  * @query-forked:            Time taken to Query vblank counters (multithreaded)
  * @query-busy:              Time taken to Query vblank counters (during V-active)
-- 
2.25.1

[igt-dev] [PATCH i-g-t 0/3] tests/kms_vblank: Test cleanup and dynamic conversion

[Swati Sharma]

Subtests are converted to dynamic subtests. Also, testplan
documentation is updated.
Review comments addressed from
https://patchwork.freedesktop.org/series/109170/

Swati Sharma (3):
  tests/kms_vblank: cosmetic changes
  tests/kms_vblank: convert test to dynamic
  tests/kms_vblank: update testplan doc

 tests/kms_vblank.c | 268 +++++++++++++++------------------------------
 1 file changed, 87 insertions(+), 181 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/3] tests/kms_vblank: convert test to dynamic

[Swati Sharma]

Convert existing subtests to dynamic subtests at pipe/output level.

v2: -Remove redundant debug prints
    -Convert all subtests into dynamic
v3: -Use pipe & output from struct data_t
v4: -Directly use data->pipe instead of using p
    -Use for_each_pipe_with_single_output()

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 tests/kms_vblank.c | 172 ++++++++++++++++++---------------------------
 1 file changed, 69 insertions(+), 103 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index bcfd8d012..507421382 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -270,10 +270,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (data->flags & RPM)
 		igt_require(igt_setup_runtime_pm(fd));
 
-	igt_info("Beginning %s on pipe %s, connector %s\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe),
-		 igt_output_name(output));
-
 	if (!(data->flags & NOHANG)) {
 		ahnd = is_i915_device(fd) ?
 			get_reloc_ahnd(fd, 0) :
@@ -314,9 +310,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (!(data->flags & NOHANG))
 		igt_post_hang_ring(fd, hang);
 
-	igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
-
 	put_ahnd(ahnd);
 
 	/* cleanup what prepare_crtc() has done */
@@ -342,63 +335,50 @@ pipe_output_combo_valid(igt_display_t *display,
 static void crtc_id_subtest(data_t *data, int fd)
 {
 	igt_display_t *display = &data->display;
-	igt_output_t *output;
-	enum pipe p;
-
-	for_each_pipe_with_valid_output(display, p, output) {
-		struct drm_event_vblank buf;
-		const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
-		unsigned crtc_id, expected_crtc_id;
-		uint64_t val;
-		union drm_wait_vblank vbl;
+	enum pipe p = data->pipe;
+	igt_output_t *output = data->output;
+	struct drm_event_vblank buf;
+	const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
+	unsigned crtc_id, expected_crtc_id;
+	uint64_t val;
+	union drm_wait_vblank vbl;
 
-		igt_display_reset(display);
+	crtc_id = display->pipes[p].crtc_id;
+	if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
+		expected_crtc_id = crtc_id;
+	else
+		expected_crtc_id = 0;
 
-		igt_output_set_pipe(output, p);
-		if (!i915_pipe_output_combo_valid(display))
-			continue;
+	prepare_crtc(data, fd, output);
 
-		igt_info("Using (pipe %s + %s) to run the subtest.\n",
-			 kmstest_pipe_name(p), igt_output_name(output));
+	memset(&vbl, 0, sizeof(vbl));
+	vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+	vbl.request.type |= pipe_id_flag;
+	vbl.request.sequence = 1;
+	igt_assert_eq(wait_vblank(fd, &vbl), 0);
 
-		crtc_id = display->pipes[p].crtc_id;
-		if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
-			expected_crtc_id = crtc_id;
-		else
-			expected_crtc_id = 0;
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		data->pipe = p;
-		prepare_crtc(data, fd, output);
+	do_or_die(drmModePageFlip(fd, crtc_id,
+				  data->primary_fb.fb_id,
+				  DRM_MODE_PAGE_FLIP_EVENT, NULL));
 
-		memset(&vbl, 0, sizeof(vbl));
-		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
-		vbl.request.type |= pipe_id_flag;
-		vbl.request.sequence = 1;
-		igt_assert_eq(wait_vblank(fd, &vbl), 0);
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-		igt_assert_eq(buf.crtc_id, expected_crtc_id);
+	if (display->is_atomic) {
+		igt_plane_t *primary = igt_output_get_plane(output, 0);
 
-		do_or_die(drmModePageFlip(fd, crtc_id,
-					  data->primary_fb.fb_id,
-					  DRM_MODE_PAGE_FLIP_EVENT, NULL));
+		igt_plane_set_fb(primary, &data->primary_fb);
+		igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
 
 		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
 		igt_assert_eq(buf.crtc_id, expected_crtc_id);
-
-		if (display->is_atomic) {
-			igt_plane_t *primary = igt_output_get_plane(output, 0);
-
-			igt_plane_set_fb(primary, &data->primary_fb);
-			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
-
-			igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-			igt_assert_eq(buf.crtc_id, expected_crtc_id);
-		}
-
-		cleanup_crtc(data, fd, output);
-		return;
 	}
+
+	cleanup_crtc(data, fd, output);
+	return;
 }
 
 static void accuracy(data_t *data, int fd, int nchildren)
@@ -576,7 +556,7 @@ static void vblank_ts_cont(data_t *data, int fd, int nchildren)
 			estimated_vblanks, seq2, seq1 + estimated_vblanks);
 }
 
-static void run_subtests_for_pipe(data_t *data)
+static void run_subtests(data_t *data)
 {
 	const struct {
 		const char *name;
@@ -610,30 +590,22 @@ static void run_subtests_for_pipe(data_t *data)
 		{ }
 	}, *m;
 
-	igt_fixture
-		igt_display_require_output_on_pipe(&data->display, data->pipe);
-
 	for (f = funcs; f->name; f++) {
 		for (m = modes; m->name; m++) {
 			if (m->flags & ~(f->valid | NOHANG))
 				continue;
 
 			igt_describe("Check if test run while hanging by introducing NOHANG flag.");
-			igt_subtest_f("pipe-%s-%s-%s",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
-				int found = 0;
-
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
-					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
+			igt_subtest_with_dynamic_f("%s-%s", f->name, m->name) {
+				for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
+					if (!pipe_output_combo_valid(&data->display, data->output, data->pipe))
 						continue;
 
-					data->flags = m->flags | NOHANG;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags | NOHANG;
+						run_test(data, f->func);
+					}
 				}
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 
 			/* Skip the -hang version if NOHANG flag is set */
@@ -641,24 +613,20 @@ static void run_subtests_for_pipe(data_t *data)
 				continue;
 
 			igt_describe("Check if injected hang is working properly.");
-			igt_subtest_f("pipe-%s-%s-%s-hang",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
+			igt_subtest_with_dynamic_f("%s-%s-hang", f->name, m->name) {
 				igt_hang_t hang;
-				int found = 0;
 
 				hang = igt_allow_hang(data->display.drm_fd, 0, 0);
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
-					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
+				for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
+					if (!pipe_output_combo_valid(&data->display, data->output, data->pipe))
 						continue;
 
-					data->flags = m->flags;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags;
+						run_test(data, f->func);
+					}
 				}
 				igt_disallow_hang(data->display.drm_fd, hang);
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 		}
 	}
@@ -668,23 +636,7 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
-	igt_display_t* display = &data->display;
-	enum pipe pipe = 0;
-	igt_output_t *output;
-
-	igt_display_reset(display);
-
-	output = igt_get_single_output_for_pipe(display, pipe);
-	igt_require(output);
-
-	data->pipe = pipe;
-	data->output = output;
-
-	igt_output_set_pipe(output, pipe);
-	igt_require(i915_pipe_output_combo_valid(display));
-
-	igt_info("Using (pipe %s + %s) to run the subtest.\n",
-		 kmstest_pipe_name(pipe), igt_output_name(output));
+	igt_output_t *output = data->output;
 
 	prepare_crtc(data, fd, output);
 
@@ -738,18 +690,32 @@ igt_main
 	}
 
 	igt_describe("Negative test for vblank request.");
-	igt_subtest("invalid")
-		invalid_subtest(&data, fd);
+	igt_subtest_with_dynamic("invalid") {
+		for_each_pipe_with_single_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.output, data.pipe))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				invalid_subtest(&data, fd);
+			break;
+		}
+	}
 
 	igt_describe("Check the vblank and flip events works with given crtc id.");
-	igt_subtest("crtc-id")
-		crtc_id_subtest(&data, fd);
+	igt_subtest_with_dynamic("crtc-id") {
+		for_each_pipe_with_single_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.output, data.pipe))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				crtc_id_subtest(&data, fd);
+		}
+	}
 
-	for_each_pipe_static(data.pipe)
-		igt_subtest_group
-			run_subtests_for_pipe(&data);
+	run_subtests(&data);
 
 	igt_fixture {
+		igt_display_fini(&data.display);
 		drm_close_driver(fd);
 	}
 }
-- 
2.25.1

[igt-dev] [PATCH i-g-t 0/3] tests/kms_vblank: Test cleanup and dynamic conversion

[Swati Sharma]

Subtests are converted to dynamic subtests. Also, testplan
documentation is updated.
Review comments addressed from
https://patchwork.freedesktop.org/series/109170/

Swati Sharma (3):
  tests/kms_vblank: cosmetic changes
  tests/kms_vblank: convert test to dynamic
  tests/kms_vblank: restrict to 2 pipes and add capability to run on all
    pipes

 tests/kms_vblank.c | 301 ++++++++++++++++++---------------------------
 1 file changed, 122 insertions(+), 179 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/3] tests/kms_vblank: convert test to dynamic

[Swati Sharma]

Convert existing subtests to dynamic subtests at pipe/output level
and update testplan documentation.

v2: -Remove redundant debug prints
    -Convert all subtests into dynamic
v3: -Use pipe & output from struct data_t
v4: -Directly use data->pipe instead of using p
v5: -Use for_each_pipe_with_valid_output()

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 tests/kms_vblank.c | 251 +++++++++++++++------------------------------
 1 file changed, 81 insertions(+), 170 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index bcfd8d012..4edd3f6bc 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -56,107 +56,63 @@
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-dpms-rpm
+ * SUBTEST: ts-continuation-dpms-rpm
  * Description: Test TS continuty with DPMS & RPM while hanging by introducing
- *              NOHANG flag on %arg[1]
+ *              NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: dpms, hang, rpm, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-dpms-suspend
+ * SUBTEST: ts-continuation-dpms-suspend
  * Description: Test TS continuty with DPMS & Suspend while hanging by introducing
- *              NOHANG flag on %arg[1]
+ *              NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: dpms, hang, suspend, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-suspend
+ * SUBTEST: ts-continuation-suspend
  * Description: Test TS continuty with Suspend while hanging by introducing NOHANG
- *              flag on %arg[1]
+ *              flag
  * Driver requirement: i915, xe
  * Functionality: hang, suspend, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-ts-continuation-modeset-rpm
+ * SUBTEST: ts-continuation-modeset-rpm
  * Description: Test TS continuty during Modeset with Suspend while hanging by
- *              introducing NOHANG flag on %arg[1]
+ *              introducing NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: hang, rpm, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
- *
- * arg[1]:
- *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
  */
 
 /**
- * SUBTEST: pipe-%s-accuracy-idle
+ * SUBTEST: accuracy-idle
  * Description: Test Accuracy of vblank events while hanging by introducing NOHANG
- *              flag on %arg[1]
+ *              flag
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
  *
- * SUBTEST: pipe-%s-%s
- * Description: Test %arg[2] while hanging by introducing NOHANG flag on %arg[1]
+ * SUBTEST: %s
+ * Description: Test %arg[1] while hanging by introducing NOHANG flag
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
  * Test category: functionality test
  *
- * SUBTEST: pipe-%s-%s-hang
- * Description: Test %arg[2] with injected hang is working properly on %arg[1]
+ * SUBTEST: %s-hang
+ * Description: Test %arg[1] with injected hang is working properly
  * Driver requirement: i915, xe
  * Functionality: hang, vblank
  * Mega feature: General Display Features
@@ -164,17 +120,6 @@
  *
  * arg[1]:
  *
- * @A:     pipe A
- * @B:     pipe B
- * @C:     pipe C
- * @D:     pipe D
- * @E:     pipe E
- * @F:     pipe F
- * @G:     pipe G
- * @H:     pipe H
- *
- * arg[2]:
- *
  * @query-idle:              Time taken to Query vblank counters
  * @query-forked:            Time taken to Query vblank counters (multithreaded)
  * @query-busy:              Time taken to Query vblank counters (during V-active)
@@ -270,10 +215,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (data->flags & RPM)
 		igt_require(igt_setup_runtime_pm(fd));
 
-	igt_info("Beginning %s on pipe %s, connector %s\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe),
-		 igt_output_name(output));
-
 	if (!(data->flags & NOHANG)) {
 		ahnd = is_i915_device(fd) ?
 			get_reloc_ahnd(fd, 0) :
@@ -314,9 +255,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
 	if (!(data->flags & NOHANG))
 		igt_post_hang_ring(fd, hang);
 
-	igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
-		 igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
-
 	put_ahnd(ahnd);
 
 	/* cleanup what prepare_crtc() has done */
@@ -342,63 +280,50 @@ pipe_output_combo_valid(igt_display_t *display,
 static void crtc_id_subtest(data_t *data, int fd)
 {
 	igt_display_t *display = &data->display;
-	igt_output_t *output;
-	enum pipe p;
-
-	for_each_pipe_with_valid_output(display, p, output) {
-		struct drm_event_vblank buf;
-		const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
-		unsigned crtc_id, expected_crtc_id;
-		uint64_t val;
-		union drm_wait_vblank vbl;
+	enum pipe p = data->pipe;
+	igt_output_t *output = data->output;
+	struct drm_event_vblank buf;
+	const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
+	unsigned crtc_id, expected_crtc_id;
+	uint64_t val;
+	union drm_wait_vblank vbl;
 
-		igt_display_reset(display);
+	crtc_id = display->pipes[p].crtc_id;
+	if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
+		expected_crtc_id = crtc_id;
+	else
+		expected_crtc_id = 0;
 
-		igt_output_set_pipe(output, p);
-		if (!i915_pipe_output_combo_valid(display))
-			continue;
+	prepare_crtc(data, fd, output);
 
-		igt_info("Using (pipe %s + %s) to run the subtest.\n",
-			 kmstest_pipe_name(p), igt_output_name(output));
+	memset(&vbl, 0, sizeof(vbl));
+	vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+	vbl.request.type |= pipe_id_flag;
+	vbl.request.sequence = 1;
+	igt_assert_eq(wait_vblank(fd, &vbl), 0);
 
-		crtc_id = display->pipes[p].crtc_id;
-		if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
-			expected_crtc_id = crtc_id;
-		else
-			expected_crtc_id = 0;
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		data->pipe = p;
-		prepare_crtc(data, fd, output);
+	do_or_die(drmModePageFlip(fd, crtc_id,
+				  data->primary_fb.fb_id,
+				  DRM_MODE_PAGE_FLIP_EVENT, NULL));
 
-		memset(&vbl, 0, sizeof(vbl));
-		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
-		vbl.request.type |= pipe_id_flag;
-		vbl.request.sequence = 1;
-		igt_assert_eq(wait_vblank(fd, &vbl), 0);
+	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+	igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
-		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-		igt_assert_eq(buf.crtc_id, expected_crtc_id);
+	if (display->is_atomic) {
+		igt_plane_t *primary = igt_output_get_plane(output, 0);
 
-		do_or_die(drmModePageFlip(fd, crtc_id,
-					  data->primary_fb.fb_id,
-					  DRM_MODE_PAGE_FLIP_EVENT, NULL));
+		igt_plane_set_fb(primary, &data->primary_fb);
+		igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
 
 		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
 		igt_assert_eq(buf.crtc_id, expected_crtc_id);
-
-		if (display->is_atomic) {
-			igt_plane_t *primary = igt_output_get_plane(output, 0);
-
-			igt_plane_set_fb(primary, &data->primary_fb);
-			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
-
-			igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
-			igt_assert_eq(buf.crtc_id, expected_crtc_id);
-		}
-
-		cleanup_crtc(data, fd, output);
-		return;
 	}
+
+	cleanup_crtc(data, fd, output);
+	return;
 }
 
 static void accuracy(data_t *data, int fd, int nchildren)
@@ -576,7 +501,7 @@ static void vblank_ts_cont(data_t *data, int fd, int nchildren)
 			estimated_vblanks, seq2, seq1 + estimated_vblanks);
 }
 
-static void run_subtests_for_pipe(data_t *data)
+static void run_subtests(data_t *data)
 {
 	const struct {
 		const char *name;
@@ -610,30 +535,22 @@ static void run_subtests_for_pipe(data_t *data)
 		{ }
 	}, *m;
 
-	igt_fixture
-		igt_display_require_output_on_pipe(&data->display, data->pipe);
-
 	for (f = funcs; f->name; f++) {
 		for (m = modes; m->name; m++) {
 			if (m->flags & ~(f->valid | NOHANG))
 				continue;
 
 			igt_describe("Check if test run while hanging by introducing NOHANG flag.");
-			igt_subtest_f("pipe-%s-%s-%s",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
-				int found = 0;
-
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
+			igt_subtest_with_dynamic_f("%s-%s", f->name, m->name) {
+				for_each_pipe_with_valid_output(&data->display, data->pipe, data->output) {
 					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
 						continue;
 
-					data->flags = m->flags | NOHANG;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags | NOHANG;
+						run_test(data, f->func);
+					}
 				}
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 
 			/* Skip the -hang version if NOHANG flag is set */
@@ -641,24 +558,20 @@ static void run_subtests_for_pipe(data_t *data)
 				continue;
 
 			igt_describe("Check if injected hang is working properly.");
-			igt_subtest_f("pipe-%s-%s-%s-hang",
-				      kmstest_pipe_name(data->pipe),
-				      f->name, m->name) {
+			igt_subtest_with_dynamic_f("%s-%s-hang", f->name, m->name) {
 				igt_hang_t hang;
-				int found = 0;
 
 				hang = igt_allow_hang(data->display.drm_fd, 0, 0);
-				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
+				for_each_pipe_with_valid_output(&data->display, data->pipe, data->output) {
 					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
 						continue;
 
-					data->flags = m->flags;
-					run_test(data, f->func);
-
-					found++;
+					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
+						data->flags = m->flags;
+						run_test(data, f->func);
+					}
 				}
 				igt_disallow_hang(data->display.drm_fd, hang);
-				igt_require_f(found, "No valid pipe/output combo found.\n");
 			}
 		}
 	}
@@ -668,23 +581,7 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
-	igt_display_t* display = &data->display;
-	enum pipe pipe = 0;
-	igt_output_t *output;
-
-	igt_display_reset(display);
-
-	output = igt_get_single_output_for_pipe(display, pipe);
-	igt_require(output);
-
-	data->pipe = pipe;
-	data->output = output;
-
-	igt_output_set_pipe(output, pipe);
-	igt_require(i915_pipe_output_combo_valid(display));
-
-	igt_info("Using (pipe %s + %s) to run the subtest.\n",
-		 kmstest_pipe_name(pipe), igt_output_name(output));
+	igt_output_t *output = data->output;
 
 	prepare_crtc(data, fd, output);
 
@@ -738,18 +635,32 @@ igt_main
 	}
 
 	igt_describe("Negative test for vblank request.");
-	igt_subtest("invalid")
-		invalid_subtest(&data, fd);
+	igt_subtest_with_dynamic("invalid") {
+		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				invalid_subtest(&data, fd);
+			break;
+		}
+	}
 
 	igt_describe("Check the vblank and flip events works with given crtc id.");
-	igt_subtest("crtc-id")
-		crtc_id_subtest(&data, fd);
+	igt_subtest_with_dynamic("crtc-id") {
+		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
+			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
+				continue;
+
+			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
+				crtc_id_subtest(&data, fd);
+		}
+	}
 
-	for_each_pipe_static(data.pipe)
-		igt_subtest_group
-			run_subtests_for_pipe(&data);
+	run_subtests(&data);
 
 	igt_fixture {
+		igt_display_fini(&data.display);
 		drm_close_driver(fd);
 	}
 }
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 2/3] tests/kms_vblank: convert test to dynamic

[Karthik B S]

Hi,

On 10/11/2023 7:01 PM, Swati Sharma wrote:
> Convert existing subtests to dynamic subtests at pipe/output level
> and update testplan documentation.
>
> v2: -Remove redundant debug prints
>      -Convert all subtests into dynamic
> v3: -Use pipe & output from struct data_t
> v4: -Directly use data->pipe instead of using p
> v5: -Use for_each_pipe_with_valid_output()
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>   tests/kms_vblank.c | 251 +++++++++++++++------------------------------
>   1 file changed, 81 insertions(+), 170 deletions(-)
>
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index bcfd8d012..4edd3f6bc 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -56,107 +56,63 @@
>    */
>   
>   /**
> - * SUBTEST: pipe-%s-ts-continuation-dpms-rpm
> + * SUBTEST: ts-continuation-dpms-rpm
>    * Description: Test TS continuty with DPMS & RPM while hanging by introducing
> - *              NOHANG flag on %arg[1]
> + *              NOHANG flag
>    * Driver requirement: i915, xe
>    * Functionality: dpms, hang, rpm, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
> - *
> - * arg[1]:
> - *
> - * @A:     pipe A
> - * @B:     pipe B
> - * @C:     pipe C
> - * @D:     pipe D
> - * @E:     pipe E
> - * @F:     pipe F
> - * @G:     pipe G
> - * @H:     pipe H
>    */
>   
>   /**
> - * SUBTEST: pipe-%s-ts-continuation-dpms-suspend
> + * SUBTEST: ts-continuation-dpms-suspend
>    * Description: Test TS continuty with DPMS & Suspend while hanging by introducing
> - *              NOHANG flag on %arg[1]
> + *              NOHANG flag
>    * Driver requirement: i915, xe
>    * Functionality: dpms, hang, suspend, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
> - *
> - * arg[1]:
> - *
> - * @A:     pipe A
> - * @B:     pipe B
> - * @C:     pipe C
> - * @D:     pipe D
> - * @E:     pipe E
> - * @F:     pipe F
> - * @G:     pipe G
> - * @H:     pipe H
>    */
>   
>   /**
> - * SUBTEST: pipe-%s-ts-continuation-suspend
> + * SUBTEST: ts-continuation-suspend
>    * Description: Test TS continuty with Suspend while hanging by introducing NOHANG
> - *              flag on %arg[1]
> + *              flag
>    * Driver requirement: i915, xe
>    * Functionality: hang, suspend, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
> - *
> - * arg[1]:
> - *
> - * @A:     pipe A
> - * @B:     pipe B
> - * @C:     pipe C
> - * @D:     pipe D
> - * @E:     pipe E
> - * @F:     pipe F
> - * @G:     pipe G
> - * @H:     pipe H
>    */
>   
>   /**
> - * SUBTEST: pipe-%s-ts-continuation-modeset-rpm
> + * SUBTEST: ts-continuation-modeset-rpm
>    * Description: Test TS continuty during Modeset with Suspend while hanging by
> - *              introducing NOHANG flag on %arg[1]
> + *              introducing NOHANG flag
>    * Driver requirement: i915, xe
>    * Functionality: hang, rpm, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
> - *
> - * arg[1]:
> - *
> - * @A:     pipe A
> - * @B:     pipe B
> - * @C:     pipe C
> - * @D:     pipe D
> - * @E:     pipe E
> - * @F:     pipe F
> - * @G:     pipe G
> - * @H:     pipe H
>    */
>   
>   /**
> - * SUBTEST: pipe-%s-accuracy-idle
> + * SUBTEST: accuracy-idle
>    * Description: Test Accuracy of vblank events while hanging by introducing NOHANG
> - *              flag on %arg[1]
> + *              flag
>    * Driver requirement: i915, xe
>    * Functionality: hang, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
>    *
> - * SUBTEST: pipe-%s-%s
> - * Description: Test %arg[2] while hanging by introducing NOHANG flag on %arg[1]
> + * SUBTEST: %s
> + * Description: Test %arg[1] while hanging by introducing NOHANG flag
>    * Driver requirement: i915, xe
>    * Functionality: hang, vblank
>    * Mega feature: General Display Features
>    * Test category: functionality test
>    *
> - * SUBTEST: pipe-%s-%s-hang
> - * Description: Test %arg[2] with injected hang is working properly on %arg[1]
> + * SUBTEST: %s-hang
> + * Description: Test %arg[1] with injected hang is working properly
>    * Driver requirement: i915, xe
>    * Functionality: hang, vblank
>    * Mega feature: General Display Features
> @@ -164,17 +120,6 @@
>    *
>    * arg[1]:
>    *
> - * @A:     pipe A
> - * @B:     pipe B
> - * @C:     pipe C
> - * @D:     pipe D
> - * @E:     pipe E
> - * @F:     pipe F
> - * @G:     pipe G
> - * @H:     pipe H
> - *
> - * arg[2]:
> - *
>    * @query-idle:              Time taken to Query vblank counters
>    * @query-forked:            Time taken to Query vblank counters (multithreaded)
>    * @query-busy:              Time taken to Query vblank counters (during V-active)
> @@ -270,10 +215,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
>   	if (data->flags & RPM)
>   		igt_require(igt_setup_runtime_pm(fd));
>   
> -	igt_info("Beginning %s on pipe %s, connector %s\n",
> -		 igt_subtest_name(), kmstest_pipe_name(data->pipe),
> -		 igt_output_name(output));
> -
>   	if (!(data->flags & NOHANG)) {
>   		ahnd = is_i915_device(fd) ?
>   			get_reloc_ahnd(fd, 0) :
> @@ -314,9 +255,6 @@ static void run_test(data_t *data, void (*testfunc)(data_t *, int, int))
>   	if (!(data->flags & NOHANG))
>   		igt_post_hang_ring(fd, hang);
>   
> -	igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
> -		 igt_subtest_name(), kmstest_pipe_name(data->pipe), igt_output_name(output));
> -
>   	put_ahnd(ahnd);
>   
>   	/* cleanup what prepare_crtc() has done */
> @@ -342,63 +280,50 @@ pipe_output_combo_valid(igt_display_t *display,
>   static void crtc_id_subtest(data_t *data, int fd)
>   {
>   	igt_display_t *display = &data->display;
> -	igt_output_t *output;
> -	enum pipe p;
> -
> -	for_each_pipe_with_valid_output(display, p, output) {
> -		struct drm_event_vblank buf;
> -		const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
> -		unsigned crtc_id, expected_crtc_id;
> -		uint64_t val;
> -		union drm_wait_vblank vbl;
> +	enum pipe p = data->pipe;
> +	igt_output_t *output = data->output;
> +	struct drm_event_vblank buf;
> +	const uint32_t pipe_id_flag = kmstest_get_vbl_flag(p);
> +	unsigned crtc_id, expected_crtc_id;
> +	uint64_t val;
> +	union drm_wait_vblank vbl;
>   
> -		igt_display_reset(display);
> +	crtc_id = display->pipes[p].crtc_id;
> +	if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
> +		expected_crtc_id = crtc_id;
> +	else
> +		expected_crtc_id = 0;
>   
> -		igt_output_set_pipe(output, p);
> -		if (!i915_pipe_output_combo_valid(display))
> -			continue;
> +	prepare_crtc(data, fd, output);
>   
> -		igt_info("Using (pipe %s + %s) to run the subtest.\n",
> -			 kmstest_pipe_name(p), igt_output_name(output));
> +	memset(&vbl, 0, sizeof(vbl));
> +	vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> +	vbl.request.type |= pipe_id_flag;
> +	vbl.request.sequence = 1;
> +	igt_assert_eq(wait_vblank(fd, &vbl), 0);
>   
> -		crtc_id = display->pipes[p].crtc_id;
> -		if (drmGetCap(display->drm_fd, DRM_CAP_CRTC_IN_VBLANK_EVENT, &val) == 0)
> -			expected_crtc_id = crtc_id;
> -		else
> -			expected_crtc_id = 0;
> +	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +	igt_assert_eq(buf.crtc_id, expected_crtc_id);
>   
> -		data->pipe = p;
> -		prepare_crtc(data, fd, output);
> +	do_or_die(drmModePageFlip(fd, crtc_id,
> +				  data->primary_fb.fb_id,
> +				  DRM_MODE_PAGE_FLIP_EVENT, NULL));
>   
> -		memset(&vbl, 0, sizeof(vbl));
> -		vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> -		vbl.request.type |= pipe_id_flag;
> -		vbl.request.sequence = 1;
> -		igt_assert_eq(wait_vblank(fd, &vbl), 0);
> +	igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> +	igt_assert_eq(buf.crtc_id, expected_crtc_id);
>   
> -		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> -		igt_assert_eq(buf.crtc_id, expected_crtc_id);
> +	if (display->is_atomic) {
> +		igt_plane_t *primary = igt_output_get_plane(output, 0);
>   
> -		do_or_die(drmModePageFlip(fd, crtc_id,
> -					  data->primary_fb.fb_id,
> -					  DRM_MODE_PAGE_FLIP_EVENT, NULL));
> +		igt_plane_set_fb(primary, &data->primary_fb);
> +		igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
>   
>   		igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
>   		igt_assert_eq(buf.crtc_id, expected_crtc_id);
> -
> -		if (display->is_atomic) {
> -			igt_plane_t *primary = igt_output_get_plane(output, 0);
> -
> -			igt_plane_set_fb(primary, &data->primary_fb);
> -			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
> -
> -			igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> -			igt_assert_eq(buf.crtc_id, expected_crtc_id);
> -		}
> -
> -		cleanup_crtc(data, fd, output);
> -		return;
>   	}
> +
> +	cleanup_crtc(data, fd, output);
> +	return;
>   }
>   
>   static void accuracy(data_t *data, int fd, int nchildren)
> @@ -576,7 +501,7 @@ static void vblank_ts_cont(data_t *data, int fd, int nchildren)
>   			estimated_vblanks, seq2, seq1 + estimated_vblanks);
>   }
>   
> -static void run_subtests_for_pipe(data_t *data)
> +static void run_subtests(data_t *data)
>   {
>   	const struct {
>   		const char *name;
> @@ -610,30 +535,22 @@ static void run_subtests_for_pipe(data_t *data)
>   		{ }
>   	}, *m;
>   
> -	igt_fixture
> -		igt_display_require_output_on_pipe(&data->display, data->pipe);
> -
>   	for (f = funcs; f->name; f++) {
>   		for (m = modes; m->name; m++) {
>   			if (m->flags & ~(f->valid | NOHANG))
>   				continue;
>   
>   			igt_describe("Check if test run while hanging by introducing NOHANG flag.");
> -			igt_subtest_f("pipe-%s-%s-%s",
> -				      kmstest_pipe_name(data->pipe),
> -				      f->name, m->name) {
> -				int found = 0;
> -
> -				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
> +			igt_subtest_with_dynamic_f("%s-%s", f->name, m->name) {
> +				for_each_pipe_with_valid_output(&data->display, data->pipe, data->output) {
>   					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
>   						continue;
>   
> -					data->flags = m->flags | NOHANG;
> -					run_test(data, f->func);
> -
> -					found++;
> +					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
> +						data->flags = m->flags | NOHANG;
> +						run_test(data, f->func);
> +					}
>   				}
> -				igt_require_f(found, "No valid pipe/output combo found.\n");
>   			}
>   
>   			/* Skip the -hang version if NOHANG flag is set */
> @@ -641,24 +558,20 @@ static void run_subtests_for_pipe(data_t *data)
>   				continue;
>   
>   			igt_describe("Check if injected hang is working properly.");
> -			igt_subtest_f("pipe-%s-%s-%s-hang",
> -				      kmstest_pipe_name(data->pipe),
> -				      f->name, m->name) {
> +			igt_subtest_with_dynamic_f("%s-%s-hang", f->name, m->name) {
>   				igt_hang_t hang;
> -				int found = 0;
>   
>   				hang = igt_allow_hang(data->display.drm_fd, 0, 0);
> -				for_each_valid_output_on_pipe(&data->display, data->pipe, data->output) {
> +				for_each_pipe_with_valid_output(&data->display, data->pipe, data->output) {
>   					if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
>   						continue;
>   
> -					data->flags = m->flags;
> -					run_test(data, f->func);
> -
> -					found++;
> +					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name) {
> +						data->flags = m->flags;
> +						run_test(data, f->func);
> +					}
>   				}
>   				igt_disallow_hang(data->display.drm_fd, hang);
> -				igt_require_f(found, "No valid pipe/output combo found.\n");
>   			}
>   		}
>   	}
> @@ -668,23 +581,7 @@ static void invalid_subtest(data_t *data, int fd)
>   {
>   	union drm_wait_vblank vbl;
>   	unsigned long valid_flags;
> -	igt_display_t* display = &data->display;
> -	enum pipe pipe = 0;
> -	igt_output_t *output;
> -
> -	igt_display_reset(display);
> -
> -	output = igt_get_single_output_for_pipe(display, pipe);
> -	igt_require(output);
> -
> -	data->pipe = pipe;
> -	data->output = output;
> -
> -	igt_output_set_pipe(output, pipe);
> -	igt_require(i915_pipe_output_combo_valid(display));
> -
> -	igt_info("Using (pipe %s + %s) to run the subtest.\n",
> -		 kmstest_pipe_name(pipe), igt_output_name(output));
> +	igt_output_t *output = data->output;
>   
>   	prepare_crtc(data, fd, output);
>   
> @@ -738,18 +635,32 @@ igt_main
>   	}
>   
>   	igt_describe("Negative test for vblank request.");
> -	igt_subtest("invalid")
> -		invalid_subtest(&data, fd);
> +	igt_subtest_with_dynamic("invalid") {
> +		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
> +			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
> +				continue;
> +
> +			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
> +				invalid_subtest(&data, fd);
> +			break;

I see that without this patch also this subtest was anyway running on 
just one pipe/output combination, so I'm fine with taking that same 
design forward. But as we're now adding this loop and then breaking 
after the first valid combination, may be we could just add a small 
comment saying why we're breaking here as this is the one subtest which 
is different from others.

With this added,

Reviewed-by: Karthik B S <karthik.b.s@intel.com>

> +		}
> +	}
>   
>   	igt_describe("Check the vblank and flip events works with given crtc id.");
> -	igt_subtest("crtc-id")
> -		crtc_id_subtest(&data, fd);
> +	igt_subtest_with_dynamic("crtc-id") {
> +		for_each_pipe_with_valid_output(&data.display, data.pipe, data.output) {
> +			if (!pipe_output_combo_valid(&data.display, data.pipe, data.output))
> +				continue;
> +
> +			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data.pipe), data.output->name)
> +				crtc_id_subtest(&data, fd);
> +		}
> +	}
>   
> -	for_each_pipe_static(data.pipe)
> -		igt_subtest_group
> -			run_subtests_for_pipe(&data);
> +	run_subtests(&data);
>   
>   	igt_fixture {
> +		igt_display_fini(&data.display);
>   		drm_close_driver(fd);
>   	}
>   }