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/dpu: fix video mode DSC INTF timing width for non 8 bit panels Date: Sun, 22 Mar 2026 04:52:40 +1000 Message-ID: In-Reply-To: <20260318-dsi-rgb101010-support-v2-3-698b7612eaeb@pm.me> References: <20260318-dsi-rgb101010-support-v2-0-698b7612eaeb@pm.me> <20260318-dsi-rgb101010-support-v2-3-698b7612eaeb@pm.me> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **The fix itself is mathematically correct.** The compressed INTF timing wi= dth should represent how many pixel-clock cycles are needed to output the c= ompressed data. The INTF outputs 3 bytes (24 bits) per pixel clock, so: ``` compressed_width =3D (width * bpp_compressed) / 24 ``` The previous formula `width * bpp_compressed / (bits_per_component * 3)` ha= ppens to equal the same thing for 8-bit panels (8=C3=973=3D24) but produces= wrong results for 10-bit panels (10=C3=973=3D30), which explains the FIFO = errors. **Consistency concern =E2=80=94 `dsi_adjust_pclk_for_compression()` at `dsi= _host.c:590-591`:** ```c int new_hdisplay =3D DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); ``` This function uses the **exact same** `bits_per_component * 3` divisor for = pclk rate adjustment. If the reasoning in this patch is correct (the diviso= r should always be 24), then this function likely needs the same fix. Not a= ddressing it could mean the pixel clock rate is calculated incorrectly for = 10-bit DSC panels, potentially causing timing issues even after this INTF f= ix. At minimum, the commit message should explain why this instance is diff= erent or add a note that it will be addressed separately. **Commit message could be improved:** The patch is described as fixing "vid= eo mode DSC INTF timing" but the rationale of "matches downstream" is somew= hat weak on its own. It would benefit from a brief explanation of *why* 24 = is correct (the INTF always outputs 3 bytes per clock regardless of source = bit depth), rather than just deferring to downstream behavior. **Nit:** This patch could arguably carry a `Fixes:` tag pointing to the com= mit that introduced the `bits_per_component * 3` formula, which would help = with backporting to stable kernels. --- Generated by Claude Code Patch Reviewer