The next revision will be all code refactoring. I moved the spec
references into the new top-level function
intel_get_texture_alignment_unit because some of the alignment
calculations occurring in the top-level function reference the spec.
Since the intel_vertical_texture_alignment_unit and
intel_horizontal_texture_alignment_unit are called from within the
top-level (and only within it), it would be assumed that the
references also apply to these functions.
On Tue, Jun 9, 2015 at 12:13 PM, Ian Romanick <i...@freedesktop.org> wrote:
> 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
