[PATCH i-g-t v2 0/7] benchmarks/kms_fb_stress: Test all possible combinations of formats

[PATCH i-g-t v2 0/7] benchmarks/kms_fb_stress: Test all possible combinations of formats

[Arthur Grillo]

This is the second version of the series. After some discussion[1],
I changed how to tackle the problem.

With the upcoming addition of YUV format to VKMS[2] it would be great to
test the their performance. So, test all possible combinations of
formats.

Also, to focus in an combination subset, add command-line options to
arbitrarily select the format of each plane and writeback.

- PATCH #1: Just a non-related assert that I found needed while developing
- PATCH #2: Log the KMS structure
- PATCH #3: Moving code, should be noop
- PATCH #4 & #5: Fix a couple of bugs found while developing patch #6
- PATCH #6: Test all possible combinations
- PATCH #7: Add command-line options

[1]: https://lore.kernel.org/all/6979cd2e-2b00-4dc4-8e41-66b435cf7ea8@riseup.net/
[2]: https://lore.kernel.org/all/20240304-yuv-v4-11-76beac8e9793@bootlin.com/ 

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
Changes in v2:
- Change how the command-line options work, now they pin the specific
  format and let the other change.
- Add a patch to test all the possible formats combination
- Add a couple of patches to fix some bugs
- Move the KMS logging patch to the beginning of the series
- Move the default_kms definition to the static struct data_t variable
- Link to v1: https://lore.kernel.org/r/20240226-kms_fb_stress-dev-v1-0-1c14942b1244@riseup.net

---
Arthur Grillo (7):
      benchmarks/kms_fb_stress: Assert that we have an supported pipe
      benchmarks/kms_fb_stress: Log the KMS structure
      benchmarks/kms_fb_stress: Move the stress procedure to its own function
      benchmarks/kms_fb_stress: Free resources on the stress procedure
      benchmarks/kms_fb_stress: Don't paint the FB's if the format is DRM_FORMAT_RGB565
      benchmarks/kms_fb_stress: Test every possible format combinations
      benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

 benchmarks/kms_fb_stress.c | 342 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 273 insertions(+), 69 deletions(-)
---
base-commit: 32369affc1fdb8194c4e7274d1d4dfd74f4aae88
change-id: 20240226-kms_fb_stress-dev-c93ae176f901

Best regards,
-- 
Arthur Grillo <arthurgrillo@riseup.net>

[PATCH i-g-t v2 1/7] benchmarks/kms_fb_stress: Assert that we have an supported pipe

[Arthur Grillo]

Some driver may not support overlay planes. Add a check for that.

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index 9a0e98bed8ad..8fea10ef069f 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -87,6 +87,27 @@ static igt_output_t *find_wb_output(struct data_t *data)
 	return NULL;
 }
 
+static void set_wb_pipe(struct data_t *data)
+{
+	enum pipe pipe = PIPE_NONE;
+
+	for_each_pipe(&data->display, pipe) {
+		igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
+
+		if (igt_pipe_count_plane_type(pipe_obj, DRM_PLANE_TYPE_OVERLAY) < 2)
+			continue;
+
+		igt_debug("Selecting pipe %s to %s\n",
+			  kmstest_pipe_name(pipe),
+			  igt_output_name(data->wb_output));
+		igt_output_set_pipe(data->wb_output, pipe);
+
+		return;
+	}
+
+	igt_assert_f(0, "No pipe supports this benchmark\n");
+}
+
 static void set_crtc_size(struct data_t *data)
 {
 	drmModeModeInfo *mode;
@@ -142,7 +163,6 @@ static struct kms_t default_kms = {
 igt_simple_main
 {
 	struct data_t data = {0};
-	enum pipe pipe = PIPE_NONE;
 	struct timespec then, now;
 	double elapsed;
 
@@ -163,24 +183,22 @@ igt_simple_main
 
 	data.wb_output = find_wb_output(&data);
 	igt_require(data.wb_output);
-
-	for_each_pipe(&data.display, pipe) {
-		igt_debug("Selecting pipe %s to %s\n",
-			  kmstest_pipe_name(pipe),
-			  igt_output_name(data.wb_output));
-		igt_output_set_pipe(data.wb_output, pipe);
-		break;
-	}
+	set_wb_pipe(&data);
 
 	set_crtc_size(&data);
 
 	gen_fbs(&data);
 
 	data.kms.primary.base = igt_output_get_plane_type(data.wb_output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(data.kms.primary.base != NULL);
+
 	data.kms.overlay_a.base = igt_output_get_plane_type_index(data.wb_output,
 								  DRM_PLANE_TYPE_OVERLAY, 0);
+	igt_assert(data.kms.overlay_a.base != NULL);
+
 	data.kms.overlay_b.base = igt_output_get_plane_type_index(data.wb_output,
 								  DRM_PLANE_TYPE_OVERLAY, 1);
+	igt_assert(data.kms.overlay_b.base != NULL);
 
 	igt_assert_eq(igt_gettime(&then), 0);
 

-- 
2.43.0

[PATCH i-g-t v2 2/7] benchmarks/kms_fb_stress: Log the KMS structure

[Arthur Grillo]

In the future, the KMS structure could be dynamically changed. So, log
the KMS structure to the user.

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index 8fea10ef069f..186e92ebaf9b 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -8,6 +8,9 @@
 #define FRAME_COUNT 100
 #define NUM_FBS 2
 
+#define RECT_FMT "%dx%d%+d%+d"
+#define RECT_ARG(r) (r)->width, (r)->height, (r)->x, (r)->y
+
 struct rect_t {
 	int x, y;
 	int width, height;
@@ -189,6 +192,22 @@ igt_simple_main
 
 	gen_fbs(&data);
 
+	igt_info("KMS structure:\n");
+	igt_info("\tCRTC:\n");
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.crtc));
+	igt_info("\tprimary:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.primary.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.primary.rect));
+	igt_info("\toverlay A:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.overlay_a.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.overlay_a.rect));
+	igt_info("\toverlay B:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.overlay_b.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.overlay_b.rect));
+	igt_info("\twriteback:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.writeback.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.writeback.rect));
+
 	data.kms.primary.base = igt_output_get_plane_type(data.wb_output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(data.kms.primary.base != NULL);
 

-- 
2.43.0

[PATCH i-g-t v2 3/7] benchmarks/kms_fb_stress: Move the stress procedure to its own function

[Arthur Grillo]

In the future, we want to do this procedure multiple times in a single
execution of the program. To allow that, move the stress procedure to
its own function. This should be noop.

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 86 +++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 40 deletions(-)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index 186e92ebaf9b..ba4af5970796 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -128,6 +128,51 @@ static void set_crtc_size(struct data_t *data)
 	igt_assert_f(0, "CRTC size %dx%d not supported\n", crtc->width, crtc->height);
 }
 
+static void stress_driver(struct data_t *data)
+{
+	struct timespec then, now;
+	double elapsed;
+
+	igt_info("KMS structure:\n");
+	igt_info("\tCRTC:\n");
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data->kms.crtc));
+	igt_info("\tprimary:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data->kms.primary.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data->kms.primary.rect));
+	igt_info("\toverlay A:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data->kms.overlay_a.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data->kms.overlay_a.rect));
+	igt_info("\toverlay B:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data->kms.overlay_b.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data->kms.overlay_b.rect));
+	igt_info("\twriteback:\n");
+	igt_info("\t\tformat: %s\n", igt_format_str(data->kms.writeback.format));
+	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data->kms.writeback.rect));
+
+	gen_fbs(data);
+
+	igt_assert_eq(igt_gettime(&then), 0);
+
+	for (int i = 0; i < FRAME_COUNT; i++) {
+		int fb_index = i % NUM_FBS;
+
+		plane_setup(&data->kms.primary, fb_index);
+
+		plane_setup(&data->kms.overlay_a, fb_index);
+
+		plane_setup(&data->kms.overlay_b, fb_index);
+
+		igt_output_set_writeback_fb(data->wb_output, &data->kms.writeback.fbs[fb_index]);
+
+		igt_display_commit2(&data->display, COMMIT_ATOMIC);
+	}
+
+	igt_assert_eq(igt_gettime(&now), 0);
+	elapsed = igt_time_elapsed(&then, &now);
+
+	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
+}
+
 static struct kms_t default_kms = {
 	.crtc = {
 		.width = 4096, .height = 2160,
@@ -166,8 +211,6 @@ static struct kms_t default_kms = {
 igt_simple_main
 {
 	struct data_t data = {0};
-	struct timespec then, now;
-	double elapsed;
 
 	data.kms = default_kms;
 
@@ -190,24 +233,6 @@ igt_simple_main
 
 	set_crtc_size(&data);
 
-	gen_fbs(&data);
-
-	igt_info("KMS structure:\n");
-	igt_info("\tCRTC:\n");
-	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.crtc));
-	igt_info("\tprimary:\n");
-	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.primary.format));
-	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.primary.rect));
-	igt_info("\toverlay A:\n");
-	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.overlay_a.format));
-	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.overlay_a.rect));
-	igt_info("\toverlay B:\n");
-	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.overlay_b.format));
-	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.overlay_b.rect));
-	igt_info("\twriteback:\n");
-	igt_info("\t\tformat: %s\n", igt_format_str(data.kms.writeback.format));
-	igt_info("\t\trectangle: " RECT_FMT "\n", RECT_ARG(&data.kms.writeback.rect));
-
 	data.kms.primary.base = igt_output_get_plane_type(data.wb_output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(data.kms.primary.base != NULL);
 
@@ -219,26 +244,7 @@ igt_simple_main
 								  DRM_PLANE_TYPE_OVERLAY, 1);
 	igt_assert(data.kms.overlay_b.base != NULL);
 
-	igt_assert_eq(igt_gettime(&then), 0);
-
-	for (int i = 0; i < FRAME_COUNT; i++) {
-		int fb_index = i % NUM_FBS;
-
-		plane_setup(&data.kms.primary, fb_index);
-
-		plane_setup(&data.kms.overlay_a, fb_index);
-
-		plane_setup(&data.kms.overlay_b, fb_index);
-
-		igt_output_set_writeback_fb(data.wb_output, &data.kms.writeback.fbs[fb_index]);
-
-		igt_display_commit2(&data.display, COMMIT_ATOMIC);
-	}
-
-	igt_assert_eq(igt_gettime(&now), 0);
-	elapsed = igt_time_elapsed(&then, &now);
-
-	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
+	stress_driver(&data);
 
 	igt_display_fini(&data.display);
 	drm_close_driver(data.fd);

-- 
2.43.0

[PATCH i-g-t v2 4/7] benchmarks/kms_fb_stress: Free resources on the stress procedure

[Arthur Grillo]

On the stress_driver(), the fbs and the sync file are not released. Fix
that by freeing all the allocated resources.

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index ba4af5970796..c3b32072384e 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -165,11 +165,30 @@ static void stress_driver(struct data_t *data)
 		igt_output_set_writeback_fb(data->wb_output, &data->kms.writeback.fbs[fb_index]);
 
 		igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
+		if (data->wb_output->writeback_out_fence_fd != -1) {
+			close(data->wb_output->writeback_out_fence_fd);
+			data->wb_output->writeback_out_fence_fd = -1;
+		}
 	}
 
 	igt_assert_eq(igt_gettime(&now), 0);
 	elapsed = igt_time_elapsed(&then, &now);
 
+	igt_plane_set_fb(data->kms.primary.base, NULL);
+	igt_plane_set_fb(data->kms.overlay_a.base, NULL);
+	igt_plane_set_fb(data->kms.overlay_b.base, NULL);
+	igt_output_set_writeback_fb(data->wb_output, NULL);
+
+	for (int i = 0; i < NUM_FBS; i++) {
+		igt_remove_fb(data->fd, &data->kms.primary.fbs[i]);
+		igt_remove_fb(data->fd, &data->kms.overlay_a.fbs[i]);
+		igt_remove_fb(data->fd, &data->kms.overlay_b.fbs[i]);
+		igt_remove_fb(data->fd, &data->kms.writeback.fbs[i]);
+	}
+
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
 	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
 }
 

-- 
2.43.0

[PATCH i-g-t v2 5/7] benchmarks/kms_fb_stress: Don't paint the FB's if the format is DRM_FORMAT_RGB565

[Arthur Grillo]

Cairo does not fully support the RGB565 due to some stride issues.
So, don't paint the FB's if the format is DRM_FORMAT_RGB565.

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 55 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index c3b32072384e..d3823908f131 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -52,20 +52,47 @@ static void gen_fbs(struct data_t *data)
 	drmModeModeInfo *mode = igt_output_get_mode(data->wb_output);
 
 	for (int i = 0; i < NUM_FBS; i++) {
-		igt_create_color_fb(data->fd, kms->primary.rect.width, kms->primary.rect.height,
-				    kms->primary.format, DRM_FORMAT_MOD_LINEAR,
-				    !i, i, i,
-				    &kms->primary.fbs[i]);
-
-		igt_create_color_fb(data->fd, kms->overlay_a.rect.width, kms->overlay_a.rect.height,
-				    kms->overlay_a.format, DRM_FORMAT_MOD_LINEAR,
-				    i, !i, i,
-				    &kms->overlay_a.fbs[i]);
-
-		igt_create_color_fb(data->fd, kms->overlay_b.rect.width, kms->overlay_b.rect.height,
-				    kms->overlay_b.format, DRM_FORMAT_MOD_LINEAR,
-				    i, i, !i,
-				    &kms->overlay_b.fbs[i]);
+		// Cairo does not fully support this format
+		if (kms->primary.format != DRM_FORMAT_RGB565) {
+			igt_create_color_fb(data->fd,
+					    kms->primary.rect.width, kms->primary.rect.height,
+					    kms->primary.format, DRM_FORMAT_MOD_LINEAR,
+					    !i, i, i,
+					    &kms->primary.fbs[i]);
+		} else {
+			igt_create_fb(data->fd,
+				      kms->primary.rect.width, kms->primary.rect.height,
+				      kms->primary.format, DRM_FORMAT_MOD_LINEAR,
+				      &kms->primary.fbs[i]);
+		}
+
+		// Cairo does not fully support this format
+		if (kms->overlay_a.format != DRM_FORMAT_RGB565) {
+			igt_create_color_fb(data->fd,
+					    kms->overlay_a.rect.width, kms->overlay_a.rect.height,
+					    kms->overlay_a.format, DRM_FORMAT_MOD_LINEAR,
+					    i, !i, i,
+					    &kms->overlay_a.fbs[i]);
+		} else {
+			igt_create_fb(data->fd,
+				      kms->overlay_a.rect.width, kms->overlay_a.rect.height,
+				      kms->overlay_a.format, DRM_FORMAT_MOD_LINEAR,
+				      &kms->overlay_a.fbs[i]);
+		}
+
+		// Cairo does not fully support this format
+		if (kms->overlay_b.format != DRM_FORMAT_RGB565) {
+			igt_create_color_fb(data->fd,
+					    kms->overlay_b.rect.width, kms->overlay_b.rect.height,
+					    kms->overlay_b.format, DRM_FORMAT_MOD_LINEAR,
+					    i, i, !i,
+					    &kms->overlay_b.fbs[i]);
+		} else {
+			igt_create_fb(data->fd,
+				      kms->overlay_b.rect.width, kms->overlay_b.rect.height,
+				      kms->overlay_b.format, DRM_FORMAT_MOD_LINEAR,
+				      &kms->overlay_b.fbs[i]);
+		}
 
 		kms->writeback.rect.width = mode->hdisplay;
 		kms->writeback.rect.height = mode->vdisplay;

-- 
2.43.0

[PATCH i-g-t v2 6/7] benchmarks/kms_fb_stress: Test every possible format combinations

[Arthur Grillo]

After some discussion[1], we realized that it would be good for the
kms_fb_stress to test all possible combinations of properties in a single
run. Currently, we only manage the formats of each plane.

So, test every possible format combinations.

[1]: https://lore.kernel.org/all/6979cd2e-2b00-4dc4-8e41-66b435cf7ea8@riseup.net/

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 5 deletions(-)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index d3823908f131..deee9d557175 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -3,6 +3,8 @@
  * Copyright © 2024 Arthur Grillo
  */
 
+#include <stdint.h>
+
 #include "igt.h"
 
 #define FRAME_COUNT 100
@@ -35,6 +37,8 @@ struct data_t {
 	int fd;
 	igt_display_t display;
 	igt_output_t *wb_output;
+	uint32_t *wb_formats;
+	size_t wb_fmt_count;
 	drmModeModeInfo *mode;
 	struct kms_t kms;
 };
@@ -117,6 +121,26 @@ static igt_output_t *find_wb_output(struct data_t *data)
 	return NULL;
 }
 
+static void get_wb_formats(struct data_t *data)
+{
+	drmModePropertyBlobRes *blob = NULL;
+	uint64_t blob_id;
+	int ret;
+
+	ret = kmstest_get_property(data->fd,
+				   data->wb_output->config.connector->connector_id,
+				   DRM_MODE_OBJECT_CONNECTOR,
+				   igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
+				   NULL, &blob_id, NULL);
+	if (ret)
+		blob = drmModeGetPropertyBlob(data->fd, blob_id);
+
+	data->wb_formats = blob->data;
+	data->wb_fmt_count = blob->length / 4;
+
+	igt_assert_f(blob, "Could not get writeback formats");
+}
+
 static void set_wb_pipe(struct data_t *data)
 {
 	enum pipe pipe = PIPE_NONE;
@@ -228,28 +252,24 @@ static struct kms_t default_kms = {
 			.x = 101, .y = 0,
 			.width = 3639, .height = 2160,
 		},
-		.format = DRM_FORMAT_XRGB8888,
 	},
 	.overlay_a = {
 		.rect = {
 			.x = 201, .y = 199,
 			.width = 3033, .height = 1777,
 		},
-		.format = DRM_FORMAT_XRGB16161616,
 	},
 	.overlay_b = {
 		.rect = {
 			.x = 1800, .y = 250,
 			.width = 1507, .height = 1400,
 		},
-		.format = DRM_FORMAT_ARGB8888,
 	},
 	.writeback = {
 		.rect = {
 			.x = 0, .y = 0,
 			// Size is to be determined at runtime
 		},
-		.format = DRM_FORMAT_XRGB8888,
 	},
 };
 
@@ -257,6 +277,9 @@ static struct kms_t default_kms = {
 igt_simple_main
 {
 	struct data_t data = {0};
+	size_t primary_fmt_count = 0;
+	size_t overlay_a_fmt_count = 0;
+	size_t overlay_b_fmt_count = 0;
 
 	data.kms = default_kms;
 
@@ -276,6 +299,7 @@ igt_simple_main
 	data.wb_output = find_wb_output(&data);
 	igt_require(data.wb_output);
 	set_wb_pipe(&data);
+	get_wb_formats(&data);
 
 	set_crtc_size(&data);
 
@@ -290,7 +314,40 @@ igt_simple_main
 								  DRM_PLANE_TYPE_OVERLAY, 1);
 	igt_assert(data.kms.overlay_b.base != NULL);
 
-	stress_driver(&data);
+	primary_fmt_count = data.kms.primary.base->format_mod_count;
+	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
+	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
+
+	for (size_t i = 0;
+	     i < primary_fmt_count *
+		 overlay_a_fmt_count *
+		 overlay_b_fmt_count *
+		 data.wb_fmt_count;
+	     i++) {
+		size_t p = (i / (overlay_a_fmt_count *
+				 overlay_b_fmt_count *
+				 data.wb_fmt_count)) % primary_fmt_count;
+
+		size_t a = (i / (overlay_b_fmt_count *
+				 data.wb_fmt_count)) % overlay_a_fmt_count;
+
+		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
+
+		size_t w = i % data.wb_fmt_count;
+
+		data.kms.primary.format = data.kms.primary.base->formats[p];
+		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
+		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
+		data.kms.writeback.format = data.wb_formats[w];
+
+		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
+			 p + 1, primary_fmt_count,
+			 a + 1, overlay_a_fmt_count,
+			 b + 1, overlay_b_fmt_count,
+			 w + 1, data.wb_fmt_count);
+
+		stress_driver(&data);
+	}
 
 	igt_display_fini(&data.display);
 	drm_close_driver(data.fd);

-- 
2.43.0

[PATCH i-g-t v2 7/7] benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

[Arthur Grillo]

Currently, the benchmark runs all the possible combinations of formats,
which can take a lot of time. Change that by adding command line options
to pin the planes or writeback formats, with the intention to decrease
the number of cases to run.

Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
---
 benchmarks/kms_fb_stress.c | 140 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 99 insertions(+), 41 deletions(-)

diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
index deee9d557175..37f60f0bfa94 100644
--- a/benchmarks/kms_fb_stress.c
+++ b/benchmarks/kms_fb_stress.c
@@ -3,6 +3,7 @@
  * Copyright © 2024 Arthur Grillo
  */
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #include "igt.h"
@@ -41,6 +42,10 @@ struct data_t {
 	size_t wb_fmt_count;
 	drmModeModeInfo *mode;
 	struct kms_t kms;
+	bool selected_primary_fmt;
+	bool selected_overlay_a_fmt;
+	bool selected_overlay_b_fmt;
+	bool selected_writeback_fmt;
 };
 
 static void plane_setup(struct plane_t *plane, int index)
@@ -243,45 +248,87 @@ static void stress_driver(struct data_t *data)
 	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
 }
 
-static struct kms_t default_kms = {
-	.crtc = {
-		.width = 4096, .height = 2160,
-	},
-	.primary = {
-		.rect = {
-			.x = 101, .y = 0,
-			.width = 3639, .height = 2160,
+static int opt_handler(int opt, int opt_index, void *_data)
+{
+	struct data_t *data = _data;
+	struct kms_t *kms =  &data->kms;
+
+	switch (opt) {
+	case 'p':
+		kms->primary.format = igt_drm_format_str_to_format(optarg);
+		data->selected_primary_fmt = true;
+		break;
+	case 'a':
+		kms->overlay_a.format = igt_drm_format_str_to_format(optarg);
+		data->selected_overlay_a_fmt = true;
+		break;
+	case 'b':
+		kms->overlay_b.format = igt_drm_format_str_to_format(optarg);
+		data->selected_overlay_b_fmt = true;
+		break;
+	case 'w':
+		kms->writeback.format = igt_drm_format_str_to_format(optarg);
+		data->selected_writeback_fmt = true;
+		break;
+	default:
+		return IGT_OPT_HANDLER_ERROR;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+static const char *help_str =
+	"  --primary-format\t\tPin the primary plane format\n"
+	"  --overlay-a-format\t\tPin the overlay A plane format\n"
+	"  --overlay-b-format\t\tPin the overlay B plane format\n"
+	"  --writeback-format\t\tPin the writeback format\n";
+
+static const struct option long_options[] = {
+	{ .name = "primary-format", .has_arg = true, .val = 'p'},
+	{ .name = "overlay-a-format", .has_arg = true, .val = 'a'},
+	{ .name = "overlay-b-format", .has_arg = true, .val = 'b'},
+	{ .name = "writeback-format", .has_arg = true, .val = 'w'},
+	{}
+};
+
+static struct data_t data = (struct data_t){
+	.kms = {
+			.crtc = {
+				.width = 4096, .height = 2160,
+			},
+		.primary = {
+			.rect = {
+				.x = 101, .y = 0,
+				.width = 3639, .height = 2160,
+			},
 		},
-	},
-	.overlay_a = {
-		.rect = {
-			.x = 201, .y = 199,
-			.width = 3033, .height = 1777,
+		.overlay_a = {
+			.rect = {
+				.x = 201, .y = 199,
+				.width = 3033, .height = 1777,
+			},
 		},
-	},
-	.overlay_b = {
-		.rect = {
-			.x = 1800, .y = 250,
-			.width = 1507, .height = 1400,
+		.overlay_b = {
+			.rect = {
+				.x = 1800, .y = 250,
+				.width = 1507, .height = 1400,
+			},
 		},
-	},
-	.writeback = {
-		.rect = {
-			.x = 0, .y = 0,
-			// Size is to be determined at runtime
+		.writeback = {
+			.rect = {
+				.x = 0, .y = 0,
+				// Size is to be determined at runtime
+			},
 		},
 	},
 };
 
-
-igt_simple_main
+igt_simple_main_args(NULL, long_options, help_str, opt_handler, &data)
 {
-	struct data_t data = {0};
 	size_t primary_fmt_count = 0;
 	size_t overlay_a_fmt_count = 0;
 	size_t overlay_b_fmt_count = 0;
-
-	data.kms = default_kms;
+	size_t wb_fmt_count = 0;
 
 	data.fd = drm_open_driver_master(DRIVER_ANY);
 
@@ -314,37 +361,48 @@ igt_simple_main
 								  DRM_PLANE_TYPE_OVERLAY, 1);
 	igt_assert(data.kms.overlay_b.base != NULL);
 
-	primary_fmt_count = data.kms.primary.base->format_mod_count;
-	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
-	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
+	primary_fmt_count = data.selected_primary_fmt ?
+			    1 : data.kms.primary.base->format_mod_count;
+	overlay_a_fmt_count = data.selected_overlay_a_fmt ?
+			      1 : data.kms.overlay_a.base->format_mod_count;
+	overlay_b_fmt_count = data.selected_overlay_b_fmt ?
+			      1 : data.kms.overlay_b.base->format_mod_count;
+	wb_fmt_count = data.selected_writeback_fmt ? 1 : data.wb_fmt_count;
 
 	for (size_t i = 0;
 	     i < primary_fmt_count *
 		 overlay_a_fmt_count *
 		 overlay_b_fmt_count *
-		 data.wb_fmt_count;
+		 wb_fmt_count;
 	     i++) {
 		size_t p = (i / (overlay_a_fmt_count *
 				 overlay_b_fmt_count *
-				 data.wb_fmt_count)) % primary_fmt_count;
+				 wb_fmt_count)) % primary_fmt_count;
 
 		size_t a = (i / (overlay_b_fmt_count *
-				 data.wb_fmt_count)) % overlay_a_fmt_count;
+				 wb_fmt_count)) % overlay_a_fmt_count;
+
+		size_t b = (i / wb_fmt_count) % overlay_a_fmt_count;
+
+		size_t w = i % wb_fmt_count;
+
+		if (data.selected_primary_fmt == false)
+			data.kms.primary.format = data.kms.primary.base->formats[p];
 
-		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
+		if (data.selected_overlay_a_fmt == false)
+			data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
 
-		size_t w = i % data.wb_fmt_count;
+		if (data.selected_overlay_b_fmt == false)
+			data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
 
-		data.kms.primary.format = data.kms.primary.base->formats[p];
-		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
-		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
-		data.kms.writeback.format = data.wb_formats[w];
+		if (data.selected_writeback_fmt == false)
+			data.kms.writeback.format = data.wb_formats[w];
 
 		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
 			 p + 1, primary_fmt_count,
 			 a + 1, overlay_a_fmt_count,
 			 b + 1, overlay_b_fmt_count,
-			 w + 1, data.wb_fmt_count);
+			 w + 1, wb_fmt_count);
 
 		stress_driver(&data);
 	}

-- 
2.43.0

✗ CI.xeBAT: failure for benchmarks/kms_fb_stress: Test all possible combinations of formats

[Patchwork]

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

== Series Details ==

Series: benchmarks/kms_fb_stress: Test all possible combinations of formats
URL   : https://patchwork.freedesktop.org/series/130770/
State : failure

== Summary ==

CI Bug Log - changes from XEIGT_7746_BAT -> XEIGTPW_10781_BAT
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with XEIGTPW_10781_BAT absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in XEIGTPW_10781_BAT, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

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

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@xe_module_load@load:
    - bat-adlp-7:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7746/bat-adlp-7/igt@xe_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10781/bat-adlp-7/igt@xe_module_load@load.html

  


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

  * IGT: IGT_7746 -> IGTPW_10781
  * Linux: xe-897-98da761379cfaa31b32641ad648f83c90a8e629e -> xe-903-8f85c978aed193f293da17e783a817844e72a569

  IGTPW_10781: 10781
  IGT_7746: 7746
  xe-897-98da761379cfaa31b32641ad648f83c90a8e629e: 98da761379cfaa31b32641ad648f83c90a8e629e
  xe-903-8f85c978aed193f293da17e783a817844e72a569: 8f85c978aed193f293da17e783a817844e72a569

== Logs ==

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

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

✗ Fi.CI.BAT: failure for benchmarks/kms_fb_stress: Test all possible combinations of formats

[Patchwork]

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

== Series Details ==

Series: benchmarks/kms_fb_stress: Test all possible combinations of formats
URL   : https://patchwork.freedesktop.org/series/130770/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14393 -> IGTPW_10781
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10781 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10781, please notify your bug team (I915-ci-infra@lists.freedesktop.org) 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_10781/index.html

Participating hosts (41 -> 38)
------------------------------

  Additional (1): bat-dg1-7 
  Missing    (4): fi-glk-j4005 bat-jsl-1 fi-snb-2520m fi-bsw-n3050 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_lmem_swapping@basic@lmem0:
    - bat-dg2-14:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14393/bat-dg2-14/igt@gem_lmem_swapping@basic@lmem0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg2-14/igt@gem_lmem_swapping@basic@lmem0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@verify-random:
    - bat-arls-2:         NOTRUN -> [SKIP][3] ([i915#10213]) +3 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_mmap@basic:
    - bat-dg1-7:          NOTRUN -> [SKIP][4] ([i915#4083])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@gem_mmap@basic.html
    - bat-arls-2:         NOTRUN -> [SKIP][5] ([i915#4083])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_mmap@basic.html

  * igt@gem_mmap_gtt@basic:
    - bat-arls-2:         NOTRUN -> [SKIP][6] ([i915#10196] / [i915#4077]) +2 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_mmap_gtt@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-arls-2:         NOTRUN -> [SKIP][7] ([i915#10197] / [i915#10211] / [i915#4079])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg1-7:          NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-dg1-7:          NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@gem_tiled_pread_basic.html
    - bat-arls-2:         NOTRUN -> [SKIP][10] ([i915#10206] / [i915#4079])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg1-7:          NOTRUN -> [SKIP][11] ([i915#6621])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@i915_pm_rps@basic-api.html
    - bat-arls-2:         NOTRUN -> [SKIP][12] ([i915#10209])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@dmabuf:
    - bat-mtlp-8:         [PASS][13] -> [DMESG-FAIL][14] ([i915#10249])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14393/bat-mtlp-8/igt@i915_selftest@live@dmabuf.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-mtlp-8/igt@i915_selftest@live@dmabuf.html

  * igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy:
    - bat-dg1-7:          NOTRUN -> [SKIP][15] ([i915#4212]) +7 other tests skip
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_addfb_basic@addfb25-x-tiled-mismatch-legacy.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-arls-2:         NOTRUN -> [SKIP][16] ([i915#10200]) +9 other tests skip
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg1-7:          NOTRUN -> [SKIP][17] ([i915#4215])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-arls-2:         NOTRUN -> [SKIP][18] ([i915#10202]) +1 other test skip
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
    - bat-dg1-7:          NOTRUN -> [SKIP][19] ([i915#4103] / [i915#4213]) +1 other test skip
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-arls-2:         NOTRUN -> [SKIP][20] ([i915#9886])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_dsc@dsc-basic.html
    - bat-dg1-7:          NOTRUN -> [SKIP][21] ([i915#3555] / [i915#3840])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-arls-2:         NOTRUN -> [SKIP][22] ([i915#10207])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_force_connector_basic@force-load-detect.html
    - bat-dg1-7:          NOTRUN -> [SKIP][23] ([fdo#109285])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_hdmi_inject@inject-audio:
    - bat-dg1-7:          NOTRUN -> [SKIP][24] ([i915#433])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_hdmi_inject@inject-audio.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-dg1-7:          NOTRUN -> [SKIP][25] ([i915#5354])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-primary-mmap-gtt@edp-1:
    - bat-arls-2:         NOTRUN -> [SKIP][26] ([i915#10196] / [i915#4077] / [i915#9688])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_psr@psr-primary-mmap-gtt@edp-1.html

  * igt@kms_psr@psr-primary-page-flip:
    - bat-dg1-7:          NOTRUN -> [SKIP][27] ([i915#9732]) +3 other tests skip
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_psr@psr-primary-page-flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-arls-2:         NOTRUN -> [SKIP][28] ([i915#10208] / [i915#8809])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-dg1-7:          NOTRUN -> [SKIP][29] ([i915#3555])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg1-7:          NOTRUN -> [SKIP][30] ([i915#3708]) +3 other tests skip
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-dg1-7:          NOTRUN -> [SKIP][31] ([i915#3708] / [i915#4077]) +1 other test skip
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-dg1-7/igt@prime_vgem@basic-fence-mmap.html
    - bat-arls-2:         NOTRUN -> [SKIP][32] ([i915#10196] / [i915#3708] / [i915#4077]) +1 other test skip
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
    - bat-arls-2:         NOTRUN -> [SKIP][33] ([i915#10212] / [i915#3708])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-read:
    - bat-arls-2:         NOTRUN -> [SKIP][34] ([i915#10214] / [i915#3708])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-write:
    - bat-arls-2:         NOTRUN -> [SKIP][35] ([i915#10216] / [i915#3708])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@gem_exec_fence@basic-await@ccs0:
    - bat-arls-2:         [ABORT][36] -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14393/bat-arls-2/igt@gem_exec_fence@basic-await@ccs0.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10781/bat-arls-2/igt@gem_exec_fence@basic-await@ccs0.html

  
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#10196]: https://gitlab.freedesktop.org/drm/intel/issues/10196
  [i915#10197]: https://gitlab.freedesktop.org/drm/intel/issues/10197
  [i915#10200]: https://gitlab.freedesktop.org/drm/intel/issues/10200
  [i915#10202]: https://gitlab.freedesktop.org/drm/intel/issues/10202
  [i915#10206]: https://gitlab.freedesktop.org/drm/intel/issues/10206
  [i915#10207]: https://gitlab.freedesktop.org/drm/intel/issues/10207
  [i915#10208]: https://gitlab.freedesktop.org/drm/intel/issues/10208
  [i915#10209]: https://gitlab.freedesktop.org/drm/intel/issues/10209
  [i915#10211]: https://gitlab.freedesktop.org/drm/intel/issues/10211
  [i915#10212]: https://gitlab.freedesktop.org/drm/intel/issues/10212
  [i915#10213]: https://gitlab.freedesktop.org/drm/intel/issues/10213
  [i915#10214]: https://gitlab.freedesktop.org/drm/intel/issues/10214
  [i915#10216]: https://gitlab.freedesktop.org/drm/intel/issues/10216
  [i915#10249]: https://gitlab.freedesktop.org/drm/intel/issues/10249
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
  [i915#9688]: https://gitlab.freedesktop.org/drm/intel/issues/9688
  [i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
  [i915#9886]: https://gitlab.freedesktop.org/drm/intel/issues/9886


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7746 -> IGTPW_10781

  CI-20190529: 20190529
  CI_DRM_14393: 8f85c978aed193f293da17e783a817844e72a569 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10781: 10781
  IGT_7746: 7746

== Logs ==

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

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

Re: [PATCH i-g-t v2 7/7] benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

[Louis Chauvet]

Le 05/03/24 - 20:15, Arthur Grillo a écrit :
> Currently, the benchmark runs all the possible combinations of formats,
> which can take a lot of time. Change that by adding command line options
> to pin the planes or writeback formats, with the intention to decrease
> the number of cases to run.
> 
> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> ---
>  benchmarks/kms_fb_stress.c | 140 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 99 insertions(+), 41 deletions(-)
> 
> diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
> index deee9d557175..37f60f0bfa94 100644
> --- a/benchmarks/kms_fb_stress.c
> +++ b/benchmarks/kms_fb_stress.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2024 Arthur Grillo
>   */
>  
> +#include <stdbool.h>
>  #include <stdint.h>
>  
>  #include "igt.h"
> @@ -41,6 +42,10 @@ struct data_t {
>  	size_t wb_fmt_count;
>  	drmModeModeInfo *mode;
>  	struct kms_t kms;
> +	bool selected_primary_fmt;
> +	bool selected_overlay_a_fmt;
> +	bool selected_overlay_b_fmt;
> +	bool selected_writeback_fmt;
>  };

Hi Arthur!

Nice work, for the next iteration of my series I will use this to run 
a full test, thanks!

If I understand correclty, when you "pin" a format, it will always use the 
first available? It's not possible to say "I want to test NV24"?

Do you think it will be difficult to add this kind of option (like 
--writeback-format in kms_writeback)?

Kind regards,
Louis Chauvet

>  static void plane_setup(struct plane_t *plane, int index)
> @@ -243,45 +248,87 @@ static void stress_driver(struct data_t *data)
>  	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
>  }
>  
> -static struct kms_t default_kms = {
> -	.crtc = {
> -		.width = 4096, .height = 2160,
> -	},
> -	.primary = {
> -		.rect = {
> -			.x = 101, .y = 0,
> -			.width = 3639, .height = 2160,
> +static int opt_handler(int opt, int opt_index, void *_data)
> +{
> +	struct data_t *data = _data;
> +	struct kms_t *kms =  &data->kms;
> +
> +	switch (opt) {
> +	case 'p':
> +		kms->primary.format = igt_drm_format_str_to_format(optarg);
> +		data->selected_primary_fmt = true;
> +		break;
> +	case 'a':
> +		kms->overlay_a.format = igt_drm_format_str_to_format(optarg);
> +		data->selected_overlay_a_fmt = true;
> +		break;
> +	case 'b':
> +		kms->overlay_b.format = igt_drm_format_str_to_format(optarg);
> +		data->selected_overlay_b_fmt = true;
> +		break;
> +	case 'w':
> +		kms->writeback.format = igt_drm_format_str_to_format(optarg);
> +		data->selected_writeback_fmt = true;
> +		break;
> +	default:
> +		return IGT_OPT_HANDLER_ERROR;
> +	}
> +
> +	return IGT_OPT_HANDLER_SUCCESS;
> +}
> +
> +static const char *help_str =
> +	"  --primary-format\t\tPin the primary plane format\n"
> +	"  --overlay-a-format\t\tPin the overlay A plane format\n"
> +	"  --overlay-b-format\t\tPin the overlay B plane format\n"
> +	"  --writeback-format\t\tPin the writeback format\n";
> +
> +static const struct option long_options[] = {
> +	{ .name = "primary-format", .has_arg = true, .val = 'p'},
> +	{ .name = "overlay-a-format", .has_arg = true, .val = 'a'},
> +	{ .name = "overlay-b-format", .has_arg = true, .val = 'b'},
> +	{ .name = "writeback-format", .has_arg = true, .val = 'w'},
> +	{}
> +};
> +
> +static struct data_t data = (struct data_t){
> +	.kms = {
> +			.crtc = {
> +				.width = 4096, .height = 2160,
> +			},
> +		.primary = {
> +			.rect = {
> +				.x = 101, .y = 0,
> +				.width = 3639, .height = 2160,
> +			},
>  		},
> -	},
> -	.overlay_a = {
> -		.rect = {
> -			.x = 201, .y = 199,
> -			.width = 3033, .height = 1777,
> +		.overlay_a = {
> +			.rect = {
> +				.x = 201, .y = 199,
> +				.width = 3033, .height = 1777,
> +			},
>  		},
> -	},
> -	.overlay_b = {
> -		.rect = {
> -			.x = 1800, .y = 250,
> -			.width = 1507, .height = 1400,
> +		.overlay_b = {
> +			.rect = {
> +				.x = 1800, .y = 250,
> +				.width = 1507, .height = 1400,
> +			},
>  		},
> -	},
> -	.writeback = {
> -		.rect = {
> -			.x = 0, .y = 0,
> -			// Size is to be determined at runtime
> +		.writeback = {
> +			.rect = {
> +				.x = 0, .y = 0,
> +				// Size is to be determined at runtime
> +			},
>  		},
>  	},
>  };
>  
> -
> -igt_simple_main
> +igt_simple_main_args(NULL, long_options, help_str, opt_handler, &data)
>  {
> -	struct data_t data = {0};
>  	size_t primary_fmt_count = 0;
>  	size_t overlay_a_fmt_count = 0;
>  	size_t overlay_b_fmt_count = 0;
> -
> -	data.kms = default_kms;
> +	size_t wb_fmt_count = 0;
>  
>  	data.fd = drm_open_driver_master(DRIVER_ANY);
>  
> @@ -314,37 +361,48 @@ igt_simple_main
>  								  DRM_PLANE_TYPE_OVERLAY, 1);
>  	igt_assert(data.kms.overlay_b.base != NULL);
>  
> -	primary_fmt_count = data.kms.primary.base->format_mod_count;
> -	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
> -	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
> +	primary_fmt_count = data.selected_primary_fmt ?
> +			    1 : data.kms.primary.base->format_mod_count;
> +	overlay_a_fmt_count = data.selected_overlay_a_fmt ?
> +			      1 : data.kms.overlay_a.base->format_mod_count;
> +	overlay_b_fmt_count = data.selected_overlay_b_fmt ?
> +			      1 : data.kms.overlay_b.base->format_mod_count;
> +	wb_fmt_count = data.selected_writeback_fmt ? 1 : data.wb_fmt_count;
>  
>  	for (size_t i = 0;
>  	     i < primary_fmt_count *
>  		 overlay_a_fmt_count *
>  		 overlay_b_fmt_count *
> -		 data.wb_fmt_count;
> +		 wb_fmt_count;
>  	     i++) {
>  		size_t p = (i / (overlay_a_fmt_count *
>  				 overlay_b_fmt_count *
> -				 data.wb_fmt_count)) % primary_fmt_count;
> +				 wb_fmt_count)) % primary_fmt_count;
>  
>  		size_t a = (i / (overlay_b_fmt_count *
> -				 data.wb_fmt_count)) % overlay_a_fmt_count;
> +				 wb_fmt_count)) % overlay_a_fmt_count;
> +
> +		size_t b = (i / wb_fmt_count) % overlay_a_fmt_count;
> +
> +		size_t w = i % wb_fmt_count;
> +
> +		if (data.selected_primary_fmt == false)
> +			data.kms.primary.format = data.kms.primary.base->formats[p];
>  
> -		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
> +		if (data.selected_overlay_a_fmt == false)
> +			data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
>  
> -		size_t w = i % data.wb_fmt_count;
> +		if (data.selected_overlay_b_fmt == false)
> +			data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
>  
> -		data.kms.primary.format = data.kms.primary.base->formats[p];
> -		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
> -		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
> -		data.kms.writeback.format = data.wb_formats[w];
> +		if (data.selected_writeback_fmt == false)
> +			data.kms.writeback.format = data.wb_formats[w];
>  
>  		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
>  			 p + 1, primary_fmt_count,
>  			 a + 1, overlay_a_fmt_count,
>  			 b + 1, overlay_b_fmt_count,
> -			 w + 1, data.wb_fmt_count);
> +			 w + 1, wb_fmt_count);
>  
>  		stress_driver(&data);
>  	}
> 
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t v2 7/7] benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

[Arthur Grillo]

On 06/03/24 14:29, Louis Chauvet wrote:
> Le 05/03/24 - 20:15, Arthur Grillo a écrit :
>> Currently, the benchmark runs all the possible combinations of formats,
>> which can take a lot of time. Change that by adding command line options
>> to pin the planes or writeback formats, with the intention to decrease
>> the number of cases to run.
>>
>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
>> ---
>>  benchmarks/kms_fb_stress.c | 140 ++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 99 insertions(+), 41 deletions(-)
>>
>> diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
>> index deee9d557175..37f60f0bfa94 100644
>> --- a/benchmarks/kms_fb_stress.c
>> +++ b/benchmarks/kms_fb_stress.c
>> @@ -3,6 +3,7 @@
>>   * Copyright © 2024 Arthur Grillo
>>   */
>>  
>> +#include <stdbool.h>
>>  #include <stdint.h>
>>  
>>  #include "igt.h"
>> @@ -41,6 +42,10 @@ struct data_t {
>>  	size_t wb_fmt_count;
>>  	drmModeModeInfo *mode;
>>  	struct kms_t kms;
>> +	bool selected_primary_fmt;
>> +	bool selected_overlay_a_fmt;
>> +	bool selected_overlay_b_fmt;
>> +	bool selected_writeback_fmt;
>>  };
> 
> Hi Arthur!
> 
> Nice work, for the next iteration of my series I will use this to run 
> a full test, thanks!

Hi Louis!

That would be great! While developing this, I ran it with your patch and
I could see a significant improvement!

> 
> If I understand correclty, when you "pin" a format, it will always use the 
> first available? It's not possible to say "I want to test NV24"?


No, you can choose what format you "pin", like:
kms_fb_stress --primary-format=NV24

With this, the primary plane format will always be NV24 and the
others will change.

Do you think this is not clear in the commit msg or help text?

> 
> Do you think it will be difficult to add this kind of option (like 
> --writeback-format in kms_writeback)?

I think the option already does what you want. If not, please reply.

Best Regards,
~Arthur Grillo

> 
> Kind regards,
> Louis Chauvet
> 
>>  static void plane_setup(struct plane_t *plane, int index)
>> @@ -243,45 +248,87 @@ static void stress_driver(struct data_t *data)
>>  	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
>>  }
>>  
>> -static struct kms_t default_kms = {
>> -	.crtc = {
>> -		.width = 4096, .height = 2160,
>> -	},
>> -	.primary = {
>> -		.rect = {
>> -			.x = 101, .y = 0,
>> -			.width = 3639, .height = 2160,
>> +static int opt_handler(int opt, int opt_index, void *_data)
>> +{
>> +	struct data_t *data = _data;
>> +	struct kms_t *kms =  &data->kms;
>> +
>> +	switch (opt) {
>> +	case 'p':
>> +		kms->primary.format = igt_drm_format_str_to_format(optarg);
>> +		data->selected_primary_fmt = true;
>> +		break;
>> +	case 'a':
>> +		kms->overlay_a.format = igt_drm_format_str_to_format(optarg);
>> +		data->selected_overlay_a_fmt = true;
>> +		break;
>> +	case 'b':
>> +		kms->overlay_b.format = igt_drm_format_str_to_format(optarg);
>> +		data->selected_overlay_b_fmt = true;
>> +		break;
>> +	case 'w':
>> +		kms->writeback.format = igt_drm_format_str_to_format(optarg);
>> +		data->selected_writeback_fmt = true;
>> +		break;
>> +	default:
>> +		return IGT_OPT_HANDLER_ERROR;
>> +	}
>> +
>> +	return IGT_OPT_HANDLER_SUCCESS;
>> +}
>> +
>> +static const char *help_str =
>> +	"  --primary-format\t\tPin the primary plane format\n"
>> +	"  --overlay-a-format\t\tPin the overlay A plane format\n"
>> +	"  --overlay-b-format\t\tPin the overlay B plane format\n"
>> +	"  --writeback-format\t\tPin the writeback format\n";
>> +
>> +static const struct option long_options[] = {
>> +	{ .name = "primary-format", .has_arg = true, .val = 'p'},
>> +	{ .name = "overlay-a-format", .has_arg = true, .val = 'a'},
>> +	{ .name = "overlay-b-format", .has_arg = true, .val = 'b'},
>> +	{ .name = "writeback-format", .has_arg = true, .val = 'w'},
>> +	{}
>> +};
>> +
>> +static struct data_t data = (struct data_t){
>> +	.kms = {
>> +			.crtc = {
>> +				.width = 4096, .height = 2160,
>> +			},
>> +		.primary = {
>> +			.rect = {
>> +				.x = 101, .y = 0,
>> +				.width = 3639, .height = 2160,
>> +			},
>>  		},
>> -	},
>> -	.overlay_a = {
>> -		.rect = {
>> -			.x = 201, .y = 199,
>> -			.width = 3033, .height = 1777,
>> +		.overlay_a = {
>> +			.rect = {
>> +				.x = 201, .y = 199,
>> +				.width = 3033, .height = 1777,
>> +			},
>>  		},
>> -	},
>> -	.overlay_b = {
>> -		.rect = {
>> -			.x = 1800, .y = 250,
>> -			.width = 1507, .height = 1400,
>> +		.overlay_b = {
>> +			.rect = {
>> +				.x = 1800, .y = 250,
>> +				.width = 1507, .height = 1400,
>> +			},
>>  		},
>> -	},
>> -	.writeback = {
>> -		.rect = {
>> -			.x = 0, .y = 0,
>> -			// Size is to be determined at runtime
>> +		.writeback = {
>> +			.rect = {
>> +				.x = 0, .y = 0,
>> +				// Size is to be determined at runtime
>> +			},
>>  		},
>>  	},
>>  };
>>  
>> -
>> -igt_simple_main
>> +igt_simple_main_args(NULL, long_options, help_str, opt_handler, &data)
>>  {
>> -	struct data_t data = {0};
>>  	size_t primary_fmt_count = 0;
>>  	size_t overlay_a_fmt_count = 0;
>>  	size_t overlay_b_fmt_count = 0;
>> -
>> -	data.kms = default_kms;
>> +	size_t wb_fmt_count = 0;
>>  
>>  	data.fd = drm_open_driver_master(DRIVER_ANY);
>>  
>> @@ -314,37 +361,48 @@ igt_simple_main
>>  								  DRM_PLANE_TYPE_OVERLAY, 1);
>>  	igt_assert(data.kms.overlay_b.base != NULL);
>>  
>> -	primary_fmt_count = data.kms.primary.base->format_mod_count;
>> -	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
>> -	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
>> +	primary_fmt_count = data.selected_primary_fmt ?
>> +			    1 : data.kms.primary.base->format_mod_count;
>> +	overlay_a_fmt_count = data.selected_overlay_a_fmt ?
>> +			      1 : data.kms.overlay_a.base->format_mod_count;
>> +	overlay_b_fmt_count = data.selected_overlay_b_fmt ?
>> +			      1 : data.kms.overlay_b.base->format_mod_count;
>> +	wb_fmt_count = data.selected_writeback_fmt ? 1 : data.wb_fmt_count;
>>  
>>  	for (size_t i = 0;
>>  	     i < primary_fmt_count *
>>  		 overlay_a_fmt_count *
>>  		 overlay_b_fmt_count *
>> -		 data.wb_fmt_count;
>> +		 wb_fmt_count;
>>  	     i++) {
>>  		size_t p = (i / (overlay_a_fmt_count *
>>  				 overlay_b_fmt_count *
>> -				 data.wb_fmt_count)) % primary_fmt_count;
>> +				 wb_fmt_count)) % primary_fmt_count;
>>  
>>  		size_t a = (i / (overlay_b_fmt_count *
>> -				 data.wb_fmt_count)) % overlay_a_fmt_count;
>> +				 wb_fmt_count)) % overlay_a_fmt_count;
>> +
>> +		size_t b = (i / wb_fmt_count) % overlay_a_fmt_count;
>> +
>> +		size_t w = i % wb_fmt_count;
>> +
>> +		if (data.selected_primary_fmt == false)
>> +			data.kms.primary.format = data.kms.primary.base->formats[p];
>>  
>> -		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
>> +		if (data.selected_overlay_a_fmt == false)
>> +			data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
>>  
>> -		size_t w = i % data.wb_fmt_count;
>> +		if (data.selected_overlay_b_fmt == false)
>> +			data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
>>  
>> -		data.kms.primary.format = data.kms.primary.base->formats[p];
>> -		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
>> -		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
>> -		data.kms.writeback.format = data.wb_formats[w];
>> +		if (data.selected_writeback_fmt == false)
>> +			data.kms.writeback.format = data.wb_formats[w];
>>  
>>  		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
>>  			 p + 1, primary_fmt_count,
>>  			 a + 1, overlay_a_fmt_count,
>>  			 b + 1, overlay_b_fmt_count,
>> -			 w + 1, data.wb_fmt_count);
>> +			 w + 1, wb_fmt_count);
>>  
>>  		stress_driver(&data);
>>  	}
>>
>> -- 
>> 2.43.0
>>

Re: [PATCH i-g-t v2 7/7] benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

[Louis Chauvet]

Le 06/03/24 - 15:14, Arthur Grillo a écrit :
> 
> 
> On 06/03/24 14:29, Louis Chauvet wrote:
> > Le 05/03/24 - 20:15, Arthur Grillo a écrit :
> >> Currently, the benchmark runs all the possible combinations of formats,
> >> which can take a lot of time. Change that by adding command line options
> >> to pin the planes or writeback formats, with the intention to decrease
> >> the number of cases to run.
> >>
> >> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
> >> ---
> >>  benchmarks/kms_fb_stress.c | 140 ++++++++++++++++++++++++++++++++-------------
> >>  1 file changed, 99 insertions(+), 41 deletions(-)
> >>
> >> diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
> >> index deee9d557175..37f60f0bfa94 100644
> >> --- a/benchmarks/kms_fb_stress.c
> >> +++ b/benchmarks/kms_fb_stress.c
> >> @@ -3,6 +3,7 @@
> >>   * Copyright © 2024 Arthur Grillo
> >>   */
> >>  
> >> +#include <stdbool.h>
> >>  #include <stdint.h>
> >>  
> >>  #include "igt.h"
> >> @@ -41,6 +42,10 @@ struct data_t {
> >>  	size_t wb_fmt_count;
> >>  	drmModeModeInfo *mode;
> >>  	struct kms_t kms;
> >> +	bool selected_primary_fmt;
> >> +	bool selected_overlay_a_fmt;
> >> +	bool selected_overlay_b_fmt;
> >> +	bool selected_writeback_fmt;
> >>  };
> > 
> > Hi Arthur!
> > 
> > Nice work, for the next iteration of my series I will use this to run 
> > a full test, thanks!
> 
> Hi Louis!
> 
> That would be great! While developing this, I ran it with your patch and
> I could see a significant improvement!
>
> > 
> > If I understand correclty, when you "pin" a format, it will always use the 
> > first available? It's not possible to say "I want to test NV24"?
> 
> 
> No, you can choose what format you "pin", like:
> kms_fb_stress --primary-format=NV24
> 
> With this, the primary plane format will always be NV24 and the
> others will change.
> 
> Do you think this is not clear in the commit msg or help text?

I missunderstood how it works because I read first the data_t structure 
and the "bool selected_*_fmt", where I did not understand why it was a 
bool and not a uint32_t (for DRM_FORMAT*). And I was too fast reading 
opt_handler, in which it is very obvious... sorry

> > 
> > Do you think it will be difficult to add this kind of option (like 
> > --writeback-format in kms_writeback)?
> 
> I think the option already does what you want. If not, please reply.

Yes, it does! The help message was not very clear for me, I read "pin" as 
"only use one format, but I don't care which one", not "use this specific 
format". Do you think something like this is better: "Use a specific DRM 
format for the * plane"?

Kind regards,
Louis Chauvet

> Best Regards,
> ~Arthur Grillo
> 
> > 
> > Kind regards,
> > Louis Chauvet
> > 
> >>  static void plane_setup(struct plane_t *plane, int index)
> >> @@ -243,45 +248,87 @@ static void stress_driver(struct data_t *data)
> >>  	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
> >>  }
> >>  
> >> -static struct kms_t default_kms = {
> >> -	.crtc = {
> >> -		.width = 4096, .height = 2160,
> >> -	},
> >> -	.primary = {
> >> -		.rect = {
> >> -			.x = 101, .y = 0,
> >> -			.width = 3639, .height = 2160,
> >> +static int opt_handler(int opt, int opt_index, void *_data)
> >> +{
> >> +	struct data_t *data = _data;
> >> +	struct kms_t *kms =  &data->kms;
> >> +
> >> +	switch (opt) {
> >> +	case 'p':
> >> +		kms->primary.format = igt_drm_format_str_to_format(optarg);
> >> +		data->selected_primary_fmt = true;
> >> +		break;
> >> +	case 'a':
> >> +		kms->overlay_a.format = igt_drm_format_str_to_format(optarg);
> >> +		data->selected_overlay_a_fmt = true;
> >> +		break;
> >> +	case 'b':
> >> +		kms->overlay_b.format = igt_drm_format_str_to_format(optarg);
> >> +		data->selected_overlay_b_fmt = true;
> >> +		break;
> >> +	case 'w':
> >> +		kms->writeback.format = igt_drm_format_str_to_format(optarg);
> >> +		data->selected_writeback_fmt = true;
> >> +		break;
> >> +	default:
> >> +		return IGT_OPT_HANDLER_ERROR;
> >> +	}
> >> +
> >> +	return IGT_OPT_HANDLER_SUCCESS;
> >> +}
> >> +
> >> +static const char *help_str =
> >> +	"  --primary-format\t\tPin the primary plane format\n"
> >> +	"  --overlay-a-format\t\tPin the overlay A plane format\n"
> >> +	"  --overlay-b-format\t\tPin the overlay B plane format\n"
> >> +	"  --writeback-format\t\tPin the writeback format\n";
> >> +
> >> +static const struct option long_options[] = {
> >> +	{ .name = "primary-format", .has_arg = true, .val = 'p'},
> >> +	{ .name = "overlay-a-format", .has_arg = true, .val = 'a'},
> >> +	{ .name = "overlay-b-format", .has_arg = true, .val = 'b'},
> >> +	{ .name = "writeback-format", .has_arg = true, .val = 'w'},
> >> +	{}
> >> +};
> >> +
> >> +static struct data_t data = (struct data_t){
> >> +	.kms = {
> >> +			.crtc = {
> >> +				.width = 4096, .height = 2160,
> >> +			},
> >> +		.primary = {
> >> +			.rect = {
> >> +				.x = 101, .y = 0,
> >> +				.width = 3639, .height = 2160,
> >> +			},
> >>  		},
> >> -	},
> >> -	.overlay_a = {
> >> -		.rect = {
> >> -			.x = 201, .y = 199,
> >> -			.width = 3033, .height = 1777,
> >> +		.overlay_a = {
> >> +			.rect = {
> >> +				.x = 201, .y = 199,
> >> +				.width = 3033, .height = 1777,
> >> +			},
> >>  		},
> >> -	},
> >> -	.overlay_b = {
> >> -		.rect = {
> >> -			.x = 1800, .y = 250,
> >> -			.width = 1507, .height = 1400,
> >> +		.overlay_b = {
> >> +			.rect = {
> >> +				.x = 1800, .y = 250,
> >> +				.width = 1507, .height = 1400,
> >> +			},
> >>  		},
> >> -	},
> >> -	.writeback = {
> >> -		.rect = {
> >> -			.x = 0, .y = 0,
> >> -			// Size is to be determined at runtime
> >> +		.writeback = {
> >> +			.rect = {
> >> +				.x = 0, .y = 0,
> >> +				// Size is to be determined at runtime
> >> +			},
> >>  		},
> >>  	},
> >>  };
> >>  
> >> -
> >> -igt_simple_main
> >> +igt_simple_main_args(NULL, long_options, help_str, opt_handler, &data)
> >>  {
> >> -	struct data_t data = {0};
> >>  	size_t primary_fmt_count = 0;
> >>  	size_t overlay_a_fmt_count = 0;
> >>  	size_t overlay_b_fmt_count = 0;
> >> -
> >> -	data.kms = default_kms;
> >> +	size_t wb_fmt_count = 0;
> >>  
> >>  	data.fd = drm_open_driver_master(DRIVER_ANY);
> >>  
> >> @@ -314,37 +361,48 @@ igt_simple_main
> >>  								  DRM_PLANE_TYPE_OVERLAY, 1);
> >>  	igt_assert(data.kms.overlay_b.base != NULL);
> >>  
> >> -	primary_fmt_count = data.kms.primary.base->format_mod_count;
> >> -	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
> >> -	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
> >> +	primary_fmt_count = data.selected_primary_fmt ?
> >> +			    1 : data.kms.primary.base->format_mod_count;
> >> +	overlay_a_fmt_count = data.selected_overlay_a_fmt ?
> >> +			      1 : data.kms.overlay_a.base->format_mod_count;
> >> +	overlay_b_fmt_count = data.selected_overlay_b_fmt ?
> >> +			      1 : data.kms.overlay_b.base->format_mod_count;
> >> +	wb_fmt_count = data.selected_writeback_fmt ? 1 : data.wb_fmt_count;
> >>  
> >>  	for (size_t i = 0;
> >>  	     i < primary_fmt_count *
> >>  		 overlay_a_fmt_count *
> >>  		 overlay_b_fmt_count *
> >> -		 data.wb_fmt_count;
> >> +		 wb_fmt_count;
> >>  	     i++) {
> >>  		size_t p = (i / (overlay_a_fmt_count *
> >>  				 overlay_b_fmt_count *
> >> -				 data.wb_fmt_count)) % primary_fmt_count;
> >> +				 wb_fmt_count)) % primary_fmt_count;
> >>  
> >>  		size_t a = (i / (overlay_b_fmt_count *
> >> -				 data.wb_fmt_count)) % overlay_a_fmt_count;
> >> +				 wb_fmt_count)) % overlay_a_fmt_count;
> >> +
> >> +		size_t b = (i / wb_fmt_count) % overlay_a_fmt_count;
> >> +
> >> +		size_t w = i % wb_fmt_count;
> >> +
> >> +		if (data.selected_primary_fmt == false)
> >> +			data.kms.primary.format = data.kms.primary.base->formats[p];
> >>  
> >> -		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
> >> +		if (data.selected_overlay_a_fmt == false)
> >> +			data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
> >>  
> >> -		size_t w = i % data.wb_fmt_count;
> >> +		if (data.selected_overlay_b_fmt == false)
> >> +			data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
> >>  
> >> -		data.kms.primary.format = data.kms.primary.base->formats[p];
> >> -		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
> >> -		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
> >> -		data.kms.writeback.format = data.wb_formats[w];
> >> +		if (data.selected_writeback_fmt == false)
> >> +			data.kms.writeback.format = data.wb_formats[w];
> >>  
> >>  		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
> >>  			 p + 1, primary_fmt_count,
> >>  			 a + 1, overlay_a_fmt_count,
> >>  			 b + 1, overlay_b_fmt_count,
> >> -			 w + 1, data.wb_fmt_count);
> >> +			 w + 1, wb_fmt_count);
> >>  
> >>  		stress_driver(&data);
> >>  	}
> >>
> >> -- 
> >> 2.43.0
> >>

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH i-g-t v2 7/7] benchmarks/kms_fb_stress: Add command line options to pin the planes or writeback formats

[Arthur Grillo]

On 06/03/24 19:50, Louis Chauvet wrote:
> Le 06/03/24 - 15:14, Arthur Grillo a écrit :
>>
>>
>> On 06/03/24 14:29, Louis Chauvet wrote:
>>> Le 05/03/24 - 20:15, Arthur Grillo a écrit :
>>>> Currently, the benchmark runs all the possible combinations of formats,
>>>> which can take a lot of time. Change that by adding command line options
>>>> to pin the planes or writeback formats, with the intention to decrease
>>>> the number of cases to run.
>>>>
>>>> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>
>>>> ---
>>>>  benchmarks/kms_fb_stress.c | 140 ++++++++++++++++++++++++++++++++-------------
>>>>  1 file changed, 99 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/benchmarks/kms_fb_stress.c b/benchmarks/kms_fb_stress.c
>>>> index deee9d557175..37f60f0bfa94 100644
>>>> --- a/benchmarks/kms_fb_stress.c
>>>> +++ b/benchmarks/kms_fb_stress.c
>>>> @@ -3,6 +3,7 @@
>>>>   * Copyright © 2024 Arthur Grillo
>>>>   */
>>>>  
>>>> +#include <stdbool.h>
>>>>  #include <stdint.h>
>>>>  
>>>>  #include "igt.h"
>>>> @@ -41,6 +42,10 @@ struct data_t {
>>>>  	size_t wb_fmt_count;
>>>>  	drmModeModeInfo *mode;
>>>>  	struct kms_t kms;
>>>> +	bool selected_primary_fmt;
>>>> +	bool selected_overlay_a_fmt;
>>>> +	bool selected_overlay_b_fmt;
>>>> +	bool selected_writeback_fmt;
>>>>  };
>>>
>>> Hi Arthur!
>>>
>>> Nice work, for the next iteration of my series I will use this to run 
>>> a full test, thanks!
>>
>> Hi Louis!
>>
>> That would be great! While developing this, I ran it with your patch and
>> I could see a significant improvement!
>>
>>>
>>> If I understand correclty, when you "pin" a format, it will always use the 
>>> first available? It's not possible to say "I want to test NV24"?
>>
>>
>> No, you can choose what format you "pin", like:
>> kms_fb_stress --primary-format=NV24
>>
>> With this, the primary plane format will always be NV24 and the
>> others will change.
>>
>> Do you think this is not clear in the commit msg or help text?
> 
> I missunderstood how it works because I read first the data_t structure 
> and the "bool selected_*_fmt", where I did not understand why it was a 
> bool and not a uint32_t (for DRM_FORMAT*). And I was too fast reading 
> opt_handler, in which it is very obvious... sorry

It's ok, it happens :)

> 
>>>
>>> Do you think it will be difficult to add this kind of option (like 
>>> --writeback-format in kms_writeback)?
>>
>> I think the option already does what you want. If not, please reply.
> 
> Yes, it does! The help message was not very clear for me, I read "pin" as 
> "only use one format, but I don't care which one", not "use this specific 
> format". Do you think something like this is better: "Use a specific DRM 
> format for the * plane"?

Sure, I can do that, I will put this on v2.

Best Regards,
~Arthur Grillo

> 
> Kind regards,
> Louis Chauvet
> 
>> Best Regards,
>> ~Arthur Grillo
>>
>>>
>>> Kind regards,
>>> Louis Chauvet
>>>
>>>>  static void plane_setup(struct plane_t *plane, int index)
>>>> @@ -243,45 +248,87 @@ static void stress_driver(struct data_t *data)
>>>>  	igt_info("Time spent in the loop with %d frames: %lfs.\n", FRAME_COUNT, elapsed);
>>>>  }
>>>>  
>>>> -static struct kms_t default_kms = {
>>>> -	.crtc = {
>>>> -		.width = 4096, .height = 2160,
>>>> -	},
>>>> -	.primary = {
>>>> -		.rect = {
>>>> -			.x = 101, .y = 0,
>>>> -			.width = 3639, .height = 2160,
>>>> +static int opt_handler(int opt, int opt_index, void *_data)
>>>> +{
>>>> +	struct data_t *data = _data;
>>>> +	struct kms_t *kms =  &data->kms;
>>>> +
>>>> +	switch (opt) {
>>>> +	case 'p':
>>>> +		kms->primary.format = igt_drm_format_str_to_format(optarg);
>>>> +		data->selected_primary_fmt = true;
>>>> +		break;
>>>> +	case 'a':
>>>> +		kms->overlay_a.format = igt_drm_format_str_to_format(optarg);
>>>> +		data->selected_overlay_a_fmt = true;
>>>> +		break;
>>>> +	case 'b':
>>>> +		kms->overlay_b.format = igt_drm_format_str_to_format(optarg);
>>>> +		data->selected_overlay_b_fmt = true;
>>>> +		break;
>>>> +	case 'w':
>>>> +		kms->writeback.format = igt_drm_format_str_to_format(optarg);
>>>> +		data->selected_writeback_fmt = true;
>>>> +		break;
>>>> +	default:
>>>> +		return IGT_OPT_HANDLER_ERROR;
>>>> +	}
>>>> +
>>>> +	return IGT_OPT_HANDLER_SUCCESS;
>>>> +}
>>>> +
>>>> +static const char *help_str =
>>>> +	"  --primary-format\t\tPin the primary plane format\n"
>>>> +	"  --overlay-a-format\t\tPin the overlay A plane format\n"
>>>> +	"  --overlay-b-format\t\tPin the overlay B plane format\n"
>>>> +	"  --writeback-format\t\tPin the writeback format\n";
>>>> +
>>>> +static const struct option long_options[] = {
>>>> +	{ .name = "primary-format", .has_arg = true, .val = 'p'},
>>>> +	{ .name = "overlay-a-format", .has_arg = true, .val = 'a'},
>>>> +	{ .name = "overlay-b-format", .has_arg = true, .val = 'b'},
>>>> +	{ .name = "writeback-format", .has_arg = true, .val = 'w'},
>>>> +	{}
>>>> +};
>>>> +
>>>> +static struct data_t data = (struct data_t){
>>>> +	.kms = {
>>>> +			.crtc = {
>>>> +				.width = 4096, .height = 2160,
>>>> +			},
>>>> +		.primary = {
>>>> +			.rect = {
>>>> +				.x = 101, .y = 0,
>>>> +				.width = 3639, .height = 2160,
>>>> +			},
>>>>  		},
>>>> -	},
>>>> -	.overlay_a = {
>>>> -		.rect = {
>>>> -			.x = 201, .y = 199,
>>>> -			.width = 3033, .height = 1777,
>>>> +		.overlay_a = {
>>>> +			.rect = {
>>>> +				.x = 201, .y = 199,
>>>> +				.width = 3033, .height = 1777,
>>>> +			},
>>>>  		},
>>>> -	},
>>>> -	.overlay_b = {
>>>> -		.rect = {
>>>> -			.x = 1800, .y = 250,
>>>> -			.width = 1507, .height = 1400,
>>>> +		.overlay_b = {
>>>> +			.rect = {
>>>> +				.x = 1800, .y = 250,
>>>> +				.width = 1507, .height = 1400,
>>>> +			},
>>>>  		},
>>>> -	},
>>>> -	.writeback = {
>>>> -		.rect = {
>>>> -			.x = 0, .y = 0,
>>>> -			// Size is to be determined at runtime
>>>> +		.writeback = {
>>>> +			.rect = {
>>>> +				.x = 0, .y = 0,
>>>> +				// Size is to be determined at runtime
>>>> +			},
>>>>  		},
>>>>  	},
>>>>  };
>>>>  
>>>> -
>>>> -igt_simple_main
>>>> +igt_simple_main_args(NULL, long_options, help_str, opt_handler, &data)
>>>>  {
>>>> -	struct data_t data = {0};
>>>>  	size_t primary_fmt_count = 0;
>>>>  	size_t overlay_a_fmt_count = 0;
>>>>  	size_t overlay_b_fmt_count = 0;
>>>> -
>>>> -	data.kms = default_kms;
>>>> +	size_t wb_fmt_count = 0;
>>>>  
>>>>  	data.fd = drm_open_driver_master(DRIVER_ANY);
>>>>  
>>>> @@ -314,37 +361,48 @@ igt_simple_main
>>>>  								  DRM_PLANE_TYPE_OVERLAY, 1);
>>>>  	igt_assert(data.kms.overlay_b.base != NULL);
>>>>  
>>>> -	primary_fmt_count = data.kms.primary.base->format_mod_count;
>>>> -	overlay_a_fmt_count = data.kms.overlay_a.base->format_mod_count;
>>>> -	overlay_b_fmt_count = data.kms.overlay_b.base->format_mod_count;
>>>> +	primary_fmt_count = data.selected_primary_fmt ?
>>>> +			    1 : data.kms.primary.base->format_mod_count;
>>>> +	overlay_a_fmt_count = data.selected_overlay_a_fmt ?
>>>> +			      1 : data.kms.overlay_a.base->format_mod_count;
>>>> +	overlay_b_fmt_count = data.selected_overlay_b_fmt ?
>>>> +			      1 : data.kms.overlay_b.base->format_mod_count;
>>>> +	wb_fmt_count = data.selected_writeback_fmt ? 1 : data.wb_fmt_count;
>>>>  
>>>>  	for (size_t i = 0;
>>>>  	     i < primary_fmt_count *
>>>>  		 overlay_a_fmt_count *
>>>>  		 overlay_b_fmt_count *
>>>> -		 data.wb_fmt_count;
>>>> +		 wb_fmt_count;
>>>>  	     i++) {
>>>>  		size_t p = (i / (overlay_a_fmt_count *
>>>>  				 overlay_b_fmt_count *
>>>> -				 data.wb_fmt_count)) % primary_fmt_count;
>>>> +				 wb_fmt_count)) % primary_fmt_count;
>>>>  
>>>>  		size_t a = (i / (overlay_b_fmt_count *
>>>> -				 data.wb_fmt_count)) % overlay_a_fmt_count;
>>>> +				 wb_fmt_count)) % overlay_a_fmt_count;
>>>> +
>>>> +		size_t b = (i / wb_fmt_count) % overlay_a_fmt_count;
>>>> +
>>>> +		size_t w = i % wb_fmt_count;
>>>> +
>>>> +		if (data.selected_primary_fmt == false)
>>>> +			data.kms.primary.format = data.kms.primary.base->formats[p];
>>>>  
>>>> -		size_t b = (i / data.wb_fmt_count) % overlay_a_fmt_count;
>>>> +		if (data.selected_overlay_a_fmt == false)
>>>> +			data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
>>>>  
>>>> -		size_t w = i % data.wb_fmt_count;
>>>> +		if (data.selected_overlay_b_fmt == false)
>>>> +			data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
>>>>  
>>>> -		data.kms.primary.format = data.kms.primary.base->formats[p];
>>>> -		data.kms.overlay_a.format = data.kms.overlay_a.base->formats[a];
>>>> -		data.kms.overlay_b.format = data.kms.overlay_b.base->formats[b];
>>>> -		data.kms.writeback.format = data.wb_formats[w];
>>>> +		if (data.selected_writeback_fmt == false)
>>>> +			data.kms.writeback.format = data.wb_formats[w];
>>>>  
>>>>  		igt_info("formats: primary: %zu/%zu overlay_a: %zu/%zu overlay_b: %zu/%zu writeback: %zu/%zu\n",
>>>>  			 p + 1, primary_fmt_count,
>>>>  			 a + 1, overlay_a_fmt_count,
>>>>  			 b + 1, overlay_b_fmt_count,
>>>> -			 w + 1, data.wb_fmt_count);
>>>> +			 w + 1, wb_fmt_count);
>>>>  
>>>>  		stress_driver(&data);
>>>>  	}
>>>>
>>>> -- 
>>>> 2.43.0
>>>>
>