From 5e8196e75650a4371b88344bef73f57343cfc76e Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 13 Sep 2022 08:52:58 +0200 Subject: [PATCH] v3dv: don't load an attachment for unaligned render area if we are not storing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the render area is not aligned to tile boundaries it means we have partially covered tiles in the framebuffer. In this case, we always need to load the tile buffer from memory in order to preserve the contents outside the render area on the tile buffer store. However, if in this scenario we know we won't be storing the tile buffer we can skip the load safely. Reviewed-by: Alejandro Piñeiro Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 25 +++++++++++++++++------ src/broadcom/vulkan/v3dv_private.h | 4 +++- src/broadcom/vulkan/v3dvx_cmd_buffer.c | 28 +++++++++++++++++++++----- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index 78ce5944311..d688e36a33c 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -1423,7 +1423,9 @@ bool v3dv_cmd_buffer_check_needs_load(const struct v3dv_cmd_buffer_state *state, VkImageAspectFlags aspect, uint32_t first_subpass_idx, - VkAttachmentLoadOp load_op) + VkAttachmentLoadOp load_op, + uint32_t last_subpass_idx, + VkAttachmentStoreOp store_op) { /* We call this with image->vk.aspects & aspect, so 0 means the aspect we are * testing does not exist in the image. @@ -1443,9 +1445,14 @@ v3dv_cmd_buffer_check_needs_load(const struct v3dv_cmd_buffer_state *state, if (state->job->is_subpass_continue) return true; - /* If the area is not aligned to tile boundaries, we always need to load */ - if (!state->tile_aligned_render_area) + /* If the area is not aligned to tile boundaries and we are going to store, + * then we need to load to preserve contents outside the render area. + */ + if (!state->tile_aligned_render_area && + v3dv_cmd_buffer_check_needs_store(state, aspect, last_subpass_idx, + store_op)) { return true; + } /* The attachment load operations must be LOAD */ return load_op == VK_ATTACHMENT_LOAD_OP_LOAD; @@ -1539,7 +1546,9 @@ cmd_buffer_subpass_check_double_buffer_mode(struct v3dv_cmd_buffer *cmd_buffer, if (v3dv_cmd_buffer_check_needs_load(state, VK_IMAGE_ASPECT_COLOR_BIT, attachment->first_subpass, - attachment->desc.loadOp)) { + attachment->desc.loadOp, + attachment->last_subpass, + attachment->desc.storeOp)) { return; } @@ -1562,14 +1571,18 @@ cmd_buffer_subpass_check_double_buffer_mode(struct v3dv_cmd_buffer *cmd_buffer, if (v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_DEPTH_BIT, ds_attachment->first_subpass, - ds_attachment->desc.loadOp)) { + ds_attachment->desc.loadOp, + ds_attachment->last_subpass, + ds_attachment->desc.storeOp)) { return; } if (v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_STENCIL_BIT, ds_attachment->first_subpass, - ds_attachment->desc.stencilLoadOp)) { + ds_attachment->desc.stencilLoadOp, + ds_attachment->last_subpass, + ds_attachment->desc.stencilStoreOp)) { return; } diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index 4427298bd76..d962dfddeb9 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -1633,7 +1633,9 @@ void v3dv_cmd_buffer_merge_barrier_state(struct v3dv_barrier_state *dst, bool v3dv_cmd_buffer_check_needs_load(const struct v3dv_cmd_buffer_state *state, VkImageAspectFlags aspect, uint32_t first_subpass_idx, - VkAttachmentLoadOp load_op); + VkAttachmentLoadOp load_op, + uint32_t last_subpass_idx, + VkAttachmentStoreOp store_op); bool v3dv_cmd_buffer_check_needs_store(const struct v3dv_cmd_buffer_state *state, VkImageAspectFlags aspect, diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index 3b283e2ad96..8f78dfb099c 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -229,11 +229,17 @@ cmd_buffer_render_pass_emit_loads(struct v3dv_cmd_buffer *cmd_buffer, attachment->first_subpass : attachment->views[layer].first_subpass; + uint32_t last_subpass = !pass->multiview_enabled ? + attachment->last_subpass : + attachment->views[layer].last_subpass; + bool needs_load = v3dv_cmd_buffer_check_needs_load(state, VK_IMAGE_ASPECT_COLOR_BIT, first_subpass, - attachment->desc.loadOp); + attachment->desc.loadOp, + last_subpass, + attachment->desc.storeOp); if (needs_load) { struct v3dv_image_view *iview = state->attachments[attachment_idx].image_view; @@ -254,17 +260,25 @@ cmd_buffer_render_pass_emit_loads(struct v3dv_cmd_buffer *cmd_buffer, ds_attachment->first_subpass : ds_attachment->views[layer].first_subpass; + uint32_t ds_last_subpass = !pass->multiview_enabled ? + ds_attachment->last_subpass : + ds_attachment->views[layer].last_subpass; + const bool needs_depth_load = v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_DEPTH_BIT, ds_first_subpass, - ds_attachment->desc.loadOp); + ds_attachment->desc.loadOp, + ds_last_subpass, + ds_attachment->desc.storeOp); const bool needs_stencil_load = v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_STENCIL_BIT, ds_first_subpass, - ds_attachment->desc.stencilLoadOp); + ds_attachment->desc.stencilLoadOp, + ds_last_subpass, + ds_attachment->desc.stencilStoreOp); if (needs_depth_load || needs_stencil_load) { struct v3dv_image_view *iview = @@ -834,7 +848,9 @@ v3dX(cmd_buffer_emit_render_pass_rcl)(struct v3dv_cmd_buffer *cmd_buffer) v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_STENCIL_BIT, ds_attachment->first_subpass, - ds_attachment->desc.stencilLoadOp); + ds_attachment->desc.stencilLoadOp, + ds_attachment->last_subpass, + ds_attachment->desc.stencilStoreOp); bool needs_stencil_store = v3dv_cmd_buffer_check_needs_store(state, @@ -1434,7 +1450,9 @@ job_update_ez_state(struct v3dv_job *job, v3dv_cmd_buffer_check_needs_load(state, ds_aspects & VK_IMAGE_ASPECT_DEPTH_BIT, ds_attachment->first_subpass, - ds_attachment->desc.loadOp); + ds_attachment->desc.loadOp, + ds_attachment->last_subpass, + ds_attachment->desc.storeOp); if (needs_depth_load) { struct v3dv_framebuffer *fb = state->framebuffer;