[Mesa-dev] [PATCH 23/29] anv/cmd_buffer: Sync clear values in begin_subpass

2017-11-27 Thread Jason Ekstrand
This is quite a bit cleaner because we now sync the clear values at the
same time as we do the fast clear.  For loading the clear values into
the surface state, we now do it once when we handle the LOAD_OP_LOAD
instead of every subpass.
---
 src/intel/vulkan/anv_private.h |   8 ++
 src/intel/vulkan/genX_cmd_buffer.c | 154 +
 2 files changed, 60 insertions(+), 102 deletions(-)

diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index f4b0f90..4137a9a 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2823,6 +2823,14 @@ anv_subpass_view_count(const struct anv_subpass *subpass)
return MAX2(1, _mesa_bitcount(subpass->view_mask));
 }
 
+static inline bool
+anv_subpass_att_is_color(const struct anv_subpass *subpass,
+ const VkAttachmentReference *att)
+{
+   return att >= subpass->color_attachments &&
+  att < subpass->color_attachments + subpass->color_count;
+}
+
 struct anv_render_pass_attachment {
/* TODO: Consider using VkAttachmentDescription instead of storing each of
 * its members individually.
diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index 0915d1a..7901b0c 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -3096,99 +3096,6 @@ cmd_buffer_subpass_transition_layouts(struct 
anv_cmd_buffer * const cmd_buffer,
}
 }
 
-/* Update the clear value dword(s) in surface state objects or the fast clear
- * state buffer entry for the color attachments used in this subpass.
- */
-static void
-cmd_buffer_subpass_sync_fast_clear_values(struct anv_cmd_buffer *cmd_buffer)
-{
-   assert(cmd_buffer && cmd_buffer->state.subpass);
-
-   const struct anv_cmd_state *state = &cmd_buffer->state;
-
-   /* Iterate through every color attachment used in this subpass. */
-   for (uint32_t i = 0; i < state->subpass->color_count; ++i) {
-
-  /* The attachment should be one of the attachments described in the
-   * render pass and used in the subpass.
-   */
-  const uint32_t a = state->subpass->color_attachments[i].attachment;
-  if (a == VK_ATTACHMENT_UNUSED)
- continue;
-
-  assert(a < state->pass->attachment_count);
-
-  /* Store some information regarding this attachment. */
-  const struct anv_attachment_state *att_state = &state->attachments[a];
-  const struct anv_image_view *iview = state->framebuffer->attachments[a];
-  const struct anv_render_pass_attachment *rp_att =
- &state->pass->attachments[a];
-
-  if (att_state->aux_usage == ISL_AUX_USAGE_NONE)
- continue;
-
-  /* The fast clear state entry must be updated if a fast clear is going to
-   * happen. The surface state must be updated if the clear value from a
-   * prior fast clear may be needed.
-   */
-  if (att_state->pending_clear_aspects && att_state->fast_clear) {
- /* Update the fast clear state entry. */
- genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
-  iview->image,
-  VK_IMAGE_ASPECT_COLOR_BIT,
-  iview->planes[0].isl.base_level,
-  true /* copy from ss */);
-
- /* Fast-clears impact whether or not a resolve will be necessary. */
- if (iview->image->planes[0].aux_usage == ISL_AUX_USAGE_CCS_E &&
- att_state->clear_color_is_zero) {
-/* This image always has the auxiliary buffer enabled. We can mark
- * the subresource as not needing a resolve because the clear color
- * will match what's in every RENDER_SURFACE_STATE object when it's
- * being used for sampling.
- */
-clear_image_needs_resolve_bits(cmd_buffer, iview->image,
-   VK_IMAGE_ASPECT_COLOR_BIT,
-   iview->planes[0].isl.base_level,
-   ANV_IMAGE_HAS_FAST_CLEAR_BIT);
- } else {
-set_image_needs_resolve_bits(cmd_buffer, iview->image,
- VK_IMAGE_ASPECT_COLOR_BIT,
- iview->planes[0].isl.base_level,
- ANV_IMAGE_HAS_FAST_CLEAR_BIT);
- }
-  } else if (rp_att->load_op == VK_ATTACHMENT_LOAD_OP_LOAD) {
- /* The attachment may have been fast-cleared in a previous render
-  * pass and the value is needed now. Update the surface state(s).
-  *
-  * TODO: Do this only once per render pass instead of every subpass.
-  */
- genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
-  iview->image,
-  VK_IMAGE_ASPECT_COLOR_BIT,
-  

Re: [Mesa-dev] [PATCH 23/29] anv/cmd_buffer: Sync clear values in begin_subpass

2018-01-12 Thread Pohjolainen, Topi
On Mon, Nov 27, 2017 at 07:06:13PM -0800, Jason Ekstrand wrote:
> This is quite a bit cleaner because we now sync the clear values at the
> same time as we do the fast clear.  For loading the clear values into
> the surface state, we now do it once when we handle the LOAD_OP_LOAD
> instead of every subpass.
> ---
>  src/intel/vulkan/anv_private.h |   8 ++
>  src/intel/vulkan/genX_cmd_buffer.c | 154 
> +
>  2 files changed, 60 insertions(+), 102 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index f4b0f90..4137a9a 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2823,6 +2823,14 @@ anv_subpass_view_count(const struct anv_subpass 
> *subpass)
> return MAX2(1, _mesa_bitcount(subpass->view_mask));
>  }
>  
> +static inline bool
> +anv_subpass_att_is_color(const struct anv_subpass *subpass,
> + const VkAttachmentReference *att)
> +{
> +   return att >= subpass->color_attachments &&
> +  att < subpass->color_attachments + subpass->color_count;
> +}
> +
>  struct anv_render_pass_attachment {
> /* TODO: Consider using VkAttachmentDescription instead of storing each of
>  * its members individually.
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 0915d1a..7901b0c 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3096,99 +3096,6 @@ cmd_buffer_subpass_transition_layouts(struct 
> anv_cmd_buffer * const cmd_buffer,
> }
>  }
>  
> -/* Update the clear value dword(s) in surface state objects or the fast clear
> - * state buffer entry for the color attachments used in this subpass.
> - */
> -static void
> -cmd_buffer_subpass_sync_fast_clear_values(struct anv_cmd_buffer *cmd_buffer)
> -{
> -   assert(cmd_buffer && cmd_buffer->state.subpass);
> -
> -   const struct anv_cmd_state *state = &cmd_buffer->state;
> -
> -   /* Iterate through every color attachment used in this subpass. */
> -   for (uint32_t i = 0; i < state->subpass->color_count; ++i) {
> -
> -  /* The attachment should be one of the attachments described in the
> -   * render pass and used in the subpass.
> -   */
> -  const uint32_t a = state->subpass->color_attachments[i].attachment;
> -  if (a == VK_ATTACHMENT_UNUSED)
> - continue;
> -
> -  assert(a < state->pass->attachment_count);
> -
> -  /* Store some information regarding this attachment. */
> -  const struct anv_attachment_state *att_state = &state->attachments[a];
> -  const struct anv_image_view *iview = 
> state->framebuffer->attachments[a];
> -  const struct anv_render_pass_attachment *rp_att =
> - &state->pass->attachments[a];
> -
> -  if (att_state->aux_usage == ISL_AUX_USAGE_NONE)
> - continue;
> -
> -  /* The fast clear state entry must be updated if a fast clear is going 
> to
> -   * happen. The surface state must be updated if the clear value from a
> -   * prior fast clear may be needed.
> -   */
> -  if (att_state->pending_clear_aspects && att_state->fast_clear) {
> - /* Update the fast clear state entry. */
> - genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> -  iview->image,
> -  VK_IMAGE_ASPECT_COLOR_BIT,
> -  iview->planes[0].isl.base_level,
> -  true /* copy from ss */);
> -
> - /* Fast-clears impact whether or not a resolve will be necessary. */
> - if (iview->image->planes[0].aux_usage == ISL_AUX_USAGE_CCS_E &&
> - att_state->clear_color_is_zero) {
> -/* This image always has the auxiliary buffer enabled. We can 
> mark
> - * the subresource as not needing a resolve because the clear 
> color
> - * will match what's in every RENDER_SURFACE_STATE object when 
> it's
> - * being used for sampling.
> - */
> -clear_image_needs_resolve_bits(cmd_buffer, iview->image,
> -   VK_IMAGE_ASPECT_COLOR_BIT,
> -   iview->planes[0].isl.base_level,
> -   ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> - } else {
> -set_image_needs_resolve_bits(cmd_buffer, iview->image,
> - VK_IMAGE_ASPECT_COLOR_BIT,
> - iview->planes[0].isl.base_level,
> - ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> - }
> -  } else if (rp_att->load_op == VK_ATTACHMENT_LOAD_OP_LOAD) {
> - /* The attachment may have been fast-cleared in a previous render
> -  * pass and the value is needed now. Update the surface state(s).

This piece of comment