[PATCH] drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory

[PATCH] drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory

[Chen-Yu Tsai]

Commit 4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
adds a new helper, which covers fetching a drm_framebuffer's GEM object
and calculating the buffer address for a given plane.

This patch uses this helper to replace our own open coded version of the
same function.

Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 16 ++-------------
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 27 ++------------------------
 2 files changed, 4 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index f08f6da55dd0..72c92203ae63 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -124,25 +124,13 @@ static void sun8i_ui_layer_update_buffer(struct sun8i_layer *layer,
 {
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_dma_object *gem;
 	dma_addr_t dma_addr;
 	u32 ch_base;
-	int bpp;
 
 	ch_base = sun8i_channel_base(layer);
 
-	/* Get the physical address of the buffer in memory */
-	gem = drm_fb_dma_get_gem_obj(fb, 0);
-
-	DRM_DEBUG_DRIVER("Using GEM @ %pad\n", &gem->dma_addr);
-
-	/* Compute the start of the displayed memory */
-	bpp = fb->format->cpp[0];
-	dma_addr = gem->dma_addr + fb->offsets[0];
-
-	/* Fixup framebuffer address for src coordinates */
-	dma_addr += (state->src.x1 >> 16) * bpp;
-	dma_addr += (state->src.y1 >> 16) * fb->pitches[0];
+	/* Get the start of the displayed memory */
+	dma_addr = drm_fb_dma_get_gem_addr(fb, state, 0);
 
 	/* Set the line width */
 	DRM_DEBUG_DRIVER("Layer line width: %d bytes\n", fb->pitches[0]);
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index ca3ab59e108d..cd8d6c2da0c7 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -197,38 +197,15 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
 	const struct drm_format_info *format = fb->format;
-	struct drm_gem_dma_object *gem;
-	u32 dx, dy, src_x, src_y;
 	dma_addr_t dma_addr;
 	u32 ch_base;
 	int i;
 
 	ch_base = sun8i_channel_base(layer);
 
-	/* Adjust x and y to be dividable by subsampling factor */
-	src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
-	src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
-
 	for (i = 0; i < format->num_planes; i++) {
-		/* Get the physical address of the buffer in memory */
-		gem = drm_fb_dma_get_gem_obj(fb, i);
-
-		DRM_DEBUG_DRIVER("Using GEM @ %pad\n", &gem->dma_addr);
-
-		/* Compute the start of the displayed memory */
-		dma_addr = gem->dma_addr + fb->offsets[i];
-
-		dx = src_x;
-		dy = src_y;
-
-		if (i > 0) {
-			dx /= format->hsub;
-			dy /= format->vsub;
-		}
-
-		/* Fixup framebuffer address for src coordinates */
-		dma_addr += dx * format->cpp[i];
-		dma_addr += dy * fb->pitches[i];
+		/* Get the start of the displayed memory */
+		dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
 
 		/* Set the line width */
 		DRM_DEBUG_DRIVER("Layer %d. line width: %d bytes\n",
-- 
2.47.3

Re: [PATCH] drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory

[Jernej Škrabec]

Dne ponedeljek, 9. marec 2026 ob 17:56:33 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> Commit 4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
> adds a new helper, which covers fetching a drm_framebuffer's GEM object
> and calculating the buffer address for a given plane.
> 
> This patch uses this helper to replace our own open coded version of the
> same function.
> 
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>

Nice!

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Claude review: drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory

[Claude Code Review Bot]

Overall Series Review

Subject: drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory
Author: Chen-Yu Tsai <wens@kernel.org>
Patches: 2
Reviewed: 2026-03-10T12:07:04.340955

---

This is a single-patch cleanup that replaces open-coded DMA address calculations in the sun4i UI and VI layer drivers with the existing `drm_fb_dma_get_gem_addr()` helper. The change is straightforward, reduces code duplication, and the helper's logic matches the replaced code. **Looks good overall, with one minor behavioral subtlety worth noting.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory

[Claude Code Review Bot]

Patch Review

**UI layer (`sun8i_ui_layer.c`) — Clean conversion:**

The old code:
```c
gem = drm_fb_dma_get_gem_obj(fb, 0);
bpp = fb->format->cpp[0];
dma_addr = gem->dma_addr + fb->offsets[0];
dma_addr += (state->src.x1 >> 16) * bpp;
dma_addr += (state->src.y1 >> 16) * fb->pitches[0];
```

is replaced by:
```c
dma_addr = drm_fb_dma_get_gem_addr(fb, state, 0);
```

For plane 0 with standard (non-block) formats, the helper computes `obj->dma_addr + offsets[0] + sample_x * char_per_block[0] + sample_y * pitches[0]`, which is equivalent. This conversion is correct.

**VI layer (`sun8i_vi_layer.c`) — Subtle behavioral difference:**

The old code explicitly aligns source coordinates to chroma subsampling boundaries before computing offsets:
```c
src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
```

The helper does not perform this alignment — it just divides by hsub/vsub directly:
```c
sample_x = (state->src_x >> 16) / h_div;
sample_y = (state->src_y >> 16) / v_div;
```

For **plane > 0** (chroma planes), these produce the same result: integer division by hsub has the same effect as masking then dividing when the coordinates are valid (aligned to subsampling boundaries). The DRM atomic check framework should validate that source coordinates are properly aligned for subsampled formats.

For **plane 0** (luma), the old code applied the alignment mask before using the coordinates (`dx = src_x` without division), effectively rounding the luma offset down to a chroma-aligned boundary. The helper does not do this alignment for plane 0 (h_div=v_div=1). In practice, for valid state, source coordinates should already be aligned, so this is a no-op difference. If anything, the old behavior of rounding down luma coordinates was slightly wrong — it could silently shift the displayed region.

**Minor nits:**
- The `DRM_DEBUG_DRIVER("Using GEM @ %pad\n", ...)` debug prints are removed. This is fine — they add noise and the helper is well-established across many drivers. Other sun4i files (`sun4i_backend.c`, `sun4i_frontend.c`) already use this helper without such debug prints.
- The unused variable `format` (alias for `fb->format`) is left in `sun8i_vi_layer_update_buffer` — it's still used for `format->num_planes` in the loop, so that's fine.

**Verdict:** The patch is correct. The alignment difference in the VI layer is not a real-world concern since DRM enforces valid coordinates.

Reviewed-by: AI Review (informational, non-binding)

---
Generated by Claude Code Patch Reviewer