On Mon, Nov 27, 2017 at 07:06:19PM -0800, Jason Ekstrand wrote:
> Even though the blorp pass looks a bit on the sketchy side, the end
> result in the Vulkan driver is very nice.  Instead of having this weird
> case where you do a fast clear and then maybe have to resolve, we just
> do the ambiguate and are done with it.  The ambiguate does exactly what
> we want of setting all the CCS values to 0 which puts it inot the
> pass-through state.

For me there wasn't enough context here to understand fully what is going on.
Looking in a tree made it clearer why we don't need to bother about the
current color values themselves.

Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>

> ---
>  src/intel/vulkan/anv_blorp.c       |  5 +++++
>  src/intel/vulkan/genX_cmd_buffer.c | 40 
> ++------------------------------------
>  2 files changed, 7 insertions(+), 38 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 45d7b12..84ac720 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1705,6 +1705,11 @@ anv_image_ccs_op(struct anv_cmd_buffer *cmd_buffer,
>                          surf.surf->format, 
> isl_to_blorp_fast_clear_op(ccs_op));
>        break;
>     case ISL_AUX_OP_AMBIGUATE:
> +      for (uint32_t a = 0; a < layer_count; a++) {
> +         const uint32_t layer = base_layer + a;
> +         blorp_ccs_ambiguate(&batch, &surf, level, layer);
> +      }
> +      break;
>     default:
>        unreachable("Unsupported CCS operation");
>     }
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index dcd5a8f..e8a4d90 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -614,17 +614,7 @@ init_fast_clear_state_entry(struct anv_cmd_buffer 
> *cmd_buffer,
>     uint32_t plane = anv_image_aspect_to_plane(image->aspects, aspect);
>     enum isl_aux_usage aux_usage = image->planes[plane].aux_usage;
>  
> -   /* The resolve flag should updated to signify that fast-clear/compression
> -    * data needs to be removed when leaving the undefined layout. Such data
> -    * may need to be removed if it would cause accesses to the color buffer
> -    * to return incorrect data. The fast clear data in CCS_D buffers should
> -    * be removed because CCS_D isn't enabled all the time.
> -    */
> -   if (aux_usage == ISL_AUX_USAGE_NONE) {
> -      set_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
> -   } else {
> -      clear_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
> -   }
> +   clear_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
>  
>     /* The fast clear value dword(s) will be copied into a surface state 
> object.
>      * Ensure that the restrictions of the fields in the dword(s) are 
> followed.
> @@ -830,7 +820,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
>                 MIN2(layer_count, anv_image_aux_layers(image, aspect, level));
>              anv_image_ccs_op(cmd_buffer, image, aspect, level,
>                               base_layer, level_layer_count,
> -                             ISL_AUX_OP_FAST_CLEAR, false);
> +                             ISL_AUX_OP_AMBIGUATE, false);
>           }
>        } else if (image->samples > 1) {
>           if (image->samples == 4 || image->samples == 16) {
> @@ -844,32 +834,6 @@ transition_color_buffer(struct anv_cmd_buffer 
> *cmd_buffer,
>                            base_layer, layer_count,
>                            ISL_AUX_OP_FAST_CLEAR, false);
>        }
> -      /* At this point, some elements of the CCS buffer may have the 
> fast-clear
> -       * bit-arrangement. As the user writes to a subresource, we need to 
> have
> -       * the associated CCS elements enter the ambiguated state. This enables
> -       * reads (implicit or explicit) to reflect the user-written data 
> instead
> -       * of the clear color. The only time such elements will not change 
> their
> -       * state as described above, is in a final layout that doesn't have CCS
> -       * enabled. In this case, we must force the associated CCS buffers of 
> the
> -       * specified range to enter the ambiguated state in advance.
> -       */
> -      if (image->samples == 1 &&
> -          image->planes[plane].aux_usage != ISL_AUX_USAGE_CCS_E &&
> -          final_layout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) {
> -         /* The CCS_D buffer may not be enabled in the final layout. Call 
> this
> -          * function again with a initial layout of COLOR_ATTACHMENT_OPTIMAL
> -          * to perform a resolve.
> -          */
> -          anv_perf_warn(cmd_buffer->device->instance, image,
> -                        "Performing an additional resolve for CCS_D layout "
> -                        "transition. Consider always leaving it on or "
> -                        "performing an ambiguation pass.");
> -         transition_color_buffer(cmd_buffer, image, aspect,
> -                                 base_level, level_count,
> -                                 base_layer, layer_count,
> -                                 VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
> -                                 final_layout);
> -      }
>        return;
>     }
>  
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to