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

[Jason Ekstrand]

On Tue, Feb 13, 2018 at 4:35 PM, Nanley Chery  wrote:

> On Mon, Feb 05, 2018 at 02:35:00PM -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/genX_cmd_buffer.c | 148 --
> ---
> >  1 file changed, 48 insertions(+), 100 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index f92e86f..4eee85a 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3392,97 +3392,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 = _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 =
> >attachments[a];
> > -  const struct anv_image_view *iview = state->framebuffer->
> attachments[a];
> > -  const struct anv_render_pass_attachment *rp_att =
> > - >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,
> > -  true /* copy from ss */);
> > -
> > - /* Fast-clears impact whether or not a resolve will be
> necessary. */
> > - if (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.
> > - */
> > -set_image_fast_clear_state(cmd_buffer, iview->image,
> > -   VK_IMAGE_ASPECT_COLOR_BIT,
> > -   ANV_FAST_CLEAR_DEFAULT_VALUE);
> > - } else {
> > -set_image_fast_clear_state(cmd_buffer, iview->image,
> > -   VK_IMAGE_ASPECT_COLOR_BIT,
> > -   ANV_FAST_CLEAR_ANY);
> > - }
> > -  } else if (rp_att->load_op == VK_ATTACHMENT_LOAD_OP_LOAD &&
> > - iview->planes[0].isl.base_level == 0 &&
> > - iview->planes[0].isl.base_array_layer == 0) {
> > - /* 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,
> > -  false /* copy to ss */);
> > -
> > - if (need_input_attachment_state(rp_att) &&
> > - att_state->input_aux_usage != ISL_AUX_USAGE_NONE) {
> > -genX(copy_fast_clear_dwords)(cmd_buffer,
> att_state->input.state,
> > - iview->image,
> > - VK_IMAGE_ASPECT_COLOR_BIT,
> > - false /* copy to ss */);
> > -

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

[Nanley Chery]

On Mon, Feb 05, 2018 at 02:35:00PM -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/genX_cmd_buffer.c | 148 
> -
>  1 file changed, 48 insertions(+), 100 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index f92e86f..4eee85a 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3392,97 +3392,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 = _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 = >attachments[a];
> -  const struct anv_image_view *iview = 
> state->framebuffer->attachments[a];
> -  const struct anv_render_pass_attachment *rp_att =
> - >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,
> -  true /* copy from ss */);
> -
> - /* Fast-clears impact whether or not a resolve will be necessary. */
> - if (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.
> - */
> -set_image_fast_clear_state(cmd_buffer, iview->image,
> -   VK_IMAGE_ASPECT_COLOR_BIT,
> -   ANV_FAST_CLEAR_DEFAULT_VALUE);
> - } else {
> -set_image_fast_clear_state(cmd_buffer, iview->image,
> -   VK_IMAGE_ASPECT_COLOR_BIT,
> -   ANV_FAST_CLEAR_ANY);
> - }
> -  } else if (rp_att->load_op == VK_ATTACHMENT_LOAD_OP_LOAD &&
> - iview->planes[0].isl.base_level == 0 &&
> - iview->planes[0].isl.base_array_layer == 0) {
> - /* 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,
> -  false /* copy to ss */);
> -
> - if (need_input_attachment_state(rp_att) &&
> - att_state->input_aux_usage != ISL_AUX_USAGE_NONE) {
> -genX(copy_fast_clear_dwords)(cmd_buffer, att_state->input.state,
> - iview->image,
> - VK_IMAGE_ASPECT_COLOR_BIT,
> - false /* copy to ss */);
> - }
> -  }
> -
> -  /* We assume that if we're starting a subpass, we're going to do some
> -   * rendering so we may end up with compressed data.
> -   */
> -  genX(cmd_buffer_mark_image_written)(cmd_buffer, iview->image,
> -