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: remove bpc > 8 entries from allow list Date: Mon, 09 Mar 2026 09:06:15 +1000 Message-ID: In-Reply-To: <20260306092518.37849-1-mitltlatltl@gmail.com> References: <20260306092518.37849-1-mitltlatltl@gmail.com> <20260306092518.37849-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 **Commit message issues:** 1. The commit message says "In upstream the msm, for bpc greater than 8 are= not supported yet" =E2=80=94 this is grammatically awkward and, more impor= tantly, **factually unclear**. The code *does* support 10/12 bpc at the DSC= parameter computation level. If there is a specific hardware or integratio= n issue that makes 10/12 bpc broken, the commit message should explain what= that issue is (e.g., incorrect packing, register programming, or observed = display corruption). 2. The updated comment claims "DSC 1.1/1.2 block" but the code at line 1848= uses `DRM_DSC_1_1_PRE_SCR` exclusively. Mentioning DSC 1.2 is misleading s= ince this code path only handles pre-SCR DSC 1.1 parameters. ```c + * In the upstream msm, only 8 bpc is supported for DSC 1.1/1.2 + * block. ``` This should say "DSC 1.1" only, consistent with: ```c /* DPU supports only pre-SCR panels */ ret =3D drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR); ``` **Functional concerns:** 3. The `Fixes:` tag implies this is a bug fix suitable for stable backport,= but no actual bug is described. Removing supported functionality is not a = "fix" =E2=80=94 it's a feature removal. If 10/12 bpc was never validated on= hardware but is otherwise correct code, a `Fixes:` tag is inappropriate. A= more accurate approach would be to document the limitation without the `Fi= xes:` tag, or to explain the specific failure mode. 4. The `rc_parameters_pre_scr` table in `drm_dsc_helper.c` has valid entrie= s for bpc=3D10. The downstream code in `dsi_populate_dsc_params()` =E2=80= =94 including `drm_dsc_set_const_params()`, `drm_dsc_set_rc_buf_thresh()`, = `drm_dsc_setup_rc_params()`, and the `line_buf_depth` calculation =E2=80=94= all work correctly with bpc values of 10 and 12. Removing them without evi= dence of breakage will regress any panels that actually use 10/12 bpc DSC. **Recommendation:** This patch should **not be applied** in its current for= m. The author should either: - Provide evidence of an actual bug caused by 10/12 bpc (display corruption= , register misconfiguration, etc.) and describe it in the commit message, or - Drop the `Fixes:` tag and reframe this as a deliberate scope reduction wi= th justification for why untested-but-correct code should be removed rather= than left in place. --- Generated by Claude Code Patch Reviewer