All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup
@ 2022-04-22  9:59 Karthik B S
  2022-04-22  9:59 ` [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic Karthik B S
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Karthik B S @ 2022-04-22  9:59 UTC (permalink / raw)
  To: igt-dev

-Convert tests to dynamic
-Replace drm function call with existing library functions
-igt_display_reset() before all subtests

v2: -Move conversion to dynamic subtest to a separate patch (Bhanu)
    -Use igt_output_get_mode to get default mode (Bhanu)
    -Add 'is_atomic' check before igt_display_commit2 (Bhanu)

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 tests/kms_async_flips.c | 194 ++++++++++++++++++++++++++++------------
 1 file changed, 137 insertions(+), 57 deletions(-)

diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
index 5e11cd43..335d6818 100644
--- a/tests/kms_async_flips.c
+++ b/tests/kms_async_flips.c
@@ -50,7 +50,7 @@ typedef struct {
 	uint32_t refresh_rate;
 	struct igt_fb bufs[4];
 	igt_display_t display;
-	drmModeConnectorPtr connector;
+	igt_output_t *output;
 	unsigned long flip_timestamp_us;
 	double flip_interval;
 	igt_pipe_crc_t *pipe_crc;
@@ -58,24 +58,9 @@ typedef struct {
 	int flip_count;
 	int frame_count;
 	bool flip_pending;
+	bool extended;
 } data_t;
 
-static drmModeConnectorPtr find_connector_for_modeset(data_t *data)
-{
-	igt_output_t *output;
-	drmModeConnectorPtr ret = NULL;
-
-	for_each_connected_output(&data->display, output) {
-		if (output->config.connector->count_modes > 0) {
-			ret = output->config.connector;
-			break;
-		}
-	}
-
-	igt_assert_f(ret, "Connector NOT found\n");
-	return ret;
-}
-
 static void flip_handler(int fd_, unsigned int sequence, unsigned int tv_sec,
 			 unsigned int tv_usec, void *_data)
 {
@@ -129,14 +114,10 @@ static void wait_flip_event(data_t *data)
 }
 
 static void make_fb(data_t *data, struct igt_fb *fb,
-		    drmModeConnectorPtr connector, int index)
+		    uint32_t width, uint32_t height, int index)
 {
-	uint32_t width, height;
 	int rec_width;
 
-	width = connector->modes[0].hdisplay;
-	height = connector->modes[0].vdisplay;
-
 	rec_width = width / (ARRAY_SIZE(data->bufs) * 2);
 
 	igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
@@ -346,9 +327,11 @@ static void test_invalid(data_t *data)
 	int ret;
 	uint32_t width, height;
 	struct igt_fb fb;
+	drmModeModeInfo *mode;
 
-	width = data->connector->modes[0].hdisplay;
-	height = data->connector->modes[0].vdisplay;
+	mode = igt_output_get_mode(data->output);
+	width = mode->hdisplay;
+	height = mode->vdisplay;
 
 	igt_require(igt_display_has_format_mod(&data->display, DRM_FORMAT_XRGB8888,
 					       I915_FORMAT_MOD_Y_TILED));
@@ -367,31 +350,60 @@ static void test_invalid(data_t *data)
 	igt_remove_fb(data->drm_fd, &fb);
 }
 
+static enum pipe get_pipe_for_output(igt_display_t *display, igt_output_t *output)
+{
+	enum pipe pipe;
+
+	for_each_pipe(display, pipe) {
+		if (igt_pipe_connector_valid(pipe, output))
+			return pipe;
+	}
+
+	igt_assert_f(false, "No pipe found for output %s\n",
+		     igt_output_name(output));
+}
+
 static void test_init(data_t *data)
 {
-	drmModeResPtr res;
-	int i, ret;
+	int i;
+	uint32_t width, height;
+	enum pipe pipe;
+	igt_plane_t *plane;
+	static uint32_t prev_output_id;
+	drmModeModeInfo *mode;
+
+	if (!prev_output_id) {
+		prev_output_id = data->output->id;
+	} else if (prev_output_id == data->output->id) {
+		return;
+	} else {
+		prev_output_id = data->output->id;
+		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
+			igt_remove_fb(data->drm_fd, &data->bufs[i]);
+	}
 
-	res = drmModeGetResources(data->drm_fd);
-	igt_assert(res);
+	igt_display_reset(&data->display);
+	igt_display_commit(&data->display);
 
-	kmstest_unset_all_crtcs(data->drm_fd, res);
+	pipe = get_pipe_for_output(&data->display, data->output);
 
-	data->connector = find_connector_for_modeset(data);
-	data->crtc_id = kmstest_find_crtc_for_connector(data->drm_fd,
-							res, data->connector, 0);
+	mode = igt_output_get_mode(data->output);
+	width = mode->hdisplay;
+	height = mode->vdisplay;
 
-	data->refresh_rate = data->connector->modes[0].vrefresh;
+	data->crtc_id = data->display.pipes[pipe].crtc_id;
+	data->refresh_rate = mode->vrefresh;
 
 	for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
-		make_fb(data, &data->bufs[i], data->connector, i);
+		make_fb(data, &data->bufs[i], width, height, i);
 
-	ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, data->bufs[0].fb_id, 0, 0,
-			     &data->connector->connector_id, 1, &data->connector->modes[0]);
+	igt_output_set_pipe(data->output, pipe);
+	plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
 
-	igt_assert(ret == 0);
+	igt_plane_set_fb(plane, &data->bufs[0]);
+	igt_plane_set_size(plane, width, height);
 
-	drmModeFreeResources(res);
+	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 }
 
 static void queue_vblank(data_t *data)
@@ -494,7 +506,8 @@ static void test_crc(data_t *data)
 	igt_draw_fill_fb(data->drm_fd, &data->bufs[!frame], 0xff0000ff);
 
 	ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, data->bufs[frame].fb_id, 0, 0,
-			     &data->connector->connector_id, 1, &data->connector->modes[0]);
+			     &data->output->config.connector->connector_id, 1,
+			     &data->output->config.connector->modes[0]);
 	igt_assert_eq(ret, 0);
 
 	data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
@@ -531,10 +544,28 @@ static void test_crc(data_t *data)
 	igt_assert_lt(data->frame_count * 2, data->flip_count);
 }
 
-igt_main
+static int opt_handler(int opt, int opt_index, void *_data)
+{
+	data_t *data = _data;
+
+	switch (opt) {
+	case 'e':
+		data->extended = true;
+		break;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+static const char help_str[] =
+	"  --e \t\tRun the extended tests\n";
+
+static data_t data;
+
+igt_main_args("e", NULL, help_str, opt_handler, &data)
 {
-	static data_t data;
 	int i;
+	igt_output_t *output;
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
@@ -548,32 +579,78 @@ igt_main
 
 	igt_describe("Verify the async flip functionality and the fps during async flips");
 	igt_subtest_group {
-		igt_fixture
-			test_init(&data);
-
 		igt_describe("Wait for page flip events in between successive asynchronous flips");
-		igt_subtest("async-flip-with-page-flip-events")
-			test_async_flip(&data, false);
+		igt_subtest("async-flip-with-page-flip-events") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_async_flip(&data, false);
+
+				if (!data.extended)
+					break;
+			}
+		}
 
 		igt_describe("Alternate between sync and async flips");
-		igt_subtest("alternate-sync-async-flip")
-			test_async_flip(&data, true);
+		igt_subtest("alternate-sync-async-flip") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_async_flip(&data, true);
+
+				if (!data.extended)
+					break;
+			}
+		}
+
 
 		igt_describe("Verify that the async flip timestamp does not coincide with either previous or next vblank");
-		igt_subtest("test-time-stamp")
-			test_timestamp(&data);
+		igt_subtest("test-time-stamp") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_timestamp(&data);
+
+				if (!data.extended)
+					break;
+			}
+		}
 
 		igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes after async flip");
-		igt_subtest("test-cursor")
-			test_cursor(&data);
+		igt_subtest("test-cursor") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_cursor(&data);
+
+				if (!data.extended)
+					break;
+			}
+		}
 
 		igt_describe("Negative case to verify if changes in fb are rejected from kernel as expected");
-		igt_subtest("invalid-async-flip")
-			test_invalid(&data);
+		igt_subtest("invalid-async-flip") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_invalid(&data);
+
+				if (!data.extended)
+					break;
+			}
+		}
 
 		igt_describe("Use CRC to verify async flip scans out the correct framebuffer");
-		igt_subtest("crc")
-			test_crc(&data);
+		igt_subtest("crc") {
+			for_each_connected_output(&data.display, output) {
+				data.output = output;
+				test_init(&data);
+				test_crc(&data);
+
+				if (!data.extended)
+					break;
+			}
+		}
 
 		igt_fixture {
 			for (i = 0; i < ARRAY_SIZE(data.bufs); i++)
@@ -581,6 +658,9 @@ igt_main
 		}
 	}
 
-	igt_fixture
+	igt_fixture {
+		igt_display_reset(&data.display);
+		igt_display_commit(&data.display);
 		igt_display_fini(&data.display);
+	}
 }
-- 
2.22.0

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

* [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic
  2022-04-22  9:59 [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Karthik B S
@ 2022-04-22  9:59 ` Karthik B S
  2022-04-22 13:55   ` Modem, Bhanuprakash
  2022-04-22 13:55 ` [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Modem, Bhanuprakash
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Karthik B S @ 2022-04-22  9:59 UTC (permalink / raw)
  To: igt-dev

Convert the subtests to dynamic at pipe/output level.

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 tests/kms_async_flips.c | 164 ++++++++++++++++++++--------------------
 1 file changed, 83 insertions(+), 81 deletions(-)

diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
index 335d6818..c00590aa 100644
--- a/tests/kms_async_flips.c
+++ b/tests/kms_async_flips.c
@@ -350,56 +350,39 @@ static void test_invalid(data_t *data)
 	igt_remove_fb(data->drm_fd, &fb);
 }
 
-static enum pipe get_pipe_for_output(igt_display_t *display, igt_output_t *output)
-{
-	enum pipe pipe;
-
-	for_each_pipe(display, pipe) {
-		if (igt_pipe_connector_valid(pipe, output))
-			return pipe;
-	}
-
-	igt_assert_f(false, "No pipe found for output %s\n",
-		     igt_output_name(output));
-}
-
-static void test_init(data_t *data)
+static void test_init(data_t *data, enum pipe pipe)
 {
 	int i;
 	uint32_t width, height;
-	enum pipe pipe;
 	igt_plane_t *plane;
 	static uint32_t prev_output_id;
 	drmModeModeInfo *mode;
 
-	if (!prev_output_id) {
-		prev_output_id = data->output->id;
-	} else if (prev_output_id == data->output->id) {
-		return;
-	} else {
-		prev_output_id = data->output->id;
-		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
-			igt_remove_fb(data->drm_fd, &data->bufs[i]);
-	}
-
-	igt_display_reset(&data->display);
-	igt_display_commit(&data->display);
-
-	pipe = get_pipe_for_output(&data->display, data->output);
-
 	mode = igt_output_get_mode(data->output);
 	width = mode->hdisplay;
 	height = mode->vdisplay;
 
+	igt_display_reset(&data->display);
+	igt_display_commit(&data->display);
+
 	data->crtc_id = data->display.pipes[pipe].crtc_id;
 	data->refresh_rate = mode->vrefresh;
 
-	for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
-		make_fb(data, &data->bufs[i], width, height, i);
-
 	igt_output_set_pipe(data->output, pipe);
 	plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
 
+	if (prev_output_id != data->output->id) {
+		prev_output_id = data->output->id;
+
+		if (data->bufs[0].fb_id) {
+			for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
+				igt_remove_fb(data->drm_fd, &data->bufs[i]);
+		}
+
+		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
+			make_fb(data, &data->bufs[i], width, height, i);
+	}
+
 	igt_plane_set_fb(plane, &data->bufs[0]);
 	igt_plane_set_size(plane, width, height);
 
@@ -566,6 +549,7 @@ igt_main_args("e", NULL, help_str, opt_handler, &data)
 {
 	int i;
 	igt_output_t *output;
+	enum pipe pipe;
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
@@ -580,75 +564,93 @@ igt_main_args("e", NULL, help_str, opt_handler, &data)
 	igt_describe("Verify the async flip functionality and the fps during async flips");
 	igt_subtest_group {
 		igt_describe("Wait for page flip events in between successive asynchronous flips");
-		igt_subtest("async-flip-with-page-flip-events") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_async_flip(&data, false);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("async-flip-with-page-flip-events") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_async_flip(&data, false);
+					}
+					if (!data.extended)
+						break;
+				}
 			}
 		}
 
 		igt_describe("Alternate between sync and async flips");
-		igt_subtest("alternate-sync-async-flip") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_async_flip(&data, true);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("alternate-sync-async-flip") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_async_flip(&data, true);
+					}
+					if (!data.extended)
+						break;
+				}
 			}
 		}
 
 
 		igt_describe("Verify that the async flip timestamp does not coincide with either previous or next vblank");
-		igt_subtest("test-time-stamp") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_timestamp(&data);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("test-time-stamp") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_timestamp(&data);
+					}
+					if (!data.extended)
+						break;
+				}
 			}
 		}
 
 		igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes after async flip");
-		igt_subtest("test-cursor") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_cursor(&data);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("test-cursor") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_cursor(&data);
+					}
+					if (!data.extended)
+						break;
+				}
 			}
 		}
 
 		igt_describe("Negative case to verify if changes in fb are rejected from kernel as expected");
-		igt_subtest("invalid-async-flip") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_invalid(&data);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("invalid-async-flip") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_invalid(&data);
+					}
+					if (!data.extended)
+						break;
+				}
 			}
 		}
 
 		igt_describe("Use CRC to verify async flip scans out the correct framebuffer");
-		igt_subtest("crc") {
-			for_each_connected_output(&data.display, output) {
-				data.output = output;
-				test_init(&data);
-				test_crc(&data);
-
-				if (!data.extended)
-					break;
+		igt_subtest_with_dynamic("crc") {
+			for_each_pipe_static(pipe) {
+				for_each_valid_output_on_pipe(&data.display, pipe, output) {
+					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
+						data.output = output;
+						test_init(&data, pipe);
+						test_crc(&data);
+					}
+					if (!data.extended)
+							break;
+				}
 			}
 		}
 
-- 
2.22.0

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

* Re: [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup
  2022-04-22  9:59 [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Karthik B S
  2022-04-22  9:59 ` [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic Karthik B S
@ 2022-04-22 13:55 ` Modem, Bhanuprakash
  2022-04-25 10:26   ` Karthik B S
  2022-04-22 15:20 ` [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,v2,1/2] " Patchwork
  2022-04-22 15:53 ` [igt-dev] ✗ Fi.CI.BAT: failure " Patchwork
  3 siblings, 1 reply; 8+ messages in thread
From: Modem, Bhanuprakash @ 2022-04-22 13:55 UTC (permalink / raw)
  To: Karthik B S, igt-dev

On Fri-22-04-2022 03:29 pm, Karthik B S wrote:
> -Convert tests to dynamic
> -Replace drm function call with existing library functions
> -igt_display_reset() before all subtests
> 
> v2: -Move conversion to dynamic subtest to a separate patch (Bhanu)
>      -Use igt_output_get_mode to get default mode (Bhanu)
>      -Add 'is_atomic' check before igt_display_commit2 (Bhanu)
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>   tests/kms_async_flips.c | 194 ++++++++++++++++++++++++++++------------
>   1 file changed, 137 insertions(+), 57 deletions(-)
> 
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
> index 5e11cd43..335d6818 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -50,7 +50,7 @@ typedef struct {
>   	uint32_t refresh_rate;
>   	struct igt_fb bufs[4];
>   	igt_display_t display;
> -	drmModeConnectorPtr connector;
> +	igt_output_t *output;
>   	unsigned long flip_timestamp_us;
>   	double flip_interval;
>   	igt_pipe_crc_t *pipe_crc;
> @@ -58,24 +58,9 @@ typedef struct {
>   	int flip_count;
>   	int frame_count;
>   	bool flip_pending;
> +	bool extended;
>   } data_t;
>   
> -static drmModeConnectorPtr find_connector_for_modeset(data_t *data)
> -{
> -	igt_output_t *output;
> -	drmModeConnectorPtr ret = NULL;
> -
> -	for_each_connected_output(&data->display, output) {
> -		if (output->config.connector->count_modes > 0) {
> -			ret = output->config.connector;
> -			break;
> -		}
> -	}
> -
> -	igt_assert_f(ret, "Connector NOT found\n");
> -	return ret;
> -}
> -
>   static void flip_handler(int fd_, unsigned int sequence, unsigned int tv_sec,
>   			 unsigned int tv_usec, void *_data)
>   {
> @@ -129,14 +114,10 @@ static void wait_flip_event(data_t *data)
>   }
>   
>   static void make_fb(data_t *data, struct igt_fb *fb,
> -		    drmModeConnectorPtr connector, int index)
> +		    uint32_t width, uint32_t height, int index)
>   {
> -	uint32_t width, height;
>   	int rec_width;
>   
> -	width = connector->modes[0].hdisplay;
> -	height = connector->modes[0].vdisplay;
> -
>   	rec_width = width / (ARRAY_SIZE(data->bufs) * 2);
>   
>   	igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
> @@ -346,9 +327,11 @@ static void test_invalid(data_t *data)
>   	int ret;
>   	uint32_t width, height;
>   	struct igt_fb fb;
> +	drmModeModeInfo *mode;
>   
> -	width = data->connector->modes[0].hdisplay;
> -	height = data->connector->modes[0].vdisplay;
> +	mode = igt_output_get_mode(data->output);
> +	width = mode->hdisplay;
> +	height = mode->vdisplay;
>   
>   	igt_require(igt_display_has_format_mod(&data->display, DRM_FORMAT_XRGB8888,
>   					       I915_FORMAT_MOD_Y_TILED));
> @@ -367,31 +350,60 @@ static void test_invalid(data_t *data)
>   	igt_remove_fb(data->drm_fd, &fb);
>   }
>   
> +static enum pipe get_pipe_for_output(igt_display_t *display, igt_output_t *output)
> +{
> +	enum pipe pipe;
> +
> +	for_each_pipe(display, pipe) {
> +		if (igt_pipe_connector_valid(pipe, output))
> +			return pipe;
> +	}
> +
> +	igt_assert_f(false, "No pipe found for output %s\n",
> +		     igt_output_name(output));
> +}
> +
>   static void test_init(data_t *data)
>   {
> -	drmModeResPtr res;
> -	int i, ret;
> +	int i;
> +	uint32_t width, height;
> +	enum pipe pipe;
> +	igt_plane_t *plane;
> +	static uint32_t prev_output_id;
> +	drmModeModeInfo *mode;
> +
> +	if (!prev_output_id) {
> +		prev_output_id = data->output->id;
> +	} else if (prev_output_id == data->output->id) {
> +		return;
> +	} else {
> +		prev_output_id = data->output->id;
> +		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> +			igt_remove_fb(data->drm_fd, &data->bufs[i]);
> +	}

I understood, this logic is to optimize fb creation. Still, I can feel 
regular method of fb creation is best option to me, b'coz crc subtest is 
filling the FBs with different content.

Subtest Start --> create fb --> do some stuff --> destroy fb --> End

>   
> -	res = drmModeGetResources(data->drm_fd);
> -	igt_assert(res);
> +	igt_display_reset(&data->display);
> +	igt_display_commit(&data->display);
>   
> -	kmstest_unset_all_crtcs(data->drm_fd, res);
> +	pipe = get_pipe_for_output(&data->display, data->output);
>   
> -	data->connector = find_connector_for_modeset(data);
> -	data->crtc_id = kmstest_find_crtc_for_connector(data->drm_fd,
> -							res, data->connector, 0);
> +	mode = igt_output_get_mode(data->output);
> +	width = mode->hdisplay;
> +	height = mode->vdisplay;
>   
> -	data->refresh_rate = data->connector->modes[0].vrefresh;
> +	data->crtc_id = data->display.pipes[pipe].crtc_id;
> +	data->refresh_rate = mode->vrefresh;
>   
>   	for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> -		make_fb(data, &data->bufs[i], data->connector, i);
> +		make_fb(data, &data->bufs[i], width, height, i);
>   
> -	ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, data->bufs[0].fb_id, 0, 0,
> -			     &data->connector->connector_id, 1, &data->connector->modes[0]);
> +	igt_output_set_pipe(data->output, pipe);
> +	plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
>   
> -	igt_assert(ret == 0);
> +	igt_plane_set_fb(plane, &data->bufs[0]);
> +	igt_plane_set_size(plane, width, height);
>   
> -	drmModeFreeResources(res);
> +	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>   }
>   
>   static void queue_vblank(data_t *data)
> @@ -494,7 +506,8 @@ static void test_crc(data_t *data)
>   	igt_draw_fill_fb(data->drm_fd, &data->bufs[!frame], 0xff0000ff);
>   
>   	ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, data->bufs[frame].fb_id, 0, 0,
> -			     &data->connector->connector_id, 1, &data->connector->modes[0]);
> +			     &data->output->config.connector->connector_id, 1,
> +			     &data->output->config.connector->modes[0]);
>   	igt_assert_eq(ret, 0);
>   
>   	data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
> @@ -531,10 +544,28 @@ static void test_crc(data_t *data)
>   	igt_assert_lt(data->frame_count * 2, data->flip_count);
>   }
>   
> -igt_main
> +static int opt_handler(int opt, int opt_index, void *_data)
> +{
> +	data_t *data = _data;
> +
> +	switch (opt) {
> +	case 'e':
> +		data->extended = true;
> +		break;
> +	}
> +
> +	return IGT_OPT_HANDLER_SUCCESS;
> +}
> +
> +static const char help_str[] =
> +	"  --e \t\tRun the extended tests\n";
> +
> +static data_t data;
> +
> +igt_main_args("e", NULL, help_str, opt_handler, &data)
>   {
> -	static data_t data;
>   	int i;
> +	igt_output_t *output;
>   
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -548,32 +579,78 @@ igt_main
>   
>   	igt_describe("Verify the async flip functionality and the fps during async flips");
>   	igt_subtest_group {
> -		igt_fixture
> -			test_init(&data);
> -
>   		igt_describe("Wait for page flip events in between successive asynchronous flips");
> -		igt_subtest("async-flip-with-page-flip-events")
> -			test_async_flip(&data, false);
> +		igt_subtest("async-flip-with-page-flip-events") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);

Please move test_init() to test_async_flip.

Ex: We can SKIP the test without test_init() if system won't support 
monotonic_timestamp

This comment is applicable in all places.

> +				test_async_flip(&data, false);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
>   
>   		igt_describe("Alternate between sync and async flips");
> -		igt_subtest("alternate-sync-async-flip")
> -			test_async_flip(&data, true);
> +		igt_subtest("alternate-sync-async-flip") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);
> +				test_async_flip(&data, true);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
> +
>   
>   		igt_describe("Verify that the async flip timestamp does not coincide with either previous or next vblank");
> -		igt_subtest("test-time-stamp")
> -			test_timestamp(&data);
> +		igt_subtest("test-time-stamp") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);
> +				test_timestamp(&data);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
>   
>   		igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes after async flip");
> -		igt_subtest("test-cursor")
> -			test_cursor(&data);
> +		igt_subtest("test-cursor") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);
> +				test_cursor(&data);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
>   
>   		igt_describe("Negative case to verify if changes in fb are rejected from kernel as expected");
> -		igt_subtest("invalid-async-flip")
> -			test_invalid(&data);
> +		igt_subtest("invalid-async-flip") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);
> +				test_invalid(&data);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
>   
>   		igt_describe("Use CRC to verify async flip scans out the correct framebuffer");
> -		igt_subtest("crc")
> -			test_crc(&data);
> +		igt_subtest("crc") {
> +			for_each_connected_output(&data.display, output) {
> +				data.output = output;
> +				test_init(&data);
> +				test_crc(&data);
> +
> +				if (!data.extended)
> +					break;
> +			}
> +		}
>   
>   		igt_fixture {
>   			for (i = 0; i < ARRAY_SIZE(data.bufs); i++)
> @@ -581,6 +658,9 @@ igt_main
>   		}
>   	}
>   
> -	igt_fixture
> +	igt_fixture {
> +		igt_display_reset(&data.display);
> +		igt_display_commit(&data.display);
>   		igt_display_fini(&data.display);
> +	}
>   }

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

* Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic
  2022-04-22  9:59 ` [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic Karthik B S
@ 2022-04-22 13:55   ` Modem, Bhanuprakash
  2022-04-25 10:30     ` Karthik B S
  0 siblings, 1 reply; 8+ messages in thread
From: Modem, Bhanuprakash @ 2022-04-22 13:55 UTC (permalink / raw)
  To: Karthik B S, igt-dev

On Fri-22-04-2022 03:29 pm, Karthik B S wrote:
> Convert the subtests to dynamic at pipe/output level.
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>   tests/kms_async_flips.c | 164 ++++++++++++++++++++--------------------
>   1 file changed, 83 insertions(+), 81 deletions(-)
> 
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
> index 335d6818..c00590aa 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -350,56 +350,39 @@ static void test_invalid(data_t *data)
>   	igt_remove_fb(data->drm_fd, &fb);
>   }
>   
> -static enum pipe get_pipe_for_output(igt_display_t *display, igt_output_t *output)
> -{
> -	enum pipe pipe;
> -
> -	for_each_pipe(display, pipe) {
> -		if (igt_pipe_connector_valid(pipe, output))
> -			return pipe;
> -	}
> -
> -	igt_assert_f(false, "No pipe found for output %s\n",
> -		     igt_output_name(output));
> -}
> -
> -static void test_init(data_t *data)
> +static void test_init(data_t *data, enum pipe pipe)
>   {
>   	int i;
>   	uint32_t width, height;
> -	enum pipe pipe;
>   	igt_plane_t *plane;
>   	static uint32_t prev_output_id;
>   	drmModeModeInfo *mode;
>   
> -	if (!prev_output_id) {
> -		prev_output_id = data->output->id;
> -	} else if (prev_output_id == data->output->id) {
> -		return;
> -	} else {
> -		prev_output_id = data->output->id;
> -		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> -			igt_remove_fb(data->drm_fd, &data->bufs[i]);
> -	}
> -
> -	igt_display_reset(&data->display);
> -	igt_display_commit(&data->display);
> -
> -	pipe = get_pipe_for_output(&data->display, data->output);
> -
>   	mode = igt_output_get_mode(data->output);
>   	width = mode->hdisplay;
>   	height = mode->vdisplay;

I think, we need to get the mode after igt_display_reset()

>   
> +	igt_display_reset(&data->display);
> +	igt_display_commit(&data->display);
> +
>   	data->crtc_id = data->display.pipes[pipe].crtc_id;
>   	data->refresh_rate = mode->vrefresh;
>   
> -	for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> -		make_fb(data, &data->bufs[i], width, height, i);
> -
>   	igt_output_set_pipe(data->output, pipe);
>   	plane = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
>   
> +	if (prev_output_id != data->output->id) {
> +		prev_output_id = data->output->id;
> +
> +		if (data->bufs[0].fb_id) {
> +			for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> +				igt_remove_fb(data->drm_fd, &data->bufs[i]);
> +		}
> +
> +		for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
> +			make_fb(data, &data->bufs[i], width, height, i);
> +	}
> +
>   	igt_plane_set_fb(plane, &data->bufs[0]);
>   	igt_plane_set_size(plane, width, height);
>   
> @@ -566,6 +549,7 @@ igt_main_args("e", NULL, help_str, opt_handler, &data)
>   {
>   	int i;
>   	igt_output_t *output;
> +	enum pipe pipe;
>   
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -580,75 +564,93 @@ igt_main_args("e", NULL, help_str, opt_handler, &data)
>   	igt_describe("Verify the async flip functionality and the fps during async flips");
>   	igt_subtest_group {
>   		igt_describe("Wait for page flip events in between successive asynchronous flips");
> -		igt_subtest("async-flip-with-page-flip-events") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_async_flip(&data, false);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("async-flip-with-page-flip-events") {
> +			for_each_pipe_static(pipe) {

So, by default we are going to run tests on all pipes with single 
output. With extended flag, we'll run on all pipe/output combinations.

I think its better to limit to single pipe (or) first & last pipes by 
default.

> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_async_flip(&data, false);
> +					}
> +					if (!data.extended)
> +						break;
> +				}
>   			}
>   		}
>   
>   		igt_describe("Alternate between sync and async flips");
> -		igt_subtest("alternate-sync-async-flip") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_async_flip(&data, true);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("alternate-sync-async-flip") {
> +			for_each_pipe_static(pipe) {
> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_async_flip(&data, true);
> +					}
> +					if (!data.extended)
> +						break;
> +				}
>   			}
>   		}
>   
>   
>   		igt_describe("Verify that the async flip timestamp does not coincide with either previous or next vblank");
> -		igt_subtest("test-time-stamp") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_timestamp(&data);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("test-time-stamp") {
> +			for_each_pipe_static(pipe) {
> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_timestamp(&data);
> +					}
> +					if (!data.extended)
> +						break;
> +				}
>   			}
>   		}
>   
>   		igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes after async flip");
> -		igt_subtest("test-cursor") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_cursor(&data);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("test-cursor") {
> +			for_each_pipe_static(pipe) {
> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_cursor(&data);
> +					}
> +					if (!data.extended)
> +						break;
> +				}
>   			}
>   		}
>   
>   		igt_describe("Negative case to verify if changes in fb are rejected from kernel as expected");
> -		igt_subtest("invalid-async-flip") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_invalid(&data);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("invalid-async-flip") {
> +			for_each_pipe_static(pipe) {
> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_invalid(&data);
> +					}
> +					if (!data.extended)
> +						break;
> +				}
>   			}
>   		}
>   
>   		igt_describe("Use CRC to verify async flip scans out the correct framebuffer");
> -		igt_subtest("crc") {
> -			for_each_connected_output(&data.display, output) {
> -				data.output = output;
> -				test_init(&data);
> -				test_crc(&data);
> -
> -				if (!data.extended)
> -					break;
> +		igt_subtest_with_dynamic("crc") {
> +			for_each_pipe_static(pipe) {
> +				for_each_valid_output_on_pipe(&data.display, pipe, output) {
> +					igt_dynamic_f("%s-pipe-%s", output->name, kmstest_pipe_name(pipe)) {
> +						data.output = output;
> +						test_init(&data, pipe);
> +						test_crc(&data);
> +					}
> +					if (!data.extended)
> +							break;
> +				}
>   			}
>   		}
>   

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

* [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,v2,1/2] tests/kms_async_flips: Test Cleanup
  2022-04-22  9:59 [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Karthik B S
  2022-04-22  9:59 ` [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic Karthik B S
  2022-04-22 13:55 ` [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Modem, Bhanuprakash
@ 2022-04-22 15:20 ` Patchwork
  2022-04-22 15:53 ` [igt-dev] ✗ Fi.CI.BAT: failure " Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2022-04-22 15:20 UTC (permalink / raw)
  To: Karthik B S; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_async_flips: Test Cleanup
URL   : https://patchwork.freedesktop.org/series/102990/
State : warning

== Summary ==

Pipeline status: FAILED.

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

test:ninja-test-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/21634992):
  Ok:                   22
  Expected Fail:         3
  Fail:                289
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1650640772:step_script
  section_start:1650640772:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1726 matching files and directories   
  Uploading artifacts as "archive" to coordinator... 201 Created  id=21634992 responseStatus=201 Created token=3rzzuoAQ
  section_end:1650640783:upload_artifacts_on_failure
  section_start:1650640783:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1650640785:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

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

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

* [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/2] tests/kms_async_flips: Test Cleanup
  2022-04-22  9:59 [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Karthik B S
                   ` (2 preceding siblings ...)
  2022-04-22 15:20 ` [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,v2,1/2] " Patchwork
@ 2022-04-22 15:53 ` Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2022-04-22 15:53 UTC (permalink / raw)
  To: Karthik B S; +Cc: igt-dev

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

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_async_flips: Test Cleanup
URL   : https://patchwork.freedesktop.org/series/102990/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11543 -> IGTPW_6981
====================================================

Summary
-------

  **FAILURE**

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

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

Participating hosts (45 -> 44)
------------------------------

  Additional (1): fi-hsw-4770 
  Missing    (2): fi-bsw-cyan fi-pnv-d510 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem_contexts:
    - fi-bdw-5557u:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@gt_timelines:
    - bat-dg1-5:          [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-dg1-5/igt@i915_selftest@live@gt_timelines.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-dg1-5/igt@i915_selftest@live@gt_timelines.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][5] ([fdo#109271]) +9 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@gem_huc_copy@huc-copy.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#2190])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][7] ([i915#4613]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_ringfill@basic-all:
    - bat-dg1-5:          [PASS][8] -> [TIMEOUT][9] ([i915#5709])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-dg1-5/igt@gem_ringfill@basic-all.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-dg1-5/igt@gem_ringfill@basic-all.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([i915#3282])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#3012])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][12] ([i915#3012])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@gt_lrc:
    - fi-bsw-n3050:       [PASS][13] -> [DMESG-FAIL][14] ([i915#2373])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-bsw-n3050/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        NOTRUN -> [INCOMPLETE][15] ([i915#4785])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-6:          [PASS][16] -> [DMESG-FAIL][17] ([i915#4494] / [i915#4957])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
    - fi-snb-2600:        [PASS][18] -> [INCOMPLETE][19] ([i915#3921])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][20] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][21] ([fdo#111827]) +8 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][22] ([i915#4070] / [i915#4103]) +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][23] ([fdo#109285] / [i915#4098])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-hsw-4770:        NOTRUN -> [SKIP][24] ([fdo#109271] / [i915#533])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][25] ([i915#4070] / [i915#533])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-rkl-11600:       NOTRUN -> [SKIP][26] ([i915#1072]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_psr@primary_mmap_gtt.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#1072]) +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@kms_psr@primary_mmap_gtt.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][28] ([i915#3555] / [i915#4098])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][29] ([i915#3301] / [i915#3708])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - fi-rkl-11600:       NOTRUN -> [SKIP][30] ([i915#3291] / [i915#3708]) +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@prime_vgem@basic-write.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][31] ([fdo#109271] / [i915#4312] / [i915#5594])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-hsw-4770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - {bat-rpls-2}:       [DMESG-WARN][32] ([i915#4391]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-rkl-11600:       [INCOMPLETE][34] ([i915#5127]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@reset:
    - {bat-rpls-1}:       [DMESG-FAIL][36] -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-rpls-1/igt@i915_selftest@live@reset.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-rpls-1/igt@i915_selftest@live@reset.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - bat-adlp-4:         [DMESG-WARN][38] ([i915#3576]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11543/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@a-edp1.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#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
  [i915#5270]: https://gitlab.freedesktop.org/drm/intel/issues/5270
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5356]: https://gitlab.freedesktop.org/drm/intel/issues/5356
  [i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
  [i915#5709]: https://gitlab.freedesktop.org/drm/intel/issues/5709


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

  * CI: CI-20190529 -> None
  * IGT: IGT_6449 -> IGTPW_6981

  CI-20190529: 20190529
  CI_DRM_11543: df51a64607842dbdc675b82f3390deefef9e562f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6981: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6981/index.html
  IGT_6449: 704da775abb83faa9324a665fe2992ab90f4ab03 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

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

* Re: [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup
  2022-04-22 13:55 ` [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Modem, Bhanuprakash
@ 2022-04-25 10:26   ` Karthik B S
  0 siblings, 0 replies; 8+ messages in thread
From: Karthik B S @ 2022-04-25 10:26 UTC (permalink / raw)
  To: Modem, Bhanuprakash, igt-dev

On 4/22/2022 7:25 PM, Modem, Bhanuprakash wrote:
> On Fri-22-04-2022 03:29 pm, Karthik B S wrote:
>> -Convert tests to dynamic
>> -Replace drm function call with existing library functions
>> -igt_display_reset() before all subtests
>>
>> v2: -Move conversion to dynamic subtest to a separate patch (Bhanu)
>>      -Use igt_output_get_mode to get default mode (Bhanu)
>>      -Add 'is_atomic' check before igt_display_commit2 (Bhanu)
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   tests/kms_async_flips.c | 194 ++++++++++++++++++++++++++++------------
>>   1 file changed, 137 insertions(+), 57 deletions(-)
>>
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index 5e11cd43..335d6818 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -50,7 +50,7 @@ typedef struct {
>>       uint32_t refresh_rate;
>>       struct igt_fb bufs[4];
>>       igt_display_t display;
>> -    drmModeConnectorPtr connector;
>> +    igt_output_t *output;
>>       unsigned long flip_timestamp_us;
>>       double flip_interval;
>>       igt_pipe_crc_t *pipe_crc;
>> @@ -58,24 +58,9 @@ typedef struct {
>>       int flip_count;
>>       int frame_count;
>>       bool flip_pending;
>> +    bool extended;
>>   } data_t;
>>   -static drmModeConnectorPtr find_connector_for_modeset(data_t *data)
>> -{
>> -    igt_output_t *output;
>> -    drmModeConnectorPtr ret = NULL;
>> -
>> -    for_each_connected_output(&data->display, output) {
>> -        if (output->config.connector->count_modes > 0) {
>> -            ret = output->config.connector;
>> -            break;
>> -        }
>> -    }
>> -
>> -    igt_assert_f(ret, "Connector NOT found\n");
>> -    return ret;
>> -}
>> -
>>   static void flip_handler(int fd_, unsigned int sequence, unsigned 
>> int tv_sec,
>>                unsigned int tv_usec, void *_data)
>>   {
>> @@ -129,14 +114,10 @@ static void wait_flip_event(data_t *data)
>>   }
>>     static void make_fb(data_t *data, struct igt_fb *fb,
>> -            drmModeConnectorPtr connector, int index)
>> +            uint32_t width, uint32_t height, int index)
>>   {
>> -    uint32_t width, height;
>>       int rec_width;
>>   -    width = connector->modes[0].hdisplay;
>> -    height = connector->modes[0].vdisplay;
>> -
>>       rec_width = width / (ARRAY_SIZE(data->bufs) * 2);
>>         igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
>> @@ -346,9 +327,11 @@ static void test_invalid(data_t *data)
>>       int ret;
>>       uint32_t width, height;
>>       struct igt_fb fb;
>> +    drmModeModeInfo *mode;
>>   -    width = data->connector->modes[0].hdisplay;
>> -    height = data->connector->modes[0].vdisplay;
>> +    mode = igt_output_get_mode(data->output);
>> +    width = mode->hdisplay;
>> +    height = mode->vdisplay;
>> igt_require(igt_display_has_format_mod(&data->display, 
>> DRM_FORMAT_XRGB8888,
>>                              I915_FORMAT_MOD_Y_TILED));
>> @@ -367,31 +350,60 @@ static void test_invalid(data_t *data)
>>       igt_remove_fb(data->drm_fd, &fb);
>>   }
>>   +static enum pipe get_pipe_for_output(igt_display_t *display, 
>> igt_output_t *output)
>> +{
>> +    enum pipe pipe;
>> +
>> +    for_each_pipe(display, pipe) {
>> +        if (igt_pipe_connector_valid(pipe, output))
>> +            return pipe;
>> +    }
>> +
>> +    igt_assert_f(false, "No pipe found for output %s\n",
>> +             igt_output_name(output));
>> +}
>> +
>>   static void test_init(data_t *data)
>>   {
>> -    drmModeResPtr res;
>> -    int i, ret;
>> +    int i;
>> +    uint32_t width, height;
>> +    enum pipe pipe;
>> +    igt_plane_t *plane;
>> +    static uint32_t prev_output_id;
>> +    drmModeModeInfo *mode;
>> +
>> +    if (!prev_output_id) {
>> +        prev_output_id = data->output->id;
>> +    } else if (prev_output_id == data->output->id) {
>> +        return;
>> +    } else {
>> +        prev_output_id = data->output->id;
>> +        for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> +            igt_remove_fb(data->drm_fd, &data->bufs[i]);
>> +    }
>
> I understood, this logic is to optimize fb creation. Still, I can feel 
> regular method of fb creation is best option to me, b'coz crc subtest 
> is filling the FBs with different content.
>
> Subtest Start --> create fb --> do some stuff --> destroy fb --> End

Thank you for the review.

Using this check to avoid duplication in fb creation as without using 
'e' option we would just need to create the fb's once. Also, this will 
help in save some execution time.
The CRC subtest will not affect other subtests as it is running at the 
end. Also, all the other subtests do not depend on what content is 
present and is actually just verifying based on commit and speed of 
async flips.
Will update this patch to use same logic as the one used in patch 2.
>
>>   -    res = drmModeGetResources(data->drm_fd);
>> -    igt_assert(res);
>> +    igt_display_reset(&data->display);
>> +    igt_display_commit(&data->display);
>>   -    kmstest_unset_all_crtcs(data->drm_fd, res);
>> +    pipe = get_pipe_for_output(&data->display, data->output);
>>   -    data->connector = find_connector_for_modeset(data);
>> -    data->crtc_id = kmstest_find_crtc_for_connector(data->drm_fd,
>> -                            res, data->connector, 0);
>> +    mode = igt_output_get_mode(data->output);
>> +    width = mode->hdisplay;
>> +    height = mode->vdisplay;
>>   -    data->refresh_rate = data->connector->modes[0].vrefresh;
>> +    data->crtc_id = data->display.pipes[pipe].crtc_id;
>> +    data->refresh_rate = mode->vrefresh;
>>         for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> -        make_fb(data, &data->bufs[i], data->connector, i);
>> +        make_fb(data, &data->bufs[i], width, height, i);
>>   -    ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, 
>> data->bufs[0].fb_id, 0, 0,
>> -                 &data->connector->connector_id, 1, 
>> &data->connector->modes[0]);
>> +    igt_output_set_pipe(data->output, pipe);
>> +    plane = igt_output_get_plane_type(data->output, 
>> DRM_PLANE_TYPE_PRIMARY);
>>   -    igt_assert(ret == 0);
>> +    igt_plane_set_fb(plane, &data->bufs[0]);
>> +    igt_plane_set_size(plane, width, height);
>>   -    drmModeFreeResources(res);
>> +    igt_display_commit2(&data->display, data->display.is_atomic ? 
>> COMMIT_ATOMIC : COMMIT_LEGACY);
>>   }
>>     static void queue_vblank(data_t *data)
>> @@ -494,7 +506,8 @@ static void test_crc(data_t *data)
>>       igt_draw_fill_fb(data->drm_fd, &data->bufs[!frame], 0xff0000ff);
>>         ret = drmModeSetCrtc(data->drm_fd, data->crtc_id, 
>> data->bufs[frame].fb_id, 0, 0,
>> -                 &data->connector->connector_id, 1, 
>> &data->connector->modes[0]);
>> + &data->output->config.connector->connector_id, 1,
>> + &data->output->config.connector->modes[0]);
>>       igt_assert_eq(ret, 0);
>>         data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
>> @@ -531,10 +544,28 @@ static void test_crc(data_t *data)
>>       igt_assert_lt(data->frame_count * 2, data->flip_count);
>>   }
>>   -igt_main
>> +static int opt_handler(int opt, int opt_index, void *_data)
>> +{
>> +    data_t *data = _data;
>> +
>> +    switch (opt) {
>> +    case 'e':
>> +        data->extended = true;
>> +        break;
>> +    }
>> +
>> +    return IGT_OPT_HANDLER_SUCCESS;
>> +}
>> +
>> +static const char help_str[] =
>> +    "  --e \t\tRun the extended tests\n";
>> +
>> +static data_t data;
>> +
>> +igt_main_args("e", NULL, help_str, opt_handler, &data)
>>   {
>> -    static data_t data;
>>       int i;
>> +    igt_output_t *output;
>>         igt_fixture {
>>           data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> @@ -548,32 +579,78 @@ igt_main
>>         igt_describe("Verify the async flip functionality and the fps 
>> during async flips");
>>       igt_subtest_group {
>> -        igt_fixture
>> -            test_init(&data);
>> -
>>           igt_describe("Wait for page flip events in between 
>> successive asynchronous flips");
>> -        igt_subtest("async-flip-with-page-flip-events")
>> -            test_async_flip(&data, false);
>> +        igt_subtest("async-flip-with-page-flip-events") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>
> Please move test_init() to test_async_flip.
>
> Ex: We can SKIP the test without test_init() if system won't support 
> monotonic_timestamp
>
> This comment is applicable in all places.

Sure. Will update the functions accordingly.

Thanks,
Karthik.B.S
>
>> + test_async_flip(&data, false);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>>             igt_describe("Alternate between sync and async flips");
>> -        igt_subtest("alternate-sync-async-flip")
>> -            test_async_flip(&data, true);
>> +        igt_subtest("alternate-sync-async-flip") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>> +                test_async_flip(&data, true);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>> +
>>             igt_describe("Verify that the async flip timestamp does 
>> not coincide with either previous or next vblank");
>> -        igt_subtest("test-time-stamp")
>> -            test_timestamp(&data);
>> +        igt_subtest("test-time-stamp") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>> +                test_timestamp(&data);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>>             igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR 
>> passes after async flip");
>> -        igt_subtest("test-cursor")
>> -            test_cursor(&data);
>> +        igt_subtest("test-cursor") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>> +                test_cursor(&data);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>>             igt_describe("Negative case to verify if changes in fb 
>> are rejected from kernel as expected");
>> -        igt_subtest("invalid-async-flip")
>> -            test_invalid(&data);
>> +        igt_subtest("invalid-async-flip") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>> +                test_invalid(&data);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>>             igt_describe("Use CRC to verify async flip scans out the 
>> correct framebuffer");
>> -        igt_subtest("crc")
>> -            test_crc(&data);
>> +        igt_subtest("crc") {
>> +            for_each_connected_output(&data.display, output) {
>> +                data.output = output;
>> +                test_init(&data);
>> +                test_crc(&data);
>> +
>> +                if (!data.extended)
>> +                    break;
>> +            }
>> +        }
>>             igt_fixture {
>>               for (i = 0; i < ARRAY_SIZE(data.bufs); i++)
>> @@ -581,6 +658,9 @@ igt_main
>>           }
>>       }
>>   -    igt_fixture
>> +    igt_fixture {
>> +        igt_display_reset(&data.display);
>> +        igt_display_commit(&data.display);
>>           igt_display_fini(&data.display);
>> +    }
>>   }
>

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

* Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic
  2022-04-22 13:55   ` Modem, Bhanuprakash
@ 2022-04-25 10:30     ` Karthik B S
  0 siblings, 0 replies; 8+ messages in thread
From: Karthik B S @ 2022-04-25 10:30 UTC (permalink / raw)
  To: Modem, Bhanuprakash, igt-dev

On 4/22/2022 7:25 PM, Modem, Bhanuprakash wrote:
> On Fri-22-04-2022 03:29 pm, Karthik B S wrote:
>> Convert the subtests to dynamic at pipe/output level.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   tests/kms_async_flips.c | 164 ++++++++++++++++++++--------------------
>>   1 file changed, 83 insertions(+), 81 deletions(-)
>>
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index 335d6818..c00590aa 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -350,56 +350,39 @@ static void test_invalid(data_t *data)
>>       igt_remove_fb(data->drm_fd, &fb);
>>   }
>>   -static enum pipe get_pipe_for_output(igt_display_t *display, 
>> igt_output_t *output)
>> -{
>> -    enum pipe pipe;
>> -
>> -    for_each_pipe(display, pipe) {
>> -        if (igt_pipe_connector_valid(pipe, output))
>> -            return pipe;
>> -    }
>> -
>> -    igt_assert_f(false, "No pipe found for output %s\n",
>> -             igt_output_name(output));
>> -}
>> -
>> -static void test_init(data_t *data)
>> +static void test_init(data_t *data, enum pipe pipe)
>>   {
>>       int i;
>>       uint32_t width, height;
>> -    enum pipe pipe;
>>       igt_plane_t *plane;
>>       static uint32_t prev_output_id;
>>       drmModeModeInfo *mode;
>>   -    if (!prev_output_id) {
>> -        prev_output_id = data->output->id;
>> -    } else if (prev_output_id == data->output->id) {
>> -        return;
>> -    } else {
>> -        prev_output_id = data->output->id;
>> -        for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> -            igt_remove_fb(data->drm_fd, &data->bufs[i]);
>> -    }
>> -
>> -    igt_display_reset(&data->display);
>> -    igt_display_commit(&data->display);
>> -
>> -    pipe = get_pipe_for_output(&data->display, data->output);
>> -
>>       mode = igt_output_get_mode(data->output);
>>       width = mode->hdisplay;
>>       height = mode->vdisplay;
>
> I think, we need to get the mode after igt_display_reset()

Thank you for the review.

Sure, will update this.

>
>>   + igt_display_reset(&data->display);
>> +    igt_display_commit(&data->display);
>> +
>>       data->crtc_id = data->display.pipes[pipe].crtc_id;
>>       data->refresh_rate = mode->vrefresh;
>>   -    for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> -        make_fb(data, &data->bufs[i], width, height, i);
>> -
>>       igt_output_set_pipe(data->output, pipe);
>>       plane = igt_output_get_plane_type(data->output, 
>> DRM_PLANE_TYPE_PRIMARY);
>>   +    if (prev_output_id != data->output->id) {
>> +        prev_output_id = data->output->id;
>> +
>> +        if (data->bufs[0].fb_id) {
>> +            for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> +                igt_remove_fb(data->drm_fd, &data->bufs[i]);
>> +        }
>> +
>> +        for (i = 0; i < ARRAY_SIZE(data->bufs); i++)
>> +            make_fb(data, &data->bufs[i], width, height, i);
>> +    }
>> +
>>       igt_plane_set_fb(plane, &data->bufs[0]);
>>       igt_plane_set_size(plane, width, height);
>>   @@ -566,6 +549,7 @@ igt_main_args("e", NULL, help_str, opt_handler, 
>> &data)
>>   {
>>       int i;
>>       igt_output_t *output;
>> +    enum pipe pipe;
>>         igt_fixture {
>>           data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> @@ -580,75 +564,93 @@ igt_main_args("e", NULL, help_str, opt_handler, 
>> &data)
>>       igt_describe("Verify the async flip functionality and the fps 
>> during async flips");
>>       igt_subtest_group {
>>           igt_describe("Wait for page flip events in between 
>> successive asynchronous flips");
>> -        igt_subtest("async-flip-with-page-flip-events") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_async_flip(&data, false);
>> -
>> -                if (!data.extended)
>> -                    break;
>> + igt_subtest_with_dynamic("async-flip-with-page-flip-events") {
>> +            for_each_pipe_static(pipe) {
>
> So, by default we are going to run tests on all pipes with single 
> output. With extended flag, we'll run on all pipe/output combinations.
>
> I think its better to limit to single pipe (or) first & last pipes by 
> default.

As this is pipe feature, I've limited the execution to just one 
output(without extended mode) and running this on all pipes.

Thanks,
Karthik.B.S
>
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_async_flip(&data, false);
>> +                    }
>> +                    if (!data.extended)
>> +                        break;
>> +                }
>>               }
>>           }
>>             igt_describe("Alternate between sync and async flips");
>> -        igt_subtest("alternate-sync-async-flip") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_async_flip(&data, true);
>> -
>> -                if (!data.extended)
>> -                    break;
>> +        igt_subtest_with_dynamic("alternate-sync-async-flip") {
>> +            for_each_pipe_static(pipe) {
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_async_flip(&data, true);
>> +                    }
>> +                    if (!data.extended)
>> +                        break;
>> +                }
>>               }
>>           }
>>               igt_describe("Verify that the async flip timestamp does 
>> not coincide with either previous or next vblank");
>> -        igt_subtest("test-time-stamp") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_timestamp(&data);
>> -
>> -                if (!data.extended)
>> -                    break;
>> +        igt_subtest_with_dynamic("test-time-stamp") {
>> +            for_each_pipe_static(pipe) {
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_timestamp(&data);
>> +                    }
>> +                    if (!data.extended)
>> +                        break;
>> +                }
>>               }
>>           }
>>             igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR 
>> passes after async flip");
>> -        igt_subtest("test-cursor") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_cursor(&data);
>> -
>> -                if (!data.extended)
>> -                    break;
>> +        igt_subtest_with_dynamic("test-cursor") {
>> +            for_each_pipe_static(pipe) {
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_cursor(&data);
>> +                    }
>> +                    if (!data.extended)
>> +                        break;
>> +                }
>>               }
>>           }
>>             igt_describe("Negative case to verify if changes in fb 
>> are rejected from kernel as expected");
>> -        igt_subtest("invalid-async-flip") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_invalid(&data);
>> -
>> -                if (!data.extended)
>> -                    break;
>> +        igt_subtest_with_dynamic("invalid-async-flip") {
>> +            for_each_pipe_static(pipe) {
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_invalid(&data);
>> +                    }
>> +                    if (!data.extended)
>> +                        break;
>> +                }
>>               }
>>           }
>>             igt_describe("Use CRC to verify async flip scans out the 
>> correct framebuffer");
>> -        igt_subtest("crc") {
>> -            for_each_connected_output(&data.display, output) {
>> -                data.output = output;
>> -                test_init(&data);
>> -                test_crc(&data);
>> -
>> -                if (!data.extended)
>> -                    break;
>> +        igt_subtest_with_dynamic("crc") {
>> +            for_each_pipe_static(pipe) {
>> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
>> +                    igt_dynamic_f("%s-pipe-%s", output->name, 
>> kmstest_pipe_name(pipe)) {
>> +                        data.output = output;
>> +                        test_init(&data, pipe);
>> +                        test_crc(&data);
>> +                    }
>> +                    if (!data.extended)
>> +                            break;
>> +                }
>>               }
>>           }
>

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

end of thread, other threads:[~2022-04-25 10:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22  9:59 [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Karthik B S
2022-04-22  9:59 ` [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_async_flips: Convert tests to dynamic Karthik B S
2022-04-22 13:55   ` Modem, Bhanuprakash
2022-04-25 10:30     ` Karthik B S
2022-04-22 13:55 ` [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_async_flips: Test Cleanup Modem, Bhanuprakash
2022-04-25 10:26   ` Karthik B S
2022-04-22 15:20 ` [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,v2,1/2] " Patchwork
2022-04-22 15:53 ` [igt-dev] ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.