[Mesa-dev] [PATCH 08/21] intel/isl: Implement correct tile size calculations for Ys/Yf

Pohjolainen, Topi topi.pohjolainen at gmail.com
Wed Feb 28 14:01:36 UTC 2018


On Tue, Feb 27, 2018 at 11:42:48AM -0800, Jason Ekstrand wrote:
> On Tue, Feb 27, 2018 at 6:13 AM, Pohjolainen, Topi <
> topi.pohjolainen at gmail.com> wrote:
> 
> > On Thu, Feb 22, 2018 at 11:06:48PM -0800, Jason Ekstrand wrote:
> > > The tile size calculations use a clever bit of math to make them short
> > > and simple.  We add unit tests to assert that they identically match the
> > > tables in the PRM.
> > > ---
> > >  src/intel/Makefile.isl.am                 |   9 +-
> > >  src/intel/isl/isl.c                       |  56 ++++++++++-
> > >  src/intel/isl/meson.build                 |  11 ++
> > >  src/intel/isl/tests/isl_tile_std_y_test.c | 160
> > ++++++++++++++++++++++++++++++
> > >  4 files changed, 230 insertions(+), 6 deletions(-)
> > >  create mode 100644 src/intel/isl/tests/isl_tile_std_y_test.c
> > >
> > > diff --git a/src/intel/Makefile.isl.am b/src/intel/Makefile.isl.am
> > > index 9525f9e..a498f2f 100644
> > > --- a/src/intel/Makefile.isl.am
> > > +++ b/src/intel/Makefile.isl.am
> > > @@ -75,7 +75,9 @@ isl/isl_format_layout.c: isl/gen_format_layout.py \
> > >  #  Tests
> > >  # ------------------------------------------------------------
> > ----------------
> > >
> > > -check_PROGRAMS += isl/tests/isl_surf_get_image_offset_test
> > > +check_PROGRAMS += \
> > > +     isl/tests/isl_surf_get_image_offset_test \
> > > +     isl/tests/isl_tile_std_y_test
> > >
> > >  TESTS += $(check_PROGRAMS)
> > >
> > > @@ -84,6 +86,11 @@ isl_tests_isl_surf_get_image_offset_test_LDADD = \
> > >       isl/libisl.la \
> > >       -lm
> > >
> > > +isl_tests_isl_tile_std_y_test_LDADD = \
> > > +     common/libintel_common.la \
> > > +     isl/libisl.la \
> > > +     -lm
> > > +
> > >  # ------------------------------------------------------------
> > ----------------
> > >
> > >  EXTRA_DIST += \
> > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > index aa56a3c..fcbe2ad 100644
> > > --- a/src/intel/isl/isl.c
> > > +++ b/src/intel/isl/isl.c
> > > @@ -217,13 +217,59 @@ isl_tiling_get_info(enum isl_tiling tiling,
> > >     case ISL_TILING_Yf:
> > >     case ISL_TILING_Ys: {
> > >        bool is_Ys = tiling == ISL_TILING_Ys;
> > > +      assert(format_bpb >= 8);
> > >
> > > -      assert(bs > 0);
> > > -      unsigned width = 1 << (6 + (ffs(bs) / 2) + (2 * is_Ys));
> > > -      unsigned height = 1 << (6 - (ffs(bs) / 2) + (2 * is_Ys));
> > > +      switch (dim) {
> > > +      case ISL_SURF_DIM_1D:
> > > +         /* See the Skylake BSpec > Memory Views > Common Surface
> > Formats >
> > > +          * Surface Layout and Tiling > 1D Surfaces > 1D Alignment
> > > +          * Requirements.
> >
> > I wonder if I'm looking the right version, under "Memory Views" there is no
> > section called "Common Surface Formats" - but under "Memory Data Formats"
> > there is such. Only there the "1D Surfaces > 1D Alignment" section is
> > pretty
> > limited - it only says:
> >
> 
> This is the problem with citing the bspec: it changes.  If you follow that
> path in the PRMs, you should get to a useful section.
> 
> 
> > 1D surfaces are not tiled, but laid out linearly in memory.
> >
> > Tiled Resource Mode     Bits per Element        Horizontal Alignment
> > TRMODE_NONE             Any                     64
> >
> > > +          */
> > > +         logical_el = (struct isl_extent4d) {
> > > +            .w = 1 << (12 - (ffs(format_bpb) - 4) + (4 * is_Ys)),
> > > +            .h = 1,
> > > +            .d = 1,
> > > +            .a = 1,
> > > +         };
> > > +         break;
> > > +
> > > +      case ISL_SURF_DIM_2D:
> > > +         /* See the Skylake BSpec > Memory Views > Common Surface
> > Formats >
> > > +          * Surface Layout and Tiling > 2D Surfaces > 2D/CUBE Alignment
> > > +          * Requirements.
> > > +          */
> > > +         logical_el = (struct isl_extent4d) {
> > > +            .w = 1 << (6 - ((ffs(format_bpb) - 4) / 2) + (2 * is_Ys)),
> > > +            .h = 1 << (6 - ((ffs(format_bpb) - 3) / 2) + (2 * is_Ys)),
> > > +            .d = 1,
> > > +            .a = 1,
> > > +         };
> >
> > In case of section "2D/CUBE Alignment" I'm having similar problem - there
> > is
> > only a simple table. The equations above, however, suggest that there is
> > more
> > to it.
> >
> 
> There are tables in the PRM.  For tile size calculations, there are these
> fairly simple (Ha!) closed-form calculations.  The unit tests below contain
> the actual tables and test that the calculations above match exactly.

Ok, PRM indeed contains the missing bits. I went trhu the math and also
double checked the unit tests:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> 
> 
> > > +
> > > +         if (is_Ys && samples > 1) {
> > > +            logical_el.w >>= (ffs(samples) - 0) / 2;
> > > +            logical_el.h >>= (ffs(samples) - 1) / 2;
> > > +            logical_el.a = samples;
> > > +         }
> > > +         break;
> > > +
> > > +      case ISL_SURF_DIM_3D:
> > > +         /* See the Skylake BSpec > Memory Views > Common Surface
> > Formats >
> > > +          * Surface Layout and Tiling > 3D Surfaces > 3D Alignment
> > > +          * Requirements.
> > > +          */
> > > +         logical_el = (struct isl_extent4d) {
> > > +            .w = 1 << (4 - ((ffs(format_bpb) - 2) / 3) + (2 * is_Ys)),
> > > +            .h = 1 << (4 - ((ffs(format_bpb) - 4) / 3) + (1 * is_Ys)),
> > > +            .d = 1 << (4 - ((ffs(format_bpb) - 3) / 3) + (1 * is_Ys)),
> > > +            .a = 1,
> > > +         };
> > > +         break;
> > > +      }
> > > +
> > > +      uint32_t tile_size_B = is_Ys ? (1 << 16) : (1 << 12);
> > >
> > > -      logical_el = isl_extent4d(width / bs, height, 1, 1);
> > > -      phys_B = isl_extent2d(width, height);
> > > +      phys_B.w = logical_el.width * bs;
> > > +      phys_B.h = tile_size_B / phys_B.w;
> > >        break;
> > >     }
> > >
> > > diff --git a/src/intel/isl/meson.build b/src/intel/isl/meson.build
> > > index 36b8b8f..ad0d5cc 100644
> > > --- a/src/intel/isl/meson.build
> > > +++ b/src/intel/isl/meson.build
> > > @@ -98,4 +98,15 @@ if with_tests
> > >        link_with : [libisl, libintel_common],
> > >      )
> > >    )
> > > +
> > > +  test(
> > > +    'isl_tile_std_y',
> > > +    executable(
> > > +      'isl_tile_std_y_test',
> > > +      'tests/isl_tile_std_y_test.c',
> > > +      dependencies : dep_m,
> > > +      include_directories : [inc_common, inc_intel],
> > > +      link_with : [libisl, libintel_common],
> > > +    )
> > > +  )
> > >  endif
> > > diff --git a/src/intel/isl/tests/isl_tile_std_y_test.c
> > b/src/intel/isl/tests/isl_tile_std_y_test.c
> > > new file mode 100644
> > > index 0000000..25053c6
> > > --- /dev/null
> > > +++ b/src/intel/isl/tests/isl_tile_std_y_test.c
> > > @@ -0,0 +1,160 @@
> > > +/*
> > > + * Copyright 2018 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > > + * copy of this software and associated documentation files (the
> > "Software"),
> > > + * to deal in the Software without restriction, including without
> > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the
> > next
> > > + * paragraph) shall be included in all copies or substantial portions
> > of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > + * DEALINGS IN THE SOFTWARE.
> > > + */
> > > +
> > > +#include <assert.h>
> > > +#include <stdbool.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +
> > > +#include "isl/isl.h"
> > > +
> > > +// An asssert that works regardless of NDEBUG.
> > > +#define t_assert(cond) \
> > > +   do { \
> > > +      if (!(cond)) { \
> > > +         fprintf(stderr, "%s:%d: assertion failed\n", __FILE__,
> > __LINE__); \
> > > +         abort(); \
> > > +      } \
> > > +   } while (0)
> > > +
> > > +static void
> > > +assert_tile_size(enum isl_tiling tiling, enum isl_surf_dim dim,
> > > +                 uint32_t bpb, uint32_t samples,
> > > +                 uint32_t w, uint32_t h, uint32_t d, uint32_t a)
> > > +{
> > > +   struct isl_tile_info tile_info;
> > > +   isl_tiling_get_info(tiling, dim, bpb, samples, &tile_info);
> > > +
> > > +   /* Sanity */
> > > +   t_assert(tile_info.tiling == tiling);
> > > +   t_assert(tile_info.format_bpb == bpb);
> > > +
> > > +   t_assert(tile_info.logical_extent_el.w == w);
> > > +   t_assert(tile_info.logical_extent_el.h == h);
> > > +   t_assert(tile_info.logical_extent_el.d == d);
> > > +   t_assert(tile_info.logical_extent_el.a == a);
> > > +
> > > +   uint32_t tile_size = (tiling == ISL_TILING_Ys) ? 64 * 1024 : 4 *
> > 1024;
> > > +
> > > +   assert(tile_size == tile_info.phys_extent_B.w *
> > > +                       tile_info.phys_extent_B.h);
> > > +
> > > +   assert(tile_size == tile_info.logical_extent_el.w *
> > > +                       tile_info.logical_extent_el.h *
> > > +                       tile_info.logical_extent_el.d *
> > > +                       tile_info.logical_extent_el.a *
> > > +                       bpb / 8);
> > > +}
> > > +
> > > +static void
> > > +test_1d_tile_dimensions()
> > > +{
> > > +#define ASSERT_1D(tiling, bpb, alignment) \
> > > +   assert_tile_size(tiling, ISL_SURF_DIM_1D, bpb, 1, alignment, 1, 1, 1)
> > > +
> > > +   ASSERT_1D(ISL_TILING_Ys,  128,  4096);
> > > +   ASSERT_1D(ISL_TILING_Ys,   64,  8192);
> > > +   ASSERT_1D(ISL_TILING_Ys,   32, 16384);
> > > +   ASSERT_1D(ISL_TILING_Ys,   16, 32768);
> > > +   ASSERT_1D(ISL_TILING_Ys,    8, 65536);
> > > +
> > > +   ASSERT_1D(ISL_TILING_Yf,  128,  256);
> > > +   ASSERT_1D(ISL_TILING_Yf,   64,  512);
> > > +   ASSERT_1D(ISL_TILING_Yf,   32, 1024);
> > > +   ASSERT_1D(ISL_TILING_Yf,   16, 2048);
> > > +   ASSERT_1D(ISL_TILING_Yf,    8, 4096);
> > > +
> > > +#undef ASSERT_1D
> > > +}
> > > +
> > > +static void
> > > +assert_2d_tile_size(enum isl_tiling tiling, uint32_t bpb,
> > > +                    uint32_t halign, uint32_t valign)
> > > +{
> > > +#define ASSERT_2D(tiling, bpb, samples, w, h, a) \
> > > +   assert_tile_size(tiling, ISL_SURF_DIM_2D, bpb, samples, w, h, 1, a)
> > > +
> > > +   /* Single sampled */
> > > +   ASSERT_2D(tiling, bpb, 1, halign, valign, 1);
> > > +
> > > +   /* Multisampled */
> > > +   if (tiling == ISL_TILING_Ys) {
> > > +      ASSERT_2D(tiling, bpb,  2, halign / 2, valign / 1,  2);
> > > +      ASSERT_2D(tiling, bpb,  4, halign / 2, valign / 2,  4);
> > > +      ASSERT_2D(tiling, bpb,  8, halign / 4, valign / 2,  8);
> > > +      ASSERT_2D(tiling, bpb, 16, halign / 4, valign / 4, 16);
> > > +   } else {
> > > +      ASSERT_2D(tiling, bpb,  2, halign, valign, 1);
> > > +      ASSERT_2D(tiling, bpb,  4, halign, valign, 1);
> > > +      ASSERT_2D(tiling, bpb,  8, halign, valign, 1);
> > > +      ASSERT_2D(tiling, bpb, 16, halign, valign, 1);
> > > +   }
> > > +
> > > +#undef ASSERT_2D
> > > +}
> > > +
> > > +static void
> > > +test_2d_tile_dimensions()
> > > +{
> > > +   assert_2d_tile_size(ISL_TILING_Ys, 128,  64,  64);
> > > +   assert_2d_tile_size(ISL_TILING_Ys,  64, 128,  64);
> > > +   assert_2d_tile_size(ISL_TILING_Ys,  32, 128, 128);
> > > +   assert_2d_tile_size(ISL_TILING_Ys,  16, 256, 128);
> > > +   assert_2d_tile_size(ISL_TILING_Ys,   8, 256, 256);
> > > +
> > > +   assert_2d_tile_size(ISL_TILING_Yf, 128,  16,  16);
> > > +   assert_2d_tile_size(ISL_TILING_Yf,  64,  32,  16);
> > > +   assert_2d_tile_size(ISL_TILING_Yf,  32,  32,  32);
> > > +   assert_2d_tile_size(ISL_TILING_Yf,  16,  64,  32);
> > > +   assert_2d_tile_size(ISL_TILING_Yf,   8,  64,  64);
> > > +}
> > > +
> > > +static void
> > > +test_3d_tile_dimensions()
> > > +{
> > > +#define ASSERT_3D(tiling, bpb, halign, valign, dalign) \
> > > +   assert_tile_size(tiling, ISL_SURF_DIM_3D, bpb, 1, halign, valign,
> > dalign, 1)
> > > +
> > > +   ASSERT_3D(ISL_TILING_Ys, 128, 16, 16, 16);
> > > +   ASSERT_3D(ISL_TILING_Ys,  64, 32, 16, 16);
> > > +   ASSERT_3D(ISL_TILING_Ys,  32, 32, 32, 16);
> > > +   ASSERT_3D(ISL_TILING_Ys,  16, 32, 32, 32);
> > > +   ASSERT_3D(ISL_TILING_Ys,   8, 64, 32, 32);
> > > +
> > > +   ASSERT_3D(ISL_TILING_Yf, 128,  4,  8,  8);
> > > +   ASSERT_3D(ISL_TILING_Yf,  64,  8,  8,  8);
> > > +   ASSERT_3D(ISL_TILING_Yf,  32,  8, 16,  8);
> > > +   ASSERT_3D(ISL_TILING_Yf,  16,  8, 16, 16);
> > > +   ASSERT_3D(ISL_TILING_Yf,   8, 16, 16, 16);
> > > +
> > > +#undef ASSERT_3D
> > > +}
> > > +
> > > +int main(void)
> > > +{
> > > +   test_1d_tile_dimensions();
> > > +   test_2d_tile_dimensions();
> > > +   test_3d_tile_dimensions();
> > > +
> > > +   return 0;
> > > +}
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >


More information about the mesa-dev mailing list