Re: [Mesa-dev] [PATCH 13/14] anv/cmd_buffer: Do subpass image transitions in begin/end_subpass

[Jason Ekstrand]

On Tue, Feb 13, 2018 at 5:34 PM, Nanley Chery  wrote:

> On Mon, Feb 05, 2018 at 02:35:02PM -0800, Jason Ekstrand wrote:
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 190 +-
> ---
> >  1 file changed, 68 insertions(+), 122 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 2d17c28..2732ef3 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3257,120 +3257,6 @@ cmd_buffer_emit_depth_stencil(struct
> anv_cmd_buffer *cmd_buffer)
> > cmd_buffer->state.hiz_enabled = info.hiz_usage == ISL_AUX_USAGE_HIZ;
> >  }
> >
> > -
> > -/**
> > - * @brief Perform any layout transitions required at the beginning
> and/or end
> > - *of the current subpass for depth buffers.
> > - *
> > - * TODO: Consider preprocessing the attachment reference array at
> render pass
> > - *   create time to determine if no layout transition is needed at
> the
> > - *   beginning and/or end of each subpass.
> > - *
> > - * @param cmd_buffer The command buffer the transition is happening
> within.
> > - * @param subpass_end If true, marks that the transition is happening
> at the
> > - *end of the subpass.
> > - */
> > -static void
> > -cmd_buffer_subpass_transition_layouts(struct anv_cmd_buffer * const
> cmd_buffer,
> > -  const bool subpass_end)
> > -{
> > -   /* We need a non-NULL command buffer. */
> > -   assert(cmd_buffer);
> > -
> > -   const struct anv_cmd_state * const cmd_state = _buffer->state;
> > -   const struct anv_subpass * const subpass = cmd_state->subpass;
> > -
> > -   /* This function must be called within a subpass. */
> > -   assert(subpass);
> > -
> > -   /* If there are attachment references, the array shouldn't be NULL.
> > -*/
> > -   if (subpass->attachment_count > 0)
> > -  assert(subpass->attachments);
> > -
> > -   /* Iterate over the array of attachment references. */
> > -   for (const struct anv_subpass_attachment *att_ref =
> subpass->attachments;
> > -att_ref < subpass->attachments + subpass->attachment_count;
> att_ref++) {
> > -
> > -  /* If the attachment is unused, we can't perform a layout
> transition. */
> > -  if (att_ref->attachment == VK_ATTACHMENT_UNUSED)
> > - continue;
> > -
> > -  /* This attachment index shouldn't go out of bounds. */
> > -  assert(att_ref->attachment < cmd_state->pass->attachment_count);
> > -
> > -  const struct anv_render_pass_attachment * const att_desc =
> > - _state->pass->attachments[att_ref->attachment];
> > -  struct anv_attachment_state * const att_state =
> > - _buffer->state.attachments[att_ref->attachment];
> > -
> > -  /* The attachment should not be used in a subpass after its last.
> */
> > -  assert(att_desc->last_subpass_idx >=
> anv_get_subpass_id(cmd_state));
> > -
> > -  if (subpass_end && anv_get_subpass_id(cmd_state) <
> > -  att_desc->last_subpass_idx) {
> > - /* We're calling this function on a buffer twice in one
> subpass and
> > -  * this is not the last use of the buffer. The layout should
> not have
> > -  * changed from the first call and no transition is necessary.
> > -  */
> > - assert(att_state->current_layout == att_ref->layout ||
> > -att_state->current_layout ==
> > -VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
> > - continue;
> > -  }
> > -
> > -  /* The attachment index must be less than the number of
> attachments
> > -   * within the framebuffer.
> > -   */
> > -  assert(att_ref->attachment < cmd_state->framebuffer->
> attachment_count);
> > -
> > -  const struct anv_image_view * const iview =
> > - cmd_state->framebuffer->attachments[att_ref->attachment];
> > -  const struct anv_image * const image = iview->image;
> > -
> > -  /* Get the appropriate target layout for this attachment. */
> > -  VkImageLayout target_layout;
> > -
> > -  /* A resolve is necessary before use as an input attachment if
> the clear
> > -   * color or auxiliary buffer usage isn't supported by the sampler.
> > -   */
> > -  const bool input_needs_resolve =
> > -(att_state->fast_clear && !att_state->clear_color_is_zero_one)
> ||
> > -att_state->input_aux_usage != att_state->aux_usage;
> > -  if (subpass_end) {
> > - target_layout = att_desc->final_layout;
> > -  } else if (iview->aspect_mask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV
> &&
> > - !input_needs_resolve) {
> > - /* Layout transitions before the final only help to enable
> sampling as
> > -  * an input attachment. If the input attachment supports
> sampling
> > -  * using the auxiliary surface, we can skip such transitions
> by making
> > -  * the target 

Re: [Mesa-dev] [PATCH 13/14] anv/cmd_buffer: Do subpass image transitions in begin/end_subpass

[Nanley Chery]

On Mon, Feb 05, 2018 at 02:35:02PM -0800, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 190 
> +
>  1 file changed, 68 insertions(+), 122 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 2d17c28..2732ef3 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3257,120 +3257,6 @@ cmd_buffer_emit_depth_stencil(struct anv_cmd_buffer 
> *cmd_buffer)
> cmd_buffer->state.hiz_enabled = info.hiz_usage == ISL_AUX_USAGE_HIZ;
>  }
>  
> -
> -/**
> - * @brief Perform any layout transitions required at the beginning and/or end
> - *of the current subpass for depth buffers.
> - *
> - * TODO: Consider preprocessing the attachment reference array at render pass
> - *   create time to determine if no layout transition is needed at the
> - *   beginning and/or end of each subpass.
> - *
> - * @param cmd_buffer The command buffer the transition is happening within.
> - * @param subpass_end If true, marks that the transition is happening at the
> - *end of the subpass.
> - */
> -static void
> -cmd_buffer_subpass_transition_layouts(struct anv_cmd_buffer * const 
> cmd_buffer,
> -  const bool subpass_end)
> -{
> -   /* We need a non-NULL command buffer. */
> -   assert(cmd_buffer);
> -
> -   const struct anv_cmd_state * const cmd_state = _buffer->state;
> -   const struct anv_subpass * const subpass = cmd_state->subpass;
> -
> -   /* This function must be called within a subpass. */
> -   assert(subpass);
> -
> -   /* If there are attachment references, the array shouldn't be NULL.
> -*/
> -   if (subpass->attachment_count > 0)
> -  assert(subpass->attachments);
> -
> -   /* Iterate over the array of attachment references. */
> -   for (const struct anv_subpass_attachment *att_ref = subpass->attachments;
> -att_ref < subpass->attachments + subpass->attachment_count; 
> att_ref++) {
> -
> -  /* If the attachment is unused, we can't perform a layout transition. 
> */
> -  if (att_ref->attachment == VK_ATTACHMENT_UNUSED)
> - continue;
> -
> -  /* This attachment index shouldn't go out of bounds. */
> -  assert(att_ref->attachment < cmd_state->pass->attachment_count);
> -
> -  const struct anv_render_pass_attachment * const att_desc =
> - _state->pass->attachments[att_ref->attachment];
> -  struct anv_attachment_state * const att_state =
> - _buffer->state.attachments[att_ref->attachment];
> -
> -  /* The attachment should not be used in a subpass after its last. */
> -  assert(att_desc->last_subpass_idx >= anv_get_subpass_id(cmd_state));
> -
> -  if (subpass_end && anv_get_subpass_id(cmd_state) <
> -  att_desc->last_subpass_idx) {
> - /* We're calling this function on a buffer twice in one subpass and
> -  * this is not the last use of the buffer. The layout should not 
> have
> -  * changed from the first call and no transition is necessary.
> -  */
> - assert(att_state->current_layout == att_ref->layout ||
> -att_state->current_layout ==
> -VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
> - continue;
> -  }
> -
> -  /* The attachment index must be less than the number of attachments
> -   * within the framebuffer.
> -   */
> -  assert(att_ref->attachment < cmd_state->framebuffer->attachment_count);
> -
> -  const struct anv_image_view * const iview =
> - cmd_state->framebuffer->attachments[att_ref->attachment];
> -  const struct anv_image * const image = iview->image;
> -
> -  /* Get the appropriate target layout for this attachment. */
> -  VkImageLayout target_layout;
> -
> -  /* A resolve is necessary before use as an input attachment if the 
> clear
> -   * color or auxiliary buffer usage isn't supported by the sampler.
> -   */
> -  const bool input_needs_resolve =
> -(att_state->fast_clear && !att_state->clear_color_is_zero_one) ||
> -att_state->input_aux_usage != att_state->aux_usage;
> -  if (subpass_end) {
> - target_layout = att_desc->final_layout;
> -  } else if (iview->aspect_mask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV &&
> - !input_needs_resolve) {
> - /* Layout transitions before the final only help to enable sampling 
> as
> -  * an input attachment. If the input attachment supports sampling
> -  * using the auxiliary surface, we can skip such transitions by 
> making
> -  * the target layout one that is CCS-aware.
> -  */
> - target_layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
> -  } else {
> - target_layout = att_ref->layout;
> -  }
> -
> -  /* Perform the layout transition. */
> -  if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT)