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: fix pclk calculation for bonded dsi Date: Mon, 09 Mar 2026 07:54:17 +1000 Message-ID: In-Reply-To: <20260308070628.482527-1-mitltlatltl@gmail.com> References: <20260308070628.482527-1-mitltlatltl@gmail.com> <20260308070628.482527-1-mitltlatltl@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness: Good** The fix correctly addresses a real rounding discrepancy. Looking at `dsi_ti= ming_setup()` (line 986-992 in the current tree), when `is_bonded_dsi` is t= rue, `hdisplay /=3D 2` occurs *before* the DSC compression calculation at l= ine 1035: ```c hdisplay =3D DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), bytes_= per_pclk); ``` So the timing path works on half the display width, meaning rounding is app= lied per-link. The pclk calculation should do the same, and this patch achi= eves that. The example in the commit message clearly demonstrates the off-b= y-one: ``` full_hdisplay =3D 1904, dsc_bpp =3D 8, bpc =3D 8 new_full_hdisplay =3D DIV_ROUND_UP(1904 * 8, 8 * 3) =3D 635 new_half_hdisplay =3D DIV_ROUND_UP(952 * 8, 8 * 3) =3D 318 new_full_display =3D 318 * 2 =3D 636 ``` **Minor observations:** 1. The unrelated comment change from "dsi" to "DSI" on line 179 is a trivia= l style fix. It's fine to include but it's cosmetic noise in what is otherw= ise a Fixes-tagged patch: ``` - * the clock rates have to be split between the two dsi controllers. + * the clock rates have to be split between the two DSI controllers. ``` 2. The commit message says "v2: add a parameter desciption" =E2=80=94 typo = "desciption" should be "description", though this is below the `---` marker= so it won't appear in the final git log. 3. There is a subtle assumption that `mode->hdisplay` is always even for bo= nded DSI (since `hdisplay /=3D 2` is integer division). This is the same as= sumption made in `dsi_timing_setup()` so it's consistent, but worth noting = that odd hdisplay values in bonded mode would silently truncate a pixel. **Overall: The patch looks correct and ready to merge.** The fix aligns the= pclk calculation with what the hardware is actually programmed to do, and = the logic is clean and well-commented. --- Generated by Claude Code Patch Reviewer