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 = &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,
> - 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,
> - VK_IMAGE_ASPECT_COLOR_BIT,
> - att_state->aux_usage,
> - iview->planes[0].isl.base_level,
> -
> iview->planes[0].isl.base_array_layer,
> - state->framebuffer->layers);
> - }
> -}
> -
> static void
> cmd_buffer_begin_subpass(struct anv_cmd_buffer *cmd_buffer,
> uint32_t subpass_id)
> @@ -3523,15 +3432,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;
>
> @@ -3565,6 +3465,25 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> 0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> base_layer++;
> layer_count--;
> +
> + genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> + image, VK_IMAGE_ASPECT_COLOR_BIT,
> + true /* copy from ss */);
> +
> + 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.
> + */
This comment is stale. Fast-clearing to zero doesn't imply that the
image always has the aux buffer enabled.
With that updated, this patch is
Reviewed-by: Nanley Chery <nanley.g.ch...@intel.com>
> + 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);
> + }
> }
>
> if (layer_count > 0) {
> @@ -3605,7 +3524,36 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> assert(att_state->pending_clear_aspects == 0);
> }
>
> + if (att_state->pending_load_aspects &
> VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
> + 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,
> + 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,
> + false /* copy to ss */);
> + }
> + }
> +
> + if (subpass->attachments[i].usage ==
> + VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
> + /* 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,
> +
> iview->planes[0].isl.base_array_layer,
> + fb->layers);
> + }
> +
> 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
> 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
