[Mesa-dev] [RFC v2 14/15] i965: refactor miptree alignment calculation code

Ian Romanick idr at freedesktop.org
Tue Jun 9 12:13:37 PDT 2015


On 06/01/2015 10:13 AM, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.chery at intel.com>
> 
> - Remove redundant checks and comments by grouping our calculations for
>   align_w and align_h wherever possible.
> - Don't pass more parameters than necessary.
> - Minor code simplifications.
> 
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_tex_layout.c | 181 +++++++++++++----------------
>  1 file changed, 83 insertions(+), 98 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 5aadd00..a54b77d 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -40,16 +40,9 @@
>  #define FILE_DEBUG_FLAG DEBUG_MIPTREE
>  
>  static unsigned int
> -intel_horizontal_texture_alignment_unit(struct brw_context *brw,
> -                                        struct intel_mipmap_tree *mt)
> +intel_horizontal_texture_alignment_unit(int gen, const struct intel_mipmap_tree *mt)
>  {
>     /**
> -    * From the "Alignment Unit Size" section of various specs, namely:
> -    * - Gen3 Spec: "Memory Data Formats" Volume,         Section 1.20.1.4
> -    * - i965 and G45 PRMs:             Volume 1,         Section 6.17.3.4.
> -    * - Ironlake and Sandybridge PRMs: Volume 1, Part 1, Section 7.18.3.4
> -    * - BSpec (for Ivybridge and slight variations in separate stencil)
> -    *

Since the rest of the spec quotation remains, why remove this part?

I also agree with Anuj's comments about splitting this into multiple
patches.

>      * +----------------------------------------------------------------------+
>      * |                                        | alignment unit width  ("i") |
>      * | Surface Property                       |-----------------------------|
> @@ -67,47 +60,19 @@ intel_horizontal_texture_alignment_unit(struct brw_context *brw,
>      * On IVB+, non-special cases can be overridden by setting the SURFACE_STATE
>      * "Surface Horizontal Alignment" field to HALIGN_4 or HALIGN_8.
>      */
> -    if (_mesa_is_format_compressed(mt->format)) {
> -       /* The hardware alignment requirements for compressed textures
> -        * happen to match the block boundaries.
> -        */
> -      unsigned int i, j;
> -      _mesa_get_format_block_size(mt->format, &i, &j);
> -
> -      /* On Gen9+ we can pick our own alignment for compressed textures but it
> -       * has to be a multiple of the block size. The minimum alignment we can
> -       * pick is 4 so we effectively have to align to 4 times the block
> -       * size
> -       */
> -      if (brw->gen >= 9)
> -         return i * 4;
> -      else
> -         return i;
> -    }
> -
> -   if (mt->format == MESA_FORMAT_S_UINT8)
> -      return 8;
> -
> -   if (brw->gen >= 7 && mt->format == MESA_FORMAT_Z_UNORM16)
> +   if (gen >= 7 && mt->format == MESA_FORMAT_Z_UNORM16)
>        return 8;
>  
> -   if (brw->gen == 8 && mt->mcs_mt && mt->num_samples <= 1)
> +   if (gen == 8 && mt->mcs_mt && mt->num_samples <= 1)
>        return 16;
>  
>     return 4;
>  }
>  
>  static unsigned int
> -intel_vertical_texture_alignment_unit(struct brw_context *brw,
> -                                      mesa_format format, bool multisampled)
> +intel_vertical_texture_alignment_unit(int gen, const struct intel_mipmap_tree *mt)
>  {
>     /**
> -    * From the "Alignment Unit Size" section of various specs, namely:
> -    * - Gen3 Spec: "Memory Data Formats" Volume,         Section 1.20.1.4
> -    * - i965 and G45 PRMs:             Volume 1,         Section 6.17.3.4.
> -    * - Ironlake and Sandybridge PRMs: Volume 1, Part 1, Section 7.18.3.4
> -    * - BSpec (for Ivybridge and slight variations in separate stencil)
> -    *
>      * +----------------------------------------------------------------------+
>      * |                                        | alignment unit height ("j") |
>      * | Surface Property                       |-----------------------------|
> @@ -124,34 +89,25 @@ intel_vertical_texture_alignment_unit(struct brw_context *brw,
>      * Where "*" means either VALIGN_2 or VALIGN_4 depending on the setting of
>      * the SURFACE_STATE "Surface Vertical Alignment" field.
>      */
> -   if (_mesa_is_format_compressed(format)) {
> -      unsigned int i, j;
> -      _mesa_get_format_block_size(format, &i, &j);
> -      /* See comment above for the horizontal alignment */
> -      return brw->gen >= 9 ? j * 4 : 4;
> -   }
> -
> -   if (format == MESA_FORMAT_S_UINT8)
> -      return brw->gen >= 7 ? 8 : 4;
>  
>     /* Broadwell only supports VALIGN of 4, 8, and 16.  The BSpec says 4
>      * should always be used, except for stencil buffers, which should be 8.
>      */
> -   if (brw->gen >= 8)
> +   if (gen >= 8)
>        return 4;
>  
> -   if (multisampled)
> +   if (mt->num_samples > 1)
>        return 4;
>  
> -   GLenum base_format = _mesa_get_format_base_format(format);
> +   const GLenum base_format = _mesa_get_format_base_format(mt->format);
>  
> -   if (brw->gen >= 6 &&
> +   if (gen >= 6 &&
>         (base_format == GL_DEPTH_COMPONENT ||
>  	base_format == GL_DEPTH_STENCIL)) {
>        return 4;
>     }
>  
> -   if (brw->gen == 7) {
> +   if (gen == 7) {
>        /* On Gen7, we prefer a vertical alignment of 4 when possible, because
>         * that allows Y tiled render targets.
>         *
> @@ -164,7 +120,7 @@ intel_vertical_texture_alignment_unit(struct brw_context *brw,
>         *
>         *     VALIGN_4 is not supported for surface format R32G32B32_FLOAT.
>         */
> -      if (base_format == GL_YCBCR_MESA || format == MESA_FORMAT_RGB_FLOAT32)
> +      if (base_format == GL_YCBCR_MESA || mt->format == MESA_FORMAT_RGB_FLOAT32)
>           return 2;
>  
>        return 4;
> @@ -174,6 +130,73 @@ intel_vertical_texture_alignment_unit(struct brw_context *brw,
>  }
>  
>  static void
> +intel_get_texture_alignment_unit(int gen, struct intel_mipmap_tree *mt)
> +{
> +   /**
> +    * From the "Alignment Unit Size" section of various specs, namely:
> +    * - Gen3 Spec: "Memory Data Formats" Volume,         Section 1.20.1.4
> +    * - i965 and G45 PRMs:             Volume 1,         Section 6.17.3.4.
> +    * - Ironlake and Sandybridge PRMs: Volume 1, Part 1, Section 7.18.3.4
> +    * - BSpec (for Ivybridge and slight variations in separate stencil)
> +    */
> +   bool gen6_hiz_or_stencil = false;
> +   if (gen == 6 && mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
> +      const GLenum base_format = _mesa_get_format_base_format(mt->format);
> +      gen6_hiz_or_stencil = _mesa_is_depth_or_stencil_format(base_format);
> +   }
> +
> +   if (gen6_hiz_or_stencil) {
> +      /* On gen6, we use ALL_SLICES_AT_EACH_LOD for stencil/hiz because the
> +       * hardware doesn't support multiple mip levels on stencil/hiz.
> +       *
> +       * PRM Vol 2, Part 1, 7.5.3 Hierarchical Depth Buffer:
> +       * "The hierarchical depth buffer does not support the LOD field"
> +       *
> +       * PRM Vol 2, Part 1, 7.5.4.1 Separate Stencil Buffer:
> +       * "The stencil depth buffer does not support the LOD field"
> +       */
> +      if (mt->format == MESA_FORMAT_S_UINT8) {
> +         /* Stencil uses W tiling, so we force W tiling alignment for the
> +          * ALL_SLICES_AT_EACH_LOD miptree layout.
> +          */
> +         mt->align_w = 64;
> +         mt->align_h = 64;
> +      } else {
> +         /* Depth uses Y tiling, so we force need Y tiling alignment for the
> +          * ALL_SLICES_AT_EACH_LOD miptree layout.
> +          */
> +         mt->align_w = 128 / mt->cpp;
> +         mt->align_h = 32;
> +      }
> +   } else if (mt->compressed) {
> +       /* The hardware alignment requirements for compressed textures
> +        * happen to match the block boundaries.
> +        */
> +      unsigned int i, j;
> +      _mesa_get_format_block_size(mt->format, &i, &j);
> +      /* On Gen9+ we can pick our own alignment for compressed textures but it
> +       * has to be a multiple of the block size. The minimum alignment we can
> +       * pick is 4 so we effectively have to align to 4 times the block
> +       * size
> +       */
> +      if (gen >= 9) {
> +         mt->align_w = i * 4;
> +         mt->align_h = j * 4;
> +      } else {
> +         mt->align_w = i;
> +         mt->align_h = 4;
> +      }
> +   } else if (mt->format == MESA_FORMAT_S_UINT8) {
> +      mt->align_w = 8;
> +      mt->align_h = gen >= 7 ? 8 : 4;
> +   } else {
> +      /* align_w and align_h must be determined independently. */
> +      mt->align_w = intel_horizontal_texture_alignment_unit(gen, mt);
> +      mt->align_h = intel_vertical_texture_alignment_unit(gen, mt);
> +   }
> +}
> +
> +static void
>  gen9_miptree_layout_1d(struct intel_mipmap_tree *mt)
>  {
>     unsigned x = 0;
> @@ -465,43 +488,7 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
>  void
>  brw_miptree_layout(struct brw_context *brw, struct intel_mipmap_tree *mt)
>  {
> -   bool multisampled = mt->num_samples > 1;
> -   bool gen6_hiz_or_stencil = false;
> -
> -   if (brw->gen == 6 && mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
> -      const GLenum base_format = _mesa_get_format_base_format(mt->format);
> -      gen6_hiz_or_stencil = _mesa_is_depth_or_stencil_format(base_format);
> -   }
> -
> -   if (gen6_hiz_or_stencil) {
> -      /* On gen6, we use ALL_SLICES_AT_EACH_LOD for stencil/hiz because the
> -       * hardware doesn't support multiple > 
mip levels on stencil/hiz.
> -       *
> -       * PRM Vol 2, Part 1, 7.5.3 Hierarchical Depth Buffer:
> -       * "The hierarchical depth buffer does not support the LOD field"
> -       *
> -       * PRM Vol 2, Part 1, 7.5.4.1 Separate Stencil Buffer:
> -       * "The stencil depth buffer does not support the LOD field"
> -       */
> -      if (mt->format == MESA_FORMAT_S_UINT8) {
> -         /* Stencil uses W tiling, so we force W tiling alignment for the
> -          * ALL_SLICES_AT_EACH_LOD miptree layout.
> -          */
> -         mt->align_w = 64;
> -         mt->align_h = 64;
> -      } else {
> -         /* Depth uses Y tiling, so we force need Y tiling alignment for the
> -          * ALL_SLICES_AT_EACH_LOD miptree layout.
> -          */
> -         mt->align_w = 128 / mt->cpp;
> -         mt->align_h = 32;
> -      }
> -   } else {
> -      mt->align_w = intel_horizontal_texture_alignment_unit(brw, mt);
> -      mt->align_h =
> -         intel_vertical_texture_alignment_unit(brw, mt->format, multisampled);
> -   }
> -
> +   intel_get_texture_alignment_unit(brw->gen, mt);
>     switch (mt->target) {
>     case GL_TEXTURE_CUBE_MAP:
>        if (brw->gen == 4) {
> @@ -547,14 +534,12 @@ brw_miptree_layout(struct brw_context *brw, struct intel_mipmap_tree *mt)
>     DBG("%s: %dx%dx%d\n", __func__,
>         mt->total_width, mt->total_height, mt->cpp);
>  
> -   /* On Gen9+ the alignment values are expressed in multiples of the block
> -    * size
> +   /* On Gen9+ the alignment values of compressed textures are expressed in
> +    * multiples of 4. See: intel_get_texture_alignment_unit()
>      */
> -   if (brw->gen >= 9) {
> -      unsigned int i, j;
> -      _mesa_get_format_block_size(mt->format, &i, &j);
> -      mt->align_w /= i;
> -      mt->align_h /= j;
> +   if (brw->gen >= 9 && mt->compressed) {
> +      mt->align_w = 4;
> +      mt->align_h = 4;
>     }
>  }
>  
> 



More information about the mesa-dev mailing list