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: halve horizontal syncs for dual LVDS output Date: Fri, 27 Feb 2026 11:46:38 +1000 Message-ID: In-Reply-To: <20260226-ti-sn65dsi83-dual-lvds-fixes-and-test-pattern-v1-2-2e15f5a9a6a0@bootlin.com> References: <20260226-ti-sn65dsi83-dual-lvds-fixes-and-test-pattern-v1-0-2e15f5a9a6a0@bootlin.com> <20260226-ti-sn65dsi83-dual-lvds-fixes-and-test-pattern-v1-2-2e15f5a9a6a0@bootlin.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Verdict: Looks good, one minor style nit.** The logic is sound. In dual LVDS mode, two pixels are output per clock cycle, so the horizontal timing values (measured in pixel clocks) need to be halved. The patch correctly divides: - `HSYNC_PULSE_WIDTH` (`mode->hsync_end - mode->hsync_start`) - `HORIZONTAL_BACK_PORCH` (`mode->htotal - mode->hsync_end`) - `HORIZONTAL_FRONT_PORCH` (`mode->hsync_start - mode->hdisplay`) Vertical timing is correctly left untouched. The division factor is neatly captured in: ```c const unsigned int dual_factor = ctx->lvds_dual_link ? 2 : 1; ``` **Minor nit**: The variable declaration ordering places `dual_factor` between `ctx` and the `const struct` declarations. The kernel style convention historically prefers reverse-christmas-tree ordering of declarations, but this is a trivial style point and doesn't affect correctness. The `ctx->lvds_dual_link` access is safe here since `ctx` is derived from the function parameter and `lvds_dual_link` is set during probe. One thing to note: the patch doesn't halve `SYNC_DELAY` (kept at the fixed value `32 + 1`). Based on the DSI Tuner tool reference, this appears intentional and correct. Fixes tag and stable Cc are appropriate. --- --- Generated by Claude Code Patch Reviewer