On 06/01/2015 10:13 AM, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.ch...@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.ch...@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;
> }
> }
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
