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/rockchip: vop2: Set correct output format for RK3576 YUV422 Date: Mon, 25 May 2026 18:48:57 +1000 Message-ID: In-Reply-To: <20260522-color-format-v15-18-21fb136c9df2@collabora.com> References: <20260522-color-format-v15-0-21fb136c9df2@collabora.com> <20260522-color-format-v15-18-21fb136c9df2@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 The nested `if/switch` in `vop2_crtc_atomic_enable`: ```c else if (vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 && vop2->version == VOP_VERSION_RK3576) switch (vcstate->output_type) { ... default: drm_err(vop2->drm, "Unknown DRM_MODE_CONNECTOR %d\n", vcstate->output_type); vop2_unlock(vop2); return; } ``` The early return in the default case after `vop2_unlock()` is correct for error handling but leaves the CRTC in a partially-enabled state. This is a hardware enable path, so returning mid-way could leave things inconsistent. An `else` after the if-else chain isn't braced, which combines with the switch to make the control flow hard to follow. This could use braces around the switch for clarity. --- Generated by Claude Code Patch Reviewer