[Mesa-dev] [PATCH v2 2/6] i965: emit BRW_NEW_AUX_STATE when we allocate aux surfaces

[Jason Ekstrand]

Given the fact that we're whacking BRW_NEW_AUX_USAGE whenever aux_state
changes, I don't think we need this.  First off, for most surfaces, the aux
buffer is created when the surface is created at which point it's never
been referenced with a SURFACE_STATE and we don't need to flag new state.
The only case where it's not allocated immediately is for single-sampled
fast-clears (CCS_D); in that case we do a fast clear and then does an
intel_miptree_set_aux_state so it will get flagged at that point.

On Fri, Sep 15, 2017 at 3:02 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:

> Fixes a regression introduced with b96313c0e1289b296d7, which removed
> BRW_NEW_BLORP for a bunch of SURFACE_STATE setup code, including render
> targets, on the basis that blorp invalidates binding tables but not
> surface states, however, at least on Broadwell, this caused a regression
> in a CTS test, which Ken and Jason tracked down to the fact that we
> are not uploading new render target surface states after allocating
> new CCS_D surfaces for fast clears (which allocation is deferred until
> an actual clear occurs).
>
> The reason this only fails in BDW is that on SKL+ we use CCS_E which
> is allocated up front so it exists in the initial surface state, the
> problem can be reproduced in these platforms too if we use
> INTEL_DEBUG=norcb to force the CCS_D path.
>
> This patch ensures that any time we create a new aux surface we
> flag BRW_NEW_AUX_STATE so we upload new surface state for it. In theory,
> we only need to do this necessarily for CCS_D because it is the only
> kind for which allocation can be deferred, all the others are allocated
> in the initial state so they will always be uploaded, but it is probably
> not a bad idea to flag all of them anyway.
>
> Credit goes to Jason and Ken for figuring out the reason for the
> regression.
>
> Fixes:
> KHR-GL45.transform_feedback.draw_xfb_test
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 32394ca3aa..8a809a7320 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -953,6 +953,8 @@ create_ccs_buf_for_image(struct brw_context *brw,
>     mt->mcs_buf->qpitch = 0;
>     mt->mcs_buf->surf = temp_ccs_surf;
>
> +   brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
> +
>     return true;
>  }
>
> @@ -1731,6 +1733,7 @@ intel_miptree_alloc_mcs(struct brw_context *brw,
>     }
>
>     mt->aux_state = aux_state;
> +   brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
>
>     intel_miptree_init_mcs(brw, mt, 0xFF);
>
> @@ -1780,6 +1783,7 @@ intel_miptree_alloc_ccs(struct brw_context *brw,
>     }
>
>     mt->aux_state = aux_state;
> +   brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
>
>     return true;
>  }
> @@ -1851,6 +1855,7 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
>        intel_miptree_level_enable_hiz(brw, mt, level);
>
>     mt->aux_state = aux_state;
> +   brw->ctx.NewDriverState |= BRW_NEW_AUX_STATE;
>
>     return true;
>  }
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170915/ea6acc0f/attachment.html>