[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

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

[Chen-Yu Tsai]

On Tue, 10 Mar 2026 00:56:33 +0800, Chen-Yu Tsai wrote:
> 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.
> 
> [...]

Applied to drm-misc-next in drm-misc, thanks!

[1/1] drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory
      commit: 79ac1c945ab885369a840da014ebc6fcde8e2ae2

Best regards,
-- 
Chen-Yu Tsai <wens@kernel.org>

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

[Chen-Yu Tsai]

Hi,

On Tue, Mar 10, 2026 at 12:56 AM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> 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);

I just realized while reading more code that there's a subtle difference
here.

The original code used state->src.x1, which is the "clipped" coordinates,
while drm_fb_dma_get_gem_addr() uses state->src_x, which is the original
coordinates.

Jernej, do you know which one the driver is supposed to use? Or maybe
it doesn't matter?


ChenYu

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

[Jernej Škrabec]

Dne sreda, 18. marec 2026 ob 10:35:35 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> Hi,
> 
> On Tue, Mar 10, 2026 at 12:56 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > 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);
> 
> I just realized while reading more code that there's a subtle difference
> here.
> 
> The original code used state->src.x1, which is the "clipped" coordinates,
> while drm_fb_dma_get_gem_addr() uses state->src_x, which is the original
> coordinates.
> 
> Jernej, do you know which one the driver is supposed to use? Or maybe
> it doesn't matter?

I would say that it doesn't matter for point of origin. Original coordinates
use uint32_t values which means origin cannot be out of screen (negative
numbers). What I found interesting is that clipped coordinates are int, e.g.
they can be negative.

Best regards,
Jernej