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

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

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