Re: [PATCH v5 2/6] drm/atomic: don't set colorop properties of inactive color pipelines

[Melissa Wen]

On 18/05/2026 13:42, Borah, Chaitanya Kumar wrote:


On 5/7/2026 12:53 AM, Melissa Wen wrote:
Reject attempts to change property values of a colorop that is not
part of an active plane color pipeline.

Suggested-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
Signed-off-by: Melissa Wen <m...@igalia.com>
---
  drivers/gpu/drm/drm_atomic_uapi.c | 34 ++++++++++++++++++++++++++-----
  1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 5bd5bf6661df..bff8d58f8f12 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1275,15 +1275,38 @@ int drm_atomic_set_property(struct 
drm_atomic_state *state,
          break;
      }
      case DRM_MODE_OBJECT_COLOROP: {
-        struct drm_colorop *colorop = obj_to_colorop(obj);
-        struct drm_colorop_state *colorop_state;
+        struct drm_colorop *active_colorop, *colorop = 
obj_to_colorop(obj);
+        struct drm_colorop_state *colorop_state = NULL;
+        struct drm_plane_state *plane_state;
  -        colorop_state = drm_atomic_get_colorop_state(state, colorop);
-        if (IS_ERR(colorop_state)) {
-            ret = PTR_ERR(colorop_state);
+        plane_state = drm_atomic_get_plane_state(state, 
colorop->plane);
+        if (IS_ERR(plane_state)) {
+            ret = PTR_ERR(plane_state);
              break;
          }

Hmmm, this creates a dependency on the call order of 
drm_atomic_set_property() for color pipeline and the color op. :(

I can move this check to drm_atomic_check_only(). This approach will 
create colorop states that will be rejected in the end, but at least it 
won't be affected by the call order.

Lemme prepare a series with this alternative approach.

Thanks for your reviews!

Melissa


Not sure if there is a good way to solve this.

  +        /* Check if the colorop obj is part of an active color 
pipeline */
+        for (active_colorop = plane_state->color_pipeline;
+             active_colorop;
+             active_colorop = active_colorop->next) {
+            if (active_colorop == colorop) {
+                colorop_state = drm_atomic_get_colorop_state(state, 
colorop);
+                if (IS_ERR(colorop_state)) {
+                    ret = PTR_ERR(colorop_state);
+                    goto err;
+                }
+                break;
+            }
+        }
+
+        if (!colorop_state) {
+            drm_dbg_atomic(prop->dev,
+                       "[COLOROP:%d:%d] not part of the active 
pipeline\n",
+                    obj->id, colorop->type);
+            ret = -EINVAL;
+            goto err;
+        }
+
          ret = drm_atomic_colorop_set_property(colorop, colorop_state,
                                file_priv, prop, prop_value);
          break;
@@ -1294,6 +1317,7 @@ int drm_atomic_set_property(struct 
drm_atomic_state *state,
          break;
      }
  +err:
      drm_property_change_valid_put(prop, ref);
      return ret;
  }