[PATCH i-g-t v5 0/6] Add writeback changes for intel-supported formats

[PATCH i-g-t v5 0/6] Add writeback changes for intel-supported formats

[Sowmiya S]

This series introduces validation for writeback functionality
across newly supported formats specific to Intel platforms.
existing functions with limited format support were retained
for compatibility. After validating new formats introduced by
the platform, additional format support was added to the
writeback test suite. Subtests leveraging common framebuffer
creation logic were consolidated and dynamic subtest generation
was implemented to efficiently handle format-specific variations.
Add negative test cases to validate format and mode.Refactored
existing test cases to improve efficiency.

Sowmiya S (6):
  tests/kms_writeback: Add support for intel-specific formats
  lib/igt_fb: Validate supported formats for crc calculation
  tests/kms_writeback: Add intel check for vendor-specific restriction
  tests/kms_writeback: Refactor format handling in commit_and_dump_fb
  tests/kms_writeback: Refactor writeback-fb-id subtest
  tests/kms_writeback: Refactor writeback-check-output subtest

 lib/igt_fb.c          |   6 +-
 tests/kms_writeback.c | 239 +++++++++++++++++++++++++-----------------
 2 files changed, 145 insertions(+), 100 deletions(-)

-- 
2.43.0

[PATCH i-g-t v5 1/6] tests/kms_writeback: Add support for intel-specific formats

[Sowmiya S]

Integrate BGR formats supported by intel
into the supported format list and verify
format before test execution.

v4: Reframe commit message. (Suraj)

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 tests/kms_writeback.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 0b968baa2..691774777 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -90,6 +90,15 @@ static data_t data;
 enum {
 	XRGB8888 = 1 << 0,
 	XRGB2101010 = 1 << 1,
+	XBGR8888 = 1 << 2,
+	XBGR2101010 = 1 << 3,
+};
+
+const uint32_t fourcc[] = {
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XBGR2101010,
 };
 
 static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
@@ -101,11 +110,6 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
 	int width, height, ret;
 	uint16_t i;
 
-	const uint32_t fourcc[] = {
-		DRM_FORMAT_XRGB8888,
-		DRM_FORMAT_XRGB2101010,
-	};
-
 	igt_output_override_mode(output, &override_mode);
 
 	width = override_mode.hdisplay;
@@ -299,7 +303,10 @@ static void fill_fb(igt_fb_t *fb, uint32_t pixel)
 	uint32_t *ptr;
 	int64_t pixel_count, i;
 
-	igt_assert(fb->drm_format == DRM_FORMAT_XRGB8888 || fb->drm_format == DRM_FORMAT_XRGB2101010);
+	igt_assert(fb->drm_format == DRM_FORMAT_XRGB8888 ||
+		   fb->drm_format == DRM_FORMAT_XRGB2101010 ||
+		   fb->drm_format == DRM_FORMAT_XBGR8888 ||
+		   fb->drm_format == DRM_FORMAT_XBGR2101010);
 
 	ptr = igt_fb_map_buffer(fb->fd, fb);
 	igt_assert(ptr);
@@ -334,7 +341,8 @@ static void writeback_sequence(igt_output_t *output, igt_plane_t *plane,
 	uint32_t clear_color = 0xffffffff;
 	igt_crc_t cleared_crc, out_expected;
 
-	if (fourcc_color == DRM_FORMAT_XRGB2101010)
+	if (fourcc_color == DRM_FORMAT_XRGB2101010 ||
+	    fourcc_color == DRM_FORMAT_XBGR2101010)
 		in_fb_colors = in_fb_colors_10bits;
 	else
 		in_fb_colors = in_fb_colors_8bits;
@@ -420,7 +428,12 @@ static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
 static void do_single_commit(igt_output_t *output, igt_plane_t *plane, igt_fb_t *in_fb,
 			      igt_fb_t *out_fb)
 {
-	uint32_t in_fb_color = 0xffff0000;
+	uint32_t in_fb_color;
+
+	if (data.format == DRM_FORMAT_XRGB8888 || data.format == DRM_FORMAT_XBGR8888)
+		in_fb_color = 0xffff0000;
+	else
+		in_fb_color = 0x3ff00000;
 
 	fill_fb(in_fb, in_fb_color);
 
-- 
2.43.0

[PATCH i-g-t v5 2/6] lib/igt_fb: Validate supported formats for crc calculation

[Sowmiya S]

Add logic to verify that provided formats
are supported before proceeding with test
execution for crc calculation

v2: Reframe commit message (Kamil)
    Separate lib changes in single commit (Kamil)
v4: Remove BGR2101010 format verification from
    fnv1a function (Suraj)

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 lib/igt_fb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d59fe133b..3a1553d48 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -5035,7 +5035,8 @@ int igt_fb_get_fnv1a_crc(struct igt_fb *fb, igt_crc_t *crc)
 	if (fb->num_planes != 1)
 		return -EINVAL;
 
-	if (fb->drm_format != DRM_FORMAT_XRGB8888 && fb->drm_format != DRM_FORMAT_XRGB2101010)
+	if (fb->drm_format != DRM_FORMAT_XRGB8888 && fb->drm_format != DRM_FORMAT_XRGB2101010 &&
+	    fb->drm_format != DRM_FORMAT_XBGR8888)
 		return -EINVAL;
 
 	ptr = igt_fb_map_buffer(fb->fd, fb);
@@ -5062,7 +5063,8 @@ int igt_fb_get_fnv1a_crc(struct igt_fb *fb, igt_crc_t *crc)
 		for (x = 0; x < fb->width; x++) {
 			uint32_t pixel = le32_to_cpu(line[x]);
 
-			if (fb->drm_format == DRM_FORMAT_XRGB8888)
+			if (fb->drm_format == DRM_FORMAT_XRGB8888 ||
+			    fb->drm_format == DRM_FORMAT_XBGR8888)
 				pixel &= 0x00ffffff;
 			else if (fb->drm_format == DRM_FORMAT_XRGB2101010)
 				pixel &= 0x3fffffff;
-- 
2.43.0

[PATCH i-g-t v5 3/6] tests/kms_writeback: Add intel check for vendor-specific restriction

[Sowmiya S]

Add intel checks for vendor specific restriction.
Disallow empty FB as writeback FB only for Intel.

v4: Restore drm_fd added in struct and access it from
    igt_display_t struct (Suraj)
v5: Revise commit message and subject with detailed
    explanation. (Suraj)

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
---
 tests/kms_writeback.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 691774777..defe43686 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -291,7 +291,7 @@ static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *
 
 	/* Zero WRITEBACK_FB_ID */
 	ret = do_writeback_test(output, 0, NULL, false);
-	igt_assert_eq(ret, 0);
+	igt_assert_eq(ret, is_intel_device(output->display->drm_fd) ? -EINVAL : 0);
 
 	/* Valid output buffer */
 	ret = do_writeback_test(output, valid_fb->fb_id, NULL, false);
@@ -406,7 +406,8 @@ static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
 
 	/* Two commits, the second with no writeback */
 	out_fbs[0] = output_fb;
-	writeback_sequence(output, plane, input_fb, out_fbs, 2, fourcc_color);
+	if (!is_intel_device(output->display->drm_fd))
+		writeback_sequence(output, plane, input_fb, out_fbs, 2, fourcc_color);
 
 	/* Two commits, both with writeback */
 	out_fbs[1] = output_fb;
-- 
2.43.0

[PATCH i-g-t v5 4/6] tests/kms_writeback: Refactor format handling in commit_and_dump_fb

[Sowmiya S]

Modify commit_and_dump_fb to select supported
formats if provided; Apply default format based
on the platform when none are specified.

v4: Add get format function to assign format
    based on vendor type as a default format.
v5: Change function name more descriptive

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
---
 tests/kms_writeback.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index defe43686..be123adcc 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -101,6 +101,13 @@ const uint32_t fourcc[] = {
 	DRM_FORMAT_XBGR2101010,
 };
 
+static uint32_t get_default_format_by_vendor(igt_display_t *display)
+{
+	if (is_intel_device(display->drm_fd))
+		return DRM_FORMAT_XBGR8888;
+	return DRM_FORMAT_XRGB8888;
+}
+
 static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
 				    drmModeModeInfo override_mode)
 {
@@ -447,7 +454,7 @@ static void do_single_commit(igt_output_t *output, igt_plane_t *plane, igt_fb_t
 }
 
 static void commit_and_dump_fb(igt_display_t *display, igt_output_t *output, igt_plane_t *plane,
-			        igt_fb_t *input_fb, drmModeModeInfo *mode)
+				igt_fb_t *input_fb, drmModeModeInfo *mode)
 {
 	cairo_surface_t *fb_surface_out;
 	char filepath_out[PATH_MAX];
@@ -456,12 +463,37 @@ static void commit_and_dump_fb(igt_display_t *display, igt_output_t *output, igt
 	char *file_name;
 	unsigned int fb_id;
 	igt_fb_t output_fb;
+	uint32_t format;
 
 	path_name = getenv("IGT_FRAME_DUMP_PATH");
 	file_name = getenv("FRAME_PNG_FILE_NAME");
 
+	if (!data.wb_fmt) {
+		format = get_default_format_by_vendor(display);
+	} else {
+		drmModePropertyBlobRes *formats_blob;
+
+		formats_blob = igt_get_writeback_formats_blob(output);
+		igt_assert_f(formats_blob, "No writeback pixel formats\n");
+		igt_assert(!(formats_blob->length % 4));
+		for (int i = 0; i < formats_blob->length; i++) {
+			format = ((uint32_t *)formats_blob->data)[i];
+			// Skip zero or unknown formats
+			if (format == 0 || strcmp(igt_format_str(format), "invalid") == 0) {
+				format = 0;
+				continue;
+			}
+			igt_debug("Supported writeback format: %s\n", igt_format_str(format));
+			if (format == data.format)
+				break;
+			format = 0;
+		}
+		drmModeFreePropertyBlob(formats_blob);
+	}
+	igt_assert_f(format, "Given format not supported\n");
+
 	fb_id = igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
-			      data.wb_fmt ? data.format : DRM_FORMAT_XRGB8888,
+			      format,
 			      igt_fb_mod_to_tiling(0), &output_fb);
 	igt_require(fb_id > 0);
 
@@ -584,7 +616,7 @@ int igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
 
 		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay,
 				      mode.vdisplay,
-				      DRM_FORMAT_XRGB8888,
+				      get_default_format_by_vendor(&display),
 				      DRM_FORMAT_MOD_LINEAR,
 				      &input_fb);
 		igt_assert(fb_id >= 0);
-- 
2.43.0

[PATCH i-g-t v5 5/6] tests/kms_writeback: Refactor writeback-fb-id subtest

[Sowmiya S]

Remove individual tests for each format for the test
writeback-fb-id and introduce dynamic subtest generation
based on the formats supported. Framebuffer creation was
now part of the subtest level based on format.

v4: squashed the FB creation function with this commit (Suraj)
v5: -Update dynamic subtest documentation (Suraj)
    -Rename input_fb to in_fb
    -Set plane to in_fb and remove unused input_fb as an arg to
    writeback-fb-id function.

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
---
 tests/kms_writeback.c | 87 +++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 36 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index be123adcc..5fff27c2b 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -51,11 +51,14 @@
  * SUBTEST: writeback-check-output
  * Description: Check writeback output with CRC validation
  *
- * SUBTEST: writeback-fb-id-XRGB2101010
- * Description: Validate WRITEBACK_FB_ID with valid and invalid options
+ * SUBTEST: writeback-fb-id-%s
+ * Description:  Validate  WRITEBACK_FB_ID with valid and invalid options
+ *               for %arg[1] format
  *
- * SUBTEST: writeback-fb-id
- * Description: Validate WRITEBACK_FB_ID with valid and invalid options
+ * arg[1]:
+ *
+ * @XBGR8888:       XBGR8888
+ * @XBGR2101010:    XBGR2101010
  *
  * SUBTEST: writeback-invalid-parameters
  * Description: Writeback has a couple of parameters linked together(output
@@ -287,7 +290,7 @@ static void test_invalid_parameters(igt_output_t *output, igt_fb_t *valid_fb, ig
 	}
 }
 
-static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *invalid_fb)
+static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb)
 {
 
 	int ret;
@@ -508,6 +511,25 @@ static void commit_and_dump_fb(igt_display_t *display, igt_output_t *output, igt
 	igt_remove_fb(display->drm_fd, &output_fb);
 }
 
+static void create_fbs(igt_display_t *display, igt_fb_t *in_fb, igt_fb_t *output_fb,
+		       uint32_t format, drmModeModeInfo *mode)
+{
+	int fb_id;
+
+	fb_id = igt_create_fb(display->drm_fd, mode->hdisplay,
+			      mode->vdisplay,
+			      format,
+			      DRM_FORMAT_MOD_LINEAR,
+			      in_fb);
+	igt_assert(fb_id >= 0);
+
+	fb_id = igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      format,
+			      DRM_FORMAT_MOD_LINEAR,
+			      output_fb);
+	igt_assert(fb_id >= 0);
+}
+
 static igt_output_t *list_writeback_modes(igt_display_t *display)
 {
 	for (int i = 0; i < display->n_outputs; i++) {
@@ -686,37 +708,30 @@ int igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
 	}
 
 	igt_describe("Validate WRITEBACK_FB_ID with valid and invalid options");
-	igt_subtest("writeback-fb-id") {
-		igt_fb_t output_fb;
-
-		igt_skip_on(data.dump_check || data.list_modes);
-		igt_skip_on_f(!(data.supported_colors & XRGB8888),"DRM_FORMAT_XRGB8888 is unsupported\n");
-		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
-				      DRM_FORMAT_XRGB8888,
-				      DRM_FORMAT_MOD_LINEAR,
-				      &output_fb);
-		igt_require(fb_id > 0);
-
-		writeback_fb_id(output, &input_fb, &output_fb);
-
-		igt_remove_fb(display.drm_fd, &output_fb);
-	}
-
-	igt_describe("Validate XRGB2101010 WRITEBACK_FB_ID with valid and invalid options");
-	igt_subtest("writeback-fb-id-XRGB2101010") {
-		igt_fb_t output_fb;
-
-		igt_skip_on(data.dump_check || data.list_modes);
-		igt_skip_on_f(!(data.supported_colors & XRGB2101010), "DRM_FORMAT_XRGB2101010 is unsupported\n");
-		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
-				      DRM_FORMAT_XRGB2101010,
-				      DRM_FORMAT_MOD_LINEAR,
-				      &output_fb);
-		igt_require(fb_id > 0);
-
-		writeback_fb_id(output, &input_fb_10bit, &output_fb);
-
-		igt_remove_fb(display.drm_fd, &output_fb);
+	igt_subtest_with_dynamic_f("writeback-fb-id") {
+		for (int i = 0; i < ARRAY_SIZE(fourcc); i++) {
+			uint32_t test_format = fourcc[i];
+
+			if (is_intel_device(display.drm_fd))
+				if (!strstr(igt_format_str(test_format), "BGR"))
+					continue;
+			igt_dynamic_f("writeback-fb-id-%s", igt_format_str(test_format)) {
+				igt_fb_t in_fb, output_fb;
+
+				igt_skip_on(data.dump_check || data.list_modes);
+				igt_skip_on_f(!(data.supported_colors & (1 << i)),
+					      "DRM_FORMAT_%s is not supported\n",
+					      igt_format_str(test_format));
+				create_fbs(&display, &in_fb, &output_fb, test_format, &mode);
+				igt_plane_set_fb(plane, &in_fb);
+				writeback_fb_id(output, &output_fb);
+				/* cleanup */
+				igt_plane_set_fb(plane, NULL);
+				igt_display_commit2(output->display, COMMIT_ATOMIC);
+				igt_remove_fb(display.drm_fd, &in_fb);
+				igt_remove_fb(display.drm_fd, &output_fb);
+			}
+		}
 	}
 
 	igt_describe("Check writeback output with CRC validation");
-- 
2.43.0

[PATCH i-g-t v5 6/6] tests/kms_writeback: Refactor writeback-check-output subtest

[Sowmiya S]

Remove individual tests for each format for the test
writeback-check-output and introduce dynamic subtest
generation based on the formats supported.Framebuffer
creation was now part of the subtest level based on
the format and removed from fixture.

v4: -create testcase for BGR8888 format only for intel
    platforms.(Suraj)
v5: -update the documentation for dynamic subtest
    -update version info (Suraj)

Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
---
 tests/kms_writeback.c | 78 +++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 5fff27c2b..8f5548f33 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -48,8 +48,13 @@
  * SUBTEST: writeback-check-output-XRGB2101010
  * Description: Check XRGB2101010 writeback output with CRC validation
  *
- * SUBTEST: writeback-check-output
- * Description: Check writeback output with CRC validation
+ * SUBTEST: writeback-check-output-%s
+ * Description: Check writeback output with CRC validation for
+ *              %arg[1] format
+ *
+ * arg[1]:
+ *
+ * @XBGR8888:       XBGR8888
  *
  * SUBTEST: writeback-fb-id-%s
  * Description:  Validate  WRITEBACK_FB_ID with valid and invalid options
@@ -494,7 +499,6 @@ static void commit_and_dump_fb(igt_display_t *display, igt_output_t *output, igt
 		drmModeFreePropertyBlob(formats_blob);
 	}
 	igt_assert_f(format, "Given format not supported\n");
-
 	fb_id = igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
 			      format,
 			      igt_fb_mod_to_tiling(0), &output_fb);
@@ -602,7 +606,7 @@ int igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
 	igt_display_t display;
 	igt_output_t *output;
 	igt_plane_t *plane;
-	igt_fb_t input_fb, input_fb_10bit;
+	igt_fb_t input_fb;
 	drmModeModeInfo mode;
 	unsigned int fb_id;
 	int ret;
@@ -643,17 +647,6 @@ int igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
 				      &input_fb);
 		igt_assert(fb_id >= 0);
 
-		if (data.supported_colors & XRGB2101010) {
-			fb_id = igt_create_fb(display.drm_fd, mode.hdisplay,
-					      mode.vdisplay,
-					      DRM_FORMAT_XRGB2101010,
-					      DRM_FORMAT_MOD_LINEAR,
-					      &input_fb_10bit);
-			igt_assert(fb_id >= 0);
-		}
-
-		igt_plane_set_fb(plane, &input_fb);
-
 		if (data.list_modes)
 			list_writeback_modes(&display);
 		if (data.dump_check)
@@ -735,46 +728,35 @@ int igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
 	}
 
 	igt_describe("Check writeback output with CRC validation");
-	igt_subtest("writeback-check-output") {
-		igt_fb_t output_fb;
-
-		igt_skip_on(data.dump_check || data.list_modes);
-		igt_skip_on_f(!(data.supported_colors & XRGB8888),"DRM_FORMAT_XRGB8888 is unsupported\n");
-		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
-				      DRM_FORMAT_XRGB8888,
-				      igt_fb_mod_to_tiling(0),
-				      &output_fb);
-		igt_require(fb_id > 0);
-
-		writeback_check_output(output, plane, &input_fb, &output_fb, DRM_FORMAT_XRGB8888);
-
-		igt_remove_fb(display.drm_fd, &output_fb);
-	}
-
-	igt_describe("Check XRGB2101010 writeback output with CRC validation");
-	igt_subtest("writeback-check-output-XRGB2101010") {
-		igt_fb_t output_fb;
-
-		igt_skip_on(data.dump_check || data.list_modes);
-		igt_skip_on_f(!(data.supported_colors & XRGB2101010), "DRM_FORMAT_XRGB2101010 is unsupported\n");
-		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
-				      DRM_FORMAT_XRGB2101010,
-				      igt_fb_mod_to_tiling(0),
-				      &output_fb);
-		igt_require(fb_id > 0);
+	igt_subtest_with_dynamic_f("writeback-check-output") {
+		for (int i = 0; i < ARRAY_SIZE(fourcc); i++) {
+			uint32_t test_format = fourcc[i];
 
-		writeback_check_output(output, plane, &input_fb_10bit, &output_fb, DRM_FORMAT_XRGB2101010);
+			if (is_intel_device(display.drm_fd))
+				if (!strstr(igt_format_str(test_format), "BGR8888"))
+					continue;
+			igt_dynamic_f("writeback-check-output-%s", igt_format_str(test_format)) {
+				igt_fb_t in_fb, output_fb;
 
-		igt_remove_fb(display.drm_fd, &output_fb);
+				igt_skip_on(data.dump_check || data.list_modes);
+				igt_skip_on_f(!(data.supported_colors & (1 << i)),
+					      "DRM_FORMAT_%s is not supported\n",
+					      igt_format_str(test_format));
+				create_fbs(&display, &in_fb, &output_fb, test_format, &mode);
+				writeback_check_output(output, plane, &in_fb,
+						       &output_fb, test_format);
+				/* cleanup */
+				igt_plane_set_fb(plane, NULL);
+				igt_display_commit2(output->display, COMMIT_ATOMIC);
+				igt_remove_fb(display.drm_fd, &output_fb);
+				igt_remove_fb(display.drm_fd, &in_fb);
+			}
+		}
 	}
 
 	igt_fixture() {
 		cleanup_writeback(&display, output);
 		igt_remove_fb(display.drm_fd, &input_fb);
-
-		if (data.supported_colors & XRGB2101010)
-			igt_remove_fb(display.drm_fd, &input_fb_10bit);
-
 		igt_display_fini(&display);
 		drm_close_driver(display.drm_fd);
 	}
-- 
2.43.0

RE: [PATCH i-g-t v5 3/6] tests/kms_writeback: Add intel check for vendor-specific restriction

[Kandpal, Suraj]

> Subject: [PATCH i-g-t v5 3/6] tests/kms_writeback: Add intel check for vendor-
> specific restriction
> 
> Add intel checks for vendor specific restriction.
> Disallow empty FB as writeback FB only for Intel.

Correct message description should be "Intel does not allow to commit fb with 0 fb id or
an empty writeback commit. Update asserts and checks to take that into account"

Update the commit subject appropriately too

With that fixed,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> 
> v4: Restore drm_fd added in struct and access it from
>     igt_display_t struct (Suraj)
> v5: Revise commit message and subject with detailed
>     explanation. (Suraj)
> 
> Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
> ---
>  tests/kms_writeback.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index
> 691774777..defe43686 100644
> --- a/tests/kms_writeback.c
> +++ b/tests/kms_writeback.c
> @@ -291,7 +291,7 @@ static void writeback_fb_id(igt_output_t *output,
> igt_fb_t *valid_fb, igt_fb_t *
> 
>  	/* Zero WRITEBACK_FB_ID */
>  	ret = do_writeback_test(output, 0, NULL, false);
> -	igt_assert_eq(ret, 0);
> +	igt_assert_eq(ret, is_intel_device(output->display->drm_fd) ? -EINVAL
> +: 0);
> 
>  	/* Valid output buffer */
>  	ret = do_writeback_test(output, valid_fb->fb_id, NULL, false); @@ -
> 406,7 +406,8 @@ static void writeback_check_output(igt_output_t *output,
> igt_plane_t *plane,
> 
>  	/* Two commits, the second with no writeback */
>  	out_fbs[0] = output_fb;
> -	writeback_sequence(output, plane, input_fb, out_fbs, 2,
> fourcc_color);
> +	if (!is_intel_device(output->display->drm_fd))
> +		writeback_sequence(output, plane, input_fb, out_fbs, 2,
> +fourcc_color);
> 
>  	/* Two commits, both with writeback */
>  	out_fbs[1] = output_fb;
> --
> 2.43.0

RE: [PATCH i-g-t v5 4/6] tests/kms_writeback: Refactor format handling in commit_and_dump_fb

[Kandpal, Suraj]

> Subject: [PATCH i-g-t v5 4/6] tests/kms_writeback: Refactor format handling
> in commit_and_dump_fb
> 
> Modify commit_and_dump_fb to select supported formats if provided; Apply
> default format based on the platform when none are specified.
> 
> v4: Add get format function to assign format
>     based on vendor type as a default format.
> v5: Change function name more descriptive
> 
> Signed-off-by: Sowmiya S <sowmiya.s@intel.com>
> ---
>  tests/kms_writeback.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index
> defe43686..be123adcc 100644
> --- a/tests/kms_writeback.c
> +++ b/tests/kms_writeback.c
> @@ -101,6 +101,13 @@ const uint32_t fourcc[] = {
>  	DRM_FORMAT_XBGR2101010,
>  };
> 
> +static uint32_t get_default_format_by_vendor(igt_display_t *display) {

Wait for my reply before you revise a series I may suggest something else. This
Helps avoid many series revision

Anyways the function name should just be get_default_fb_format no need for vendor

With that fixed LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>


> +	if (is_intel_device(display->drm_fd))
> +		return DRM_FORMAT_XBGR8888;
> +	return DRM_FORMAT_XRGB8888;
> +}
> +
>  static bool check_writeback_config(igt_display_t *display, igt_output_t
> *output,
>  				    drmModeModeInfo override_mode)
>  {
> @@ -447,7 +454,7 @@ static void do_single_commit(igt_output_t *output,
> igt_plane_t *plane, igt_fb_t  }
> 
>  static void commit_and_dump_fb(igt_display_t *display, igt_output_t
> *output, igt_plane_t *plane,
> -			        igt_fb_t *input_fb, drmModeModeInfo *mode)
> +				igt_fb_t *input_fb, drmModeModeInfo
> *mode)
>  {
>  	cairo_surface_t *fb_surface_out;
>  	char filepath_out[PATH_MAX];
> @@ -456,12 +463,37 @@ static void commit_and_dump_fb(igt_display_t
> *display, igt_output_t *output, igt
>  	char *file_name;
>  	unsigned int fb_id;
>  	igt_fb_t output_fb;
> +	uint32_t format;
> 
>  	path_name = getenv("IGT_FRAME_DUMP_PATH");
>  	file_name = getenv("FRAME_PNG_FILE_NAME");
> 
> +	if (!data.wb_fmt) {
> +		format = get_default_format_by_vendor(display);
> +	} else {
> +		drmModePropertyBlobRes *formats_blob;
> +
> +		formats_blob = igt_get_writeback_formats_blob(output);
> +		igt_assert_f(formats_blob, "No writeback pixel formats\n");
> +		igt_assert(!(formats_blob->length % 4));
> +		for (int i = 0; i < formats_blob->length; i++) {
> +			format = ((uint32_t *)formats_blob->data)[i];
> +			// Skip zero or unknown formats
> +			if (format == 0 || strcmp(igt_format_str(format),
> "invalid") == 0) {
> +				format = 0;
> +				continue;
> +			}
> +			igt_debug("Supported writeback format: %s\n",
> igt_format_str(format));
> +			if (format == data.format)
> +				break;
> +			format = 0;
> +		}
> +		drmModeFreePropertyBlob(formats_blob);
> +	}
> +	igt_assert_f(format, "Given format not supported\n");
> +
>  	fb_id = igt_create_fb(display->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> -			      data.wb_fmt ? data.format :
> DRM_FORMAT_XRGB8888,
> +			      format,
>  			      igt_fb_mod_to_tiling(0), &output_fb);
>  	igt_require(fb_id > 0);
> 
> @@ -584,7 +616,7 @@ int igt_main_args("b:c:f:dl", long_options, help_str,
> opt_handler, NULL)
> 
>  		fb_id = igt_create_fb(display.drm_fd, mode.hdisplay,
>  				      mode.vdisplay,
> -				      DRM_FORMAT_XRGB8888,
> +				      get_default_format_by_vendor(&display),
>  				      DRM_FORMAT_MOD_LINEAR,
>  				      &input_fb);
>  		igt_assert(fb_id >= 0);
> --
> 2.43.0