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 gets dropped. See further down:
> - *
> - * 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,
> - iview->planes[0].isl.base_level,
> - 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,
> - iview->planes[0].isl.base_level,
> - 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,
> - VK_IMAGE_ASPECT_COLOR_BIT,
> - att_state->aux_usage,
> - iview->planes[0].isl.base_level);
> - }
> -}
> -
> static void
> cmd_buffer_begin_subpass(struct anv_cmd_buffer *cmd_buffer,
> uint32_t subpass_id)
> @@ -3218,15 +3125,6 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> */
> cmd_buffer_subpass_transition_layouts(cmd_buffer, false);
>
> - /* Update clear values *after* performing automatic layout transitions.
> - * This ensures that transitions from the UNDEFINED layout have had a
> chance
> - * to populate the clear value buffer with the correct values for the
> - * LOAD_OP_LOAD loadOp and that the fast-clears will update the buffer
> - * without the aforementioned layout transition overwriting the fast-clear
> - * value.
> - */
> - cmd_buffer_subpass_sync_fast_clear_values(cmd_buffer);
> -
> VkRect2D render_area = cmd_buffer->state.render_area;
> struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
>
> @@ -3254,6 +3152,30 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> iview->planes[0].isl.base_array_layer,
> fb->layers,
> ISL_AUX_OP_FAST_CLEAR, false);
> +
> + genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> + 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 (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 {
> anv_image_clear_color(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> att_state->aux_usage,
> @@ -3292,7 +3214,35 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> assert(att_state->pending_clear_aspects == 0);
> }
>
> + if (att_state->pending_load_aspects) {
I think that comment would fit here. Anyway, with or without:
Reviewed-by: Topi Pohjolainen <[email protected]>
> + if (att_state->aux_usage != ISL_AUX_USAGE_NONE) {
> + genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> + image, VK_IMAGE_ASPECT_COLOR_BIT,
> + iview->planes[0].isl.base_level,
> + false /* copy to ss */);
> + }
> +
> + if (need_input_attachment_state(&cmd_state->pass->attachments[a]) &&
> + att_state->input_aux_usage != ISL_AUX_USAGE_NONE) {
> + genX(copy_fast_clear_dwords)(cmd_buffer, att_state->input.state,
> + image, VK_IMAGE_ASPECT_COLOR_BIT,
> + iview->planes[0].isl.base_level,
> + false /* copy to ss */);
> + }
> + }
> +
> + if (anv_subpass_att_is_color(subpass, &subpass->attachments[i])) {
> + /* 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,
> + VK_IMAGE_ASPECT_COLOR_BIT,
> + att_state->aux_usage,
> +
> iview->planes[0].isl.base_level);
> + }
> +
> att_state->pending_clear_aspects = 0;
> + att_state->pending_load_aspects = 0;
> }
>
> cmd_buffer_emit_depth_stencil(cmd_buffer);
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
