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/rockchip: vop2: Set correct output format for RK3576 YUV422
Date: Sun, 12 Apr 2026 11:01:43 +1000
Message-ID: <review-patch15-20260409-color-format-v12-15-ce84e1817a27@collabora.com>
In-Reply-To: <20260409-color-format-v12-15-ce84e1817a27@collabora.com>
References: <20260409-color-format-v12-0-ce84e1817a27@collabora.com> <20260409-color-format-v12-15-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

**Style issue**: The `else if` chain has an embedded switch without braces:

```c
else if (vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 &&
	 vop2->version == VOP_VERSION_RK3576)
	switch (vcstate->output_type) {
	case DRM_MODE_CONNECTOR_DisplayPort:
	...
	}
else
	out_mode = vcstate->output_mode;
```

This is technically valid C but reads poorly. The `switch` inside a braceless `else if` is unusual and could be misread. Adding braces around the else-if body would improve readability. This is a style nit but worth mentioning given kernel coding standards generally prefer braces when the body is non-trivial.

The `default:` case in the switch does `vop2_unlock(vop2); return;` -- make sure the caller doesn't expect cleanup after this point. The early return skips the rest of `vop2_crtc_atomic_enable()`, which could leave the hardware in a partially configured state for an unknown connector type. However, this is a defensive error path that shouldn't be hit in practice.

R-b from Andy Yan.

**Verdict**: Functional but add braces. Consider whether the early return is safe vs. all subsequent register writes.

---

---
Generated by Claude Code Patch Reviewer