[Mesa-dev] [PATCH v2 1/2] isl: introduce depth pitch query function

Lionel Landwerlin lionel.g.landwerlin at intel.com
Fri Dec 9 16:45:15 UTC 2016


On 08/12/16 19:19, Jason Ekstrand wrote:
> On Dec 8, 2016 8:48 AM, "Lionel Landwerlin" <llandwerlin at gmail.com 
> <mailto:llandwerlin at gmail.com>> wrote:
>
>     v2: add lod level argument (Jason)
>         return 0 for any lod level > 0 (Jason)
>         return 0 for any surface not 3D (Jason)
>
>
> I'd rather have ISL assert these than just silently return 0.  That 
> way it's clear they make no sense. We can have a dimension check in 
> the Vulkan driver where it calls this function.
>
>     Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com
>     <mailto:lionel.g.landwerlin at intel.com>>
>     ---
>      src/intel/Makefile.isl.am <http://Makefile.isl.am>         |  10 +-
>      src/intel/isl/isl.c                                | 28 +++
>      src/intel/isl/isl.h                                | 11 +
>      src/intel/isl/tests/.gitignore                     |  1 +
>      .../tests/isl_surf_get_image_depth_pitch_test.c | 245
>     +++++++++++++++++++++
>      5 files changed, 294 insertions(+), 1 deletion(-)
>      create mode 100644
>     src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
>
>     diff --git a/src/intel/Makefile.isl.am <http://Makefile.isl.am>
>     b/src/intel/Makefile.isl.am <http://Makefile.isl.am>
>     index 5a317f522b..eb788f4a13 100644
>     --- a/src/intel/Makefile.isl.am <http://Makefile.isl.am>
>     +++ b/src/intel/Makefile.isl.am <http://Makefile.isl.am>
>     @@ -67,10 +67,18 @@ 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_depth_pitch_test \
>     +       isl/tests/isl_surf_get_image_offset_test
>
>      TESTS += $(check_PROGRAMS)
>
>     +isl_tests_isl_surf_get_image_depth_pitch_test_LDADD = \
>     +       common/libintel_common.la <http://libintel_common.la> \
>     +       isl/libisl.la <http://libisl.la> \
>     +     
>      $(top_builddir)/src/mesa/drivers/dri/i965/libi965_compiler.la
>     <http://libi965_compiler.la> \
>     +       -lm
>     +
>      isl_tests_isl_surf_get_image_offset_test_LDADD = \
>             common/libintel_common.la <http://libintel_common.la> \
>             isl/libisl.la <http://libisl.la> \
>     diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
>     index 82ab68dc65..0d61cd7cdc 100644
>     --- a/src/intel/isl/isl.c
>     +++ b/src/intel/isl/isl.c
>     @@ -1874,3 +1874,31 @@ isl_surf_get_depth_format(const struct
>     isl_device *dev,
>            return 5; /* D16_UNORM */
>         }
>      }
>     +
>     +uint32_t
>     +isl_surf_get_depth_pitch(const struct isl_device *device,
>
>
> Could you please put some units on this function?
>
>     +                         const struct isl_surf *surf,
>     +                         uint32_t level)
>     +{
>     +   switch (surf->dim_layout) {
>     +   case ISL_DIM_LAYOUT_GEN9_1D:
>     +   case ISL_DIM_LAYOUT_GEN4_2D:
>     +      return 0;
>
>
> This isn't right.  On Sky Lake and above, 3D surfaces have the GEN4_2D 
> layout.  The depth pitch does make sense here and it's equal to the 
> array pitch for all miplevels.

So should that return the array pitch for GEN4_2D layout just on Skylake 
or other generations too?

>
>     +   case ISL_DIM_LAYOUT_GEN4_3D: {
>     +      if (level > 0)
>     +         return 0;
>     +
>     +      if (surf->tiling == ISL_TILING_LINEAR)
>     +         return surf->row_pitch * surf->phys_level0_sa.h;
>     +
>     +      struct isl_tile_info tile_info;
>     +      isl_surf_get_tile_info(device, surf, &tile_info);
>     +
>     +      return surf->row_pitch * isl_align(surf->phys_level0_sa.h,
>     +  surf->image_alignment_el.h);
>
>
> This calculation isn't right.  In both cases, it should simply be the 
> height of lod0 aligned to the surface vertical alignment.  It has 
> nothing to do with tiling so fat as I know.
>
>     +      }
>     +   default:
>     +      unreachable("bad isl_dim_layout");
>     +      break;
>     +   }
>     +}
>     diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
>     index 07368f9bcf..7c033f380c 100644
>     --- a/src/intel/isl/isl.h
>     +++ b/src/intel/isl/isl.h
>     @@ -1388,10 +1388,21 @@ isl_surf_get_array_pitch_sa_rows(const
>     struct isl_surf *surf)
>      static inline uint32_t
>      isl_surf_get_array_pitch(const struct isl_surf *surf)
>      {
>     +   if (surf->dim_layout == ISL_DIM_LAYOUT_GEN4_3D)
>     +      return 0;
>         return isl_surf_get_array_pitch_sa_rows(surf) * surf->row_pitch;
>      }
>
>      /**
>     + * Pitch between depth slices, in bytes (for 2D images, this
>     should be
>     + * equivalent to isl_surf_get_array_pitch()).
>     + */
>     +uint32_t
>     +isl_surf_get_depth_pitch(const struct isl_device *device,
>     +                         const struct isl_surf *surf,
>     +                         uint32_t level);
>     +
>     +/**
>       * Calculate the offset, in units of surface samples, to a
>     subimage in the
>       * surface.
>       *
>     diff --git a/src/intel/isl/tests/.gitignore
>     b/src/intel/isl/tests/.gitignore
>     index ba70ecfbee..e90b4a4a97 100644
>     --- a/src/intel/isl/tests/.gitignore
>     +++ b/src/intel/isl/tests/.gitignore
>     @@ -1 +1,2 @@
>     +/isl_surf_get_image_depth_pitch_test
>      /isl_surf_get_image_offset_test
>     diff --git
>     a/src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
>     b/src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
>     new file mode 100644
>     index 0000000000..d59ad9b2a9
>     --- /dev/null
>     +++ b/src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
>     @@ -0,0 +1,245 @@
>     +/*
>     + * Copyright 2016 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 "common/gen_device_info.h"
>     +#include "isl/isl.h"
>     +#include "isl/isl_priv.h"
>     +
>     +#define BDW_GT2_DEVID 0x161a
>     +
>     +// 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
>     +test_bdw_2d_r8g8b8a8_unorm_42x43_levels09_tiley0(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_2D,
>     +                      .format = ISL_FORMAT_R8G8B8A8_UNORM,
>     +                      .width = 42,
>     +                      .height = 43,
>     +                      .depth = 1,
>     +                      .levels = 9,
>     +                      .array_len = 1,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_Y0_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch == 0);
>     +}
>     +
>     +static void
>     +test_bdw_3d_r8g8b8a8_unorm_256x256x256_levels09_tiley0(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_3D,
>     +                      .format = ISL_FORMAT_R8G8B8A8_UNORM,
>     +                      .width = 256,
>     +                      .height = 256,
>     +                      .depth = 256,
>     +                      .levels = 9,
>     +                      .array_len = 1,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_Y0_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch >= 256 * 256 * 4);
>     +   t_assert(depth_pitch % 4096 == 0);
>     +}
>     +
>     +static void
>     +test_bdw_3d_r8g8b8a8_unorm_256x256x256_levels09_linear(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_3D,
>     +                      .format = ISL_FORMAT_R8G8B8A8_UNORM,
>     +                      .width = 256,
>     +                      .height = 256,
>     +                      .depth = 256,
>     +                      .levels = 1,
>     +                      .array_len = 1,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_LINEAR_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch >= 256 * 256 * 4);
>     +}
>     +
>     +static void
>     +test_bdw_3d_r8g8b8_unorm_256x256x256_levels09_linear(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_3D,
>     +                      .format = ISL_FORMAT_R8G8B8_UNORM,
>     +                      .width = 256,
>     +                      .height = 256,
>     +                      .depth = 256,
>     +                      .levels = 1,
>     +                      .array_len = 1,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_LINEAR_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch >= 256 * 256 * 3);
>     +}
>     +
>     +static void
>     +test_bdw_2d_r8g8b8a8_unorm_256x256x256_levels09_linear(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_2D,
>     +                      .format = ISL_FORMAT_R8G8B8A8_UNORM,
>     +                      .width = 256,
>     +                      .height = 256,
>     +                      .depth = 1,
>     +                      .levels = 1,
>     +                      .array_len = 256,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_LINEAR_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch == 0);
>     +}
>     +
>     +static void
>     +test_bdw_1d_r8g8b8a8_unorm_256_levels09_linear(void)
>     +{
>     +   bool ok;
>     +
>     +   struct gen_device_info devinfo;
>     +   t_assert(gen_get_device_info(BDW_GT2_DEVID, &devinfo));
>     +
>     +   struct isl_device dev;
>     +   isl_device_init(&dev, &devinfo, /*bit6_swizzle*/ false);
>     +
>     +   struct isl_surf surf;
>     +   ok = isl_surf_init(&dev, &surf,
>     +                      .dim = ISL_SURF_DIM_1D,
>     +                      .format = ISL_FORMAT_R8G8B8A8_UNORM,
>     +                      .width = 256,
>     +                      .height = 1,
>     +                      .depth = 1,
>     +                      .levels = 1,
>     +                      .array_len = 1,
>     +                      .samples = 1,
>     +                      .usage = ISL_SURF_USAGE_TEXTURE_BIT |
>     +  ISL_SURF_USAGE_DISABLE_AUX_BIT,
>     +                      .tiling_flags = ISL_TILING_LINEAR_BIT);
>     +   t_assert(ok);
>     +
>     +   uint32_t depth_pitch =
>     +      isl_surf_get_depth_pitch(&dev, &surf, 0);
>     +
>     +   t_assert(depth_pitch == 0);
>     +}
>     +
>     +int main(void)
>     +{
>     +   test_bdw_2d_r8g8b8a8_unorm_42x43_levels09_tiley0();
>     +   test_bdw_3d_r8g8b8a8_unorm_256x256x256_levels09_tiley0();
>     +   test_bdw_3d_r8g8b8a8_unorm_256x256x256_levels09_linear();
>     +   test_bdw_3d_r8g8b8_unorm_256x256x256_levels09_linear();
>     +   test_bdw_2d_r8g8b8a8_unorm_256x256x256_levels09_linear();
>     +   test_bdw_1d_r8g8b8a8_unorm_256_levels09_linear();
>     +}
>     --
>     2.11.0
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161209/a4f7372e/attachment-0001.html>


More information about the mesa-dev mailing list