[Mesa-dev] [PATCH] intel/blorp: Be more conservative about copying clear colors

[Rafael Antognolli]

On Fri, Jan 04, 2019 at 01:07:07PM -0600, Jason Ekstrand wrote:
> In 92eb5bbc68d7324 we attempted to avoid copying clear colors whenever
> we weren't doing a resolve.  However, this broke MSAA resolves because
> we need the clear color in the source.  This patch makes blorp much more
> conservative such that it only avoids the clear color copy if either
> aux_usage == NONE or it's explicitly doing a fast-clear.

Ah, nice! I think I tried to fix this by not setting the
surface->clear_color_addr.buffer in some cases, but I think it broke
some crucible test.  I don't remember the details of how, though.

Anyway, this looks better, and I assume it doesn't break anything.

Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>

> Fixes: 92eb5bbc68d7 "intel/blorp: Only copy clear color when doing..."
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107728
> Cc: Rafael Antognolli <rafael.antognolli at intel.com>
> ---
>  src/intel/blorp/blorp_genX_exec.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h
> index 9010b03fb67..29afe8ac78b 100644
> --- a/src/intel/blorp/blorp_genX_exec.h
> +++ b/src/intel/blorp/blorp_genX_exec.h
> @@ -1326,7 +1326,7 @@ blorp_emit_memcpy(struct blorp_batch *batch,
>  static void
>  blorp_emit_surface_state(struct blorp_batch *batch,
>                           const struct brw_blorp_surface_info *surface,
> -                         enum isl_aux_op op,
> +                         enum isl_aux_op aux_op,
>                           void *state, uint32_t state_offset,
>                           const bool color_write_disables[4],
>                           bool is_render_target)
> @@ -1382,7 +1382,7 @@ blorp_emit_surface_state(struct blorp_batch *batch,
>                            surface->aux_addr, *aux_addr);
>     }
>  
> -   if (surface->clear_color_addr.buffer) {
> +   if (aux_usage != ISL_AUX_USAGE_NONE && surface->clear_color_addr.buffer) {
>  #if GEN_GEN >= 10
>        assert((surface->clear_color_addr.offset & 0x3f) == 0);
>        uint32_t *clear_addr = state + isl_dev->ss.clear_color_state_offset;
> @@ -1390,7 +1390,10 @@ blorp_emit_surface_state(struct blorp_batch *batch,
>                            isl_dev->ss.clear_color_state_offset,
>                            surface->clear_color_addr, *clear_addr);
>  #elif GEN_GEN >= 7
> -      if (op == ISL_AUX_OP_FULL_RESOLVE || op == ISL_AUX_OP_PARTIAL_RESOLVE) {
> +      /* Fast clears just whack the AUX surface and don't actually use the
> +       * clear color for anything.  We can avoid the MI memcpy on that case.
> +       */
> +      if (aux_op != ISL_AUX_OP_FAST_CLEAR) {
>           struct blorp_address dst_addr = blorp_get_surface_base_address(batch);
>           dst_addr.offset += state_offset + isl_dev->ss.clear_value_offset;
>           blorp_emit_memcpy(batch, dst_addr, surface->clear_color_addr,
> -- 
> 2.20.1
>