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/msm/dsi: Add support for RGB101010 pixel format Date: Sun, 22 Mar 2026 04:52:40 +1000 Message-ID: In-Reply-To: <20260318-dsi-rgb101010-support-v2-2-698b7612eaeb@pm.me> References: <20260318-dsi-rgb101010-support-v2-0-698b7612eaeb@pm.me> <20260318-dsi-rgb101010-support-v2-2-698b7612eaeb@pm.me> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **`has_rgb30` flag approach:** Using a per-config boolean flag is a reasonable approach given the version-guarding requirement. The check in `dsi_host_attach()` is clean: ```c + if (dsi->format == MIPI_DSI_FMT_RGB101010 && + !msm_host->cfg_hnd->cfg->has_rgb30) { + DRM_DEV_ERROR(&msm_host->pdev->dev, + "RGB101010 not supported on this DSI controller\n"); + return -EINVAL; + } ``` **Missing configs:** The patch sets `has_rgb30 = true` for msm8998, sdm660, sdm845, sm8550, sm8650, kaanapali, sc7280, and sa8775p. However, the cover letter says this is guarded behind `MSM_DSI_6G_VER >= V2.1.0`, yet `msm8998_dsi_cfg` and `sdm660_dsi_cfg` are V1.x era configs. Are these actually V2.1.0+? If the hardware truly requires V2.1.0+, then marking msm8998 (which is 6G v1.x based) as `has_rgb30 = true` may be incorrect. The commit message and the actual flag settings should be reconciled. **Register XML changes are correct:** Extending `VID_CFG0.DST_FORMAT` from bits [4:5] to [4:6] and adding `VID_DST_FORMAT_RGB101010 = 4` and `CMD_DST_FORMAT_RGB101010 = 9` looks appropriate. The comment `` is a helpful annotation. **Format mapping additions** in `dsi_get_vid_fmt()` and `dsi_get_cmd_fmt()` are straightforward and correct. --- --- Generated by Claude Code Patch Reviewer