Claude review: Add new general DRM property "color format"

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

Overall Series Review

Subject: Add new general DRM property "color format"
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Patches: 30
Reviewed: 2026-04-12T11:01:40.104255

---

This is a well-structured 25-patch series (v12) that adds a new "color format" DRM connector property, allowing userspace to explicitly request RGB, YUV 4:4:4, YUV 4:2:2, YUV 4:2:0, or AUTO output. The series is now in its 12th revision and has accumulated Reviewed-by tags from Maxime Ripard, Dmitry Baryshkov, Cristian Ciocaltea, and Andy Yan on various patches.

**Architecture**: The design uses two separate enums -- `drm_output_color_format` (internal, hardware-level) and `drm_connector_color_format` (userspace-facing, with AUTO=0). This separation is clean and avoids disturbing existing internal enum users. The AUTO behavior is well-defined for HDMI (RGB first, YUV420 fallback) and documented for non-HDMI (first working bridge chain format). The approach of bubbling up errors to userspace rather than silently downgrading explicit requests is the right call.

**Coverage**: Three driver implementations (amdgpu, i915, rockchip dw-hdmi-qp) plus core DRM framework changes. MST is explicitly excluded for amdgpu and i915, which is sensible given format negotiation complexity with heterogeneous sinks. Extensive KUnit tests cover the HDMI state helper, bridge chain format selection, and mode_valid changes.

**Concerns**:
1. `hdmi_colorformats` and `dp_colorformats` are identical bitmasks -- one could be removed or they should diverge if DP truly has different constraints.
2. The amdgpu patch has a complex fallback in the else branch of `fill_stream_properties_from_drm_display_mode` that re-checks AUTO and 420-only, which is somewhat fragile.
3. Patch 15 (rockchip VOP2 YUV422) has a missing-braces style issue on an else-if chain with an embedded switch.
4. The `DRM_CONNECTOR_COLOR_FORMAT_AUTO` case in patch 6's switch is deliberately unreachable but uses `drm_warn` + fallthrough, which is a bit unusual.

Overall this is in good shape for a v12. The test coverage is strong, the documentation is thorough, and the design decisions are well-justified in commit messages.

---

---
Generated by Claude Code Patch Reviewer