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

2016-12-06 Thread Jason Ekstrand 
On Tue, Dec 6, 2016 at 3:05 AM, Lionel Landwerlin 
wrote:

> Signed-off-by: Lionel Landwerlin 
> ---
>  src/intel/Makefile.isl.am  |  10 +-
>  src/intel/isl/isl.c|  25 +++
>  src/intel/isl/isl.h|   8 +
>  src/intel/isl/tests/.gitignore |   1 +
>  .../tests/isl_surf_get_image_depth_pitch_test.c| 248
> +
>  5 files changed, 291 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 b/src/intel/Makefile.isl.am
> index 5a317f522b..eb788f4a13 100644
> --- a/src/intel/Makefile.isl.am
> +++ b/src/intel/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 \
> +   isl/libisl.la \
> +   $(top_builddir)/src/mesa/drivers/dri/i965/libi965_compiler.la \
> +   -lm
> +
>  isl_tests_isl_surf_get_image_offset_test_LDADD = \
> common/libintel_common.la \
> isl/libisl.la \
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 82ab68dc65..8c3aed92ac 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -1874,3 +1874,28 @@ 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,
> + const struct isl_surf *surf)
> +{
> +   switch (surf->dim_layout) {
> +   case ISL_DIM_LAYOUT_GEN9_1D:
> +   case ISL_DIM_LAYOUT_GEN4_2D:
> +  return isl_surf_get_array_pitch(surf);
> +   case ISL_DIM_LAYOUT_GEN4_3D: {
> +  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, _info);
> +
> +  return isl_align(surf->row_pitch * surf->phys_level0_sa.h,
> +   tile_info.phys_extent_B.width *
> +   tile_info.phys_extent_B.height);
> +  }
>

Sorry, but this still isn't correct :/  What you really want is to align it
to the surf->image_align_el.h

What is the motivation for this query?  To claim to have a useful depth
pitch is 80% a lie on Broadwell and earlier so I'd prefer to not encourage
apps with one.  At the very least, this function should take a LOD and
assert that it's 0 for GEN4_3D surfaces so that users of ISL don't think
that it returns a useful value on older hardware.


> +   default:
> +  unreachable("bad isl_dim_layout");
> +  break;
> +   }
> +}
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 07368f9bcf..3ec76fabc9 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -1392,6 +1392,14 @@ isl_surf_get_array_pitch(const struct isl_surf
> *surf)
>  }
>
>  /**
> + * 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);
> +
> +/**
>   * 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 00..725d1e2b48
> --- /dev/null
> +++ b/src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
> @@ -0,0 +1,248 @@
> +/*
> + * 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
> + *

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

2016-12-06 Thread Lionel Landwerlin 
Signed-off-by: Lionel Landwerlin 
---
 src/intel/Makefile.isl.am  |  10 +-
 src/intel/isl/isl.c|  25 +++
 src/intel/isl/isl.h|   8 +
 src/intel/isl/tests/.gitignore |   1 +
 .../tests/isl_surf_get_image_depth_pitch_test.c| 248 +
 5 files changed, 291 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 b/src/intel/Makefile.isl.am
index 5a317f522b..eb788f4a13 100644
--- a/src/intel/Makefile.isl.am
+++ b/src/intel/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 \
+   isl/libisl.la \
+   $(top_builddir)/src/mesa/drivers/dri/i965/libi965_compiler.la \
+   -lm
+
 isl_tests_isl_surf_get_image_offset_test_LDADD = \
common/libintel_common.la \
isl/libisl.la \
diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 82ab68dc65..8c3aed92ac 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -1874,3 +1874,28 @@ 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,
+ const struct isl_surf *surf)
+{
+   switch (surf->dim_layout) {
+   case ISL_DIM_LAYOUT_GEN9_1D:
+   case ISL_DIM_LAYOUT_GEN4_2D:
+  return isl_surf_get_array_pitch(surf);
+   case ISL_DIM_LAYOUT_GEN4_3D: {
+  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, _info);
+
+  return isl_align(surf->row_pitch * surf->phys_level0_sa.h,
+   tile_info.phys_extent_B.width *
+   tile_info.phys_extent_B.height);
+  }
+   default:
+  unreachable("bad isl_dim_layout");
+  break;
+   }
+}
diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index 07368f9bcf..3ec76fabc9 100644
--- a/src/intel/isl/isl.h
+++ b/src/intel/isl/isl.h
@@ -1392,6 +1392,14 @@ isl_surf_get_array_pitch(const struct isl_surf *surf)
 }
 
 /**
+ * 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);
+
+/**
  * 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 00..725d1e2b48
--- /dev/null
+++ b/src/intel/isl/tests/isl_surf_get_image_depth_pitch_test.c
@@ -0,0 +1,248 @@
+/*
+ * 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 
+#include 
+#include 
+#include 
+
+#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