[Mesa-dev] [RFC v2 14/15] i965: refactor miptree alignment calculation code
Anuj Phogat
anuj.phogat at gmail.com
Mon Jun 8 14:46:29 PDT 2015
On Mon, Jun 1, 2015 at 10:13 AM, Nanley Chery <nanleychery at gmail.com> 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)
> - *
> * +----------------------------------------------------------------------+
> * | | 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;
> }
> }
>
> --
> 2.4.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
You're adding functional changes in this patch other than code
refactoring. It will be easier to review if you split up the patch
in two: 1. All code refactoring 2. All functional changes for
compressed textures. FYI, I have upstreamed few patches
touching this file, and few more are going up soon. You'll
need to rebase this patch on top of my changes. Looking at
your patch this should not be a problem. Let me know if you
face any issues.
More information about the mesa-dev
mailing list