[Mesa-dev] [PATCH 6/6] i965/gen8+: Add aux buffer alignment assertions

[Chad Versace]

On Thu 28 May 2015, Ben Widawsky wrote:
> This helped find the incorrect HALIGN values from the previous patches.
> 
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  src/mesa/drivers/dri/i965/gen8_surface_state.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 672fc70..c8965db 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -178,6 +178,8 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
>     if (mt->mcs_mt) {
>        aux_mt = mt->mcs_mt;
>        aux_mode = GEN8_SURFACE_AUX_MODE_MCS;
> +      assert(brw->gen < 9 || mt->align_w == 16);
> +      assert(brw->gen < 8 || mt->num_samples > 0 || mt->align_w == 16);

When I saw the gen8 assertion, I was unsure if you were asserting that
the miptree satisfied a purely software expectation or that the miptree
also satifisfied a hardware requirement. I'd like to see a PRM quote
here to make it clear:

   The Broadwell PRM, RENDER_SURFACE_STATE.SurfaceHorizontalAlignment,
   says "When MCS is enabled for non-MSRT, HALIGN_16 must be used".

>     uint32_t *surf = allocate_surface_state(brw, surf_offset, surf_index);
> @@ -391,6 +393,8 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
>     if (mt->mcs_mt) {
>        aux_mt = mt->mcs_mt;
>        aux_mode = GEN8_SURFACE_AUX_MODE_MCS;
> +      assert(brw->gen < 9 || mt->align_w == 16);
> +      assert(brw->gen < 8 || mt->num_samples > 0 || mt->align_w == 16);
>     }

Same here.

With some PRM quotes sprinkled in, this patch is
Reviewed-by: Chad Versace <chad.versace at intel.com>