From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/i915: Implement the "color format" DRM property
Date: Sun, 12 Apr 2026 11:01:42 +1000
Message-ID: <review-patch11-20260409-color-format-v12-11-ce84e1817a27@collabora.com>
In-Reply-To: <20260409-color-format-v12-11-ce84e1817a27@collabora.com>
References: <20260409-color-format-v12-0-ce84e1817a27@collabora.com> <20260409-color-format-v12-11-ce84e1817a27@collabora.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

Full i915 implementation. Renames existing `compute_formats` to `compute_formats_auto`, adds a new `compute_formats` dispatcher:

```c
switch (conn_state->color_format) {
case DRM_CONNECTOR_COLOR_FORMAT_RGB444:
	return intel_dp_compute_output_format(encoder, crtc_state, conn_state,
					      respect_downstream_limits,
					      INTEL_OUTPUT_FORMAT_RGB);
case DRM_CONNECTOR_COLOR_FORMAT_YCBCR444:
	return intel_dp_compute_output_format(encoder, crtc_state, conn_state,
					      respect_downstream_limits,
					      INTEL_OUTPUT_FORMAT_YCBCR444);
case DRM_CONNECTOR_COLOR_FORMAT_YCBCR420:
	...
```

**Notes**:

1. i915 registers with RGB444|YCBCR444|YCBCR420 -- **no YUV422**. This is a deliberate omission (not all Intel hardware supports YUV422 output). This is the correct approach -- only advertise what the source can do.

2. The `MISSING_CASE(conn_state->color_format)` in the default case is idiomatic i915 style. Good.

3. The `YCBCR422` case is absent from the switch, so if userspace tries to set it, the `MISSING_CASE` default will fire. But since YUV422 is not in the property's enum list, userspace can't actually set it. Correct.

4. Both HDMI and DP paths use the same dispatcher pattern. Clean.

**Verdict**: Good.

---

---
Generated by Claude Code Patch Reviewer