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: bridge: ti-sn65dsi83: Disable video burst mode for LVDS stability Date: Thu, 28 May 2026 12:36:19 +1000 Message-ID: In-Reply-To: <20260527085736.920390-3-tessolveupstream@gmail.com> References: <20260527085736.920390-1-tessolveupstream@gmail.com> <20260527085736.920390-3-tessolveupstream@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 **Verdict: Acceptable but with a reservation about regression risk.** The change removes `MIPI_DSI_MODE_VIDEO_BURST`: ```c - dsi->mode_flags =3D MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | - MIPI_DSI_MODE_VIDEO_NO_HSA | MIPI_DSI_MODE_NO_EOT_PACKET; + dsi->mode_flags =3D MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_NO_HSA | + MIPI_DSI_MODE_NO_EOT_PACKET; ``` Without `MIPI_DSI_MODE_VIDEO_BURST`, the DSI link will operate in non-burst= sync-event mode. The justification =E2=80=94 that burst mode depends on co= ntinuous clocking behavior from the DSI host and may be unreliable =E2=80= =94 is plausible but somewhat generic. **Points to consider:** 1. **Regression risk:** Unlike patch 1 (where the logic was clearly contrad= ictory), burst mode is a valid DSI mode and this driver has shipped with it= since its introduction. Removing it could cause issues on platforms where = burst mode is currently working and non-burst mode is not =E2=80=94 differe= nt DSI host controllers may behave differently. The Tested-by tags are from= only two platforms. 2. **The commit message is honest** about the nature of the problem ("depen= ding on the host controller implementation"), which is good. However, it wo= uld benefit from naming the specific DSI host controller or SoC where the i= nstability was observed to help future readers understand the scope. 3. **No `Fixes:` tag** is present on either patch. If this is considered a = bug fix (which the title "for LVDS stability" implies), a Fixes tag pointin= g to the original commit that added burst mode would be appropriate for bac= kport tracking. 4. **`MIPI_DSI_MODE_VIDEO_NO_HSA` and `MIPI_DSI_MODE_NO_EOT_PACKET` are ret= ained.** This is fine =E2=80=94 `NO_HSA` is a reasonable optimization when = the bridge programs hsync width via register, and `NO_EOT_PACKET` is common= ly used with TI bridges. The code change itself is trivially correct =E2=80=94 it's just flag remova= l and reformatting the line continuation. No functional issues with the imp= lementation. --- Generated by Claude Code Patch Reviewer