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

[Mikko Perttunen <mperttunen@nvidia.com>]

On Tuesday, October 20, 2026 2:59 PM Aaron Kling wrote:
> On Tue, Oct 20, 2026 at 12:08 AM Mikko Perttunen <mperttunen@nvidia.com> wrote:
> >
> > On Monday, October 19, 2026 7:03 AM Aaron Kling via B4 Relay wrote:
> > > From: Aaron Kling <webgeek1234@gmail.com>
> > >
> > > 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@gmail.com>
> > > ---
> > >  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.

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
> > >
> > >
> > >
> >
> >
> >
> >