[Mesa-dev] [PATCH 1/2] i965/gen7: Prefer vertical alignment of 4 when possible.

[Kenneth Graunke]

On 11/16/2013 12:01 AM, Paul Berry wrote:
> On 15 November 2013 19:26, Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>> wrote:
> 
>     On 11/15/2013 01:18 PM, Paul Berry wrote:
>     > Gen6+ allows for color buffers to use a vertical alignment of either 4
>     > or 2.  Previously we defaulted to 2.  This may have caused problems on
>     > Gen7 because Y-tiled render targets are not allowed to use a vertical
>     > alignment of 2.
>     >
>     > This patch changes the vertical alignment to 4 on Gen7, except for the
>     > few formats where a vertical alignment of 2 is required.
>     > ---
>     >  src/mesa/drivers/dri/i965/brw_tex_layout.c | 25
>     ++++++++++++++++++++++---
>     >  1 file changed, 22 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>     b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>     > index d05dbeb..2c81eed 100644
>     > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>     > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>     > @@ -105,11 +105,11 @@ intel_vertical_texture_alignment_unit(struct
>     brw_context *brw,
>     >      * | Depth Buffer                           |  2  |  2  |  2
>      |  4  |  4  |
>     >      * | Separate Stencil Buffer                | N/A | N/A | N/A
>     |  4  |  8  |
>     >      * | Multisampled (4x or 8x) render target  | N/A | N/A | N/A
>     |  4  |  4  |
>     > -    * | All Others                             |  2  |  2  |  2
>      |  2  |  2  |
>     > +    * | All Others                             |  2  |  2  |  2
>      |  *  |  *  |
>     >      *
>     +----------------------------------------------------------------------+
>     >      *
>     > -    * On SNB+, non-special cases can be overridden by setting the
>     SURFACE_STATE
>     > -    * "Surface Vertical Alignment" field to VALIGN_2 or VALIGN_4.
>     > +    * 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))
>     >        return 4;
>     > @@ -128,6 +128,25 @@ intel_vertical_texture_alignment_unit(struct
>     brw_context *brw,
>     >        return 4;
>     >     }
>     >
>     > +   if (brw->gen == 7) {
>     > +      /* On Gen7, we prefer a vertical alignment of 4 when
>     possible, because
>     > +       * that allows Y tiled render targets.
>     > +       *
>     > +       * From the Ivy Bridge PRM, Vol4 Part1 2.12.2.1
>     (SURFACE_STATE for most
>     > +       * messages), on p64, under the heading "Surface Vertical
>     Alignment":
>     > +       *
>     > +       *     Value of 1 [VALIGN_4] is not supported for format
>     YCRCB_NORMAL
>     > +       *     (0x182), YCRCB_SWAPUVY (0x183), YCRCB_SWAPUV
>     (0x18f), YCRCB_SWAPY
>     > +       *     (0x190)
>     > +       *
>     > +       *     VALIGN_4 is not supported for surface format
>     R32G32B32_FLOAT.
>     > +       */
>     > +      if (base_format == GL_YCBCR_MESA || format ==
>     MESA_FORMAT_RGB_FLOAT32)
>     > +         return 2;
>     > +
>     > +      return 4;
>     > +   }
>     > +
>     >     return 2;
>     >  }
> 
>     We use Y-tiling on Gen6 as well.
> 
> 
> 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.

Huh.  Strange, but you appear to be correct.

Patch 1 is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>