[PATCH v4 02/11] drm/atomic: reject colorop update from inactive color pipeline

[Melissa Wen <mwen@igalia.com>]

A colorop can only be changed while it is part of an active color
pipeline, or in the same commit that activates or deactivates that
pipeline. Enforce this contract by checking that the colorop belongs to
the color pipeline of a plane in its current, new or old state, and
rejecting the state change otherwise. Do the check in
drm_atomic_check_only() rather than when the colorop property is set, so
it doesn't depend on the order userspace sets properties within a
commit.

Link: https://lore.kernel.org/dri-devel/20260519211111.228303-1-mwen@igalia.com/
Suggested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Acked-by: Harry Wentland <harry.wentland@amd.com> #v3
Signed-off-by: Melissa Wen <mwen@igalia.com>

---
v4:
- skip check when just duplicating state for suspend/resume persistence
- rewrite commit message and better explain the change (John H.)
---
 drivers/gpu/drm/drm_atomic.c | 76 ++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f00df28e2051..86e4348cad58 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -943,6 +943,71 @@ drm_atomic_add_pipeline_colorops(struct drm_atomic_commit *state,
 	return 0;
 }
 
+/*
+ * drm_atomic_colorop_check - check new colorop state
+ * @new_colorop_state: new colorop state to check
+ *
+ * Ensure that the colorop in @new_colorop_state belongs to an active color
+ * pipeline, i.e. it's in the chain of colorops set to the color_pipeline
+ * property of current, old or new plane state.
+ *
+ * Userspace is allowed to finalize colorop's settings in the same commit that
+ * deactivates its pipeline, and the changes should persist for when that
+ * pipeline is reactivated later. So changes to colorop in the old plane
+ * state's pipeline are accepted even though it won't drive hardware updates.
+ *
+ * Skip this check for duplicated state, since all colorop states must persist
+ * in suspend/resume regardless of whether it belongs to an active color
+ * pipeline or not.
+ *
+ * Returns: 0 on success, -EINVAL otherwise.
+ */
+static int drm_atomic_colorop_check(const struct drm_colorop_state *new_colorop_state)
+{
+	struct drm_atomic_commit *state = new_colorop_state->state;
+	struct drm_plane *plane = new_colorop_state->colorop->plane;
+	struct drm_plane_state *new_plane_state, *old_plane_state;
+	struct drm_colorop *colorop;
+
+	if (state->duplicated)
+		return 0;
+
+	/* Not a plane colorop */
+	if (!plane)
+		return 0;
+
+	new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+
+	/* No changes in the plane state. Check current-committed plane state */
+	if (!new_plane_state) {
+		drm_for_each_colorop_in_pipeline(colorop, plane->state->color_pipeline)
+			if (colorop == new_colorop_state->colorop)
+				return 0;
+		return -EINVAL;
+	}
+
+	if (WARN_ON(!old_plane_state))
+		return -EINVAL;
+
+	/* Check if the colorop is active in the new plane state */
+	drm_for_each_colorop_in_pipeline(colorop, new_plane_state->color_pipeline)
+		if (colorop == new_colorop_state->colorop)
+			return 0;
+
+	/* Same color pipeline as new; no point walking old. Colorop isn't active */
+	if (new_plane_state->color_pipeline == old_plane_state->color_pipeline)
+		return -EINVAL;
+
+	/* Check if the colorop was active in the old plane state */
+	drm_for_each_colorop_in_pipeline(colorop, old_plane_state->color_pipeline)
+		if (colorop == new_colorop_state->colorop)
+			return 0;
+
+	/* Colorop is not part of an active color pipeline. */
+	return -EINVAL;
+}
+
 static void drm_atomic_colorop_print_state(struct drm_printer *p,
 					   const struct drm_colorop_state *state)
 {
@@ -1748,6 +1813,8 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
 	struct drm_plane *plane;
 	struct drm_plane_state *old_plane_state;
 	struct drm_plane_state *new_plane_state;
+	struct drm_colorop *colorop;
+	struct drm_colorop_state *new_colorop_state;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc_state *new_crtc_state;
@@ -1764,6 +1831,15 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
 			requested_crtc |= drm_crtc_mask(crtc);
 	}
 
+	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+		ret = drm_atomic_colorop_check(new_colorop_state);
+		if (ret) {
+			drm_dbg_atomic(dev, "[COLOROP:%d:%d] isn't in an active color pipeline.\n",
+				       colorop->base.id, colorop->type);
+			return ret;
+		}
+	}
+
 	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
 		ret = drm_atomic_plane_check(old_plane_state, new_plane_state);
 		if (ret) {
-- 
2.53.0