* [PATCH v1 0/1] gpu/drm: bridge: tc358768: Add delay after PLL setup
@ 2026-05-20 9:26 Svyatoslav Ryhel
2026-05-20 9:26 ` [PATCH v1 1/1] " Svyatoslav Ryhel
2026-05-25 12:05 ` Claude Code Review Bot
0 siblings, 2 replies; 4+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-20 9:26 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Svyatoslav Ryhel
Cc: dri-devel, linux-kernel
After tc358768_setup_pll() enables PLL_CKEN and the lock indicator comes
up, the DSI register sequence runs near immediately. On TF700T this
results in the bridge claiming PLL lock but producing no DSI video output:
the panel powers up, the backlight comes on, but the framebuffer is never
scanned out.
Insert a small settling delay between PLL setup and the rest of the
attach.
Svyatoslav Ryhel (1):
gpu/drm: bridge: tc358768: Add delay after PLL setup
drivers/gpu/drm/bridge/tc358768.c | 3 +++
1 file changed, 3 insertions(+)
--
2.51.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v1 1/1] gpu/drm: bridge: tc358768: Add delay after PLL setup
2026-05-20 9:26 [PATCH v1 0/1] gpu/drm: bridge: tc358768: Add delay after PLL setup Svyatoslav Ryhel
@ 2026-05-20 9:26 ` Svyatoslav Ryhel
2026-05-25 12:05 ` Claude review: " Claude Code Review Bot
2026-05-25 12:05 ` Claude Code Review Bot
1 sibling, 1 reply; 4+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-20 9:26 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Svyatoslav Ryhel
Cc: dri-devel, linux-kernel
After tc358768_setup_pll() enables PLL_CKEN and the lock indicator comes
up, the DSI register sequence runs near immediately. On TF700T this
results in the bridge claiming PLL lock but producing no DSI video output:
the panel powers up, the backlight comes on, but the framebuffer is never
scanned out.
Insert a small settling delay between PLL setup and the rest of the
attach.
Suggested-by: Michele Brocco <brocco@perceptos.de>
Tested-by: Michele Brocco <brocco@perceptos.de> # TF700T
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/gpu/drm/bridge/tc358768.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 0f2820b37302..17abb24a7181 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -824,6 +824,9 @@ static void tc358768_bridge_atomic_pre_enable(struct drm_bridge *bridge,
return;
}
+ /* Allow the PLL to settle */
+ msleep(20);
+
drm_display_mode_to_videomode(mode, &vm);
dsiclk = priv->dsiclk;
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Claude review: gpu/drm: bridge: tc358768: Add delay after PLL setup
2026-05-20 9:26 [PATCH v1 0/1] gpu/drm: bridge: tc358768: Add delay after PLL setup Svyatoslav Ryhel
2026-05-20 9:26 ` [PATCH v1 1/1] " Svyatoslav Ryhel
@ 2026-05-25 12:05 ` Claude Code Review Bot
1 sibling, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 12:05 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: gpu/drm: bridge: tc358768: Add delay after PLL setup
Author: Svyatoslav Ryhel <clamor95@gmail.com>
Patches: 2
Reviewed: 2026-05-25T22:05:01.975192
---
This is a single-patch series adding a 20ms `msleep()` delay after PLL setup in the TC358768 DSI bridge driver. The fix addresses a real hardware issue on the TF700T (ASUS Transformer Pad Infinity) where the PLL reports lock but the clock hasn't fully stabilized, resulting in a blank screen.
The change is small, low-risk, and placed at the correct location. The existing `tc358768_setup_pll()` already has a 1-2ms `usleep_range()` for PLL lock, then enables `PLL_CKEN`. The new delay gives the clock output time to settle after `PLL_CKEN` is asserted — a distinct and valid concern from the lock wait itself. The patch has a Tested-by tag on the affected hardware.
**Verdict**: Looks reasonable. One minor concern worth raising.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: gpu/drm: bridge: tc358768: Add delay after PLL setup
2026-05-20 9:26 ` [PATCH v1 1/1] " Svyatoslav Ryhel
@ 2026-05-25 12:05 ` Claude Code Review Bot
0 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 12:05 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**The fix itself is correct in placement.** Looking at the PLL setup sequence:
```c
/* wait for lock */
usleep_range(1000, 2000);
/* FRS[11:10] LBWS[9:8] CKEN[4] PLL_CKEN[4] RESETB[1] EN[0] */
tc358768_write(priv, TC358768_PLLCTL1,
(frs << 10) | (0x2 << 8) | BIT(4) | BIT(1) | BIT(0));
```
The existing 1-2ms delay waits for the PLL to lock before enabling `PLL_CKEN` (BIT(4)). The new 20ms `msleep()` is placed *after* `tc358768_setup_pll()` returns — i.e., after `PLL_CKEN` is enabled — giving the clock output time to propagate and stabilize before the DSI register configuration begins. This is a reasonable hardware settling requirement.
**Minor concerns:**
1. **Delay magnitude**: 20ms is a fairly generous delay. The commit message says "small settling delay" but doesn't reference a datasheet timing spec. It would be good to know whether 20ms is derived from the TC358768 datasheet or found empirically. If empirical, a comment noting the origin (e.g., "empirically determined on TF700T") would help future maintainers understand whether this can be tightened. That said, 20ms in a `pre_enable` path is unlikely to cause user-visible latency issues — modeset paths are inherently slow.
2. **`msleep` vs `usleep_range`**: The kernel convention is to use `usleep_range()` for delays under ~20ms. At exactly 20ms, `msleep()` is acceptable per `Documentation/timers/timers-howto.rst`, but `usleep_range(20000, 25000)` would be slightly more idiomatic and give the scheduler more flexibility. This is a stylistic nit, not a blocking issue.
3. **No datasheet reference**: The TC358768 datasheet likely has a timing spec for the PLL settling time after `PLL_CKEN` assertion. Referencing it (or noting the absence of one) would strengthen the commit message.
**No correctness issues.** The patch is safe to apply — it adds latency only in the modeset path, affects only the TC358768 bridge, and has hardware testing confirmation.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-25 12:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-20 9:26 [PATCH v1 0/1] gpu/drm: bridge: tc358768: Add delay after PLL setup Svyatoslav Ryhel
2026-05-20 9:26 ` [PATCH v1 1/1] " Svyatoslav Ryhel
2026-05-25 12:05 ` Claude review: " Claude Code Review Bot
2026-05-25 12:05 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox