[igt-dev] [PATCH] lib/igt_fb: Support Tile Size for AMD non linear modifiers.

[Mark Yacoub]

On Fri, Jun 18, 2021 at 6:40 AM Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
>
> On Fri, Jun 18, 2021 at 1:45 AM Bas Nieuwenhuizen
> <bas at basnieuwenhuizen.nl> wrote:
> >
> > Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> >
> > though I'm not sure how useful it is without also appropriately
> > catching the stride weirdness wrt amdgpu.
> >
> > On Mon, Jun 7, 2021 at 6:13 PM Mark Yacoub <markyacoub at chromium.org> wrote:
> > >
> > > friendly ping.
> > >
> > > On Tue, Apr 13, 2021 at 12:02 PM Mark Yacoub <markyacoub at chromium.org> wrote:
> > > >
> > > > From: Mark Yacoub <markyacoub at google.com>
> > > >
> > > > Calculate the width and height of the tile on amdgpu when the modifier
> > > > is not linear.
> > > >
> > > > Cc: bas at basnieuwenhuizen.nl
> > > > Cc: daniel.vetter at ffwll.ch
> > > > Signed-off-by: markyacoub at chromium.org
> > > > ---
> > > >  lib/igt_fb.c | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 4ded7e78..3d1fdae5 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -396,6 +396,19 @@ void igt_get_fb_tile_size(int fd, uint64_t modifier, int fb_bpp,
> > > >                 vc4_modifier_param = fourcc_mod_broadcom_param(modifier);
> > > >                 modifier = fourcc_mod_broadcom_mod(modifier);
> > > >         }
> > > > +       // For all non-linear modifiers, AMD uses 64 KiB tiles
> > > > +       else if (IS_AMD_FMT_MOD(modifier)) {
> > > > +               igt_require_amdgpu(fd);
> > > > +               const int bytes_per_pixel = fb_bpp / 8;
> > > > +               const int format_log2 = log2(bytes_per_pixel);
> > > > +               const int pixel_log2 = log2(64 * 1024) - format_log2;
> > > > +               const int width_log2 = (pixel_log2 + 1) / 2;
> > > > +               const int height_log2 = pixel_log2 - width_log2;
> > > > +
> > > > +               *width_ret = 1 << width_log2;
>
> Actually looking at the documentation of this function width_ret
> should be the width in bytes, not pixels, so we should probably use
> "bytes_per_pixel << width_log2".
>
Thanks! updated and sent v2: Thanks! updated it and sent v2:
https://patchwork.freedesktop.org/patch/440283/?series=89017&rev=2
> > > > +               *height_ret = 1 << height_log2;
> > > > +               return;
> > > > +       }
> > > >
> > > >         switch (modifier) {
> > > >         case LOCAL_DRM_FORMAT_MOD_NONE:
> > > > --
> > > > 2.31.1.295.g9ea45b61b8-goog
> > > >