public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v3] drm: shmobile: Fix white screen after resume when LCDC is stopped
@ 2026-03-19  8:31 phucduc.bui
  2026-03-21 18:28 ` Claude review: " Claude Code Review Bot
  2026-03-21 18:28 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: phucduc.bui @ 2026-03-19  8:31 UTC (permalink / raw)
  Cc: phucduc.bui, airlied, dri-devel, geert, laurent.pinchart,
	linux-kernel, linux-renesas-soc, maarten.lankhorst, mripard,
	simona, tzimmermann, wsa+renesas, Geert Uytterhoeven

From: bui duc phuc <phucduc.bui@gmail.com>

The LCDC on R8A7740 may show a completely white screen after resuming
from suspend (s2idle or s2ram).

After resume, both Set A and Set B registers are reset to 0. As the
Frame End interrupt is not generated while the controller is stopped
(DO=0), the hardware register switching mechanism is not triggered.
Consequently, Set A remains at 0x00000000 even though a valid Start
Address is written to Set B, resulting in a white screen.

This is a timing-dependent race condition. In some configurations,
debug options slow down the resume path enough for a Frame End
interrupt to occur, which can mask the issue.

Fix this by priming both register sets with the Start Address while the
controller is stopped, ensuring a valid base address is available
immediately after resume.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes:
 - v3 
   Update commit message, add Tested-by and Reviewed-by tags 
 - v2 
   Fix incorrect use of lcdc_write_mirror() for LDSA2R in
   the DO=0 path; use lcdc_write() to update both register
   sets as intended.

 .../gpu/drm/renesas/shmobile/shmob_drm_plane.c  | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
index b61fda52d17a..23c8489dba71 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
@@ -70,6 +70,7 @@ static void shmob_drm_primary_plane_setup(struct shmob_drm_plane *splane,
 	struct shmob_drm_plane_state *sstate = to_shmob_plane_state(state);
 	struct shmob_drm_device *sdev = to_shmob_device(splane->base.dev);
 	struct drm_framebuffer *fb = state->fb;
+	u32 ldcnt2r;
 
 	/* TODO: Handle YUV colorspaces. Hardcode REC709 for now. */
 	lcdc_write(sdev, LDDFR, sstate->format->lddfr | LDDFR_CF1);
@@ -78,11 +79,19 @@ static void shmob_drm_primary_plane_setup(struct shmob_drm_plane *splane,
 	/* Word and long word swap. */
 	lcdc_write(sdev, LDDDSR, sstate->format->ldddsr);
 
-	lcdc_write_mirror(sdev, LDSA1R, sstate->dma[0]);
-	if (shmob_drm_format_is_yuv(sstate->format))
-		lcdc_write_mirror(sdev, LDSA2R, sstate->dma[1]);
+	ldcnt2r = lcdc_read(sdev, LDCNT2R);
+
+	if (ldcnt2r & LDCNT2R_DO) {
+		lcdc_write_mirror(sdev, LDSA1R, sstate->dma[0]);
+		if (shmob_drm_format_is_yuv(sstate->format))
+			lcdc_write_mirror(sdev, LDSA2R, sstate->dma[1]);
 
-	lcdc_write(sdev, LDRCNTR, lcdc_read(sdev, LDRCNTR) ^ LDRCNTR_MRS);
+		lcdc_write(sdev, LDRCNTR, lcdc_read(sdev, LDRCNTR) ^ LDRCNTR_MRS);
+	} else {
+		lcdc_write(sdev, LDSA1R, sstate->dma[0]);
+		if (shmob_drm_format_is_yuv(sstate->format))
+			lcdc_write(sdev, LDSA2R, sstate->dma[1]);
+	}
 }
 
 static void shmob_drm_overlay_plane_setup(struct shmob_drm_plane *splane,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm: shmobile: Fix white screen after resume when LCDC is stopped
  2026-03-19  8:31 [PATCH v3] drm: shmobile: Fix white screen after resume when LCDC is stopped phucduc.bui
  2026-03-21 18:28 ` Claude review: " Claude Code Review Bot
@ 2026-03-21 18:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:28 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm: shmobile: Fix white screen after resume when LCDC is stopped
Author: phucduc.bui@gmail.com
Patches: 1
Reviewed: 2026-03-22T04:28:52.005399

---

This is a single, well-motivated bugfix patch for the shmobile LCDC driver. The problem is clearly described: after suspend/resume, both register sets (A and B) are reset to 0, but the original code only wrote to the mirror (Set B) and relied on a Frame End interrupt to trigger the hardware register switching. When the controller is stopped (DO=0), no Frame End interrupt occurs, so Set A stays at 0 and the display shows white.

The fix is correct and minimal. It checks whether the display output is active (LDCNT2R_DO) and uses the appropriate write path:
- **DO=1 (running):** Use `lcdc_write_mirror()` + toggle `LDRCNTR_MRS` as before (the normal double-buffered update path).
- **DO=0 (stopped):** Use `lcdc_write()` which writes to both Set A and Set B directly, ensuring valid addresses are present when output is re-enabled.

The patch has Tested-by and Reviewed-by from the subsystem maintainer (Geert Uytterhoeven). It looks ready to merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm: shmobile: Fix white screen after resume when LCDC is stopped
  2026-03-19  8:31 [PATCH v3] drm: shmobile: Fix white screen after resume when LCDC is stopped phucduc.bui
@ 2026-03-21 18:28 ` Claude Code Review Bot
  2026-03-21 18:28 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The logic correctly distinguishes between the running and stopped states:

```c
ldcnt2r = lcdc_read(sdev, LDCNT2R);

if (ldcnt2r & LDCNT2R_DO) {
    lcdc_write_mirror(sdev, LDSA1R, sstate->dma[0]);
    ...
    lcdc_write(sdev, LDRCNTR, lcdc_read(sdev, LDRCNTR) ^ LDRCNTR_MRS);
} else {
    lcdc_write(sdev, LDSA1R, sstate->dma[0]);
    ...
}
```

When DO=0, `lcdc_write()` is used which writes to both the base register and `reg + LCDC_SIDE_B_OFFSET` (since LDSA1R and LDSA2R are banked registers per `lcdc_is_banked()`). This primes both register sets, avoiding the white screen.

When DO=1, the existing mirror-write + MRS toggle path is preserved, maintaining the correct double-buffered update behavior during normal operation.

**Minor observations:**
- The `LDRCNTR` toggle is correctly omitted in the DO=0 path since there's no need to request a register set switch when the controller is stopped.
- The YUV handling (LDSA2R) is consistently handled in both branches.
- No concerns about atomicity — in the DO=0 case the controller is stopped so there's no race with hardware reading registers.

**Commit message:** Clear, well-written, explains the root cause and the fix. The changelog for v2/v3 is properly formatted below the `---` separator.

**Verdict: No issues found.** This is a clean, correct, and well-tested fix.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-21 18:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-19  8:31 [PATCH v3] drm: shmobile: Fix white screen after resume when LCDC is stopped phucduc.bui
2026-03-21 18:28 ` Claude review: " Claude Code Review Bot
2026-03-21 18:28 ` 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