[Mesa-dev] [PATCH v3] i965/skl: Fix the qpitch value

[Anuj Phogat]

On Fri, Mar 13, 2015 at 9:29 AM, Neil Roberts <neil at linux.intel.com> wrote:
> On Skylake the qpitch value is uploaded as part of the surface state
> so we don't need to add the extra rows that are done for other
> generations. However for 3D textures it needs to be aligned to the
> tile height and for depth/stencil textures it needs to be a multiple
> of 8. Unlike previous generations the qpitch is measured as a multiple
> of the block size for compressed surfaces. When the horizontal mipmap
> layout is used for 1D textures then the qpitch is measured in pixels
> instead of rows.
>
> v2: Align the depth/stencil textures to a multiple of 8
> v3: Add an assert that ALL_SLICES_AT_EACH_LOD is not used. Ignore the
>     vertical alignment when picking the qpitch for 1D_ARRAY textures.
>
> ---
>
> Here's a v3 which adds an assert about never using
> ALL_SLICES_AT_EACH_LOAD. I've also squashed in the patch about
> ignoring the vertical alignment for 1D_ARRAY textures.
>
>  src/mesa/drivers/dri/i965/brw_tex_layout.c    | 62 ++++++++++++++++++++++-----
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++--
>  2 files changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 5e70cd2..a67bdd6 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -300,24 +300,66 @@ static void
>  brw_miptree_layout_texture_array(struct brw_context *brw,
>                                  struct intel_mipmap_tree *mt)
>  {
> -   int h0, h1;
>     unsigned height = mt->physical_height0;
>     bool layout_1d = use_linear_1d_layout(brw, mt);
> -
> -   h0 = ALIGN(mt->physical_height0, mt->align_h);
> -   h1 = ALIGN(minify(mt->physical_height0, 1), mt->align_h);
> -   if (mt->array_layout == ALL_SLICES_AT_EACH_LOD)
> -      mt->qpitch = h0;
> -   else
> -      mt->qpitch = (h0 + h1 + (brw->gen >= 7 ? 12 : 11) * mt->align_h);
> -
> -   int physical_qpitch = mt->compressed ? mt->qpitch / 4 : mt->qpitch;
> +   int physical_qpitch;
>
>     if (layout_1d)
>        gen9_miptree_layout_1d(mt);
>     else
>        brw_miptree_layout_2d(mt);
>
> +   if (layout_1d) {
> +      physical_qpitch = 1;
> +      /* When using the horizontal layout the qpitch specifies the distance in
> +       * pixels between array slices. The total_width is forced to be a
> +       * multiple of the horizontal alignment in brw_miptree_layout_1d (in
> +       * this case it's always 64). The vertical alignment is ignored.
> +       */
> +      mt->qpitch = mt->total_width;
> +   } else if (brw->gen >= 9) {
> +      GLenum base_format;
> +
> +      /* ALL_SLICES_AT_EACH_LOD isn't supported on Gen8+ but this code will
> +       * effectively end up with a packed qpitch anyway whenever
> +       * mt->first_level == mt->last_level.
> +       */
> +      assert(mt->msaa_layout != ALL_SLICES_AT_EACH_LOD);
mt->array_layout != ALL_SLICES_AT_EACH_LOD

> +
> +      /* On Gen9 we can pick whatever qpitch we like as long as it's aligned
> +       * to the vertical alignment so we don't need to add any extra rows.
> +       */
> +      mt->qpitch = mt->total_height;
> +
> +      /* If the surface might be used as a stencil buffer or HiZ buffer then
> +       * it needs to be a multiple of 8.
> +       */
> +      base_format = _mesa_get_format_base_format(mt->format);
> +      if (_mesa_is_depth_or_stencil_format(base_format))
> +         mt->qpitch = ALIGN(mt->qpitch, 8);
> +
> +      /* 3D textures need to be aligned to the tile height. At this point we
> +       * don't know which tiling will be used so let's just align it to 32
> +       */
> +      if (mt->target == GL_TEXTURE_3D)
> +         mt->qpitch = ALIGN(mt->qpitch, 32);
> +
> +      /* Unlike previous generations the qpitch is now a multiple of the
> +       * compressed block size so physical_qpitch matches mt->qpitch.
> +       */
> +      physical_qpitch = mt->qpitch;
> +   } else {
> +      int h0 = ALIGN(mt->physical_height0, mt->align_h);
> +      int h1 = ALIGN(minify(mt->physical_height0, 1), mt->align_h);
> +
> +      if (mt->array_layout == ALL_SLICES_AT_EACH_LOD)
> +         mt->qpitch = h0;
> +      else
> +         mt->qpitch = (h0 + h1 + (brw->gen >= 7 ? 12 : 11) * mt->align_h);
> +
> +      physical_qpitch = mt->compressed ? mt->qpitch / 4 : mt->qpitch;
> +   }
> +
>     for (unsigned level = mt->first_level; level <= mt->last_level; level++) {
>        unsigned img_height;
>        img_height = ALIGN(height, mt->align_h);
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 41b6036..158f0eb 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -403,10 +403,14 @@ struct intel_mipmap_tree
>     enum miptree_array_layout array_layout;
>
>     /**
> -    * The distance in rows between array slices in an uncompressed surface.
> +    * The distance in between array slices.
>      *
> -    * For compressed surfaces, slices are stored closer together physically;
> -    * the real distance is (qpitch / block height).
> +    * The value is the one that is sent in the surface state. The actual
> +    * meaning depends on certain criteria. Usually it is simply the number of
> +    * uncompressed rows between each slice. However on Gen9+ for compressed
> +    * surfaces it is the number of blocks. For 1D array surfaces that have the
> +    * mipmap tree stored horizontally it is the number of pixels between each
> +    * slice.
>      */
>     uint32_t qpitch;
>
> --
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

With above comment fixed:
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>