[PATCH i-g-t v4 8/8] lib/igt_kms: add macros to iterate color pipelines and colorops

Walking a plane's color pipelines, and the colorop chain of a given
pipeline, is open-coded in six places across lib and the colorop tests.
Add for_each_color_pipeline() to iterate over all color pipelines
supported by a plane and for_each_colorop_in_pipeline() to iterate over
all colorops of a pipeline, together with the igt_colorop_next() helper
the latter builds on, and convert the existing walks.

The chain walk in igt_fill_plane_color_pipelines() is left as is: it
discovers colorops while walking, so igt_find_colorop() cannot resolve
them yet.

Suggested-by: Jani Nikula <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
---

v4:
- new patch, suggested by Jani
---
 lib/igt_kms.c              | 52 ++++++++++++++++++++------------------
 lib/igt_kms.h              |  9 +++++++
 tests/kms_colorop.c        | 17 +++++--------
 tests/kms_colorop_helper.c | 40 ++++++++++-------------------
 tests/kms_properties.c     | 17 ++++---------
 5 files changed, 62 insertions(+), 73 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e9a14c991..7a21404b8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -815,6 +815,21 @@ igt_colorop_t *igt_find_colorop(igt_display_t *display, uint32_t id)
 	return NULL;
 }
 
+/**
+ * igt_colorop_next:
+ * @display: a pointer to an #igt_display_t structure
+ * @colorop: Current colorop.
+ *
+ * Returns: the colorop pointed to by @colorop's NEXT property, or NULL at the
+ * end of the chain.
+ */
+igt_colorop_t *igt_colorop_next(igt_display_t *display, igt_colorop_t *colorop)
+{
+	uint32_t next = igt_colorop_get_prop(display, colorop, IGT_COLOROP_NEXT);
+
+	return igt_find_colorop(display, next);
+}
+
 /*
  * Retrieve all the properies specified in props_name and store them into
  * colorop->props.
@@ -3809,13 +3824,14 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_crtc_t *crtc,
  * Add colorop properties
  */
 static void
-igt_atomic_prepare_colorop_commit(igt_colorop_t *colorop, igt_crtc_t *crtc,
+igt_atomic_prepare_colorop_commit(igt_colorop_t *color_pipeline, igt_crtc_t *crtc,
 				  drmModeAtomicReq *req)
 {
 	igt_display_t *display = crtc->display;
-	int i, next_val;
+	igt_colorop_t *colorop;
+	int i;
 
-	while (colorop) {
+	for_each_colorop_in_pipeline(display, color_pipeline, colorop) {
 		LOG(display,
 		    "populating colorop data: %s.%d\n",
 		    igt_crtc_name(crtc),
@@ -3837,11 +3853,6 @@ igt_atomic_prepare_colorop_commit(igt_colorop_t *colorop, igt_crtc_t *crtc,
 							colorop->props[i],
 							colorop->values[i]));
 		}
-
-		/* get next colorop */
-		next_val = igt_colorop_get_prop(display, colorop,
-						IGT_COLOROP_NEXT);
-		colorop = igt_find_colorop(display, next_val);
 	}
 }
 
@@ -4414,18 +4425,16 @@ bool igt_plane_check_prop_is_mutable(igt_plane_t *plane,
  */
 bool igt_plane_is_valid_colorop(igt_plane_t *plane, igt_colorop_t *colorop)
 {
-	int i;
-	bool found = false;
+	igt_colorop_t *color_pipeline;
 
-	for (i = 0; i < plane->num_color_pipelines; i++) {
-		if (plane->color_pipelines[i] == colorop) {
-			found = true;
-			break;
-		}
+	for_each_color_pipeline(plane, color_pipeline) {
+		if (color_pipeline == colorop)
+			return true;
 	}
 
-	return found;
+	return false;
 }
+
 /**
  * igt_plane_set_color_pipeline:
  * @plane: Target plane.
@@ -4925,15 +4934,10 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
 				 * so already-committed property values aren't re-emitted on
 				 * the next commit.
 				 */
-				colorop = plane->assigned_color_pipeline;
-				while (colorop) {
-					uint32_t next_val;
-
+				for_each_colorop_in_pipeline(display,
+							     plane->assigned_color_pipeline,
+							     colorop)
 					colorop->changed = 0;
-					next_val = igt_colorop_get_prop(display, colorop,
-									IGT_COLOROP_NEXT);
-					colorop = igt_find_colorop(display, next_val);
-				}
 
 				fd = plane->values[IGT_PLANE_IN_FENCE_FD];
 				if (fd != -1)
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index ace8c2b1f..b4b666796 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1022,6 +1022,15 @@ uint64_t igt_colorop_get_prop(igt_display_t *display, igt_colorop_t *colorop, en
 		igt_colorop_set_prop_changed(colorop, prop); \
 	} while (0)
 
+igt_colorop_t *igt_colorop_next(igt_display_t *display, igt_colorop_t *colorop);
+
+#define for_each_color_pipeline(plane, color_pipeline) \
+	for (int i__ = 0; i__ < (plane)->num_color_pipelines && \
+	     ((color_pipeline) = (plane)->color_pipelines[i__], true); i__++)
+
+#define for_each_colorop_in_pipeline(display, pipeline, colorop) \
+	for ((colorop) = (pipeline); (colorop); \
+	     (colorop) = igt_colorop_next((display), (colorop)))
 
 extern bool igt_colorop_has_prop_enum_value(igt_colorop_t *colorop,
 					    enum igt_atomic_colorop_properties prop,
diff --git a/tests/kms_colorop.c b/tests/kms_colorop.c
index 8648bd7ce..ae67d4f4c 100644
--- a/tests/kms_colorop.c
+++ b/tests/kms_colorop.c
@@ -296,10 +296,8 @@ static void colorop_plane_test(igt_display_t *display,
 static void check_plane_colorop_ids(igt_display_t *display)
 {
 	igt_plane_t *plane;
-	int colorop_idx;
-	igt_colorop_t *next;
+	igt_colorop_t *colorop, *color_pipeline;
 	igt_crtc_t *crtc;
-	int prop_val = 0;
 
 	/* Use hash tables to track drm_planes and unique IDs */
 	GHashTable *plane_set = g_hash_table_new(g_direct_hash, g_direct_equal);
@@ -314,18 +312,15 @@ static void check_plane_colorop_ids(igt_display_t *display)
 
 			g_hash_table_add(plane_set, GINT_TO_POINTER(plane->drm_plane->plane_id));
 
-			for (colorop_idx = 0; colorop_idx < plane->num_color_pipelines; colorop_idx++) {
-				next = plane->color_pipelines[colorop_idx];
-				while (next) {
+			for_each_color_pipeline(plane, color_pipeline) {
+				for_each_colorop_in_pipeline(display, color_pipeline, colorop) {
 					/* Check if the ID already exists in the set */
-					if (g_hash_table_contains(id_set, GINT_TO_POINTER(next->id))) {
+					if (g_hash_table_contains(id_set, GINT_TO_POINTER(colorop->id))) {
 						igt_fail_on_f(true, "Duplicate colorop ID %u found on plane %d\n",
-						next->id, plane->drm_plane->plane_id);
+						colorop->id, plane->drm_plane->plane_id);
 					}
 
-					g_hash_table_add(id_set, GINT_TO_POINTER(next->id));
-					prop_val = igt_colorop_get_prop(display, next, IGT_COLOROP_NEXT);
-					next = igt_find_colorop(display, prop_val);
+					g_hash_table_add(id_set, GINT_TO_POINTER(colorop->id));
 				}
 			}
 		}
diff --git a/tests/kms_colorop_helper.c b/tests/kms_colorop_helper.c
index 707661378..192502c18 100644
--- a/tests/kms_colorop_helper.c
+++ b/tests/kms_colorop_helper.c
@@ -230,29 +230,25 @@ static bool can_use_colorop(igt_display_t *display, igt_colorop_t *colorop, kms_
  * colorops[] to it.
  */
 static bool map_to_pipeline(igt_display_t *display,
-			    igt_colorop_t *colorop,
+			    igt_colorop_t *color_pipeline,
 			    kms_colorop_t *colorops[])
 {
-	igt_colorop_t *next = colorop;
+	igt_colorop_t *colorop;
 	kms_colorop_t *current_op;
 	int i = 0;
-	int prop_val = 0;
 
 	current_op = colorops[i];
 	i++;
 	igt_require(current_op);
 
-	while (next) {
-		if (can_use_colorop(display, next, current_op)) {
-			current_op->colorop = next;
+	for_each_colorop_in_pipeline(display, color_pipeline, colorop) {
+		if (can_use_colorop(display, colorop, current_op)) {
+			current_op->colorop = colorop;
 			current_op = colorops[i];
 			i++;
 			if (!current_op)
 				break;
 		}
-		prop_val = igt_colorop_get_prop(display, next,
-						IGT_COLOROP_NEXT);
-		next = igt_find_colorop(display, prop_val);
 	}
 
 	if (current_op) {
@@ -272,18 +268,16 @@ igt_colorop_t *get_color_pipeline(igt_display_t *display,
 			          igt_plane_t *plane,
 				  kms_colorop_t *colorops[])
 {
-	igt_colorop_t *colorop = NULL;
-	int i;
+	igt_colorop_t *color_pipeline;
 
 	/* go through all color pipelines */
-	for (i = 0; i < plane->num_color_pipelines; ++i) {
-		if (map_to_pipeline(display, plane->color_pipelines[i], colorops)) {
-			colorop = plane->color_pipelines[i];
-			break;
+	for_each_color_pipeline(plane, color_pipeline) {
+		if (map_to_pipeline(display, color_pipeline, colorops)) {
+			return color_pipeline;
 		}
 	}
 
-	return colorop;
+	return NULL;
 }
 
 static void fill_custom_1dlut(igt_display_t *display, kms_colorop_t *colorop)
@@ -374,8 +368,7 @@ void set_color_pipeline(igt_display_t *display,
 			kms_colorop_t *colorops[],
 			igt_colorop_t *color_pipeline)
 {
-	igt_colorop_t *next;
-	int prop_val = 0;
+	igt_colorop_t *colorop;
 	int i;
 
 	igt_plane_set_color_pipeline(plane, color_pipeline);
@@ -384,17 +377,12 @@ void set_color_pipeline(igt_display_t *display,
 		set_colorop(display, colorops[i]);
 
 	/* set unused ops in pipeline to bypass */
-	next = color_pipeline;
 	i = 0;
-	while (next) {
-		if (!colorops[i] || colorops[i]->colorop != next)
-			igt_colorop_set_prop_value(next, IGT_COLOROP_BYPASS, 1);
+	for_each_colorop_in_pipeline(display, color_pipeline, colorop) {
+		if (!colorops[i] || colorops[i]->colorop != colorop)
+			igt_colorop_set_prop_value(colorop, IGT_COLOROP_BYPASS, 1);
 		else
 			i++;
-
-		prop_val = igt_colorop_get_prop(display, next,
-						IGT_COLOROP_NEXT);
-		next = igt_find_colorop(display, prop_val);
 	}
 }
 
diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index 764c77963..1395f10ae 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -239,9 +239,7 @@ static void run_colorop_property_tests(igt_display_t *display,
 {
 	struct igt_fb fb, afb;
 	igt_plane_t *plane;
-	igt_colorop_t *colorop;
-	int i;
-	int colorop_id = 0;
+	igt_colorop_t *colorop, *color_pipeline;
 
 	prepare_crtc(display, crtc, output,
 		     &fb);
@@ -268,23 +266,18 @@ static void run_colorop_property_tests(igt_display_t *display,
 		}
 
 		/* iterate over all color pipelines on plane */
-		for (i = 0; i < plane->num_color_pipelines; ++i) {
-			/* iterate over all colorops in pipeline*/
-			colorop = plane->color_pipelines[i];
-			igt_plane_set_color_pipeline(plane, colorop);
+		for_each_color_pipeline(plane, color_pipeline) {
+			igt_plane_set_color_pipeline(plane, color_pipeline);
 			igt_display_commit2(display, COMMIT_ATOMIC);
 
-			while (colorop) {
+			/* iterate over all colorops in pipeline*/
+			for_each_colorop_in_pipeline(display, color_pipeline, colorop) {
 				igt_info("Testing colorop properties on %s.#%d.#%d-%s (output: %s)\n",
 					  igt_crtc_name(crtc), plane->index,
 					  colorop->id,
 					  kmstest_plane_type_name(plane->type), output->name);
 				test_properties(display->drm_fd, DRM_MODE_OBJECT_COLOROP, colorop->id,
 						atomic, display->has_plane_color_pipeline);
-
-				colorop_id = igt_colorop_get_prop(display, colorop,
-								IGT_COLOROP_NEXT);
-				colorop = igt_find_colorop(display, colorop_id);
 			}
 		}
 
-- 
2.53.0