Re: [PATCH v2 1/2] drm/tegra: Support swapping chroma planes

From: Aaron Kling

Date: Tue Oct 20 2026 - 04:13:25 EST

On Tue, Oct 20, 2026 at 1:33 AM Mikko Perttunen <mperttunen@xxxxxxxxxx> wrote:
>
> On Tuesday, October 20, 2026 2:59 PM Aaron Kling wrote:
> > On Tue, Oct 20, 2026 at 12:08 AM Mikko Perttunen <mperttunen@xxxxxxxxxx> wrote:
> > >
> > > On Monday, October 19, 2026 7:03 AM Aaron Kling via B4 Relay wrote:
> > > > From: Aaron Kling <webgeek1234@xxxxxxxxx>
> > > >
> > > > The Tegra display controller supports some yuv types, but not their
> > > > chroma swapped variants. For example, yuv420 is supported but yvu420 is
> > > > not. This can be efficiently worked around by just passing the hardware
> > > > said chroma planes backwards. Add a flag to the plane state struct and
> > > > handling to both tegradc and nvdisplay paths.
> > > >
> > > > Signed-off-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/tegra/dc.c | 6 +++++-
> > > > drivers/gpu/drm/tegra/hub.c | 14 +++++++++-----
> > > > drivers/gpu/drm/tegra/plane.c | 5 ++++-
> > > > drivers/gpu/drm/tegra/plane.h | 3 ++-
> > > > 4 files changed, 20 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > > > index b0bfa946e6979..39c2e920d76a6 100644
> > > > --- a/drivers/gpu/drm/tegra/dc.c
> > > > +++ b/drivers/gpu/drm/tegra/dc.c
> > > > @@ -740,7 +740,8 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
> > > >
> > > > err = tegra_plane_format(new_plane_state->fb->format->format,
> > > > &plane_state->format,
> > > > - &plane_state->swap);
> > > > + &plane_state->swap,
> > > > + &plane_state->swap_uv);
> > > > if (err < 0)
> > > > return err;
> > > >
> > > > @@ -872,6 +873,9 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
> > > > window.stride[i] = fb->pitches[i];
> > > > }
> > > >
> > > > + if (tegra_plane_state->swap_uv)
> > > > + swap(window.base[1], window.base[2]);
> > > > +
> > > > tegra_dc_setup_window(p, &window);
> > > > }
> > > >
> > > > diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> > > > index 448f49f3a7d7e..6427155ac4fdf 100644
> > > > --- a/drivers/gpu/drm/tegra/hub.c
> > > > +++ b/drivers/gpu/drm/tegra/hub.c
> > > > @@ -440,7 +440,8 @@ static int tegra_shared_plane_atomic_check(struct drm_plane *plane,
> > > >
> > > > err = tegra_plane_format(new_plane_state->fb->format->format,
> > > > &plane_state->format,
> > > > - &plane_state->swap);
> > > > + &plane_state->swap,
> > > > + &plane_state->swap_uv);
> > > > if (err < 0)
> > > > return err;
> > > >
> > > > @@ -664,24 +665,27 @@ static void tegra_shared_plane_atomic_update(struct drm_plane *plane,
> > > > tegra_plane_writel(p, value, DC_WIN_PLANAR_STORAGE);
> > > >
> > > > if (yuv && planes > 1) {
> > > > - base = tegra_plane_state->iova[1] + fb->offsets[1];
> > > > + uint8_t u = tegra_plane_state->swap_uv ? 2 : 1;
> > > > + uint8_t v = tegra_plane_state->swap_uv ? 1 : 2;
> > > > +
> > > > + base = tegra_plane_state->iova[u] + fb->offsets[u];
> > > > base |= addr_flag;
> > > >
> > > > tegra_plane_writel(p, upper_32_bits(base), DC_WINBUF_START_ADDR_HI_U);
> > > > tegra_plane_writel(p, lower_32_bits(base), DC_WINBUF_START_ADDR_U);
> > > >
> > > > if (planes > 2) {
> > > > - base = tegra_plane_state->iova[2] + fb->offsets[2];
> > > > + base = tegra_plane_state->iova[v] + fb->offsets[v];
> > > > base |= addr_flag;
> > > >
> > > > tegra_plane_writel(p, upper_32_bits(base), DC_WINBUF_START_ADDR_HI_V);
> > > > tegra_plane_writel(p, lower_32_bits(base), DC_WINBUF_START_ADDR_V);
> > > > }
> > > >
> > > > - value = PITCH_U(fb->pitches[1]);
> > > > + value = PITCH_U(fb->pitches[u]);
> > > >
> > > > if (planes > 2)
> > > > - value |= PITCH_V(fb->pitches[2]);
> > > > + value |= PITCH_V(fb->pitches[v]);
> > > >
> > > > tegra_plane_writel(p, value, DC_WIN_PLANAR_STORAGE_UV);
> > > > } else {
> > >
> > > I noticed DC_WIN_PRECOMP_WGRP_PARAMS has a SWAP_UV bit. This is already
> > > defined in dc.h. So we can use that for the hub.c implementation instead
> > > of manually swapping.
> >
> > Oh. I figured there had to be something like that in one of the
> > generations, but my grep-fu failed me. Even more embarrassing that I
> > ended up using the exact same name. I will update and verify this and
> > send a new revision.
> >
> > > Otherwise everything looks good to me. We could also enable the swap_uv
> > > logic for other UV formats.
> >
> > Eh, other than asking for the entire patch to be rewritten over the
> > last two revisions, it looks good. :P And alright, I'll plumb yvu422
> > and yvu444 in the new revision as well. However, I don't have test
> > cases for those.
>
> These are minor revisions, pretty much cosmetic! :)
>
> If you have access to the libdrm modetest command, you can use it to
> test -- e.g.
>
> sudo modetest -M tegra -s 78:1920x1080 \
> -P 44@43:896x504+40+288@YU12 \
> -P 48@43:896x504+984+288@YV12
>
> (the ID numbers may differ) to set up a 1920x1080 mode with two overlay
> planes one with each plane ordering.

I'm still mostly focused on android. If there's something where I'm
concerned there's a difference between android and a Linux rootfs or
such, I'll test against a simple cli rootfs. But I don't have a full
graphical Linux setup for testing. At one point I did try to compile
modetest for aosp, but I don't think I made it work.

Aaron

> BTW, I believe the hardware swap bit also supports packed and semiplanar
> formats -- you can add those too if you'd like or leave them out and we
> can add them later if needed.
>
> Mikko
>
> >
> > Aaron
> >
> > > Cheers
> > > Mikko
> > >
> > > > diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> > > > index 2148b0c59815c..757bc552809fe 100644
> > > > --- a/drivers/gpu/drm/tegra/plane.c
> > > > +++ b/drivers/gpu/drm/tegra/plane.c
> > > > @@ -59,6 +59,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane)
> > > > copy->tiling = state->tiling;
> > > > copy->format = state->format;
> > > > copy->swap = state->swap;
> > > > + copy->swap_uv = state->swap_uv;
> > > > copy->reflect_x = state->reflect_x;
> > > > copy->reflect_y = state->reflect_y;
> > > > copy->opaque = state->opaque;
> > > > @@ -319,11 +320,13 @@ int tegra_plane_state_add(struct tegra_plane *plane,
> > > > return 0;
> > > > }
> > > >
> > > > -int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
> > > > +int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool *swap_uv)
> > > > {
> > > > /* assume no swapping of fetched data */
> > > > if (swap)
> > > > *swap = BYTE_SWAP_NOSWAP;
> > > > + if (swap_uv)
> > > > + *swap_uv = false;
> > > >
> > > > switch (fourcc) {
> > > > case DRM_FORMAT_ARGB4444:
> > > > diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
> > > > index e33a581e6490e..dba5f1b254e98 100644
> > > > --- a/drivers/gpu/drm/tegra/plane.h
> > > > +++ b/drivers/gpu/drm/tegra/plane.h
> > > > @@ -49,6 +49,7 @@ struct tegra_plane_state {
> > > > struct tegra_bo_tiling tiling;
> > > > u32 format;
> > > > u32 swap;
> > > > + bool swap_uv;
> > > >
> > > > bool reflect_x;
> > > > bool reflect_y;
> > > > @@ -88,7 +89,7 @@ void tegra_plane_cleanup_fb(struct drm_plane *plane,
> > > > int tegra_plane_state_add(struct tegra_plane *plane,
> > > > struct drm_plane_state *state);
> > > >
> > > > -int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);
> > > > +int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool *swap_uv);
> > > > bool tegra_plane_format_is_indexed(unsigned int format);
> > > > bool tegra_plane_format_is_yuv(unsigned int format, unsigned int *planes, unsigned int *bpc);
> > > > int tegra_plane_setup_legacy_state(struct tegra_plane *tegra,
> > > >
> > > > --
> > > > 2.54.0
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
>
>
>
>