[PATCH 0/3] drm/bridge: ti-sn65dsi83: two fixes + add test pattern

[PATCH 0/3] drm/bridge: ti-sn65dsi83: two fixes + add test pattern

[Luca Ceresoli]

This series fixes two bugs in the driver code and adds support for enabling
the test pattern output from userspace.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Luca Ceresoli (3):
      drm/bridge: ti-sn65dsi83: fix CHA_DSI_CLK_RANGE rounding
      drm/bridge: ti-sn65dsi83: halve horizontal syncs for dual LVDS output
      drm/bridge: ti-sn65dsi83: add test pattern generation support

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)
---
base-commit: 36d9579fed6c9429aa172f77bd28c58696ce8e2b
change-id: 20260226-ti-sn65dsi83-dual-lvds-fixes-and-test-pattern-f5ff67e1900c

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH 1/3] drm/bridge: ti-sn65dsi83: fix CHA_DSI_CLK_RANGE rounding

[Luca Ceresoli]

The DSI frequency must be in the range:

  (CHA_DSI_CLK_RANGE * 5 MHz) <= DSI freq < ((CHA_DSI_CLK_RANGE + 1) * 5 MHz)

So the register value shouldpoint to the lower range value, but
DIV_ROUND_UP() rounds the division to the higher range value, resulting in
an excess of 1 (unless the frequency is an exact multiple of 5 MHz).

For example for a 437100000 MHz clock CHA_DSI_CLK_RANGE should be 87 (0x57):

  (87 * 5 = 435) <= 437.1 < (88 * 5 = 440)

but current code returns 88 (0x58).

Fix the computation by removing the DIV_ROUND_UP().

Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index f6736b4457bb..d2a81175d279 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -351,9 +351,9 @@ static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx,
 	 *  DSI_CLK = mode clock * bpp / dsi_data_lanes / 2
 	 * the 2 is there because the bus is DDR.
 	 */
-	return DIV_ROUND_UP(clamp((unsigned int)mode->clock *
-			    mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
-			    ctx->dsi->lanes / 2, 40000U, 500000U), 5000U);
+	return clamp((unsigned int)mode->clock *
+		     mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
+		     ctx->dsi->lanes / 2, 40000U, 500000U) / 5000U;
 }
 
 static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx)

-- 
2.53.0

[PATCH 2/3] drm/bridge: ti-sn65dsi83: halve horizontal syncs for dual LVDS output

[Luca Ceresoli]

Dual LVDS output (available on the SN65DSI84) requires HSYNC_PULSE_WIDTH
and HORIZONTAL_BACK_PORCH to be divided by two with respect to the values
used for single LVDS output.

While not clearly stated in the datasheet, this is needed according to the
DSI Tuner [0] output. It also makes sense intuitively because in dual LVDS
output two pixels at a time are output and so the output clock is half of
the pixel clock.

Some dual-LVDS panels refuse to show any picture without this fix.

Divide by two HORIZONTAL_FRONT_PORCH too, even though this register is used
only for test pattern generation which is not currently implemented by this
driver.

[0] https://www.ti.com/tool/DSI-TUNER

Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index d2a81175d279..17a885244e1e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -517,6 +517,7 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 					struct drm_atomic_state *state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+	const unsigned int dual_factor = ctx->lvds_dual_link ? 2 : 1;
 	const struct drm_bridge_state *bridge_state;
 	const struct drm_crtc_state *crtc_state;
 	const struct drm_display_mode *mode;
@@ -653,18 +654,18 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 	/* 32 + 1 pixel clock to ensure proper operation */
 	le16val = cpu_to_le16(32 + 1);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &le16val, 2);
-	le16val = cpu_to_le16(mode->hsync_end - mode->hsync_start);
+	le16val = cpu_to_le16((mode->hsync_end - mode->hsync_start) / dual_factor);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
 			  &le16val, 2);
 	le16val = cpu_to_le16(mode->vsync_end - mode->vsync_start);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
 			  &le16val, 2);
 	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
-		     mode->htotal - mode->hsync_end);
+		     (mode->htotal - mode->hsync_end) / dual_factor);
 	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_BACK_PORCH,
 		     mode->vtotal - mode->vsync_end);
 	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
-		     mode->hsync_start - mode->hdisplay);
+		     (mode->hsync_start - mode->hdisplay) / dual_factor);
 	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
 		     mode->vsync_start - mode->vdisplay);
 	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);

-- 
2.53.0

[PATCH 3/3] drm/bridge: ti-sn65dsi83: add test pattern generation support

[Luca Ceresoli]

Generation of a test pattern output is a useful tool for panel bringup and
debugging, and very simple to support with this chip.

The value of REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW needs to be divided by two
for the test pattern to work in dual LVDS mode. While not clearly stated in
the datasheet, this is needed according to the DSI Tuner [0] output. And
some dual-LVDS panels refuse to show any picture without this division by
two.

[0] https://www.ti.com/tool/DSI-TUNER

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 17a885244e1e..ddc8b5e1dd15 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -114,6 +114,7 @@
 #define REG_VID_CHA_HORIZONTAL_FRONT_PORCH	0x38
 #define REG_VID_CHA_VERTICAL_FRONT_PORCH	0x3a
 #define REG_VID_CHA_TEST_PATTERN		0x3c
+#define  REG_VID_CHA_TEST_PATTERN_EN		BIT(4)
 /* IRQ registers */
 #define REG_IRQ_GLOBAL				0xe0
 #define  REG_IRQ_GLOBAL_IRQ_EN			BIT(0)
@@ -134,6 +135,9 @@
 #define  REG_IRQ_STAT_CHA_SOT_BIT_ERR		BIT(2)
 #define  REG_IRQ_STAT_CHA_PLL_UNLOCK		BIT(0)
 
+static bool sn65dsi83_test_pattern;
+module_param_named(test_pattern, sn65dsi83_test_pattern, bool, 0644);
+
 enum sn65dsi83_channel {
 	CHANNEL_A,
 	CHANNEL_B
@@ -645,7 +649,11 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 			  REG_LVDS_LANE_CHB_LVDS_TERM : 0));
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
 
-	le16val = cpu_to_le16(mode->hdisplay);
+	/*
+	 * Active line length needs to be halved for test pattern
+	 * generation in dual LVDS output.
+	 */
+	le16val = cpu_to_le16(mode->hdisplay / (sn65dsi83_test_pattern ? 2 : 1));
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
 			  &le16val, 2);
 	le16val = cpu_to_le16(mode->vdisplay);
@@ -668,7 +676,8 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
 		     (mode->hsync_start - mode->hdisplay) / dual_factor);
 	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
 		     mode->vsync_start - mode->vdisplay);
-	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
+	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN,
+		     sn65dsi83_test_pattern ? REG_VID_CHA_TEST_PATTERN_EN : 0);
 
 	/* Enable PLL */
 	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);

-- 
2.53.0

Claude review: drm/bridge: ti-sn65dsi83: two fixes + add test pattern

[Claude Code Review Bot]

Overall Series Review

Subject: drm/bridge: ti-sn65dsi83: two fixes + add test pattern
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>
Patches: 4
Reviewed: 2026-02-27T11:46:38.387323

---

This is a clean, well-focused 3-patch series for the TI SN65DSI83/84 DSI-to-LVDS bridge driver. Patches 1 and 2 are straightforward bug fixes with good explanations. Patch 3 adds a debug feature (test pattern generation via module parameter) that is useful for panel bringup but has one logic bug where the `hdisplay` halving is applied unconditionally regardless of whether dual LVDS mode is active.

The series is well-ordered: fix the clock range rounding, fix the dual-LVDS horizontal timing, then build on the `dual_factor` variable to add test pattern support.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: ti-sn65dsi83: fix CHA_DSI_CLK_RANGE rounding

[Claude Code Review Bot]

Patch Review

**Verdict: Looks good.**

The fix is correct. The register encoding is:

```
(CHA_DSI_CLK_RANGE * 5 MHz) <= DSI_CLK < ((CHA_DSI_CLK_RANGE + 1) * 5 MHz)
```

This means the register value should be the floor of `DSI_CLK / 5 MHz`. The old code:

```c
return DIV_ROUND_UP(clamp(..., 40000U, 500000U), 5000U);
```

rounds up, producing an off-by-one for any frequency that isn't an exact multiple of 5 MHz. The new code:

```c
return clamp(..., 40000U, 500000U) / 5000U;
```

uses truncating integer division, which is the correct floor operation here. The example in the commit message (437.1 MHz → register 87 vs incorrect 88) clearly demonstrates the issue.

Fixes tag and stable Cc are appropriate.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: ti-sn65dsi83: halve horizontal syncs for dual LVDS output

[Claude Code Review Bot]

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

Claude review: drm/bridge: ti-sn65dsi83: add test pattern generation support

[Claude Code Review Bot]

Patch Review

**Verdict: Has a bug in the hdisplay halving logic.**

**Bug — hdisplay halved unconditionally instead of only for dual LVDS:**

The commit message says:
> "Active line length needs to be halved for test pattern generation **in dual LVDS output**."

But the code halves `hdisplay` whenever the test pattern is enabled, regardless of whether dual LVDS mode is active:

```c
le16val = cpu_to_le16(mode->hdisplay / (sn65dsi83_test_pattern ? 2 : 1));
```

On an SN65DSI83 (single LVDS only), enabling the test pattern would incorrectly halve the active line length. This should use the `dual_factor` introduced in patch 2:

```c
le16val = cpu_to_le16(mode->hdisplay / (sn65dsi83_test_pattern ? dual_factor : 1));
```

This way single LVDS (`dual_factor == 1`) gets no division, and dual LVDS (`dual_factor == 2`) gets the correct halving.

**Missing MODULE_PARM_DESC:**

The module parameter has no description:

```c
static bool sn65dsi83_test_pattern;
module_param_named(test_pattern, sn65dsi83_test_pattern, bool, 0644);
```

Adding a `MODULE_PARM_DESC` would help users discover and understand the parameter (e.g., `MODULE_PARM_DESC(test_pattern, "Enable LVDS test pattern output");`).

**Minor — global state for per-device feature:**

The module parameter is a file-scope `static bool`, meaning it applies to all SN65DSI83/84 instances in the system. For a debug/bringup feature this is probably acceptable, but worth noting. A per-device debugfs entry would be cleaner if there are multi-bridge use cases.

**Minor — runtime writability vs. effect:**

The parameter is 0644 (writable at runtime), but changes only take effect on the next `atomic_pre_enable` (i.e., next modeset). This is expected behavior but could surprise users. The `MODULE_PARM_DESC` could note this.

---
Generated by Claude Code Patch Reviewer