From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/atomic: reject colorop update from inactive color pipeline Date: Mon, 25 May 2026 22:24:20 +1000 Message-ID: In-Reply-To: <20260519211111.228303-3-mwen@igalia.com> References: <20260519211111.228303-1-mwen@igalia.com> <20260519211111.228303-3-mwen@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Status: Minor observation, otherwise good** Adds `drm_atomic_colorop_check()` that verifies every colorop in the new state belongs to the active color pipeline. The check is done in `drm_atomic_check_only()` to avoid ordering dependencies at property-set time. The fallback logic is reasonable: ```c color_pipeline = new_plane_state ? new_plane_state->color_pipeline : new_colorop_state->colorop->plane->state->color_pipeline; ``` When the plane isn't part of this atomic commit, it reads from the current committed state. This is safe because `drm_atomic_check_only` runs with appropriate locking. **Minor**: The function returns `-EINVAL` with only a `drm_dbg_atomic` message. Since this is a user-visible error path (userspace set a colorop property on a colorop not in the active pipeline), the error code seems appropriate. It might be helpful to also log which plane and pipeline were expected, but that's a polish item. --- --- Generated by Claude Code Patch Reviewer