[RFT,v2,0/5] drm: Add and use drm_fb_dma_get_gem_clipped_addr() helper

Message ID 20260916033327.3054126-1-wenst@chromium.org (mailing list archive)
Headers
Series drm: Add and use drm_fb_dma_get_gem_clipped_addr() helper |

Message

Chen-Yu Tsai Sept. 16, 2026, 3:33 a.m. UTC
Hi,

This is v2 of my drm_fb_dma_get_gem_clipped_addr() series.

Changes since v1:
- Add and use new drm_framebuffer_get_block_offset() helper (Thomas)


This series adds a helper to retrieve the buffer starting address of a
"clipped" framebuffer. This contrasts with drm_fb_dma_get_gem_addr(),
which gives the address of the full buffer.

Some drivers program their hardware with clipped dimensions, so they
should be using the clipped buffer address as well, unless the hardware
can advance the scanout directly. (Side note: many drivers still use
the non-clipped dimensions.)

While at it, also pull out the offset calculation of drm_fb_dma_get_gem_addr()
into a separate helper in drm_framebuffer.[ch], thereby separating
responsibilities.

The sun4i driver was recently incorrectly converted to use the unclipped
drm_fb_dma_get_gem_addr() helper. This broke offsets into subsampled
pixel groups, but also exposed the mismatch between the dimensions used
vs the buffer address. Two other drivers were also touched.


Patch 1 adds a new helper to return the byte offset into a framebuffer
for the start of the pixel block of the given pixel coordinates.

Patch 2 adds the new helper to return the buffer address based on
clipped coordinates.

Patch 3 switches the sun4i driver to the new helper, and fixes the
luma plane buffer address offset for subsampled YUV formats.

Patch 4 converts the imx/dc driver to use the new helper. This fixes a
mismatch between the programmed coordinates and the buffer address.

Patch 5 replaces the open coded buffer address calculation in the
imx/dcss driver with the new helper. Existing behavior, which might be
wrong, is preserved.


Please help test. The series is only compile tested on my end. The sun4i
changes should revert its behavior to before the drm_fb_dma_get_gem_addr()
was adopted. The imx/dcss changes should not have any behavioral
difference.


Thanks
ChenYu

Chen-Yu Tsai (5):
  drm: Split framebuffer pixel offset calculation from
    drm_fb_dma_get_gem_addr()
  drm/fb-dma-helper: Add drm_fb_dma_get_gem_clipped_addr()
  drm/sun4i: layers: Fix VI buffer address for clipped offsets
  drm/imx/dc: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
  drm/imx/dcss: plane: Switch to drm_fb_dma_get_gem_clipped_addr()

 drivers/gpu/drm/drm_fb_dma_helper.c    | 61 ++++++++++++++------------
 drivers/gpu/drm/drm_framebuffer.c      | 45 +++++++++++++++++++
 drivers/gpu/drm/imx/dc/dc-plane.c      |  4 +-
 drivers/gpu/drm/imx/dcss/dcss-plane.c  | 34 ++++++--------
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c |  2 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 16 ++++++-
 include/drm/drm_fb_dma_helper.h        |  4 ++
 include/drm/drm_framebuffer.h          |  3 ++
 8 files changed, 118 insertions(+), 51 deletions(-)
  

Comments

Icenowy Zheng Sept. 17, 2026, 11:15 a.m. UTC | #1
在 2026-09-16三的 11:33 +0800,Chen-Yu Tsai写道:
> Hi,
> 
> This is v2 of my drm_fb_dma_get_gem_clipped_addr() series.
> 
> Changes since v1:
> - Add and use new drm_framebuffer_get_block_offset() helper (Thomas)
> 
> 
> This series adds a helper to retrieve the buffer starting address of
> a
> "clipped" framebuffer. This contrasts with drm_fb_dma_get_gem_addr(),
> which gives the address of the full buffer.

Should vs_fb_get_dma_addr() in verisilicon/vs_plane.c be replaced with
this helper too?

I implemented manual framebuffer offset addition here.

Thanks,
Icenowy

> 
> Some drivers program their hardware with clipped dimensions, so they
> should be using the clipped buffer address as well, unless the
> hardware
> can advance the scanout directly. (Side note: many drivers still use
> the non-clipped dimensions.)
> 
> While at it, also pull out the offset calculation of
> drm_fb_dma_get_gem_addr()
> into a separate helper in drm_framebuffer.[ch], thereby separating
> responsibilities.
> 
> The sun4i driver was recently incorrectly converted to use the
> unclipped
> drm_fb_dma_get_gem_addr() helper. This broke offsets into subsampled
> pixel groups, but also exposed the mismatch between the dimensions
> used
> vs the buffer address. Two other drivers were also touched.
> 
> 
> Patch 1 adds a new helper to return the byte offset into a
> framebuffer
> for the start of the pixel block of the given pixel coordinates.
> 
> Patch 2 adds the new helper to return the buffer address based on
> clipped coordinates.
> 
> Patch 3 switches the sun4i driver to the new helper, and fixes the
> luma plane buffer address offset for subsampled YUV formats.
> 
> Patch 4 converts the imx/dc driver to use the new helper. This fixes
> a
> mismatch between the programmed coordinates and the buffer address.
> 
> Patch 5 replaces the open coded buffer address calculation in the
> imx/dcss driver with the new helper. Existing behavior, which might
> be
> wrong, is preserved.
> 
> 
> Please help test. The series is only compile tested on my end. The
> sun4i
> changes should revert its behavior to before the
> drm_fb_dma_get_gem_addr()
> was adopted. The imx/dcss changes should not have any behavioral
> difference.
> 
> 
> Thanks
> ChenYu
> 
> Chen-Yu Tsai (5):
>   drm: Split framebuffer pixel offset calculation from
>     drm_fb_dma_get_gem_addr()
>   drm/fb-dma-helper: Add drm_fb_dma_get_gem_clipped_addr()
>   drm/sun4i: layers: Fix VI buffer address for clipped offsets
>   drm/imx/dc: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
>   drm/imx/dcss: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
> 
>  drivers/gpu/drm/drm_fb_dma_helper.c    | 61 ++++++++++++++----------
> --
>  drivers/gpu/drm/drm_framebuffer.c      | 45 +++++++++++++++++++
>  drivers/gpu/drm/imx/dc/dc-plane.c      |  4 +-
>  drivers/gpu/drm/imx/dcss/dcss-plane.c  | 34 ++++++--------
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c |  2 +-
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 16 ++++++-
>  include/drm/drm_fb_dma_helper.h        |  4 ++
>  include/drm/drm_framebuffer.h          |  3 ++
>  8 files changed, 118 insertions(+), 51 deletions(-)
  
Chen-Yu Tsai Sept. 17, 2026, 11:42 a.m. UTC | #2
On Thu, Sep 17, 2026 at 7:16 PM Icenowy Zheng <uwu@icenowy.me> wrote:
>
> 在 2026-09-16三的 11:33 +0800,Chen-Yu Tsai写道:
> > Hi,
> >
> > This is v2 of my drm_fb_dma_get_gem_clipped_addr() series.
> >
> > Changes since v1:
> > - Add and use new drm_framebuffer_get_block_offset() helper (Thomas)
> >
> >
> > This series adds a helper to retrieve the buffer starting address of
> > a
> > "clipped" framebuffer. This contrasts with drm_fb_dma_get_gem_addr(),
> > which gives the address of the full buffer.
>
> Should vs_fb_get_dma_addr() in verisilicon/vs_plane.c be replaced with
> this helper too?
>
> I implemented manual framebuffer offset addition here.

Didn't I replace vs_fb_get_dma_addr() with drm_fb_dma_get_gem_addr()
already? At the time only primary and cursor planes were supported by
the driver. The primary plane can't be clipped, and the cursor plane
had some custom clipping, but seemed to want the unclipped address.


ChenYu

> Thanks,
> Icenowy
>
> >
> > Some drivers program their hardware with clipped dimensions, so they
> > should be using the clipped buffer address as well, unless the
> > hardware
> > can advance the scanout directly. (Side note: many drivers still use
> > the non-clipped dimensions.)
> >
> > While at it, also pull out the offset calculation of
> > drm_fb_dma_get_gem_addr()
> > into a separate helper in drm_framebuffer.[ch], thereby separating
> > responsibilities.
> >
> > The sun4i driver was recently incorrectly converted to use the
> > unclipped
> > drm_fb_dma_get_gem_addr() helper. This broke offsets into subsampled
> > pixel groups, but also exposed the mismatch between the dimensions
> > used
> > vs the buffer address. Two other drivers were also touched.
> >
> >
> > Patch 1 adds a new helper to return the byte offset into a
> > framebuffer
> > for the start of the pixel block of the given pixel coordinates.
> >
> > Patch 2 adds the new helper to return the buffer address based on
> > clipped coordinates.
> >
> > Patch 3 switches the sun4i driver to the new helper, and fixes the
> > luma plane buffer address offset for subsampled YUV formats.
> >
> > Patch 4 converts the imx/dc driver to use the new helper. This fixes
> > a
> > mismatch between the programmed coordinates and the buffer address.
> >
> > Patch 5 replaces the open coded buffer address calculation in the
> > imx/dcss driver with the new helper. Existing behavior, which might
> > be
> > wrong, is preserved.
> >
> >
> > Please help test. The series is only compile tested on my end. The
> > sun4i
> > changes should revert its behavior to before the
> > drm_fb_dma_get_gem_addr()
> > was adopted. The imx/dcss changes should not have any behavioral
> > difference.
> >
> >
> > Thanks
> > ChenYu
> >
> > Chen-Yu Tsai (5):
> >   drm: Split framebuffer pixel offset calculation from
> >     drm_fb_dma_get_gem_addr()
> >   drm/fb-dma-helper: Add drm_fb_dma_get_gem_clipped_addr()
> >   drm/sun4i: layers: Fix VI buffer address for clipped offsets
> >   drm/imx/dc: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
> >   drm/imx/dcss: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
> >
> >  drivers/gpu/drm/drm_fb_dma_helper.c    | 61 ++++++++++++++----------
> > --
> >  drivers/gpu/drm/drm_framebuffer.c      | 45 +++++++++++++++++++
> >  drivers/gpu/drm/imx/dc/dc-plane.c      |  4 +-
> >  drivers/gpu/drm/imx/dcss/dcss-plane.c  | 34 ++++++--------
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c |  2 +-
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 16 ++++++-
> >  include/drm/drm_fb_dma_helper.h        |  4 ++
> >  include/drm/drm_framebuffer.h          |  3 ++
> >  8 files changed, 118 insertions(+), 51 deletions(-)
  
Icenowy Zheng Sept. 17, 2026, 11:51 a.m. UTC | #3
在 2026-09-17四的 19:42 +0800,Chen-Yu Tsai写道:
> On Thu, Sep 17, 2026 at 7:16 PM Icenowy Zheng <uwu@icenowy.me> wrote:
> > 
> > 在 2026-09-16三的 11:33 +0800,Chen-Yu Tsai写道:
> > > Hi,
> > > 
> > > This is v2 of my drm_fb_dma_get_gem_clipped_addr() series.
> > > 
> > > Changes since v1:
> > > - Add and use new drm_framebuffer_get_block_offset() helper
> > > (Thomas)
> > > 
> > > 
> > > This series adds a helper to retrieve the buffer starting address
> > > of
> > > a
> > > "clipped" framebuffer. This contrasts with
> > > drm_fb_dma_get_gem_addr(),
> > > which gives the address of the full buffer.
> > 
> > Should vs_fb_get_dma_addr() in verisilicon/vs_plane.c be replaced
> > with
> > this helper too?
> > 
> > I implemented manual framebuffer offset addition here.
> 
> Didn't I replace vs_fb_get_dma_addr() with drm_fb_dma_get_gem_addr()

Yes, it seems so. I checked newest rc, but this change is in drm-misc-
next. Sorry for the noise.

> already? At the time only primary and cursor planes were supported by
> the driver. The primary plane can't be clipped, and the cursor plane
> had some custom clipping, but seemed to want the unclipped address.

Yes it looks like thedrm_fb_dma_get_gem_addr() helper already handled
the non-clipping source offset.

Thanks,
Icenowy

> 
> 
> ChenYu
> 
> > Thanks,
> > Icenowy
> > 
> > > 
> > > Some drivers program their hardware with clipped dimensions, so
> > > they
> > > should be using the clipped buffer address as well, unless the
> > > hardware
> > > can advance the scanout directly. (Side note: many drivers still
> > > use
> > > the non-clipped dimensions.)
> > > 
> > > While at it, also pull out the offset calculation of
> > > drm_fb_dma_get_gem_addr()
> > > into a separate helper in drm_framebuffer.[ch], thereby
> > > separating
> > > responsibilities.
> > > 
> > > The sun4i driver was recently incorrectly converted to use the
> > > unclipped
> > > drm_fb_dma_get_gem_addr() helper. This broke offsets into
> > > subsampled
> > > pixel groups, but also exposed the mismatch between the
> > > dimensions
> > > used
> > > vs the buffer address. Two other drivers were also touched.
> > > 
> > > 
> > > Patch 1 adds a new helper to return the byte offset into a
> > > framebuffer
> > > for the start of the pixel block of the given pixel coordinates.
> > > 
> > > Patch 2 adds the new helper to return the buffer address based on
> > > clipped coordinates.
> > > 
> > > Patch 3 switches the sun4i driver to the new helper, and fixes
> > > the
> > > luma plane buffer address offset for subsampled YUV formats.
> > > 
> > > Patch 4 converts the imx/dc driver to use the new helper. This
> > > fixes
> > > a
> > > mismatch between the programmed coordinates and the buffer
> > > address.
> > > 
> > > Patch 5 replaces the open coded buffer address calculation in the
> > > imx/dcss driver with the new helper. Existing behavior, which
> > > might
> > > be
> > > wrong, is preserved.
> > > 
> > > 
> > > Please help test. The series is only compile tested on my end.
> > > The
> > > sun4i
> > > changes should revert its behavior to before the
> > > drm_fb_dma_get_gem_addr()
> > > was adopted. The imx/dcss changes should not have any behavioral
> > > difference.
> > > 
> > > 
> > > Thanks
> > > ChenYu
> > > 
> > > Chen-Yu Tsai (5):
> > >   drm: Split framebuffer pixel offset calculation from
> > >     drm_fb_dma_get_gem_addr()
> > >   drm/fb-dma-helper: Add drm_fb_dma_get_gem_clipped_addr()
> > >   drm/sun4i: layers: Fix VI buffer address for clipped offsets
> > >   drm/imx/dc: plane: Switch to drm_fb_dma_get_gem_clipped_addr()
> > >   drm/imx/dcss: plane: Switch to
> > > drm_fb_dma_get_gem_clipped_addr()
> > > 
> > >  drivers/gpu/drm/drm_fb_dma_helper.c    | 61 ++++++++++++++------
> > > ----
> > > --
> > >  drivers/gpu/drm/drm_framebuffer.c      | 45 +++++++++++++++++++
> > >  drivers/gpu/drm/imx/dc/dc-plane.c      |  4 +-
> > >  drivers/gpu/drm/imx/dcss/dcss-plane.c  | 34 ++++++--------
> > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c |  2 +-
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 16 ++++++-
> > >  include/drm/drm_fb_dma_helper.h        |  4 ++
> > >  include/drm/drm_framebuffer.h          |  3 ++
> > >  8 files changed, 118 insertions(+), 51 deletions(-)