<div dir="ltr">On 15 November 2013 19:26, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 11/15/2013 01:18 PM, Paul Berry wrote:<br>
> Gen6+ allows for color buffers to use a vertical alignment of either 4<br>
> or 2.  Previously we defaulted to 2.  This may have caused problems on<br>
> Gen7 because Y-tiled render targets are not allowed to use a vertical<br>
> alignment of 2.<br>
><br>
> This patch changes the vertical alignment to 4 on Gen7, except for the<br>
> few formats where a vertical alignment of 2 is required.<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_tex_layout.c | 25 ++++++++++++++++++++++---<br>
>  1 file changed, 22 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c<br>
> index d05dbeb..2c81eed 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c<br>
> @@ -105,11 +105,11 @@ intel_vertical_texture_alignment_unit(struct brw_context *brw,<br>
>      * | Depth Buffer                           |  2  |  2  |  2  |  4  |  4  |<br>
>      * | Separate Stencil Buffer                | N/A | N/A | N/A |  4  |  8  |<br>
>      * | Multisampled (4x or 8x) render target  | N/A | N/A | N/A |  4  |  4  |<br>
> -    * | All Others                             |  2  |  2  |  2  |  2  |  2  |<br>
> +    * | All Others                             |  2  |  2  |  2  |  *  |  *  |<br>
>      * +----------------------------------------------------------------------+<br>
>      *<br>
> -    * On SNB+, non-special cases can be overridden by setting the SURFACE_STATE<br>
> -    * "Surface Vertical Alignment" field to VALIGN_2 or VALIGN_4.<br>
> +    * Where "*" means either VALIGN_2 or VALIGN_4 depending on the setting of<br>
> +    * the SURFACE_STATE "Surface Vertical Alignment" field.<br>
>      */<br>
>     if (_mesa_is_format_compressed(format))<br>
>        return 4;<br>
> @@ -128,6 +128,25 @@ intel_vertical_texture_alignment_unit(struct brw_context *brw,<br>
>        return 4;<br>
>     }<br>
><br>
> +   if (brw->gen == 7) {<br>
> +      /* On Gen7, we prefer a vertical alignment of 4 when possible, because<br>
> +       * that allows Y tiled render targets.<br>
> +       *<br>
> +       * From the Ivy Bridge PRM, Vol4 Part1 2.12.2.1 (SURFACE_STATE for most<br>
> +       * messages), on p64, under the heading "Surface Vertical Alignment":<br>
> +       *<br>
> +       *     Value of 1 [VALIGN_4] is not supported for format YCRCB_NORMAL<br>
> +       *     (0x182), YCRCB_SWAPUVY (0x183), YCRCB_SWAPUV (0x18f), YCRCB_SWAPY<br>
> +       *     (0x190)<br>
> +       *<br>
> +       *     VALIGN_4 is not supported for surface format R32G32B32_FLOAT.<br>
> +       */<br>
> +      if (base_format == GL_YCBCR_MESA || format == MESA_FORMAT_RGB_FLOAT32)<br>
> +         return 2;<br>
> +<br>
> +      return 4;<br>
> +   }<br>
> +<br>
>     return 2;<br>
>  }<br>
<br>
</div></div>We use Y-tiling on Gen6 as well.<br>
<br>
</blockquote></div><br></div><div class="gmail_extra">Yes, but the restriction that Y-tiled render targets must use a vertical alignment of 4 is not present on Gen6, so there would be no benefit to applying this logic to Gen6.<br>
</div></div>