[Mesa-dev] [v2 21/39] intel/isl/gen6/hack: Use hiz vertical alignment of 16 instead of 8

[Jason Ekstrand]

On Wed, May 3, 2017 at 2:22 AM, Topi Pohjolainen <topi.pohjolainen at gmail.com
> wrote:

> Looking PRMs (SNB, IVB) it also looks to me that the height of hiz
> buffer would need to be half the height of depth. How this is taken
> into account in i965 legacy or isl is unclear to me also.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/intel/isl/isl_gen6.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/isl/isl_gen6.c b/src/intel/isl/isl_gen6.c
> index 19430e9..c78a558 100644
> --- a/src/intel/isl/isl_gen6.c
> +++ b/src/intel/isl/isl_gen6.c
> @@ -146,6 +146,39 @@ isl_gen6_choose_image_alignment_el(const struct
> isl_device *dev,
>     *image_align_el = isl_extent3d(4, 2, 1);
>  }
>
> +static uint32_t
> +get_valign(const struct isl_extent3d *image_align_sa, enum isl_tiling
> tiling)
>

Why are you making this an override function?  Why not just adjust
isl_choose_image_alignment_el in isl.c to return isl_extent3d(2, 4, 1) on
gen6?


> +{
> +   assert(tiling == ISL_TILING_W || tiling == ISL_TILING_HIZ);
> +
> +   if (tiling == ISL_TILING_W)
> +      return image_align_sa->h;
> +
> +   /* Using simply:
> +    *    array_pitch = d * isl_align_npot(h, image_align_sa.h);
> +    *
> +    * results into individual slices of array to be aligned by 8
> +    * (image_align_sa.h) rows. In practise, this upsets piglit test:
> +    *    tex-miplevel-selection "texture()" 2DArrayShadow -auto -fbo
> +    *
> +    * Using 16 instead seems to work.
> +    *
> +    * From the Sandrybridge PRM (2011-05), Volume 2, Part 1, Section 7.5.3
> +    * Hierarchical Depth Buffer:
> +    *
> +    *   ceiling(Z_height / 8) * 4 * Z_Depth
> +    *
> +    * This, however, results into even tighter packing than align by
> eight.
>

It's 8, it's just weird...  The way the docs describe it is as a
translation from Z pixels to bytes where

hiz_width_B = align(Z_width, 16)
hiz_height_rows = align(Z_height, 8) / 2

What the hardware docs don't say is that HiZ surfaces work in 8x4 128bpb
blocks with 16x16 blocks per tile.  This is the way ISL does the
calculations.  With this scheme and an alignment of 2x2 blocks, you get the
same calculation though it's a bit more round-about.  On gen8+, this is
exactly correct to get the HiZ layout including miplevels.  On gen7, HiZ
isn't supported for arrayed or multi-LOD surfaces without craziness.

On gen6... I have no idea.  The docs don't seem to mention qpitch for HiZ
and, to my knowledge, the windows GL driver doesn't support layered
rendering on SNB.  I think it's safe to say that we're breaking new ground
here.  You could probably hack up Chad's HiZ dumping scripts to try and
figure out what the hardware actually does.  Or, we could just go with
something we know works. :-)

+    *
> +    * It should be noted that i965 GL driver used alignment of even
> greater
> +    * alignment of 32 before switching to this here. Also important to
> see is
> +    * that this only affects the amount of padding between arrays of
> slices
> +    * of two subsequent levels. Driver doesn't try to offset to slices
> within
> +    * an array, driver only offsets to levels, i.e., to first slice of
> array.
> +    */
> +   return 16;
> +}
> +
>  void
>  get_image_offset_sa_gen6_back_to_back(const struct isl_surf *surf,
>                                        uint32_t level,
> @@ -154,6 +187,9 @@ get_image_offset_sa_gen6_back_to_back(const struct
> isl_surf *surf,
>                                        uint32_t *x_offset_sa,
>                                        uint32_t *y_offset_sa)
>  {
> +   /* Alignment of individual slices may be broken. */
> +   assert(surf->tiling != ISL_TILING_HIZ ||
> +          (logical_array_layer == 0 && logical_z_offset_px == 0));
>     assert(level < surf->levels);
>
>     const struct isl_extent3d image_align_sa =
> @@ -169,7 +205,7 @@ get_image_offset_sa_gen6_back_to_back(const struct
> isl_surf *surf,
>                           isl_minify(surf->phys_level0_sa.depth, i) :
>                           surf->phys_level0_sa.array_len;
>        const unsigned h = isl_minify(surf->phys_level0_sa.height, i);
> -      y += d * isl_align_npot(h, image_align_sa.h);
> +      y += d * isl_align_npot(h, get_valign(&image_align_sa,
> surf->tiling));
>
>        /* Align on tile boundary so that driver can offset without
> intra-tile
>         * offsets.
> @@ -208,7 +244,7 @@ isl_gen6_calc_back_to_back_total_h(const struct
> isl_extent4d *phys_level0_sa,
>                           isl_minify(phys_level0_sa->depth, i) :
>                           phys_level0_sa->array_len;
>        const unsigned h = isl_minify(phys_level0_sa->height, i);
> -      total_h += d * isl_align_npot(h, image_align_sa->h);
> +      total_h += d * isl_align_npot(h, get_valign(image_align_sa,
> tiling));
>
>        /* Align on tile boundary so that driver can offset without
> intra-tile
>         * offsets.
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170508/a1677438/attachment-0001.html>