Claude review: drm/atomic: only add states of active or transient active colorops

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Purpose:** Replaces the call to `drm_atomic_add_affected_colorops()` in `drm_atomic_add_affected_planes()` with a new `drm_atomic_add_pipeline_colorops()` that only walks the old and new pipeline chains rather than iterating over every colorop on the plane.

**Code quality:** The new function is well-structured with clear early-exit optimization when old and new pipelines are the same.

**Issue 1 — WARN_ON condition may be too strict:**

```c
if (WARN_ON(!new_plane_state || !old_plane_state))
    return -EINVAL;
```

In `drm_atomic_add_affected_planes()`, `drm_atomic_get_plane_state()` was just called, which creates both old and new plane state entries. So `new_plane_state` being NULL here should indeed be impossible. However, `old_plane_state` is also guaranteed since `drm_atomic_get_plane_state()` always sets both old and new. The WARN_ON is defensive and fine, just noting it's arguably unnecessary.

**Issue 2 — docstring update in `drm_atomic_add_affected_colorops` is incomplete:**

The patch updates the doc comment for `drm_atomic_add_affected_colorops()`:

```c
- * 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
+ * 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.
```

But the function still adds **all** colorops for the plane (via `drm_for_each_colorop`), not just pipeline-active ones. This is correct for its remaining caller (`drm_atomic_helper_duplicate_state` in Patch 2), but the doc should clarify that this adds *all* colorops, not just those in the active pipeline. This would help future readers understand the distinction from `drm_atomic_add_pipeline_colorops`.

**Otherwise looks correct.** The implementation properly handles the pipeline transition case (old pipeline != new pipeline) by walking both chains.

---

---
Generated by Claude Code Patch Reviewer