On 5/26/26 08:17, Melissa Wen wrote:
Only consider affected colorop states those that are part of an active
color pipeline or a pipeline that is about to be activated or
deactivated in the same atomic commit, i.e., colorop is in the chain of
old/new plane color pipeline property. To cover color_pipeline
deactivation, remove the condition for plane_state->color_pipeline.

Signed-off-by: Melissa Wen <[email protected]>
---
  drivers/gpu/drm/drm_atomic.c | 67 +++++++++++++++++++++++++++++++-----
  1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 170de30c28ae..4fb3a23e862a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -812,6 +812,59 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,
        return 0;
  }
+/*
+ * This function walks old and new plane state color pipelines and adds all
+ * colorops in use by @plane to the atomic configuration @state. This is useful
+ * when an atomic commit needs to check all currently enabled or about to be
+ * enabled colorop on @plane, e.g. when changing the mode. This also avoids
+ * including colorop states that are not part of the atomic state.
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
+ * then the w/w mutex code has detected a deadlock and the entire atomic
+ * sequence must be restarted. All other errors are fatal.
+ */
+static int
+drm_atomic_add_pipeline_colorops(struct drm_atomic_commit *state,
+                                struct drm_plane *plane)
+{
+       struct drm_colorop *colorop;
+       struct drm_colorop_state *colorop_state;
+       struct drm_plane_state *new_plane_state, *old_plane_state;
+
+       new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+       old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+
+       if (WARN_ON(!new_plane_state || !old_plane_state))
+               return -EINVAL;
+
+       drm_dbg_atomic(plane->dev,
+                      "Adding old+new pipeline colorops for [PLANE:%d:%s]\n",
+                      plane->base.id, plane->name);
+
+       for (colorop = new_plane_state->color_pipeline;
+            colorop;
+            colorop = colorop->next) {

This for-loop is used 5 times in this patchset. How about a macro in drm_colorop.h?

#define drm_for_each_colorop_in_pipeline(colorop, pipeline) \
    for ((colorop) = (pipeline); (colorop); (colorop) = (colorop)->next)

+               colorop_state = drm_atomic_get_colorop_state(state, colorop);
+               if (IS_ERR(colorop_state))
+                       return PTR_ERR(colorop_state);
+       }
+
+       /* Same color pipeline as new; no point walking old. */
+       if (new_plane_state->color_pipeline == old_plane_state->color_pipeline)
+               return 0;
+
+       for (colorop = old_plane_state->color_pipeline;
+            colorop;
+            colorop = colorop->next) {
+               colorop_state = drm_atomic_get_colorop_state(state, colorop);
+               if (IS_ERR(colorop_state))
+                       return PTR_ERR(colorop_state);
+       }
+
+       return 0;
+}
+
  static void drm_atomic_colorop_print_state(struct drm_printer *p,
                                           const struct drm_colorop_state 
*state)
  {
@@ -1591,11 +1644,9 @@ drm_atomic_add_affected_planes(struct drm_atomic_commit 
*state,
                if (IS_ERR(plane_state))
                        return PTR_ERR(plane_state);
- if (plane_state->color_pipeline) {
-                       ret = drm_atomic_add_affected_colorops(state, plane);
-                       if (ret)
-                               return ret;
-               }
+               ret = drm_atomic_add_pipeline_colorops(state, plane);
+               if (ret)
+                       return ret;
        }
        return 0;
  }
@@ -1607,10 +1658,8 @@ EXPORT_SYMBOL(drm_atomic_add_affected_planes);
   * @plane: DRM plane
   *
   * This function walks the current configuration and adds all colorops
- * currently used by @plane to the atomic configuration @state. This is useful
- * when an atomic commit also needs to check all currently enabled colorop on
- * @plane, e.g. when changing the mode. It's also useful when re-enabling a 
plane
- * to avoid special code to force-enable all colorops.
+ * currently used by @plane to the atomic configuration @state. It's useful
+ * when re-enabling a plane to avoid special code to force-enable all colorops.
   *
   * Since acquiring a colorop state will always also acquire the w/w mutex of 
the
   * current plane for that colorop (if there is any) adding all the colorop 
states for

Reply via email to