[Mesa-dev] [PATCH 17/22] anv: Perform HiZ resolves only on layout transitions
Jason Ekstrand
jason at jlekstrand.net
Thu Jan 12 04:54:01 UTC 2017
On Wed, Jan 11, 2017 at 5:55 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> This is a better mapping to the Vulkan API and improves performance in
> all tested workloads.
>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
> src/intel/vulkan/anv_blorp.c | 48 ++-------------------------------
> src/intel/vulkan/genX_cmd_buffer.c | 54 ++++++++++++++++++++++++++++++
> +-------
> 2 files changed, 46 insertions(+), 56 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 9919ac7ea0..5d410f7d86 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1579,52 +1579,8 @@ anv_gen8_hiz_op_resolve(struct anv_cmd_buffer
> *cmd_buffer,
> image->aux_usage != ISL_AUX_USAGE_HIZ)
> return;
>
> - const struct anv_cmd_state *cmd_state = &cmd_buffer->state;
> - const uint32_t ds = cmd_state->subpass->depth_stencil_attachment;
> -
> - /* Section 7.4. of the Vulkan 1.0.27 spec states:
> - *
> - * "The render area must be contained within the framebuffer
> dimensions."
> - *
> - * Therefore, the only way the extent of the render area can match
> that of
> - * the image view is if the render area offset equals (0, 0).
> - */
> - const bool full_surface_op =
> - cmd_state->render_area.extent.width == image->extent.width
> &&
> - cmd_state->render_area.extent.height ==
> image->extent.height;
> - if (full_surface_op)
> - assert(cmd_state->render_area.offset.x == 0 &&
> - cmd_state->render_area.offset.y == 0);
> -
> - /* Check the subpass index to determine if skipping a resolve is
> allowed */
> - const uint32_t subpass_idx = cmd_state->subpass -
> cmd_state->pass->subpasses;
> - switch (op) {
> - case BLORP_HIZ_OP_DEPTH_RESOLVE:
> - if (cmd_buffer->state.pass->attachments[ds].store_op !=
> - VK_ATTACHMENT_STORE_OP_STORE &&
> - subpass_idx == cmd_state->pass->subpass_count - 1)
> - return;
> - break;
> - case BLORP_HIZ_OP_HIZ_RESOLVE:
> - /* If the render area covers the entire surface *and* load_op is
> either
> - * CLEAR or DONT_CARE then the previous contents of the depth buffer
> - * will be entirely discarded. In this case, we can skip the HiZ
> - * resolve.
> - *
> - * If the render area is not the full surface, we need to do
> - * the resolve because otherwise data outside the render area may
> get
> - * garbled by the resolve at the end of the render pass.
> - */
> - if (full_surface_op &&
> - cmd_buffer->state.pass->attachments[ds].load_op !=
> - VK_ATTACHMENT_LOAD_OP_LOAD && subpass_idx == 0)
> - return;
> - break;
> - case BLORP_HIZ_OP_DEPTH_CLEAR:
> - case BLORP_HIZ_OP_NONE:
> - unreachable("Invalid HiZ OP");
> - }
> -
> + assert(op == BLORP_HIZ_OP_HIZ_RESOLVE ||
> + op == BLORP_HIZ_OP_DEPTH_RESOLVE);
>
> struct blorp_batch batch;
> blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer, 0);
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 1793c4df26..447baa08b2 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -510,7 +510,13 @@ genX(cmd_buffer_setup_attachments)(struct
> anv_cmd_buffer *cmd_buffer,
> state->attachments[i].aux_usage,
> state->attachments[i].color_rt_state);
> } else {
> - state->attachments[i].aux_usage = iview->image->aux_usage;
> + if (iview->image->aux_usage == ISL_AUX_USAGE_HIZ &&
> + iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {
> + state->attachments[i].aux_usage =
> + layout_to_hiz_usage(att->initial_layout);
> + } else {
> + state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE;
> + }
> state->attachments[i].input_aux_usage = ISL_AUX_USAGE_NONE;
> }
>
> @@ -915,6 +921,13 @@ void genX(CmdPipelineBarrier)(
> for (uint32_t i = 0; i < imageMemoryBarrierCount; i++) {
> src_flags |= pImageMemoryBarriers[i].srcAccessMask;
> dst_flags |= pImageMemoryBarriers[i].dstAccessMask;
> + ANV_FROM_HANDLE(anv_image, image, pImageMemoryBarriers[i].image);
> + if (pImageMemoryBarriers[i].subresourceRange.aspectMask &
> + VK_IMAGE_ASPECT_DEPTH_BIT) {
> + transition_depth_buffer(cmd_buffer, image,
> + pImageMemoryBarriers[i].oldLayout,
> + pImageMemoryBarriers[i].newLayout);
> + }
> }
>
> enum anv_pipe_bits pipe_bits = 0;
> @@ -2297,9 +2310,16 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer
> *cmd_buffer,
> const struct anv_image_view *iview =
> anv_cmd_buffer_get_depth_stencil_view(cmd_buffer);
>
> - if (iview) {
> - anv_gen8_hiz_op_resolve(cmd_buffer, iview->image,
> - BLORP_HIZ_OP_HIZ_RESOLVE);
> + if (iview && iview->image->aux_usage == ISL_AUX_USAGE_HIZ &&
> + iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {
>
I don't think you want the "iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT"
part. The image view aspect doesn't affect depth stencil attachments, only
input attachments and textures.
> + const uint32_t ds = subpass->depth_stencil_attachment;
> + transition_depth_buffer(cmd_buffer, iview->image,
> + cmd_buffer->state.attachments[
> ds].current_layout,
> + cmd_buffer->state.subpass->
> depth_stencil_layout);
> + cmd_buffer->state.attachments[ds].current_layout =
> + cmd_buffer->state.subpass->depth_stencil_layout;
> + cmd_buffer->state.attachments[ds].aux_usage =
> + layout_to_hiz_usage(cmd_buffer->state.subpass->depth_
> stencil_layout);
> }
>
> cmd_buffer_emit_depth_stencil(cmd_buffer);
> @@ -2337,9 +2357,16 @@ void genX(CmdNextSubpass)(
> const struct anv_image_view *iview =
> anv_cmd_buffer_get_depth_stencil_view(cmd_buffer);
>
> - if (iview) {
> - anv_gen8_hiz_op_resolve(cmd_buffer, iview->image,
> - BLORP_HIZ_OP_DEPTH_RESOLVE);
> + if (iview && iview->image->aux_usage == ISL_AUX_USAGE_HIZ &&
> + iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {
>
Same comment here
> + const uint32_t ds = cmd_buffer->state.subpass->
> depth_stencil_attachment;
> +
> + if (cmd_buffer->state.subpass - cmd_buffer->state.pass->subpasses
> ==
> + cmd_buffer->state.pass->attachments[ds].last_subpass_idx) {
> + transition_depth_buffer(cmd_buffer, iview->image,
> + cmd_buffer->state.attachments[
> ds].current_layout,
> + cmd_buffer->state.pass->
> attachments[ds].final_layout);
> + }
> }
>
> anv_cmd_buffer_resolve_subpass(cmd_buffer);
> @@ -2354,9 +2381,16 @@ void genX(CmdEndRenderPass)(
> const struct anv_image_view *iview =
> anv_cmd_buffer_get_depth_stencil_view(cmd_buffer);
>
> - if (iview) {
> - anv_gen8_hiz_op_resolve(cmd_buffer, iview->image,
> - BLORP_HIZ_OP_DEPTH_RESOLVE);
> + if (iview && iview->image->aux_usage == ISL_AUX_USAGE_HIZ &&
> + iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {
>
And here.
> + const uint32_t ds = cmd_buffer->state.subpass->
> depth_stencil_attachment;
> +
> + if (cmd_buffer->state.subpass - cmd_buffer->state.pass->subpasses
> ==
> + cmd_buffer->state.pass->attachments[ds].last_subpass_idx) {
> + transition_depth_buffer(cmd_buffer, iview->image,
> + cmd_buffer->state.attachments[
> ds].current_layout,
> + cmd_buffer->state.pass->
> attachments[ds].final_layout);
> + }
> }
>
> anv_cmd_buffer_resolve_subpass(cmd_buffer);
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170111/61301558/attachment-0001.html>
More information about the mesa-dev
mailing list