[PATCH i-g-t v5 5/8] lib/igt_kms: extend igt_plane_set_color_pipeline to accept Bypass

[Melissa Wen <mwen@igalia.com>]

"Bypass" is just the COLOR_PIPELINE enum value with no colorop object
behind it, so make igt_plane_set_color_pipeline accept NULL and set
"Bypass" instead of making callers poke the property directly. Convert
its callers, igt_plane_reset() included, and drop the now unused
set_color_pipeline_bypass() from kms_colorop_helper.

Keep assigned_color_pipeline pointing at the outgoing pipeline when NULL
is passed: the kernel accepts a colorop update whose pipeline is in the
plane's old state, so reset_colorops() must still be able to finalize
colorop settings in the same commit that deactivates them. No functional
change.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---

v3:
- new patch, replaces the open-coded "Bypass" setting in patch 6
---
 lib/igt_kms.c                                 | 20 +++++++++++++------
 .../chamelium/kms_chamelium_color_pipeline.c  |  2 +-
 tests/kms_color_pipeline.c                    |  2 +-
 tests/kms_colorop.c                           |  6 +++---
 tests/kms_colorop_helper.c                    |  5 -----
 tests/kms_colorop_helper.h                    |  1 -
 6 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6039ad18a..cabe60a31 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2708,7 +2708,7 @@ static void igt_plane_reset(igt_plane_t *plane)
 		igt_plane_set_prop_value(plane, IGT_PLANE_HOTSPOT_Y, 0);
 
 	if (igt_plane_has_prop(plane, IGT_PLANE_COLOR_PIPELINE))
-		igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_PIPELINE, "Bypass");
+		igt_plane_set_color_pipeline(plane, NULL);
 
 	igt_plane_clear_prop_changed(plane, IGT_PLANE_IN_FENCE_FD);
 	plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
@@ -4430,17 +4430,25 @@ bool igt_plane_is_valid_colorop(igt_plane_t *plane, igt_colorop_t *colorop)
 /**
  * igt_plane_set_color_pipeline:
  * @plane: Target plane.
- * @colorop: Colorop to set as color pipeline.
+ * @colorop: Colorop to set as color pipeline, or NULL for "Bypass".
  *
  * This function sets the given @colorop as color pipeline on @plane, or fails
- * the test if it's an invalid color pipeline for the plane.
+ * the test if it's an invalid color pipeline for the plane. Passing NULL sets
+ * the plane color pipeline to "Bypass" but keeps the previously assigned
+ * pipeline, so that pending colorop changes are still submitted with the
+ * commit that deactivates it, which the kernel accepts because the colorop is
+ * in the plane's old state.
  */
 void igt_plane_set_color_pipeline(igt_plane_t *plane, igt_colorop_t *colorop)
 {
-	igt_assert(igt_plane_is_valid_colorop(plane, colorop));
+	igt_assert(!colorop || igt_plane_is_valid_colorop(plane, colorop));
 
-	plane->assigned_color_pipeline = colorop;
-	igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_PIPELINE, colorop->name);
+	if (colorop)
+		plane->assigned_color_pipeline = colorop;
+
+	igt_plane_set_prop_enum(plane,
+				IGT_PLANE_COLOR_PIPELINE,
+				colorop ? colorop->name : "Bypass");
 }
 
 /**
diff --git a/tests/chamelium/kms_chamelium_color_pipeline.c b/tests/chamelium/kms_chamelium_color_pipeline.c
index db6107221..5738c6dd1 100644
--- a/tests/chamelium/kms_chamelium_color_pipeline.c
+++ b/tests/chamelium/kms_chamelium_color_pipeline.c
@@ -161,7 +161,7 @@ static void _test_plane_colorops(data_t *data,
 	chamelium_destroy_frame_dump(frame);
 
 	/* Cleanup */
-	set_color_pipeline_bypass(plane);
+	igt_plane_set_color_pipeline(plane, NULL);
 	reset_colorops(colorops);
 
 	igt_plane_set_fb(plane, NULL);
diff --git a/tests/kms_color_pipeline.c b/tests/kms_color_pipeline.c
index 78860a845..f71416ce1 100644
--- a/tests/kms_color_pipeline.c
+++ b/tests/kms_color_pipeline.c
@@ -168,7 +168,7 @@ static void _test_plane_colorops(data_t *data,
 	igt_assert_crc_equal(crc_ref, &crc_pipe);
 
 	/* Cleanup per-test state */
-	set_color_pipeline_bypass(plane);
+	igt_plane_set_color_pipeline(plane, NULL);
 	reset_colorops(colorops);
 	igt_plane_set_fb(plane, NULL);
 	igt_display_commit_atomic(&data->display, 0, NULL);
diff --git a/tests/kms_colorop.c b/tests/kms_colorop.c
index 732b9d57f..2bee1eecd 100644
--- a/tests/kms_colorop.c
+++ b/tests/kms_colorop.c
@@ -287,7 +287,7 @@ static void colorop_plane_test(igt_display_t *display,
 
 	/* reset color pipeline*/
 
-	set_color_pipeline_bypass(plane);
+	igt_plane_set_color_pipeline(plane, NULL);
 
 	/* Commit */
 	igt_plane_set_fb(plane, input_fb);
@@ -315,7 +315,7 @@ static void colorop_plane_test(igt_display_t *display,
 
 	if (!colorops[0]) {
 		/* bypass test */
-		set_color_pipeline_bypass(plane);
+		igt_plane_set_color_pipeline(plane, NULL);
 	} else {
 		/* get COLOR_PIPELINE enum */
 		color_pipeline = get_color_pipeline(display, plane, colorops);
@@ -343,7 +343,7 @@ static void colorop_plane_test(igt_display_t *display,
 	/* Test bypass transition if requested */
 	if (verify_bypass) {
 		/* reset color pipeline*/
-		set_color_pipeline_bypass(plane);
+		igt_plane_set_color_pipeline(plane, NULL);
 
 		/* Commit */
 		igt_plane_set_fb(plane, input_fb);
diff --git a/tests/kms_colorop_helper.c b/tests/kms_colorop_helper.c
index da234410e..a109c1b06 100644
--- a/tests/kms_colorop_helper.c
+++ b/tests/kms_colorop_helper.c
@@ -450,11 +450,6 @@ void set_color_pipeline(igt_display_t *display,
 	}
 }
 
-void set_color_pipeline_bypass(igt_plane_t *plane)
-{
-	igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_PIPELINE, "Bypass");
-}
-
 static void reset_colorop(kms_colorop_t *colorop)
 {
 	igt_assert(colorop->colorop);
diff --git a/tests/kms_colorop_helper.h b/tests/kms_colorop_helper.h
index 9a1477666..2a0e3799f 100644
--- a/tests/kms_colorop_helper.h
+++ b/tests/kms_colorop_helper.h
@@ -114,7 +114,6 @@ void set_color_pipeline(igt_display_t *display,
 			igt_plane_t *plane,
 			kms_colorop_t *colorops[],
 			igt_colorop_t *color_pipeline);
-void set_color_pipeline_bypass(igt_plane_t *plane);
 void reset_colorops(kms_colorop_t *colorops[]);
 
 #endif /* __KMS_COLOROP_HELPER_H__ */
-- 
2.53.0