All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [i-g-t V2 0/8] Revert Color patches
@ 2022-09-22  5:11 Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 1/8] Revert "tests/kms_color: fix crc assert condition" Bhanuprakash Modem
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

As we are suspecting dynamic subtests are causing regression,
revert all the way back to before the dynamic subtest conversion.

Bhanuprakash Modem (8):
  Revert "tests/kms_color: fix crc assert condition"
  Revert "tests/kms: Fix to use max_bpc constraint helper"
  Revert "tests/kms_color: Fix multiple failures in deep-color tests"
  Revert "tests/kms_color: Fix memory leaks"
  Revert "tests/kms_color: Test Cleanup"
  Revert "tests/kms_color: Convert tests to dynamic"
  Revert "tests/kms_color_chamelium: Convert tests to dynamic"
  tests/kms_color: Fix crc compare check in CTM tests

 tests/chamelium/kms_color_chamelium.c | 1034 +++++++++++++------------
 tests/kms_color.c                     |  665 ++++++++--------
 tests/kms_color_helper.h              |    2 -
 tests/kms_dither.c                    |   25 +-
 tests/kms_hdr.c                       |   44 +-
 5 files changed, 908 insertions(+), 862 deletions(-)

--
2.37.3

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

* [igt-dev] [i-g-t V2 1/8] Revert "tests/kms_color: fix crc assert condition"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 2/8] Revert "tests/kms: Fix to use max_bpc constraint helper" Bhanuprakash Modem
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit c27dcb9e0baca05abce8527c245b513f7a8f1059.
---
 tests/kms_color.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index ab285af8..c202547e 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -99,7 +99,7 @@ static bool test_pipe_degamma(data_t *data,
 	 * Verify that the CRC of the software computed output is
 	 * equal to the CRC of the degamma LUT transformation output.
 	 */
-	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
+	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
 
 	disable_degamma(primary->pipe);
 	igt_plane_set_fb(primary, NULL);
@@ -188,7 +188,7 @@ static bool test_pipe_gamma(data_t *data,
 	 * Verify that the CRC of the software computed output is
 	 * equal to the CRC of the gamma LUT transformation output.
 	 */
-	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
+	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
 
 	disable_gamma(primary->pipe);
 	igt_plane_set_fb(primary, NULL);
@@ -289,7 +289,7 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	 * Verify that the CRC of the software computed output is
 	 * equal to the CRC of the gamma LUT transformation output.
 	 */
-	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
+	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
 
 	/* Reset output. */
 	for (i = 1; i < legacy_lut_size; i++)
@@ -534,7 +534,7 @@ static bool test_pipe_ctm(data_t *data,
 	 * Verify that the CRC of the software computed output is
 	 * equal to the CRC of the CTM matrix transformation output.
 	 */
-	ret &= igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
+	ret &= !igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
 
 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 2/8] Revert "tests/kms: Fix to use max_bpc constraint helper"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 1/8] Revert "tests/kms_color: fix crc assert condition" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 3/8] Revert "tests/kms_color: Fix multiple failures in deep-color tests" Bhanuprakash Modem
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit a37df4b538ae7b5bb1bb47be4804b4787410325d.
---
 tests/kms_color.c  | 24 +++++++++++++++++++++++-
 tests/kms_dither.c | 25 ++++++++++++++++++++++++-
 tests/kms_hdr.c    | 44 +++++++++++++++++++++++++++++---------------
 3 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index c202547e..8c15c7be 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -660,6 +660,28 @@ static void test_pipe_limited_range_ctm(data_t *data,
 }
 #endif
 
+static bool i915_clock_constraint(data_t *data, enum pipe pipe, int bpc)
+{
+	igt_output_t *output = data->output;
+	drmModeConnector *connector = output->config.connector;
+
+	igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
+
+	for_each_connector_mode(output) {
+		igt_output_override_mode(output, &connector->modes[j__]);
+		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+		if (!igt_check_output_bpc_equal(data->drm_fd, pipe,
+						data->output->name, bpc))
+			continue;
+
+		return true;
+	}
+
+	igt_output_override_mode(output, NULL);
+	return false;
+}
+
 static void
 prep_pipe(data_t *data, enum pipe p)
 {
@@ -823,7 +845,7 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
 
 		if (is_i915_device(data->drm_fd) &&
-		    !igt_max_bpc_constraint(&data->display, p, output, 10))
+		    !i915_clock_constraint(data, p, 10))
 			continue;
 
 		data->color_depth = 10;
diff --git a/tests/kms_dither.c b/tests/kms_dither.c
index 43a25e20..3a44e797 100644
--- a/tests/kms_dither.c
+++ b/tests/kms_dither.c
@@ -97,6 +97,29 @@ static dither_status_t get_dither_state(data_t *data, enum pipe pipe)
 	return status;
 }
 
+static bool i915_clock_constraint(data_t *data, enum pipe pipe,
+				  igt_output_t *output, int bpc)
+{
+	drmModeConnector *connector = output->config.connector;
+	igt_display_t *display = &data->display;
+
+	igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
+
+	for_each_connector_mode(output) {
+		igt_output_override_mode(output, &connector->modes[j__]);
+		igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+
+		if (!igt_check_output_bpc_equal(data->drm_fd, pipe,
+						output->name, bpc))
+			continue;
+
+		return true;
+	}
+
+	igt_output_override_mode(output, NULL);
+	return false;
+}
+
 static void test_dithering(data_t *data, enum pipe pipe,
 			   igt_output_t *output,
 			   int fb_bpc, int fb_format,
@@ -129,7 +152,7 @@ static void test_dithering(data_t *data, enum pipe pipe,
 	if (ret)
 		goto cleanup;
 
-	constraint = igt_max_bpc_constraint(display, pipe, output, output_bpc);
+	constraint = i915_clock_constraint(data, pipe, output, output_bpc);
 	if (!constraint)
 		goto cleanup;
 
diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
index e2650f51..6790f26f 100644
--- a/tests/kms_hdr.c
+++ b/tests/kms_hdr.c
@@ -130,7 +130,6 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 					  INTEL_PIPE_CRC_SOURCE_AUTO);
 
 	igt_output_set_pipe(data->output, data->pipe_id);
-	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
 
 	data->w = data->mode->hdisplay;
 	data->h = data->mode->vdisplay;
@@ -205,6 +204,33 @@ static bool has_max_bpc(igt_output_t *output)
 	       igt_output_get_prop(output, IGT_CONNECTOR_MAX_BPC);
 }
 
+static bool i915_clock_constraint(data_t *data, int bpc)
+{
+	igt_output_t *output = data->output;
+	drmModeConnector *connector = output->config.connector;
+
+	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, bpc);
+	igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
+
+	for_each_connector_mode(output) {
+		igt_output_override_mode(output, &connector->modes[j__]);
+		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+		if (!igt_check_output_bpc_equal(data->fd, data->pipe_id,
+						data->output->name, bpc))
+			continue;
+
+		data->mode = igt_output_get_mode(output);
+		data->w = data->mode->hdisplay;
+		data->h = data->mode->vdisplay;
+
+		return true;
+	}
+
+	test_fini(data);
+	return false;
+}
+
 static void test_bpc_switch(data_t *data, uint32_t flags)
 {
 	igt_display_t *display = &data->display;
@@ -224,14 +250,8 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
 				prepare_test(data, output, pipe);
 
 				if (is_i915_device(data->fd) &&
-				    !igt_max_bpc_constraint(display, pipe, output, 10)) {
-					test_fini(data);
+				    !i915_clock_constraint(data, 10))
 					break;
-				}
-
-				data->mode = igt_output_get_mode(output);
-				data->w = data->mode->hdisplay;
-				data->h = data->mode->vdisplay;
 
 				igt_dynamic_f("pipe-%s-%s",
 					      kmstest_pipe_name(pipe), output->name)
@@ -550,14 +570,8 @@ static void test_hdr(data_t *data, uint32_t flags)
 				prepare_test(data, output, pipe);
 
 				if (is_i915_device(data->fd) &&
-				    !igt_max_bpc_constraint(display, pipe, output, 10)) {
-					test_fini(data);
+				    !i915_clock_constraint(data, 10))
 					break;
-				}
-
-				data->mode = igt_output_get_mode(output);
-				data->w = data->mode->hdisplay;
-				data->h = data->mode->vdisplay;
 
 				igt_dynamic_f("pipe-%s-%s",
 					      kmstest_pipe_name(pipe), output->name) {
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 3/8] Revert "tests/kms_color: Fix multiple failures in deep-color tests"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 1/8] Revert "tests/kms_color: fix crc assert condition" Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 2/8] Revert "tests/kms: Fix to use max_bpc constraint helper" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 4/8] Revert "tests/kms_color: Fix memory leaks" Bhanuprakash Modem
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit 333ca8f6be43044d71ecf15d37a3e88e7a3ce821.
---
 tests/kms_color.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 8c15c7be..6ea721f1 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -839,7 +839,6 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		if (!panel_supports_deep_color(data->drm_fd, output->name))
 			continue;
 
-		igt_display_reset(&data->display);
 		igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
 		igt_output_set_pipe(output, p);
 		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
@@ -851,10 +850,7 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		data->color_depth = 10;
 		data->drm_format = DRM_FORMAT_XRGB2101010;
 		data->output = output;
-
-		data->mode = malloc(sizeof(drmModeModeInfo));
-		igt_assert(data->mode);
-		memcpy(data->mode, igt_output_get_mode(data->output), sizeof(drmModeModeInfo));
+		data->mode = igt_output_get_mode(data->output);
 
 		igt_dynamic_f("pipe-%s-%s-gamma", kmstest_pipe_name(p), output->name) {
 			igt_display_reset(&data->display);
@@ -888,8 +884,6 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 			igt_assert(ret);
 		}
 
-		free(data->mode);
-
 		break;
 	}
 
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 4/8] Revert "tests/kms_color: Fix memory leaks"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (2 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 3/8] Revert "tests/kms_color: Fix multiple failures in deep-color tests" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 5/8] Revert "tests/kms_color: Test Cleanup" Bhanuprakash Modem
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit cd21565a5fffc451e2cae5f1a0fe20104dc2a2cf.
---
 tests/kms_color.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 6ea721f1..0b03fb9a 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -397,13 +397,8 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 	drmModeFreeCrtc(kms_crtc);
 
 	red_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
-	igt_assert(red_lut);
-
 	green_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
-	igt_assert(green_lut);
-
 	blue_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
-	igt_assert(blue_lut);
 
 	for (i = 0; i < legacy_lut_size; i++)
 		red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
@@ -432,17 +427,13 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 			   lut[i].blue == 0xffff);
 	drmModeFreePropertyBlob(blob);
 
-	free(red_lut);
-	free(green_lut);
-	free(blue_lut);
-
-end:
 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
 	igt_display_commit(&data->display);
 
 	free_lut(degamma_linear);
 	free_lut(gamma_zero);
+end:
 	return ret;
 }
 
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 5/8] Revert "tests/kms_color: Test Cleanup"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (3 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 4/8] Revert "tests/kms_color: Fix memory leaks" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 6/8] Revert "tests/kms_color: Convert tests to dynamic" Bhanuprakash Modem
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit 1a3ffecd400b8f82c35745fa2e07992f6bdeede2.
---
 tests/kms_color.c        | 96 ++++++++++++++++------------------------
 tests/kms_color_helper.h |  1 -
 2 files changed, 39 insertions(+), 58 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 0b03fb9a..85f3b32d 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -37,7 +37,7 @@ static bool test_pipe_degamma(data_t *data,
 		{ 0.0, 1.0, 0.0 },
 		{ 0.0, 0.0, 1.0 }
 	};
-	drmModeModeInfo *mode = data->mode;
+	drmModeModeInfo *mode;
 	struct igt_fb fb_modeset, fb;
 	igt_crc_t crc_fullgamma, crc_fullcolors;
 	int fb_id, fb_modeset_id;
@@ -50,7 +50,7 @@ static bool test_pipe_degamma(data_t *data,
 	degamma_full = generate_table_max(data->degamma_lut_size);
 
 	igt_output_set_pipe(output, primary->pipe->pipe);
-	igt_output_override_mode(output, mode);
+	mode = igt_output_get_mode(output);
 
 	/* Create a framebuffer at the size of the output. */
 	fb_id = igt_create_fb(data->drm_fd,
@@ -83,8 +83,7 @@ static bool test_pipe_degamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
-	/*
-	 * Draw a gradient with degamma LUT to remap all
+	/* Draw a gradient with degamma LUT to remap all
 	 * values to max red/green/blue.
 	 */
 	paint_gradient_rectangles(data, mode, red_green_blue, &fb);
@@ -95,8 +94,7 @@ static bool test_pipe_degamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
-	/*
-	 * Verify that the CRC of the software computed output is
+	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the degamma LUT transformation output.
 	 */
 	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
@@ -129,7 +127,7 @@ static bool test_pipe_gamma(data_t *data,
 		{ 0.0, 1.0, 0.0 },
 		{ 0.0, 0.0, 1.0 }
 	};
-	drmModeModeInfo *mode = data->mode;
+	drmModeModeInfo *mode;
 	struct igt_fb fb_modeset, fb;
 	igt_crc_t crc_fullgamma, crc_fullcolors;
 	int fb_id, fb_modeset_id;
@@ -140,7 +138,7 @@ static bool test_pipe_gamma(data_t *data,
 	gamma_full = generate_table_max(data->gamma_lut_size);
 
 	igt_output_set_pipe(output, primary->pipe->pipe);
-	igt_output_override_mode(output, mode);
+	mode = igt_output_get_mode(output);
 
 	/* Create a framebuffer at the size of the output. */
 	fb_id = igt_create_fb(data->drm_fd,
@@ -173,8 +171,7 @@ static bool test_pipe_gamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
-	/*
-	 * Draw a gradient with gamma LUT to remap all values
+	/* Draw a gradient with gamma LUT to remap all values
 	 * to max red/green/blue.
 	 */
 	paint_gradient_rectangles(data, mode, red_green_blue, &fb);
@@ -184,8 +181,7 @@ static bool test_pipe_gamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
-	/*
-	 * Verify that the CRC of the software computed output is
+	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the gamma LUT transformation output.
 	 */
 	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
@@ -210,7 +206,7 @@ static bool test_pipe_gamma(data_t *data,
 static bool test_pipe_legacy_gamma(data_t *data,
 				   igt_plane_t *primary)
 {
-	igt_output_t *output = data->output;
+	igt_output_t *output;
 	igt_display_t *display = &data->display;
 	color_t red_green_blue[] = {
 		{ 1.0, 0.0, 0.0 },
@@ -220,7 +216,7 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	drmModeCrtc *kms_crtc;
 	uint32_t i, legacy_lut_size;
 	uint16_t *red_lut, *green_lut, *blue_lut;
-	drmModeModeInfo *mode = data->mode;
+	drmModeModeInfo *mode;
 	struct igt_fb fb_modeset, fb;
 	igt_crc_t crc_fullgamma, crc_fullcolors;
 	int fb_id, fb_modeset_id;
@@ -234,8 +230,11 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	green_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
 	blue_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
 
+	output = igt_get_single_output_for_pipe(&data->display, primary->pipe->pipe);
+	igt_require(output);
+
 	igt_output_set_pipe(output, primary->pipe->pipe);
-	igt_output_override_mode(output, mode);
+	mode = igt_output_get_mode(output);
 
 	/* Create a framebuffer at the size of the output. */
 	fb_id = igt_create_fb(data->drm_fd,
@@ -268,8 +267,7 @@ static bool test_pipe_legacy_gamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
-	/*
-	 * Draw a gradient with gamma LUT to remap all values
+	/* Draw a gradient with gamma LUT to remap all values
 	 * to max red/green/blue.
 	 */
 	paint_gradient_rectangles(data, mode, red_green_blue, &fb);
@@ -285,8 +283,7 @@ static bool test_pipe_legacy_gamma(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
-	/*
-	 * Verify that the CRC of the software computed output is
+	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the gamma LUT transformation output.
 	 */
 	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
@@ -301,7 +298,6 @@ static bool test_pipe_legacy_gamma(data_t *data,
 
 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
 	igt_remove_fb(data->drm_fd, &fb);
 	igt_remove_fb(data->drm_fd, &fb_modeset);
 
@@ -330,7 +326,7 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 	uint16_t *red_lut, *green_lut, *blue_lut;
 	struct drm_color_lut *lut;
 	drmModePropertyBlobPtr blob;
-	igt_output_t *output = data->output;
+	igt_output_t *output;
 	bool ret = true;
 
 	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
@@ -339,6 +335,9 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 		degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_zero = generate_table_zero(data->gamma_lut_size);
 
+	output = igt_get_single_output_for_pipe(&data->display, primary->pipe->pipe);
+	igt_require(output);
+
 	igt_output_set_pipe(output, primary->pipe->pipe);
 
 	/* Ensure we have a clean state to start with. */
@@ -347,11 +346,9 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 	disable_gamma(primary->pipe);
 	igt_display_commit(&data->display);
 
-	/*
-	 * Set a degama & gamma LUT and a CTM using the
+	/* Set a degama & gamma LUT and a CTM using the
 	 * properties and verify the content of the
-	 * properties.
-	 */
+	 * properties. */
 	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
 		set_degamma(data, primary->pipe, degamma_linear);
 	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
@@ -387,11 +384,9 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 	if(!ret)
 		goto end;
 
-	/*
-	 * Set a gamma LUT using the legacy ioctl and verify
+	/* Set a gamma LUT using the legacy ioctl and verify
 	 * the content of the GAMMA_LUT property is changed
-	 * and that CTM and DEGAMMA_LUT are empty.
-	 */
+	 * and that CTM and DEGAMMA_LUT are empty. */
 	kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
 	legacy_lut_size = kms_crtc->gamma_size;
 	drmModeFreeCrtc(kms_crtc);
@@ -429,7 +424,6 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 
 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
 
 	free_lut(degamma_linear);
 	free_lut(gamma_zero);
@@ -456,7 +450,7 @@ static bool test_pipe_ctm(data_t *data,
 	igt_output_t *output = data->output;
 	bool ret = true;
 	igt_display_t *display = &data->display;
-	drmModeModeInfo *mode = data->mode;
+	drmModeModeInfo *mode;
 	struct igt_fb fb_modeset, fb;
 	igt_crc_t crc_software, crc_hardware;
 	int fb_id, fb_modeset_id;
@@ -467,7 +461,7 @@ static bool test_pipe_ctm(data_t *data,
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
 	igt_output_set_pipe(output, primary->pipe->pipe);
-	igt_output_override_mode(output, mode);
+	mode = igt_output_get_mode(output);
 
 	/* Create a framebuffer at the size of the output. */
 	fb_id = igt_create_fb(data->drm_fd,
@@ -521,15 +515,13 @@ static bool test_pipe_ctm(data_t *data,
 			    display->pipes[primary->pipe->pipe].crtc_offset);
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_hardware);
 
-	/*
-	 * Verify that the CRC of the software computed output is
+	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the CTM matrix transformation output.
 	 */
 	ret &= !igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
 
 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
 	igt_remove_fb(data->drm_fd, &fb);
 	igt_remove_fb(data->drm_fd, &fb_modeset);
 
@@ -711,12 +703,17 @@ static void test_setup(data_t *data, enum pipe p)
 	igt_display_require_output_on_pipe(&data->display, p);
 	data->output = igt_get_single_output_for_pipe(&data->display, p);
 	igt_require(data->output);
-
-	igt_display_reset(&data->display);
 }
 
 static void test_cleanup(data_t *data)
 {
+	igt_plane_t *primary = data->primary;
+
+	disable_degamma(primary->pipe);
+	disable_gamma(primary->pipe);
+	disable_ctm(primary->pipe);
+	igt_display_commit(&data->display);
+
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = NULL;
 }
@@ -727,13 +724,10 @@ run_gamma_degamma_tests_for_pipe(data_t *data, enum pipe p,
 {
 	test_setup(data, p);
 
-	/*
-	 * We assume an 8bits depth per color for degamma/gamma LUTs
-	 * for CRC checks with framebuffer references.
-	 */
+	/* We assume an 8bits depth per color for degamma/gamma LUTs
+	 * for CRC checks with framebuffer references. */
 	data->color_depth = 8;
 	data->drm_format = DRM_FORMAT_XRGB8888;
-	data->mode = igt_output_get_mode(data->output);
 
 	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name)
 		igt_assert(test_t(data, data->primary));
@@ -763,7 +757,6 @@ run_ctm_tests_for_pipe(data_t *data, enum pipe p,
 	data->color_depth = 8;
 	delta = 1.0 / (1 << data->color_depth);
 	data->drm_format = DRM_FORMAT_XRGB8888;
-	data->mode = igt_output_get_mode(data->output);
 
 	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name) {
 		bool success = false;
@@ -830,6 +823,9 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		if (!panel_supports_deep_color(data->drm_fd, output->name))
 			continue;
 
+		data->color_depth = 10;
+		data->drm_format = DRM_FORMAT_XRGB2101010;
+		data->output = output;
 		igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
 		igt_output_set_pipe(output, p);
 		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
@@ -838,15 +834,7 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		    !i915_clock_constraint(data, p, 10))
 			continue;
 
-		data->color_depth = 10;
-		data->drm_format = DRM_FORMAT_XRGB2101010;
-		data->output = output;
-		data->mode = igt_output_get_mode(data->output);
-
 		igt_dynamic_f("pipe-%s-%s-gamma", kmstest_pipe_name(p), output->name) {
-			igt_display_reset(&data->display);
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
-
 			ret = test_pipe_gamma(data, data->primary);
 
 			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
@@ -854,9 +842,6 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		}
 
 		igt_dynamic_f("pipe-%s-%s-degamma", kmstest_pipe_name(p), output->name) {
-			igt_display_reset(&data->display);
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
-
 			ret = test_pipe_degamma(data, data->primary);
 
 			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
@@ -864,9 +849,6 @@ run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
 		}
 
 		igt_dynamic_f("pipe-%s-%s-ctm", kmstest_pipe_name(p), output->name) {
-			igt_display_reset(&data->display);
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
-
 			ret = test_pipe_ctm(data, data->primary,
 					    red_green_blue,
 					    blue_green_blue, ctm);
diff --git a/tests/kms_color_helper.h b/tests/kms_color_helper.h
index f0ae30e3..2ea15bcd 100644
--- a/tests/kms_color_helper.h
+++ b/tests/kms_color_helper.h
@@ -51,7 +51,6 @@ typedef struct {
 	igt_pipe_crc_t *pipe_crc;
 	igt_output_t *output;
 	igt_plane_t *primary;
-	drmModeModeInfo *mode;
 
 	uint32_t drm_format;
 	uint32_t color_depth;
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 6/8] Revert "tests/kms_color: Convert tests to dynamic"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (4 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 5/8] Revert "tests/kms_color: Test Cleanup" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 7/8] Revert "tests/kms_color_chamelium: " Bhanuprakash Modem
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit d61e4598142e41fe5eb13295802ecf935bba85bc.
---
 tests/kms_color.c        | 556 ++++++++++++++++++---------------------
 tests/kms_color_helper.h |   1 -
 2 files changed, 259 insertions(+), 298 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 85f3b32d..ba06947b 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -203,7 +203,7 @@ static bool test_pipe_gamma(data_t *data,
  * gamma LUT and verify we have the same CRC as drawing solid color rectangles
  * with linear legacy gamma LUT.
  */
-static bool test_pipe_legacy_gamma(data_t *data,
+static void test_pipe_legacy_gamma(data_t *data,
 				   igt_plane_t *primary)
 {
 	igt_output_t *output;
@@ -220,7 +220,6 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	struct igt_fb fb_modeset, fb;
 	igt_crc_t crc_fullgamma, crc_fullcolors;
 	int fb_id, fb_modeset_id;
-	bool ret;
 
 	kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
 	legacy_lut_size = kms_crtc->gamma_size;
@@ -286,7 +285,7 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the gamma LUT transformation output.
 	 */
-	ret = !igt_skip_crc_compare || igt_check_crc_equal(&crc_fullgamma, &crc_fullcolors);
+	igt_assert_crc_equal(&crc_fullgamma, &crc_fullcolors);
 
 	/* Reset output. */
 	for (i = 1; i < legacy_lut_size; i++)
@@ -304,15 +303,13 @@ static bool test_pipe_legacy_gamma(data_t *data,
 	free(red_lut);
 	free(green_lut);
 	free(blue_lut);
-
-	return ret;
 }
 
 /*
  * Verify that setting the legacy gamma LUT resets the gamma LUT set
  * through the GAMMA_LUT property.
  */
-static bool test_pipe_legacy_gamma_reset(data_t *data,
+static void test_pipe_legacy_gamma_reset(data_t *data,
 					 igt_plane_t *primary)
 {
 	const double ctm_identity[] = {
@@ -327,7 +324,6 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 	struct drm_color_lut *lut;
 	drmModePropertyBlobPtr blob;
 	igt_output_t *output;
-	bool ret = true;
 
 	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
 
@@ -377,12 +373,10 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 				    data->gamma_lut_size));
 	lut = (struct drm_color_lut *) blob->data;
 	for (i = 0; i < data->gamma_lut_size; i++)
-		ret &=(lut[i].red == 0 &&
+		igt_assert(lut[i].red == 0 &&
 			   lut[i].green == 0 &&
 			   lut[i].blue == 0);
 	drmModeFreePropertyBlob(blob);
-	if(!ret)
-		goto end;
 
 	/* Set a gamma LUT using the legacy ioctl and verify
 	 * the content of the GAMMA_LUT property is changed
@@ -417,7 +411,7 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 				    legacy_lut_size));
 	lut = (struct drm_color_lut *) blob->data;
 	for (i = 0; i < legacy_lut_size; i++)
-		ret &= (lut[i].red == 0xffff &&
+		igt_assert(lut[i].red == 0xffff &&
 			   lut[i].green == 0xffff &&
 			   lut[i].blue == 0xffff);
 	drmModeFreePropertyBlob(blob);
@@ -427,8 +421,6 @@ static bool test_pipe_legacy_gamma_reset(data_t *data,
 
 	free_lut(degamma_linear);
 	free_lut(gamma_zero);
-end:
-	return ret;
 }
 
 /*
@@ -685,99 +677,130 @@ prep_pipe(data_t *data, enum pipe p)
 	}
 }
 
-static void test_setup(data_t *data, enum pipe p)
+static void
+run_tests_for_pipe(data_t *data, enum pipe p)
 {
 	igt_pipe_t *pipe;
+	igt_plane_t *primary;
+	double delta;
+	int i;
+	color_t red_green_blue[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 0.0, 0.0, 1.0 }
+	};
 
-	prep_pipe(data, p);
-	igt_require_pipe_crc(data->drm_fd);
+	igt_fixture {
+		prep_pipe(data, p);
 
-	pipe = &data->display.pipes[p];
-	igt_require(pipe->n_planes >= 0);
+		igt_require_pipe_crc(data->drm_fd);
 
-	data->primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
-	data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
-					  data->primary->pipe->pipe,
-					  INTEL_PIPE_CRC_SOURCE_AUTO);
+		pipe = &data->display.pipes[p];
+		igt_require(pipe->n_planes >= 0);
 
-	igt_display_require_output_on_pipe(&data->display, p);
-	data->output = igt_get_single_output_for_pipe(&data->display, p);
-	igt_require(data->output);
-}
+		primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
 
-static void test_cleanup(data_t *data)
-{
-	igt_plane_t *primary = data->primary;
+		data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
+						  primary->pipe->pipe,
+						  INTEL_PIPE_CRC_SOURCE_AUTO);
 
-	disable_degamma(primary->pipe);
-	disable_gamma(primary->pipe);
-	disable_ctm(primary->pipe);
-	igt_display_commit(&data->display);
-
-	igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = NULL;
-}
-
-static void
-run_gamma_degamma_tests_for_pipe(data_t *data, enum pipe p,
-				 bool (*test_t)(data_t*, igt_plane_t*))
-{
-	test_setup(data, p);
+		igt_display_require_output_on_pipe(&data->display, p);
+		data->output = igt_get_single_output_for_pipe(&data->display, p);
+		igt_require(data->output);
+	}
 
 	/* We assume an 8bits depth per color for degamma/gamma LUTs
 	 * for CRC checks with framebuffer references. */
 	data->color_depth = 8;
+	delta = 1.0 / (1 << data->color_depth);
 	data->drm_format = DRM_FORMAT_XRGB8888;
 
-	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name)
-		igt_assert(test_t(data, data->primary));
+	igt_describe("Check the color transformation from red to blue");
+	igt_subtest_f("pipe-%s-ctm-red-to-blue", kmstest_pipe_name(p)) {
+		color_t blue_green_blue[] = {
+			{ 0.0, 0.0, 1.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 0.0, 0.0, 0.0,
+				0.0, 1.0, 0.0,
+				1.0, 0.0, 1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 blue_green_blue, ctm));
+	}
 
-	test_cleanup(data);
-}
+	igt_describe("Check the color transformation from green to red");
+	igt_subtest_f("pipe-%s-ctm-green-to-red", kmstest_pipe_name(p)) {
+		color_t red_red_blue[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 1.0, 1.0, 0.0,
+				0.0, 0.0, 0.0,
+				0.0, 0.0, 1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 red_red_blue, ctm));
+	}
 
-static void
-run_ctm_tests_for_pipe(data_t *data, enum pipe p,
-		       color_t *expected_colors,
-		       double *ctm,
-		       int iter)
-{
-	double delta;
-	color_t red_green_blue[] = {
-		{ 1.0, 0.0, 0.0 },
-		{ 0.0, 1.0, 0.0 },
-		{ 0.0, 0.0, 1.0 }
-	};
+	igt_describe("Check the color transformation from blue to red");
+	igt_subtest_f("pipe-%s-ctm-blue-to-red", kmstest_pipe_name(p)) {
+		color_t red_green_red[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 1.0, 0.0, 0.0 }
+		};
+		double ctm[] = { 1.0, 0.0, 1.0,
+				0.0, 1.0, 0.0,
+				0.0, 0.0, 0.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 red_green_red, ctm));
+	}
 
-	test_setup(data, p);
+	/* We tests a few values around the expected result because
+	 * the it depends on the hardware we're dealing with, we can
+	 * either get clamped or rounded values and we also need to
+	 * account for odd number of items in the LUTs. */
+	igt_describe("Check the color transformation for 0.25 transparency");
+	igt_subtest_f("pipe-%s-ctm-0-25", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.25, 0.0,  0.0,
+				 0.0,  0.25, 0.0,
+				 0.0,  0.0,  0.25 };
+		bool success = false;
 
-	/*
-	 * We assume an 8bits depth per color for degamma/gamma LUTs
-	 * for CRC checks with framebuffer references.
-	 */
-	data->color_depth = 8;
-	delta = 1.0 / (1 << data->color_depth);
-	data->drm_format = DRM_FORMAT_XRGB8888;
+		for (i = 0; i < 5; i++) {
+			expected_colors[0].r =
+				expected_colors[1].g =
+				expected_colors[2].b =
+				0.25 + delta * (i - 2);
+			if (test_pipe_ctm(data, primary, red_green_blue,
+					  expected_colors, ctm)) {
+				success = true;
+				break;
+			}
+		}
+		igt_assert(success);
+	}
 
-	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name) {
+	igt_describe("Check the color transformation for 0.5 transparency");
+	igt_subtest_f("pipe-%s-ctm-0-5", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.5, 0.0, 0.0,
+				 0.0, 0.5, 0.0,
+				 0.0, 0.0, 0.5 };
 		bool success = false;
-		int i;
 
-		if (!iter)
-			success = test_pipe_ctm(data, data->primary, red_green_blue,
-						expected_colors, ctm);
-
-		/*
-		 * We tests a few values around the expected result because
-		 * it depends on the hardware we're dealing with, we can either
-		 * get clamped or rounded values and we also need to account
-		 * for odd number of items in the LUTs.
-		 */
-		for (i = 0; i < iter; i++) {
+		for (i = 0; i < 5; i++) {
 			expected_colors[0].r =
 				expected_colors[1].g =
 				expected_colors[2].b =
 				0.5 + delta * (i - 2);
-			if (test_pipe_ctm(data, data->primary, red_green_blue,
+			if (test_pipe_ctm(data, primary, red_green_blue,
 					  expected_colors, ctm)) {
 				success = true;
 				break;
@@ -786,249 +809,186 @@ run_ctm_tests_for_pipe(data_t *data, enum pipe p,
 		igt_assert(success);
 	}
 
-	test_cleanup(data);
-}
+	igt_describe("Check the color transformation for 0.75 transparency");
+	igt_subtest_f("pipe-%s-ctm-0-75", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.75, 0.0,  0.0,
+				 0.0,  0.75, 0.0,
+				 0.0,  0.0,  0.75 };
+		bool success = false;
 
-static void
-run_deep_color_tests_for_pipe(data_t *data, enum pipe p)
-{
-	igt_output_t *output;
-	color_t blue_green_blue[] = {
-		{ 0.0, 0.0, 1.0 },
-		{ 0.0, 1.0, 0.0 },
-		{ 0.0, 0.0, 1.0 }
-	};
-	color_t red_green_blue[] = {
-		{ 1.0, 0.0, 0.0 },
-		{ 0.0, 1.0, 0.0 },
-		{ 0.0, 0.0, 1.0 }
-	};
-	double ctm[] = { 0.0, 0.0, 0.0,
-			 0.0, 1.0, 0.0,
-			 1.0, 0.0, 1.0 };
+		for (i = 0; i < 7; i++) {
+			expected_colors[0].r =
+				expected_colors[1].g =
+				expected_colors[2].b =
+				0.75 + delta * (i - 3);
+			if (test_pipe_ctm(data, primary, red_green_blue,
+						expected_colors, ctm)) {
+				success = true;
+				break;
+			}
+		}
+		igt_assert(success);
+	}
 
-	if (is_i915_device(data->drm_fd))
-		igt_require_f((intel_display_ver(data->devid) >= 11),
-				"At least GEN 11 is required to validate Deep-color.\n");
+	igt_describe("Check the color transformation for maximum transparency");
+	igt_subtest_f("pipe-%s-ctm-max", kmstest_pipe_name(p)) {
+		color_t full_rgb[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 100.0,   0.0,   0.0,
+				 0.0,   100.0,   0.0,
+				 0.0,     0.0, 100.0 };
+
+		/* CherryView generates values on 10bits that we
+		 * produce with an 8 bits per color framebuffer. */
+		igt_require(!IS_CHERRYVIEW(data->devid));
+
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 full_rgb, ctm));
+	}
 
-	test_setup(data, p);
+	igt_describe("Check the color transformation for negative transparency");
+	igt_subtest_f("pipe-%s-ctm-negative", kmstest_pipe_name(p)) {
+		color_t all_black[] = {
+			{ 0.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 0.0 }
+		};
+		double ctm[] = { -1.0,  0.0,  0.0,
+				 0.0, -1.0,  0.0,
+				 0.0,  0.0, -1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 all_black, ctm));
+	}
 
-	for_each_valid_output_on_pipe(&data->display, p, output) {
-		uint64_t max_bpc = get_max_bpc(output);
-		bool ret;
+#if 0
+	igt_subtest_f("pipe-%s-ctm-limited-range", kmstest_pipe_name(p))
+		test_pipe_limited_range_ctm(data, primary);
+#endif
 
-		if (!max_bpc)
-			continue;
+	igt_describe("Verify that degamma LUT transformation works correctly");
+	igt_subtest_f("pipe-%s-degamma", kmstest_pipe_name(p))
+		igt_assert(test_pipe_degamma(data, primary));
 
-		if (!panel_supports_deep_color(data->drm_fd, output->name))
-			continue;
+	igt_describe("Verify that gamma LUT transformation works correctly");
+	igt_subtest_f("pipe-%s-gamma", kmstest_pipe_name(p))
+		igt_assert(test_pipe_gamma(data, primary));
 
-		data->color_depth = 10;
-		data->drm_format = DRM_FORMAT_XRGB2101010;
-		data->output = output;
-		igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
-		igt_output_set_pipe(output, p);
-		igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	igt_describe("Verify that legacy gamma LUT transformation works correctly");
+	igt_subtest_f("pipe-%s-legacy-gamma", kmstest_pipe_name(p))
+		test_pipe_legacy_gamma(data, primary);
 
-		if (is_i915_device(data->drm_fd) &&
-		    !i915_clock_constraint(data, p, 10))
-			continue;
+	igt_describe("Verify that setting the legacy gamma LUT resets the gamma LUT set through "
+			"GAMMA_LUT property");
+	igt_subtest_f("pipe-%s-legacy-gamma-reset", kmstest_pipe_name(p))
+		test_pipe_legacy_gamma_reset(data, primary);
 
-		igt_dynamic_f("pipe-%s-%s-gamma", kmstest_pipe_name(p), output->name) {
-			ret = test_pipe_gamma(data, data->primary);
+	igt_fixture
+		igt_require(data->display.is_atomic);
 
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
-			igt_assert(ret);
-		}
+	igt_describe("Verify that deep color works correctly");
+	igt_subtest_with_dynamic_f("pipe-%s-deep-color", kmstest_pipe_name(p)) {
+		igt_output_t *output;
+		color_t blue_green_blue[] = {
+			{ 0.0, 0.0, 1.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 0.0, 0.0, 0.0,
+				0.0, 1.0, 0.0,
+				1.0, 0.0, 1.0 };
+
+		if (is_i915_device(data->drm_fd))
+			igt_require_f((intel_display_ver(data->devid) >= 11),
+					"At least GEN 11 is required to validate Deep-color.\n");
+
+		for_each_valid_output_on_pipe(&data->display, p, output) {
+			uint64_t max_bpc = get_max_bpc(output);
+			bool ret;
+
+			if (!max_bpc)
+				continue;
+
+			if (!panel_supports_deep_color(data->drm_fd, output->name))
+				continue;
+
+			data->color_depth = 10;
+			data->drm_format = DRM_FORMAT_XRGB2101010;
+			data->output = output;
+			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, 10);
+			igt_output_set_pipe(output, p);
+			igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+			if (is_i915_device(data->drm_fd) &&
+			    !i915_clock_constraint(data, p, 10))
+				continue;
+
+			igt_dynamic_f("gamma-%s", output->name) {
+				ret = test_pipe_gamma(data, primary);
+
+				igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
+				igt_assert(ret);
+			}
 
-		igt_dynamic_f("pipe-%s-%s-degamma", kmstest_pipe_name(p), output->name) {
-			ret = test_pipe_degamma(data, data->primary);
+			igt_dynamic_f("degamma-%s", output->name) {
+				ret = test_pipe_degamma(data, primary);
 
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
-			igt_assert(ret);
-		}
+				igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
+				igt_assert(ret);
+			}
 
-		igt_dynamic_f("pipe-%s-%s-ctm", kmstest_pipe_name(p), output->name) {
-			ret = test_pipe_ctm(data, data->primary,
-					    red_green_blue,
-					    blue_green_blue, ctm);
+			igt_dynamic_f("ctm-%s", output->name) {
+				ret = test_pipe_ctm(data, primary,
+						    red_green_blue,
+						    blue_green_blue, ctm);
 
-			igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
-			igt_assert(ret);
-		}
+				igt_output_set_prop_value(output, IGT_CONNECTOR_MAX_BPC, max_bpc);
+				igt_assert(ret);
+			}
 
-		break;
+			break;
+		}
 	}
 
-	test_cleanup(data);
-}
-
-static void
-run_invalid_tests_for_pipe(data_t *data)
-{
-	enum pipe pipe;
-	struct {
-		const char *name;
-		void (*test_t) (data_t *data, enum pipe pipe);
-		const char *desc;
-	} tests[] = {
-		{ "invalid-gamma-lut-sizes", invalid_gamma_lut_sizes,
-			"Negative check for invalid gamma lut sizes" },
-
-		{ "invalid-degamma-lut-sizes", invalid_degamma_lut_sizes,
-			"Negative check for invalid degamma lut sizes" },
-
-		{ "invalid-ctm-matrix-sizes", invalid_ctm_matrix_sizes,
-			"Negative check for color tranformation matrix sizes" },
-	};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(tests); i++) {
-		igt_describe_f("%s", tests[i].desc);
-		igt_subtest_with_dynamic_f("%s", tests[i].name) {
-			for_each_pipe(&data->display, pipe) {
-				prep_pipe(data, pipe);
+	igt_fixture {
+		disable_degamma(primary->pipe);
+		disable_gamma(primary->pipe);
+		disable_ctm(primary->pipe);
+		igt_display_commit(&data->display);
 
-				igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe))
-					tests[i].test_t(data, pipe);
-			}
-		}
+		igt_pipe_crc_free(data->pipe_crc);
+		data->pipe_crc = NULL;
 	}
 }
 
 static void
-run_tests_for_pipe(data_t *data)
+run_invalid_tests_for_pipe(data_t *data, enum pipe p)
 {
-	enum pipe pipe;
-	struct {
-		const char *name;
-		bool (*test_t)(data_t*, igt_plane_t*);
-		const char *desc;
-	} gamma_degamma_tests[] = {
-		{ "degamma", test_pipe_degamma,
-		  "Verify that degamma LUT transformation works correctly" },
-
-		{ "gamma", test_pipe_gamma,
-		  "Verify that gamma LUT transformation works correctly" },
-
-		{ "legacy-gamma", test_pipe_legacy_gamma,
-		  "Verify that legacy gamma LUT transformation works correctly" },
-
-		{ "legacy-gamma-reset", test_pipe_legacy_gamma_reset,
-		  "Verify that setting the legacy gamma LUT resets the gamma LUT set through GAMMA_LUT property" },
-	};
-	struct {
-		const char *name;
-		int iter;
-		color_t colors[3];
-		double ctm[9];
-		const char *desc;
-	} ctm_tests[] = {
-		{ "ctm-red-to-blue", 0,
-			{{ 0.0, 0.0, 1.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 0.0, 0.0, 0.0,
-		    0.0, 1.0, 0.0,
-		    1.0, 0.0, 1.0 },
-		  "Check the color transformation from red to blue"
-		},
-		{ "ctm-green-to-red", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 1.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 1.0, 1.0, 0.0,
-		    0.0, 0.0, 0.0,
-		    0.0, 0.0, 1.0 },
-		  "Check the color transformation from green to red"
-		},
-		{ "ctm-blue-to-red", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 1.0, 0.0, 0.0 }},
-		  { 1.0, 0.0, 1.0,
-		    0.0, 1.0, 0.0,
-		    0.0, 0.0, 0.0 },
-		  "Check the color transformation from blue to red"
-		},
-		{ "ctm-max", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 100.0, 0.0, 0.0,
-		    0.0, 100.0, 0.0,
-		    0.0, 0.0, 100.0 },
-		  "Check the color transformation for maximum transparency"
-		},
-		{ "ctm-negative", 0,
-			{{ 0.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 0.0 }},
-		  { -1.0, 0.0, 0.0,
-		    0.0, -1.0, 0.0,
-		    0.0, 0.0, -1.0 },
-		  "Check the color transformation for negative transparency"
-		},
-		{ "ctm-0-25", 5,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.25, 0.0,  0.0,
-		    0.0,  0.25, 0.0,
-		    0.0,  0.0,  0.25 },
-		  "Check the color transformation for 0.25 transparency"
-		},
-		{ "ctm-0-50", 5,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.5,  0.0,  0.0,
-		    0.0,  0.5,  0.0,
-		    0.0,  0.0,  0.5 },
-		  "Check the color transformation for 0.5 transparency"
-		},
-		{ "ctm-0-75", 7,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.75, 0.0,  0.0,
-		    0.0,  0.75, 0.0,
-		    0.0,  0.0,  0.75 },
-		  "Check the color transformation for 0.75 transparency"
-		},
-	};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(gamma_degamma_tests); i++) {
-		igt_describe_f("%s", gamma_degamma_tests[i].desc);
-		igt_subtest_with_dynamic_f("%s", gamma_degamma_tests[i].name) {
-			for_each_pipe(&data->display, pipe) {
-				run_gamma_degamma_tests_for_pipe(data, pipe,
-								 gamma_degamma_tests[i].test_t);
-			}
-		}
-	}
+	igt_fixture
+		prep_pipe(data, p);
 
-	for (i = 0; i < ARRAY_SIZE(ctm_tests); i++) {
-		igt_describe_f("%s", ctm_tests[i].desc);
-		igt_subtest_with_dynamic_f("%s", ctm_tests[i].name) {
-			for_each_pipe(&data->display, pipe) {
-				run_ctm_tests_for_pipe(data, pipe,
-						       ctm_tests[i].colors,
-						       ctm_tests[i].ctm,
-						       ctm_tests[i].iter);
-			}
-		}
-	}
+	igt_describe("Negative check for invalid gamma lut sizes");
+	igt_subtest_f("pipe-%s-invalid-gamma-lut-sizes", kmstest_pipe_name(p))
+		invalid_gamma_lut_sizes(data, p);
 
-	igt_fixture
-		igt_require(data->display.is_atomic);
+	igt_describe("Negative check for invalid degamma lut sizes");
+	igt_subtest_f("pipe-%s-invalid-degamma-lut-sizes", kmstest_pipe_name(p))
+		invalid_degamma_lut_sizes(data, p);
 
-	igt_describe("Verify that deep color works correctly");
-	igt_subtest_with_dynamic("deep-color") {
-		for_each_pipe(&data->display, pipe) {
-			run_deep_color_tests_for_pipe(data, pipe);
-		}
-	}
+	igt_describe("Negative check for color tranformation matrix sizes");
+	igt_subtest_f("pipe-%s-invalid-ctm-matrix-sizes", kmstest_pipe_name(p))
+		invalid_ctm_matrix_sizes(data, p);
 }
 
 igt_main
 {
 	data_t data = {};
+	enum pipe pipe;
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
@@ -1039,11 +999,13 @@ igt_main
 		igt_display_require(&data.display, data.drm_fd);
 	}
 
-	igt_subtest_group
-		run_tests_for_pipe(&data);
+	for_each_pipe_static(pipe) {
+		igt_subtest_group
+			run_tests_for_pipe(&data, pipe);
 
-	igt_subtest_group
-		run_invalid_tests_for_pipe(&data);
+		igt_subtest_group
+			run_invalid_tests_for_pipe(&data, pipe);
+	}
 
 	igt_fixture {
 		igt_display_fini(&data.display);
diff --git a/tests/kms_color_helper.h b/tests/kms_color_helper.h
index 2ea15bcd..cc07f5ee 100644
--- a/tests/kms_color_helper.h
+++ b/tests/kms_color_helper.h
@@ -50,7 +50,6 @@ typedef struct {
 	igt_display_t display;
 	igt_pipe_crc_t *pipe_crc;
 	igt_output_t *output;
-	igt_plane_t *primary;
 
 	uint32_t drm_format;
 	uint32_t color_depth;
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 7/8] Revert "tests/kms_color_chamelium: Convert tests to dynamic"
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (5 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 6/8] Revert "tests/kms_color: Convert tests to dynamic" Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests Bhanuprakash Modem
  2022-09-22 15:28 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Revert Color patches Patchwork
  8 siblings, 0 replies; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

This reverts commit a98a66399db9939b16e1cb3b47055f400834affb.
---
 tests/chamelium/kms_color_chamelium.c | 1034 +++++++++++++------------
 1 file changed, 547 insertions(+), 487 deletions(-)

diff --git a/tests/chamelium/kms_color_chamelium.c b/tests/chamelium/kms_color_chamelium.c
index 678931aa..76f82d6d 100644
--- a/tests/chamelium/kms_color_chamelium.c
+++ b/tests/chamelium/kms_color_chamelium.c
@@ -31,88 +31,115 @@ IGT_TEST_DESCRIPTION("Test Color Features at Pipe level using Chamelium to verif
  * degamma LUT and verify we have the same frame dump as drawing solid color
  * rectangles with linear degamma LUT.
  */
-static bool test_pipe_degamma(data_t *data,
-			      igt_plane_t *primary,
-			      struct chamelium_port *port)
+static void test_pipe_degamma(data_t *data,
+			      igt_plane_t *primary)
 {
-	igt_output_t *output = data->output;
-	gamma_lut_t *degamma_full;
-	drmModeModeInfo *mode = data->mode;
-	struct igt_fb fb_modeset, fb, fbref;
-	struct chamelium_frame_dump *frame_fullcolors;
-	int fb_id, fb_modeset_id, fbref_id;
+	igt_output_t *output;
+	gamma_lut_t *degamma_linear, *degamma_full;
 	color_t red_green_blue[] = {
 		{ 1.0, 0.0, 0.0 },
 		{ 0.0, 1.0, 0.0 },
 		{ 0.0, 0.0, 1.0 }
 	};
-	bool ret;
 
-	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
+	int i;
+	struct chamelium_port *port;
+	char *connected_ports[4];
 
-	degamma_full = generate_table_max(data->degamma_lut_size);
+	for (i = 0; i < data->port_count; i++)
+		connected_ports[i] =
+			(char *) chamelium_port_get_name(data->ports[i]);
 
-	igt_output_set_pipe(output, primary->pipe->pipe);
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
 
-	/* Create a framebuffer at the size of the output. */
-	fb_id = igt_create_fb(data->drm_fd,
-			      mode->hdisplay,
-			      mode->vdisplay,
-			      DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR,
-			      &fb);
-	igt_assert(fb_id);
+	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
+	degamma_full = generate_table_max(data->degamma_lut_size);
 
-	fb_modeset_id = igt_create_fb(data->drm_fd,
+	for_each_valid_output_on_pipe(&data->display,
+				      primary->pipe->pipe,
+				      output) {
+		drmModeModeInfo *mode;
+		struct igt_fb fb_modeset, fb, fbref;
+		struct chamelium_frame_dump *frame_fullcolors;
+		int fb_id, fb_modeset_id, fbref_id;
+		bool valid_output = false;
+
+		for (i = 0; i < data->port_count; i++)
+			valid_output |=
+				(strcmp(output->name, connected_ports[i]) == 0);
+		if (!valid_output)
+			continue;
+		else
+			for (i = 0; i < data->port_count; i++)
+				if (strcmp(output->name,
+					   connected_ports[i]) == 0)
+					port = data->ports[i];
+
+		igt_output_set_pipe(output, primary->pipe->pipe);
+		mode = igt_output_get_mode(output);
+
+		/* Create a framebuffer at the size of the output. */
+		fb_id = igt_create_fb(data->drm_fd,
 				      mode->hdisplay,
 				      mode->vdisplay,
 				      DRM_FORMAT_XRGB8888,
 				      DRM_FORMAT_MOD_LINEAR,
-				      &fb_modeset);
-	igt_assert(fb_modeset_id);
-
-	fbref_id = igt_create_fb(data->drm_fd,
-				 mode->hdisplay,
-				 mode->vdisplay,
-				 DRM_FORMAT_XRGB8888,
-				 DRM_FORMAT_MOD_LINEAR,
-				 &fbref);
-	igt_assert(fbref_id);
-
-	igt_plane_set_fb(primary, &fb_modeset);
-	disable_ctm(primary->pipe);
-	disable_gamma(primary->pipe);
-	igt_display_commit(&data->display);
-
-	/* Draw solid colors with linear degamma transformation. */
-	paint_rectangles(data, mode, red_green_blue, &fbref);
-
-	/* Draw a gradient with degamma LUT to remap all
-	 * values to max red/green/blue.
-	 */
-	paint_gradient_rectangles(data, mode, red_green_blue, &fb);
-	igt_plane_set_fb(primary, &fb);
-	set_degamma(data, primary->pipe, degamma_full);
-	igt_display_commit(&data->display);
-	chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
-	frame_fullcolors =
-		chamelium_read_captured_frame(data->chamelium, 0);
-
-	/* Verify that the framebuffer reference of the software
-	 * computed output is equal to the frame dump of the degamma
-	 * LUT transformation output.
-	 */
-	ret = chamelium_frame_match_or_dump(data->chamelium, port,
-					    frame_fullcolors, &fbref,
-					    CHAMELIUM_CHECK_ANALOG);
-
-	disable_degamma(primary->pipe);
-	igt_plane_set_fb(primary, NULL);
-	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
-	free_lut(degamma_full);
+				      &fb);
+		igt_assert(fb_id);
+
+		fb_modeset_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      DRM_FORMAT_MOD_LINEAR,
+					      &fb_modeset);
+		igt_assert(fb_modeset_id);
+
+		fbref_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      DRM_FORMAT_MOD_LINEAR,
+					      &fbref);
+		igt_assert(fbref_id);
+
+		igt_plane_set_fb(primary, &fb_modeset);
+		disable_ctm(primary->pipe);
+		disable_gamma(primary->pipe);
+		set_degamma(data, primary->pipe, degamma_linear);
+		igt_display_commit(&data->display);
 
-	return ret;
+		/* Draw solid colors with linear degamma transformation. */
+		paint_rectangles(data, mode, red_green_blue, &fbref);
+
+		/* Draw a gradient with degamma LUT to remap all
+		 * values to max red/green/blue.
+		 */
+		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
+		igt_plane_set_fb(primary, &fb);
+		set_degamma(data, primary->pipe, degamma_full);
+		igt_display_commit(&data->display);
+		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
+		frame_fullcolors =
+			chamelium_read_captured_frame(data->chamelium, 0);
+
+		/* Verify that the framebuffer reference of the software
+		 * computed output is equal to the frame dump of the degamma
+		 * LUT transformation output.
+		 */
+		chamelium_frame_match_or_dump(data->chamelium, port,
+					      frame_fullcolors, &fbref,
+					      CHAMELIUM_CHECK_ANALOG);
+
+		disable_degamma(primary->pipe);
+		igt_plane_set_fb(primary, NULL);
+		igt_output_set_pipe(output, PIPE_NONE);
+		igt_display_commit(&data->display);
+	}
+
+	free_lut(degamma_linear);
+	free_lut(degamma_full);
 }
 
 /*
@@ -120,88 +147,111 @@ static bool test_pipe_degamma(data_t *data,
  * gamma LUT and verify we have the same frame dump as drawing solid
  * color rectangles.
  */
-static bool test_pipe_gamma(data_t *data,
-			    igt_plane_t *primary,
-			    struct chamelium_port *port)
+static void test_pipe_gamma(data_t *data,
+			    igt_plane_t *primary)
 {
-	igt_output_t *output = data->output;
+	igt_output_t *output;
 	gamma_lut_t *gamma_full;
-	drmModeModeInfo *mode = data->mode;
-	struct igt_fb fb_modeset, fb, fbref;
-	struct chamelium_frame_dump *frame_fullcolors;
-	int fb_id, fb_modeset_id, fbref_id;
 	color_t red_green_blue[] = {
 		{ 1.0, 0.0, 0.0 },
 		{ 0.0, 1.0, 0.0 },
 		{ 0.0, 0.0, 1.0 }
 	};
-	bool ret;
 
-	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
+	int i;
+	struct chamelium_port *port;
+	char *connected_ports[4];
 
-	gamma_full = generate_table_max(data->gamma_lut_size);
+	for (i = 0; i < data->port_count; i++)
+		connected_ports[i] =
+			(char *) chamelium_port_get_name(data->ports[i]);
 
-	igt_output_set_pipe(output, primary->pipe->pipe);
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
 
-	/* Create a framebuffer at the size of the output. */
-	fb_id = igt_create_fb(data->drm_fd,
-			      mode->hdisplay,
-			      mode->vdisplay,
-			      DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR,
-			      &fb);
-	igt_assert(fb_id);
+	gamma_full = generate_table_max(data->gamma_lut_size);
 
-	fb_modeset_id = igt_create_fb(data->drm_fd,
+	for_each_valid_output_on_pipe(&data->display,
+				      primary->pipe->pipe,
+				      output) {
+		drmModeModeInfo *mode;
+		struct igt_fb fb_modeset, fb, fbref;
+		struct chamelium_frame_dump *frame_fullcolors;
+		int fb_id, fb_modeset_id, fbref_id;
+		bool valid_output = false;
+
+		for (i = 0; i < data->port_count; i++)
+			valid_output |=
+				(strcmp(output->name, connected_ports[i]) == 0);
+		if (!valid_output)
+			continue;
+		else
+			for (i = 0; i < data->port_count; i++)
+				if (strcmp(output->name,
+					   connected_ports[i]) == 0)
+					port = data->ports[i];
+
+		igt_output_set_pipe(output, primary->pipe->pipe);
+		mode = igt_output_get_mode(output);
+
+		/* Create a framebuffer at the size of the output. */
+		fb_id = igt_create_fb(data->drm_fd,
 				      mode->hdisplay,
 				      mode->vdisplay,
 				      DRM_FORMAT_XRGB8888,
 				      DRM_FORMAT_MOD_LINEAR,
-				      &fb_modeset);
-	igt_assert(fb_modeset_id);
-
-	fbref_id = igt_create_fb(data->drm_fd,
-			      mode->hdisplay,
-			      mode->vdisplay,
-			      DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR,
-			      &fbref);
-	igt_assert(fbref_id);
-
-	igt_plane_set_fb(primary, &fbref);
-	disable_ctm(primary->pipe);
-	disable_degamma(primary->pipe);
-	set_gamma(data, primary->pipe, gamma_full);
-	igt_display_commit(&data->display);
-
-	/* Draw solid colors with no gamma transformation. */
-	paint_rectangles(data, mode, red_green_blue, &fbref);
-
-	/* Draw a gradient with gamma LUT to remap all values
-	 * to max red/green/blue.
-	 */
-	paint_gradient_rectangles(data, mode, red_green_blue, &fb);
-	igt_plane_set_fb(primary, &fb);
-	igt_display_commit(&data->display);
-	chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
-	frame_fullcolors =
-		chamelium_read_captured_frame(data->chamelium, 0);
-
-	/* Verify that the framebuffer reference of the software computed
-	 * output is equal to the frame dump of the degamma LUT
-	 * transformation output.
-	 */
-	ret = chamelium_frame_match_or_dump(data->chamelium, port,
-					    frame_fullcolors, &fbref,
-					    CHAMELIUM_CHECK_ANALOG);
-
-	disable_gamma(primary->pipe);
-	igt_plane_set_fb(primary, NULL);
-	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
-	free_lut(gamma_full);
+				      &fb);
+		igt_assert(fb_id);
+
+		fb_modeset_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      DRM_FORMAT_MOD_LINEAR,
+					      &fb_modeset);
+		igt_assert(fb_modeset_id);
+
+		fbref_id = igt_create_fb(data->drm_fd,
+				      mode->hdisplay,
+				      mode->vdisplay,
+				      DRM_FORMAT_XRGB8888,
+				      DRM_FORMAT_MOD_LINEAR,
+				      &fbref);
+		igt_assert(fbref_id);
 
-	return ret;
+		igt_plane_set_fb(primary, &fb_modeset);
+		disable_ctm(primary->pipe);
+		disable_degamma(primary->pipe);
+		set_gamma(data, primary->pipe, gamma_full);
+		igt_display_commit(&data->display);
+
+		/* Draw solid colors with no gamma transformation. */
+		paint_rectangles(data, mode, red_green_blue, &fbref);
+
+		/* Draw a gradient with gamma LUT to remap all values
+		 * to max red/green/blue.
+		 */
+		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
+		igt_plane_set_fb(primary, &fb);
+		igt_display_commit(&data->display);
+		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
+		frame_fullcolors =
+			chamelium_read_captured_frame(data->chamelium, 0);
+
+		/* Verify that the framebuffer reference of the software computed
+		 * output is equal to the frame dump of the degamma LUT
+		 * transformation output.
+		 */
+		chamelium_frame_match_or_dump(data->chamelium, port,
+					      frame_fullcolors, &fbref,
+					      CHAMELIUM_CHECK_ANALOG);
+
+		disable_gamma(primary->pipe);
+		igt_plane_set_fb(primary, NULL);
+		igt_output_set_pipe(output, PIPE_NONE);
+		igt_display_commit(&data->display);
+	}
+
+	free_lut(gamma_full);
 }
 
 /*
@@ -212,97 +262,119 @@ static bool test_pipe_ctm(data_t *data,
 			  igt_plane_t *primary,
 			  color_t *before,
 			  color_t *after,
-			  double *ctm_matrix,
-			  struct chamelium_port *port)
+			  double *ctm_matrix)
 {
 	gamma_lut_t *degamma_linear, *gamma_linear;
-	igt_output_t *output = data->output;
-	drmModeModeInfo *mode = data->mode;
-	struct igt_fb fb_modeset, fb, fbref;
-	struct chamelium_frame_dump *frame_hardware;
-	int fb_id, fb_modeset_id, fbref_id;
+	igt_output_t *output;
+
+	int i;
 	bool ret = true;
+	struct chamelium_port *port;
+	char *connected_ports[4];
 
 	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
 
+	for (i = 0; i < data->port_count; i++)
+		connected_ports[i] =
+			(char *) chamelium_port_get_name(data->ports[i]);
+
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
-	igt_output_set_pipe(output, primary->pipe->pipe);
-
-	/* Create a framebuffer at the size of the output. */
-	fb_id = igt_create_fb(data->drm_fd,
-			      mode->hdisplay,
-			      mode->vdisplay,
-			      DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR,
-			      &fb);
-	igt_assert(fb_id);
-
-	fb_modeset_id = igt_create_fb(data->drm_fd,
+	for_each_valid_output_on_pipe(&data->display,
+				      primary->pipe->pipe,
+				      output) {
+		drmModeModeInfo *mode;
+		struct igt_fb fb_modeset, fb, fbref;
+		struct chamelium_frame_dump *frame_hardware;
+		int fb_id, fb_modeset_id, fbref_id;
+		bool valid_output = false;
+
+		for (i = 0; i < data->port_count; i++)
+			valid_output |=
+				(strcmp(output->name, connected_ports[i]) == 0);
+		if (!valid_output)
+			continue;
+		else
+			for (i = 0; i < data->port_count; i++)
+				if (strcmp(output->name,
+					   connected_ports[i]) == 0)
+					port = data->ports[i];
+
+		igt_output_set_pipe(output, primary->pipe->pipe);
+		mode = igt_output_get_mode(output);
+
+		/* Create a framebuffer at the size of the output. */
+		fb_id = igt_create_fb(data->drm_fd,
 				      mode->hdisplay,
 				      mode->vdisplay,
 				      DRM_FORMAT_XRGB8888,
 				      DRM_FORMAT_MOD_LINEAR,
-				      &fb_modeset);
-	igt_assert(fb_modeset_id);
-
-	fbref_id = igt_create_fb(data->drm_fd,
-				 mode->hdisplay,
-				 mode->vdisplay,
-				 DRM_FORMAT_XRGB8888,
-				 DRM_FORMAT_MOD_LINEAR,
-				 &fbref);
-	igt_assert(fbref_id);
+				      &fb);
+		igt_assert(fb_id);
+
+		fb_modeset_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      DRM_FORMAT_MOD_LINEAR,
+					      &fb_modeset);
+		igt_assert(fb_modeset_id);
+
+		fbref_id = igt_create_fb(data->drm_fd,
+				      mode->hdisplay,
+				      mode->vdisplay,
+				      DRM_FORMAT_XRGB8888,
+				      DRM_FORMAT_MOD_LINEAR,
+				      &fbref);
+		igt_assert(fbref_id);
+
+		igt_plane_set_fb(primary, &fb_modeset);
+
+		if (memcmp(before, after, sizeof(color_t))) {
+			set_degamma(data, primary->pipe, degamma_linear);
+			set_gamma(data, primary->pipe, gamma_linear);
+		} else {
+			/* Disable Degamma and Gamma for ctm max test */
+			disable_degamma(primary->pipe);
+			disable_gamma(primary->pipe);
+		}
 
-	igt_plane_set_fb(primary, &fb_modeset);
+		disable_ctm(primary->pipe);
+		igt_display_commit(&data->display);
 
-	if (memcmp(before, after, sizeof(color_t))) {
-		set_degamma(data, primary->pipe, degamma_linear);
-		set_gamma(data, primary->pipe, gamma_linear);
-	} else {
-		/* Disable Degamma and Gamma for ctm max test */
-		disable_degamma(primary->pipe);
-		disable_gamma(primary->pipe);
-	}
+		paint_rectangles(data, mode, after, &fbref);
 
-	disable_ctm(primary->pipe);
-	igt_display_commit(&data->display);
+		/* With CTM transformation. */
+		paint_rectangles(data, mode, before, &fb);
+		igt_plane_set_fb(primary, &fb);
+		set_ctm(primary->pipe, ctm_matrix);
+		igt_display_commit(&data->display);
+		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
+		frame_hardware =
+			chamelium_read_captured_frame(data->chamelium, 0);
 
-	paint_rectangles(data, mode, after, &fbref);
+		/* Verify that the framebuffer reference of the software
+		 * computed output is equal to the frame dump of the CTM
+		 * matrix transformation output.
+		 */
+		ret &= chamelium_frame_match_or_dump(data->chamelium, port,
+						     frame_hardware,
+						     &fbref,
+						     CHAMELIUM_CHECK_ANALOG);
 
-	/* With CTM transformation. */
-	paint_rectangles(data, mode, before, &fb);
-	igt_plane_set_fb(primary, &fb);
-	set_ctm(primary->pipe, ctm_matrix);
-	igt_display_commit(&data->display);
-	chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
-	frame_hardware =
-		chamelium_read_captured_frame(data->chamelium, 0);
+		igt_plane_set_fb(primary, NULL);
+		igt_output_set_pipe(output, PIPE_NONE);
+	}
 
-	/* Verify that the framebuffer reference of the software
-	 * computed output is equal to the frame dump of the CTM
-	 * matrix transformation output.
-	 */
-	ret &= chamelium_frame_match_or_dump(data->chamelium, port,
-					     frame_hardware,
-					     &fbref,
-					     CHAMELIUM_CHECK_ANALOG);
-
-	igt_plane_set_fb(primary, NULL);
-	disable_degamma(primary->pipe);
-	disable_gamma(primary->pipe);
-	igt_output_set_pipe(output, PIPE_NONE);
-	igt_display_commit(&data->display);
 	free_lut(degamma_linear);
 	free_lut(gamma_linear);
 
 	return ret;
 }
 
-static bool test_pipe_limited_range_ctm(data_t *data,
-					igt_plane_t *primary,
-					struct chamelium_port *port)
+static void test_pipe_limited_range_ctm(data_t *data,
+					igt_plane_t *primary)
 {
 	double limited_result = 235.0 / 255.0;
 	color_t red_green_blue_limited[] = {
@@ -319,355 +391,342 @@ static bool test_pipe_limited_range_ctm(data_t *data,
 			0.0, 1.0, 0.0,
 			0.0, 0.0, 1.0 };
 	gamma_lut_t *degamma_linear, *gamma_linear;
-	igt_output_t *output = data->output;
-	drmModeModeInfo *mode = data->mode;
-	struct igt_fb fb_modeset, fb, fbref;
-	struct chamelium_frame_dump *frame_limited;
-	int fb_id, fb_modeset_id, fbref_id;
-	bool ret = false;
+	igt_output_t *output;
+	bool has_broadcast_rgb_output = false;
+
+	int i;
+	struct chamelium_port *port;
+	char *connected_ports[4];
 
 	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
 
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
-	igt_output_set_pipe(output, primary->pipe->pipe);
-
-	/* Create a framebuffer at the size of the output. */
-	fb_id = igt_create_fb(data->drm_fd,
-			      mode->hdisplay,
-			      mode->vdisplay,
-			      DRM_FORMAT_XRGB8888,
-			      DRM_FORMAT_MOD_LINEAR,
-			      &fb);
-	igt_assert(fb_id);
-
-	fb_modeset_id = igt_create_fb(data->drm_fd,
+	for (i = 0; i < data->port_count; i++)
+		connected_ports[i] =
+			(char *) chamelium_port_get_name(data->ports[i]);
+
+	for_each_valid_output_on_pipe(&data->display,
+				      primary->pipe->pipe,
+				      output) {
+		drmModeModeInfo *mode;
+		struct igt_fb fb_modeset, fb, fbref;
+		struct chamelium_frame_dump *frame_limited;
+		int fb_id, fb_modeset_id, fbref_id;
+		bool valid_output = false;
+
+		for (i = 0; i < data->port_count; i++)
+			valid_output |=
+				(strcmp(output->name, connected_ports[i]) == 0);
+		if (!valid_output)
+			continue;
+		else
+			for (i = 0; i < data->port_count; i++)
+				if (strcmp(output->name,
+				    connected_ports[i]) == 0)
+					port = data->ports[i];
+
+		if (!igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
+			continue;
+
+		has_broadcast_rgb_output = true;
+
+		igt_output_set_pipe(output, primary->pipe->pipe);
+		mode = igt_output_get_mode(output);
+
+		/* Create a framebuffer at the size of the output. */
+		fb_id = igt_create_fb(data->drm_fd,
 				      mode->hdisplay,
 				      mode->vdisplay,
 				      DRM_FORMAT_XRGB8888,
 				      DRM_FORMAT_MOD_LINEAR,
-				      &fb_modeset);
-	igt_assert(fb_modeset_id);
-
-	fbref_id = igt_create_fb(data->drm_fd,
-				 mode->hdisplay,
-				 mode->vdisplay,
-				 DRM_FORMAT_XRGB8888,
-				 DRM_FORMAT_MOD_LINEAR,
-				 &fbref);
-	igt_assert(fbref_id);
-
-	igt_plane_set_fb(primary, &fb_modeset);
-
-	set_degamma(data, primary->pipe, degamma_linear);
-	set_gamma(data, primary->pipe, gamma_linear);
-	set_ctm(primary->pipe, ctm);
-
-	igt_output_set_prop_value(output,
-				  IGT_CONNECTOR_BROADCAST_RGB,
-				  BROADCAST_RGB_FULL);
-	paint_rectangles(data, mode, red_green_blue_limited, &fb);
-	igt_plane_set_fb(primary, &fb);
-	igt_display_commit(&data->display);
-
-	/* Set the output into limited range. */
-	igt_output_set_prop_value(output,
-				  IGT_CONNECTOR_BROADCAST_RGB,
-				  BROADCAST_RGB_16_235);
-	paint_rectangles(data, mode, red_green_blue_full, &fb);
-
-	/* And reset.. */
-	igt_output_set_prop_value(output,
-				  IGT_CONNECTOR_BROADCAST_RGB,
-				  BROADCAST_RGB_FULL);
-	igt_plane_set_fb(primary, NULL);
-	igt_output_set_pipe(output, PIPE_NONE);
-	chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
-	frame_limited =
-		chamelium_read_captured_frame(data->chamelium, 0);
-
-
-	/* Verify that the framebuffer reference of the software
-	 * computed output is equal to the frame dump of the CTM
-	 * matrix transformation output.
-	 */
-	ret = chamelium_frame_match_or_dump(data->chamelium, port,
-					    frame_limited, &fbref,
-					    CHAMELIUM_CHECK_ANALOG);
-
-	free_lut(gamma_linear);
-	free_lut(degamma_linear);
+				      &fb);
+		igt_assert(fb_id);
+
+		fb_modeset_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      DRM_FORMAT_MOD_LINEAR,
+					      &fb_modeset);
+		igt_assert(fb_modeset_id);
+
+		fbref_id = igt_create_fb(data->drm_fd,
+				      mode->hdisplay,
+				      mode->vdisplay,
+				      DRM_FORMAT_XRGB8888,
+				      DRM_FORMAT_MOD_LINEAR,
+				      &fbref);
+		igt_assert(fbref_id);
 
-	return ret;
-}
+		igt_plane_set_fb(primary, &fb_modeset);
 
-static void
-prep_pipe(data_t *data, enum pipe p)
-{
-	igt_require_pipe(&data->display, p);
+		set_degamma(data, primary->pipe, degamma_linear);
+		set_gamma(data, primary->pipe, gamma_linear);
+		set_ctm(primary->pipe, ctm);
+
+		igt_output_set_prop_value(output,
+					  IGT_CONNECTOR_BROADCAST_RGB,
+					  BROADCAST_RGB_FULL);
+		paint_rectangles(data, mode, red_green_blue_limited, &fb);
+		igt_plane_set_fb(primary, &fb);
+		igt_display_commit(&data->display);
+
+		/* Set the output into limited range. */
+		igt_output_set_prop_value(output,
+					  IGT_CONNECTOR_BROADCAST_RGB,
+					  BROADCAST_RGB_16_235);
+		paint_rectangles(data, mode, red_green_blue_full, &fb);
+
+		/* And reset.. */
+		igt_output_set_prop_value(output,
+					  IGT_CONNECTOR_BROADCAST_RGB,
+					  BROADCAST_RGB_FULL);
+		igt_plane_set_fb(primary, NULL);
+		igt_output_set_pipe(output, PIPE_NONE);
+		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
+		frame_limited =
+			chamelium_read_captured_frame(data->chamelium, 0);
+
+
+		/* Verify that the framebuffer reference of the software
+		 * computed output is equal to the frame dump of the CTM
+		 * matrix transformation output.
+		 */
+		chamelium_frame_match_or_dump(data->chamelium, port,
+					      frame_limited, &fbref,
+					      CHAMELIUM_CHECK_ANALOG);
 
-	if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)) {
-		data->degamma_lut_size =
-			igt_pipe_obj_get_prop(&data->display.pipes[p],
-					      IGT_CRTC_DEGAMMA_LUT_SIZE);
-		igt_assert_lt(0, data->degamma_lut_size);
 	}
 
-	if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE)) {
-		data->gamma_lut_size =
-			igt_pipe_obj_get_prop(&data->display.pipes[p],
-					      IGT_CRTC_GAMMA_LUT_SIZE);
-		igt_assert_lt(0, data->gamma_lut_size);
-	}
+	free_lut(gamma_linear);
+	free_lut(degamma_linear);
+
+	igt_require(has_broadcast_rgb_output);
 }
 
-static int test_setup(data_t *data, enum pipe p)
+static void
+run_tests_for_pipe(data_t *data, enum pipe p)
 {
 	igt_pipe_t *pipe;
-	int i = 0;
+	igt_plane_t *primary;
+	double delta;
+	int i;
+	color_t red_green_blue[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 0.0, 0.0, 1.0 }
+	};
 
-	igt_display_reset(&data->display);
-	prep_pipe(data, p);
+	igt_fixture {
 
-	pipe = &data->display.pipes[p];
-	igt_require(pipe->n_planes >= 0);
+		igt_require_pipe(&data->display, p);
 
-	data->primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		pipe = &data->display.pipes[p];
+		igt_require(pipe->n_planes >= 0);
 
-	for_each_valid_output_on_pipe(&data->display, p, data->output) {
-		for (i = 0; i < data->port_count; i++) {
-			if (strcmp(data->output->name,
-				   chamelium_port_get_name(data->ports[i])) == 0)
-				return i;
-		}
-	}
+		primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
 
-	return 0;
-}
+		if (igt_pipe_obj_has_prop(&data->display.pipes[p],
+					  IGT_CRTC_DEGAMMA_LUT_SIZE)) {
+			data->degamma_lut_size =
+				igt_pipe_obj_get_prop(&data->display.pipes[p],
+						IGT_CRTC_DEGAMMA_LUT_SIZE);
+			igt_assert_lt(0, data->degamma_lut_size);
+		}
 
-static void
-run_gamma_degamma_tests_for_pipe(data_t *data, enum pipe p,
-		bool (*test_t)(data_t*, igt_plane_t*, struct chamelium_port*))
-{
-	int port_idx = test_setup(data, p);
+		if (igt_pipe_obj_has_prop(&data->display.pipes[p],
+					  IGT_CRTC_GAMMA_LUT_SIZE)) {
+			data->gamma_lut_size =
+				igt_pipe_obj_get_prop(&data->display.pipes[p],
+						      IGT_CRTC_GAMMA_LUT_SIZE);
+			igt_assert_lt(0, data->gamma_lut_size);
+		}
 
-	igt_require(port_idx);
+		igt_display_require_output_on_pipe(&data->display, p);
+	}
 
 	data->color_depth = 8;
-	data->drm_format = DRM_FORMAT_XRGB8888;
-	data->mode = igt_output_get_mode(data->output);
+	delta = 1.0 / (1 << data->color_depth);
 
-	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name)
-		igt_assert(test_t(data, data->primary, data->ports[port_idx]));
-}
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-red-to-blue", kmstest_pipe_name(p)) {
+		color_t blue_green_blue[] = {
+			{ 0.0, 0.0, 1.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 0.0, 0.0, 0.0,
+				0.0, 1.0, 0.0,
+				1.0, 0.0, 1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 blue_green_blue, ctm));
+	}
 
-static void
-run_ctm_tests_for_pipe(data_t *data, enum pipe p,
-		       color_t *expected_colors,
-		       double *ctm,
-		       int iter)
-{
-	double delta;
-	color_t red_green_blue[] = {
-		{ 1.0, 0.0, 0.0 },
-		{ 0.0, 1.0, 0.0 },
-		{ 0.0, 0.0, 1.0 }
-	};
-	int port_idx = test_setup(data, p);
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-green-to-red", kmstest_pipe_name(p)) {
+		color_t red_red_blue[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 1.0, 1.0, 0.0,
+				0.0, 0.0, 0.0,
+				0.0, 0.0, 1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 red_red_blue, ctm));
+	}
 
-	igt_require(port_idx);
-	/*
-	 * CherryView generates values on 10bits that we
-	 * produce with an 8 bits per color framebuffer.
-	 */
-	if (expected_colors[0].r == 1.0 && ctm[0] == 100)
-		igt_require(!IS_CHERRYVIEW(data->devid));
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-blue-to-red", kmstest_pipe_name(p)) {
+		color_t red_green_red[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 1.0, 0.0, 0.0 }
+		};
+		double ctm[] = { 1.0, 0.0, 1.0,
+				0.0, 1.0, 0.0,
+				0.0, 0.0, 0.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 red_green_red, ctm));
+	}
 
-	/*
-	 * We assume an 8bits depth per color for degamma/gamma LUTs
-	 * for CRC checks with framebuffer references.
+	/* We tests a few values around the expected result because
+	 * the it depends on the hardware we're dealing with, we can
+	 * either get clamped or rounded values and we also need to
+	 * account for odd number of items in the LUTs.
 	 */
-	data->color_depth = 8;
-	delta = 1.0 / (1 << data->color_depth);
-	data->drm_format = DRM_FORMAT_XRGB8888;
-	data->mode = igt_output_get_mode(data->output);
-
-	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name) {
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-0-25", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.25, 0.0,  0.0,
+				 0.0,  0.25, 0.0,
+				 0.0,  0.0,  0.25 };
 		bool success = false;
-		int i;
-
-		if (!iter)
-			success = test_pipe_ctm(data, data->primary,
-						red_green_blue,
-						expected_colors, ctm,
-						data->ports[port_idx]);
-
-		/*
-		 * We tests a few values around the expected result because
-		 * it depends on the hardware we're dealing with, we can either
-		 * get clamped or rounded values and we also need to account
-		 * for odd number of items in the LUTs.
-		 */
-		for (i = 0; i < iter; i++) {
+
+		for (i = 0; i < 5; i++) {
 			expected_colors[0].r =
 				expected_colors[1].g =
 				expected_colors[2].b =
-				0.5 + delta * (i - 2);
-			if (test_pipe_ctm(data, data->primary,
-					  red_green_blue, expected_colors,
-					  ctm, data->ports[port_idx])) {
+				0.25 + delta * (i - 2);
+			if(test_pipe_ctm(data, primary, red_green_blue,
+					 expected_colors, ctm)) {
 				success = true;
 				break;
 			}
 		}
 		igt_assert(success);
 	}
-}
-
-static void
-run_limited_range_ctm_test_for_pipe(data_t *data, enum pipe p,
-		bool (*test_t)(data_t*, igt_plane_t*, struct chamelium_port*))
-{
-	int port_idx = test_setup(data, p);
-
-	igt_require(port_idx);
-	igt_require(igt_output_has_prop(data->output, IGT_CONNECTOR_BROADCAST_RGB));
 
-	data->color_depth = 8;
-	data->drm_format = DRM_FORMAT_XRGB8888;
-	data->mode = igt_output_get_mode(data->output);
-
-	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(p), data->output->name)
-		igt_assert(test_t(data, data->primary, data->ports[port_idx]));
-}
-
-static void
-run_tests_for_pipe(data_t *data)
-{
-	enum pipe pipe;
-	struct {
-		const char *name;
-		bool (*test_t)(data_t*, igt_plane_t*, struct chamelium_port*);
-		const char *desc;
-	} gamma_degamma_tests[] = {
-		{ "degamma", test_pipe_degamma,
-		  "Verify that degamma LUT transformation works correctly" },
-
-		{ "gamma", test_pipe_gamma,
-		  "Verify that gamma LUT transformation works correctly" },
-	};
-	struct {
-		const char *name;
-		int iter;
-		color_t colors[3];
-		double ctm[9];
-		const char *desc;
-	} ctm_tests[] = {
-		{ "ctm-red-to-blue", 0,
-			{{ 0.0, 0.0, 1.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 0.0, 0.0, 0.0,
-		    0.0, 1.0, 0.0,
-		    1.0, 0.0, 1.0 },
-		  "Check the color transformation from red to blue"
-		},
-		{ "ctm-green-to-red", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 1.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 1.0, 1.0, 0.0,
-		    0.0, 0.0, 0.0,
-		    0.0, 0.0, 1.0 },
-		  "Check the color transformation from green to red"
-		},
-		{ "ctm-blue-to-red", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 1.0, 0.0, 0.0 }},
-		  { 1.0, 0.0, 1.0,
-		    0.0, 1.0, 0.0,
-		    0.0, 0.0, 0.0 },
-		  "Check the color transformation from blue to red"
-		},
-		{ "ctm-max", 0,
-			{{ 1.0, 0.0, 0.0 },
-			 { 0.0, 1.0, 0.0 },
-			 { 0.0, 0.0, 1.0 }},
-		  { 100.0, 0.0, 0.0,
-		    0.0, 100.0, 0.0,
-		    0.0, 0.0, 100.0 },
-		  "Check the color transformation for maximum transparency"
-		},
-		{ "ctm-negative", 0,
-			{{ 0.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 0.0 },
-			 { 0.0, 0.0, 0.0 }},
-		  { -1.0, 0.0, 0.0,
-		    0.0, -1.0, 0.0,
-		    0.0, 0.0, -1.0 },
-		  "Check the color transformation for negative transparency"
-		},
-		{ "ctm-0-25", 5,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.25, 0.0,  0.0,
-		    0.0,  0.25, 0.0,
-		    0.0,  0.0,  0.25 },
-		  "Check the color transformation for 0.25 transparency"
-		},
-		{ "ctm-0-50", 5,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.5,  0.0,  0.0,
-		    0.0,  0.5,  0.0,
-		    0.0,  0.0,  0.5 },
-		  "Check the color transformation for 0.5 transparency"
-		},
-		{ "ctm-0-75", 7,
-			{{ 0.0, }, { 0.0, }, { 0.0, }},
-		  { 0.75, 0.0,  0.0,
-		    0.0,  0.75, 0.0,
-		    0.0,  0.0,  0.75 },
-		  "Check the color transformation for 0.75 transparency"
-		},
-	};
-	int i;
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-0-5", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.5, 0.0, 0.0,
+				 0.0, 0.5, 0.0,
+				 0.0, 0.0, 0.5 };
+		bool success = false;
 
-	for (i = 0; i < ARRAY_SIZE(gamma_degamma_tests); i++) {
-		igt_describe_f("%s", gamma_degamma_tests[i].desc);
-		igt_subtest_with_dynamic_f("%s", gamma_degamma_tests[i].name) {
-			for_each_pipe(&data->display, pipe) {
-				run_gamma_degamma_tests_for_pipe(data, pipe,
-								 gamma_degamma_tests[i].test_t);
+		for (i = 0; i < 5; i++) {
+			expected_colors[0].r =
+				expected_colors[1].g =
+				expected_colors[2].b =
+				0.5 + delta * (i - 2);
+			if(test_pipe_ctm(data, primary, red_green_blue,
+					 expected_colors, ctm)) {
+				success = true;
+				break;
 			}
 		}
+		igt_assert(success);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(ctm_tests); i++) {
-		igt_describe_f("%s", ctm_tests[i].desc);
-		igt_subtest_with_dynamic_f("%s", ctm_tests[i].name) {
-			for_each_pipe(&data->display, pipe) {
-				run_ctm_tests_for_pipe(data, pipe,
-						       ctm_tests[i].colors,
-						       ctm_tests[i].ctm,
-						       ctm_tests[i].iter);
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-0-75", kmstest_pipe_name(p)) {
+		color_t expected_colors[] = {
+			{ 0.0, }, { 0.0, }, { 0.0, }
+		};
+		double ctm[] = { 0.75, 0.0,  0.0,
+				 0.0,  0.75, 0.0,
+				 0.0,  0.0,  0.75 };
+		bool success = false;
+
+		for (i = 0; i < 7; i++) {
+			expected_colors[0].r =
+				expected_colors[1].g =
+				expected_colors[2].b =
+				0.75 + delta * (i - 3);
+			if(test_pipe_ctm(data, primary, red_green_blue,
+					 expected_colors, ctm)) {
+				success = true;
+				break;
 			}
 		}
+		igt_assert(success);
 	}
 
 	igt_describe("Compare after applying ctm matrix & identity matrix");
-	igt_subtest_with_dynamic("ctm-limited-range") {
-		for_each_pipe(&data->display, pipe) {
-			run_limited_range_ctm_test_for_pipe(data, pipe,
-							    test_pipe_limited_range_ctm);
-		}
+	igt_subtest_f("pipe-%s-ctm-max", kmstest_pipe_name(p)) {
+		color_t full_rgb[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 }
+		};
+		double ctm[] = { 100.0,   0.0,   0.0,
+				 0.0,   100.0,   0.0,
+				 0.0,     0.0, 100.0 };
+
+		/* CherryView generates values on 10bits that we
+		 * produce with an 8 bits per color framebuffer.
+		 */
+		igt_require(!IS_CHERRYVIEW(data->devid));
+
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 full_rgb, ctm));
+	}
+
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-negative", kmstest_pipe_name(p)) {
+		color_t all_black[] = {
+			{ 0.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 0.0 },
+			{ 0.0, 0.0, 0.0 }
+		};
+		double ctm[] = { -1.0,  0.0,  0.0,
+				 0.0, -1.0,  0.0,
+				 0.0,  0.0, -1.0 };
+		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+					 all_black, ctm));
 	}
 
+	igt_describe("Compare after applying ctm matrix & identity matrix");
+	igt_subtest_f("pipe-%s-ctm-limited-range", kmstest_pipe_name(p))
+		test_pipe_limited_range_ctm(data, primary);
+
+	igt_describe("Compare maxed out gamma LUT and solid color linear LUT");
+	igt_subtest_f("pipe-%s-degamma", kmstest_pipe_name(p))
+		test_pipe_degamma(data, primary);
+
+	igt_describe("Compare maxed out gamma LUT and solid color linear LUT");
+	igt_subtest_f("pipe-%s-gamma", kmstest_pipe_name(p))
+		test_pipe_gamma(data, primary);
+
+	igt_fixture {
+		disable_degamma(primary->pipe);
+		disable_gamma(primary->pipe);
+		disable_ctm(primary->pipe);
+		igt_display_commit(&data->display);
+	}
 }
 
 igt_main
 {
 	data_t data = {};
+	enum pipe pipe;
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
@@ -692,8 +751,9 @@ igt_main
 		kmstest_set_vt_graphics_mode();
 	}
 
-	igt_subtest_group
-		run_tests_for_pipe(&data);
+	for_each_pipe_static(pipe)
+		igt_subtest_group
+			run_tests_for_pipe(&data, pipe);
 
 	igt_fixture {
 		igt_display_fini(&data.display);
-- 
2.37.3

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

* [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (6 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 7/8] Revert "tests/kms_color_chamelium: " Bhanuprakash Modem
@ 2022-09-22  5:11 ` Bhanuprakash Modem
  2022-09-22  5:49   ` Ville Syrjälä
  2022-09-22 15:28 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Revert Color patches Patchwork
  8 siblings, 1 reply; 16+ messages in thread
From: Bhanuprakash Modem @ 2022-09-22  5:11 UTC (permalink / raw)
  To: igt-dev

!igt_skip_crc_compare || igt_check_crc_equal() is always true
which is not correct.

Fixes: 1a42910d
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 tests/kms_color.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index ba06947b..e2462aa3 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -510,7 +510,7 @@ static bool test_pipe_ctm(data_t *data,
 	/* Verify that the CRC of the software computed output is
 	 * equal to the CRC of the CTM matrix transformation output.
 	 */
-	ret &= !igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
+	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);

 	igt_plane_set_fb(primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
--
2.37.3

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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests Bhanuprakash Modem
@ 2022-09-22  5:49   ` Ville Syrjälä
  2022-09-22  6:18     ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-22  5:49 UTC (permalink / raw)
  To: Bhanuprakash Modem; +Cc: igt-dev

On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> !igt_skip_crc_compare || igt_check_crc_equal() is always true
> which is not correct.
> 
> Fixes: 1a42910d

Hang on. Now you're saying the regression in detecting actual failures
was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
igt_check_crc_equal()") rather than the dynamic subtest conversion
commit (which is what the previous Fixes line claimed)?

So apparently the test may have been broken for 1.5 years now and
no one realize it.

Hmm. And idea just came to me. Could we introduce some kind of fairly
generic (say, igt_fb level) knob that would attempt to introduce a real
crc mismatch into all (or at least most) tests? Eg. maybe igt_fb could
frob around with the pixels a bit or something? This would at least
allow one to easily verify that the test *may* start to fail when
turning that knob on. Whether it actually starts to fail for a given
pixel frobbry isn't 100% guaraneed though since we could just end up
with a crc collision by dumb luck. Ideally we would have some way to
guarantee the failure, and then we would just make CI run every test
a second time with knob turned on to validate that the test can
actually detect the failure. But maybe this is a pipe dream.

> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  tests/kms_color.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index ba06947b..e2462aa3 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -510,7 +510,7 @@ static bool test_pipe_ctm(data_t *data,
>  	/* Verify that the CRC of the software computed output is
>  	 * equal to the CRC of the CTM matrix transformation output.
>  	 */
> -	ret &= !igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
> +	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
> 
>  	igt_plane_set_fb(primary, NULL);
>  	igt_output_set_pipe(output, PIPE_NONE);
> --
> 2.37.3

-- 
Ville Syrjälä
Intel

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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22  5:49   ` Ville Syrjälä
@ 2022-09-22  6:18     ` Ville Syrjälä
  2022-09-22 10:25       ` Petri Latvala
  2022-09-22 19:06       ` Lyude Paul
  0 siblings, 2 replies; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-22  6:18 UTC (permalink / raw)
  To: Bhanuprakash Modem; +Cc: igt-dev, Sarvela, Tomi P, Petri Latvala

On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
> On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> > !igt_skip_crc_compare || igt_check_crc_equal() is always true
> > which is not correct.
> > 
> > Fixes: 1a42910d
> 
> Hang on. Now you're saying the regression in detecting actual failures
> was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
> igt_check_crc_equal()") rather than the dynamic subtest conversion
> commit (which is what the previous Fixes line claimed)?
> 
> So apparently the test may have been broken for 1.5 years now and
> no one realize it.

Looks like the tests were in fact failing (at least on some hw)
before and CI did notice the change in the behaviour:
https://patchwork.freedesktop.org/series/88075/#rev2

But it did not raise any kind of stink about that, and still
flagged the whole thing as an overall success. I presume
Lyude neglected to look at the individual results in any detail
once the overall succees was indicated (kinda dangerous thing
to do with our CI it seems).

So why did CI not present that change in behaviour more
prominently?

Is it because we went from FAIL to apparent SUCCESS
and that's always considered sort of fine. I suppose flagging
the thing a failure because something seems to have gotten
fixed would be weird, but maybe it should at least present
such changes in behaviour at the top of the list.

Or is it because it attributed that change to these bugs
https://gitlab.freedesktop.org/drm/intel/-/issues/1149
https://gitlab.freedesktop.org/drm/intel/-/issues/315
Does CI consider everything with an associated bug a potential
ping pong and just basically hides any change it the result
in that big list of CI noise in the report (which this series 
hit a lot)? If so we need to get that fixed. Expected fail
should be handled differently from a known ping pong.

-- 
Ville Syrjälä
Intel

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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22  6:18     ` Ville Syrjälä
@ 2022-09-22 10:25       ` Petri Latvala
  2022-09-22 19:06       ` Lyude Paul
  1 sibling, 0 replies; 16+ messages in thread
From: Petri Latvala @ 2022-09-22 10:25 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev, Sarvela, Tomi P

On Thu, Sep 22, 2022 at 09:18:06AM +0300, Ville Syrjälä wrote:
> On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> > > !igt_skip_crc_compare || igt_check_crc_equal() is always true
> > > which is not correct.
> > > 
> > > Fixes: 1a42910d
> > 
> > Hang on. Now you're saying the regression in detecting actual failures
> > was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
> > igt_check_crc_equal()") rather than the dynamic subtest conversion
> > commit (which is what the previous Fixes line claimed)?
> > 
> > So apparently the test may have been broken for 1.5 years now and
> > no one realize it.
> 
> Looks like the tests were in fact failing (at least on some hw)
> before and CI did notice the change in the behaviour:
> https://patchwork.freedesktop.org/series/88075/#rev2
> 
> But it did not raise any kind of stink about that, and still
> flagged the whole thing as an overall success. I presume
> Lyude neglected to look at the individual results in any detail
> once the overall succees was indicated (kinda dangerous thing
> to do with our CI it seems).

The previous run was full of notruns, making the results all kinds of
noisy =(

> 
> So why did CI not present that change in behaviour more
> prominently?
> 
> Is it because we went from FAIL to apparent SUCCESS
> and that's always considered sort of fine. I suppose flagging
> the thing a failure because something seems to have gotten
> fixed would be weird, but maybe it should at least present
> such changes in behaviour at the top of the list.

Are you saying the "Possible fixes" section should come before "Issues
hit"? Reasonable ask.

But the "Possible fixes" section in that result had 18 tests change
state to PASS. I suppose it's possible to just ctrl+f for kms_color
there but to a non-i915 developer, how should one have noticed
something is amiss? In a results email of ten billion status
changes. As you said, the overall verdict of "success" sounds like
something that could be followed.

> Or is it because it attributed that change to these bugs
> https://gitlab.freedesktop.org/drm/intel/-/issues/1149
> https://gitlab.freedesktop.org/drm/intel/-/issues/315
> Does CI consider everything with an associated bug a potential
> ping pong and just basically hides any change it the result
> in that big list of CI noise in the report (which this series 
> hit a lot)? If so we need to get that fixed. Expected fail
> should be handled differently from a known ping pong.

Ah, the meat of the matter. The kms_color results indeed went from
#1149 and #315 to PASS. Which possibly should have caused some alarm
bells. The question is how to add that logic.

At this time we don't have the possibility of marking XFAIL in
cibuglog to raise alarms on a test going PASS when it shouldn't. I
suspect the need isn't that great for that functionality, a rough
guesstimate for the amount of such bug reports is two, these kms_color
ones. Having that functionality for SKIP results would be a nightmare
to maintain, for skips that depend on, say, the display configuration.


-- 
Petri Latvala

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

* [igt-dev] ✗ Fi.CI.BUILD: failure for Revert Color patches
  2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
                   ` (7 preceding siblings ...)
  2022-09-22  5:11 ` [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests Bhanuprakash Modem
@ 2022-09-22 15:28 ` Patchwork
  8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-09-22 15:28 UTC (permalink / raw)
  To: Modem, Bhanuprakash; +Cc: igt-dev

== Series Details ==

Series: Revert Color patches
URL   : https://patchwork.freedesktop.org/series/108864/
State : failure

== Summary ==

Applying: Revert "tests/kms_color: fix crc assert condition"
Applying: Revert "tests/kms: Fix to use max_bpc constraint helper"
Applying: Revert "tests/kms_color: Fix multiple failures in deep-color tests"
Applying: Revert "tests/kms_color: Fix memory leaks"
Applying: Revert "tests/kms_color: Test Cleanup"
Applying: Revert "tests/kms_color: Convert tests to dynamic"
Applying: Revert "tests/kms_color_chamelium: Convert tests to dynamic"
Using index info to reconstruct a base tree...
M	tests/chamelium/kms_color_chamelium.c
Falling back to patching base and 3-way merge...
Auto-merging tests/chamelium/kms_color_chamelium.c
CONFLICT (content): Merge conflict in tests/chamelium/kms_color_chamelium.c
Patch failed at 0007 Revert "tests/kms_color_chamelium: Convert tests to dynamic"
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22  6:18     ` Ville Syrjälä
  2022-09-22 10:25       ` Petri Latvala
@ 2022-09-22 19:06       ` Lyude Paul
  2022-09-22 19:40         ` Ville Syrjälä
  1 sibling, 1 reply; 16+ messages in thread
From: Lyude Paul @ 2022-09-22 19:06 UTC (permalink / raw)
  To: Ville Syrjälä, Bhanuprakash Modem
  Cc: igt-dev, Sarvela, Tomi P, Petri Latvala

On Thu, 2022-09-22 at 09:18 +0300, Ville Syrjälä wrote:
> On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> > > !igt_skip_crc_compare || igt_check_crc_equal() is always true
> > > which is not correct.
> > > 
> > > Fixes: 1a42910d
> > 
> > Hang on. Now you're saying the regression in detecting actual failures
> > was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
> > igt_check_crc_equal()") rather than the dynamic subtest conversion
> > commit (which is what the previous Fixes line claimed)?
> > 
> > So apparently the test may have been broken for 1.5 years now and
> > no one realize it.
> 
> Looks like the tests were in fact failing (at least on some hw)
> before and CI did notice the change in the behaviour:
> https://patchwork.freedesktop.org/series/88075/#rev2
> 
> But it did not raise any kind of stink about that, and still
> flagged the whole thing as an overall success. I presume
> Lyude neglected to look at the individual results in any detail
> once the overall succees was indicated (kinda dangerous thing
> to do with our CI it seems).

sheesh, I'll definitely keep this in mind for the future. Glad it at least
wasn't a clear cut case of me just missing the CI output entirely lol…

fwiw, feel fine to revert this change. it was a minor nitpick

> 
> So why did CI not present that change in behaviour more
> prominently?
> 
> Is it because we went from FAIL to apparent SUCCESS
> and that's always considered sort of fine. I suppose flagging
> the thing a failure because something seems to have gotten
> fixed would be weird, but maybe it should at least present
> such changes in behaviour at the top of the list.
> 
> Or is it because it attributed that change to these bugs
> https://gitlab.freedesktop.org/drm/intel/-/issues/1149
> https://gitlab.freedesktop.org/drm/intel/-/issues/315
> Does CI consider everything with an associated bug a potential
> ping pong and just basically hides any change it the result
> in that big list of CI noise in the report (which this series 
> hit a lot)? If so we need to get that fixed. Expected fail
> should be handled differently from a known ping pong.
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22 19:06       ` Lyude Paul
@ 2022-09-22 19:40         ` Ville Syrjälä
  2022-09-25 11:46           ` Modem, Bhanuprakash
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2022-09-22 19:40 UTC (permalink / raw)
  To: Lyude Paul; +Cc: igt-dev, Sarvela, Tomi P, Petri Latvala

On Thu, Sep 22, 2022 at 03:06:22PM -0400, Lyude Paul wrote:
> On Thu, 2022-09-22 at 09:18 +0300, Ville Syrjälä wrote:
> > On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> > > > !igt_skip_crc_compare || igt_check_crc_equal() is always true
> > > > which is not correct.
> > > > 
> > > > Fixes: 1a42910d
> > > 
> > > Hang on. Now you're saying the regression in detecting actual failures
> > > was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
> > > igt_check_crc_equal()") rather than the dynamic subtest conversion
> > > commit (which is what the previous Fixes line claimed)?
> > > 
> > > So apparently the test may have been broken for 1.5 years now and
> > > no one realize it.
> > 
> > Looks like the tests were in fact failing (at least on some hw)
> > before and CI did notice the change in the behaviour:
> > https://patchwork.freedesktop.org/series/88075/#rev2
> > 
> > But it did not raise any kind of stink about that, and still
> > flagged the whole thing as an overall success. I presume
> > Lyude neglected to look at the individual results in any detail
> > once the overall succees was indicated (kinda dangerous thing
> > to do with our CI it seems).
> 
> sheesh, I'll definitely keep this in mind for the future. Glad it at least
> wasn't a clear cut case of me just missing the CI output entirely lol…
> 
> fwiw, feel fine to revert this change. it was a minor nitpick

I guess some kind of mass revert is still needed, or someone
needs to just figure out why the test is now broken. And I'm
not talking about the failures on icl+ (or maybe glk+) since
those are apparently expected. But the idle runs are now
showing the test failing on absolutely all machines, so
clearly a real regression has managed to sneak in.

-- 
Ville Syrjälä
Intel

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

* Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests
  2022-09-22 19:40         ` Ville Syrjälä
@ 2022-09-25 11:46           ` Modem, Bhanuprakash
  0 siblings, 0 replies; 16+ messages in thread
From: Modem, Bhanuprakash @ 2022-09-25 11:46 UTC (permalink / raw)
  To: Ville Syrjälä, Lyude Paul
  Cc: igt-dev, Sarvela, Tomi P, Petri Latvala

On Fri-23-09-2022 01:10 am, Ville Syrjälä wrote:
> On Thu, Sep 22, 2022 at 03:06:22PM -0400, Lyude Paul wrote:
>> On Thu, 2022-09-22 at 09:18 +0300, Ville Syrjälä wrote:
>>> On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
>>>> On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
>>>>> !igt_skip_crc_compare || igt_check_crc_equal() is always true
>>>>> which is not correct.
>>>>>
>>>>> Fixes: 1a42910d
>>>>
>>>> Hang on. Now you're saying the regression in detecting actual failures
>>>> was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
>>>> igt_check_crc_equal()") rather than the dynamic subtest conversion
>>>> commit (which is what the previous Fixes line claimed)?
>>>>
>>>> So apparently the test may have been broken for 1.5 years now and
>>>> no one realize it.
>>>
>>> Looks like the tests were in fact failing (at least on some hw)
>>> before and CI did notice the change in the behaviour:
>>> https://patchwork.freedesktop.org/series/88075/#rev2
>>>
>>> But it did not raise any kind of stink about that, and still
>>> flagged the whole thing as an overall success. I presume
>>> Lyude neglected to look at the individual results in any detail
>>> once the overall succees was indicated (kinda dangerous thing
>>> to do with our CI it seems).
>>
>> sheesh, I'll definitely keep this in mind for the future. Glad it at least
>> wasn't a clear cut case of me just missing the CI output entirely lol…
>>
>> fwiw, feel fine to revert this change. it was a minor nitpick
> 
> I guess some kind of mass revert is still needed, or someone
> needs to just figure out why the test is now broken. And I'm
> not talking about the failures on icl+ (or maybe glk+) since
> those are apparently expected. But the idle runs are now
> showing the test failing on absolutely all machines, so
> clearly a real regression has managed to sneak in.

 From this series (or the same series in try-bot [1]), it is clear that 
this regression is not due to the dynamic subtests.

I'll try to figure out why the test is failing.

[1] https://patchwork.freedesktop.org/series/108837/

- Bhanu

> 

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

end of thread, other threads:[~2022-09-25 11:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22  5:11 [igt-dev] [i-g-t V2 0/8] Revert Color patches Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 1/8] Revert "tests/kms_color: fix crc assert condition" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 2/8] Revert "tests/kms: Fix to use max_bpc constraint helper" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 3/8] Revert "tests/kms_color: Fix multiple failures in deep-color tests" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 4/8] Revert "tests/kms_color: Fix memory leaks" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 5/8] Revert "tests/kms_color: Test Cleanup" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 6/8] Revert "tests/kms_color: Convert tests to dynamic" Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 7/8] Revert "tests/kms_color_chamelium: " Bhanuprakash Modem
2022-09-22  5:11 ` [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests Bhanuprakash Modem
2022-09-22  5:49   ` Ville Syrjälä
2022-09-22  6:18     ` Ville Syrjälä
2022-09-22 10:25       ` Petri Latvala
2022-09-22 19:06       ` Lyude Paul
2022-09-22 19:40         ` Ville Syrjälä
2022-09-25 11:46           ` Modem, Bhanuprakash
2022-09-22 15:28 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Revert Color patches Patchwork

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