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.pohjolai...@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.pohjolai...@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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
