[Mesa-dev] [PATCH v3 05/16] anv/cmd_buffer: Restrict fast clears in the GENERAL layout

Jason Ekstrand jason at jlekstrand.net
Mon Jul 10 16:28:05 UTC 2017


On Wed, Jun 28, 2017 at 2:14 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> v2: Remove ::first_subpass_layout assertion (Jason Ekstrand).
> v3: Allow some fast clears in the GENERAL layout.
>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/intel/vulkan/anv_pass.c        | 22 ++++++++++++++++++++++
>  src/intel/vulkan/anv_private.h     |  2 ++
>  src/intel/vulkan/genX_cmd_buffer.c | 17 ++++++++++++++++-
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c
> index 1b30c1409d..ab0733fc10 100644
> --- a/src/intel/vulkan/anv_pass.c
> +++ b/src/intel/vulkan/anv_pass.c
> @@ -34,6 +34,16 @@ num_subpass_attachments(const VkSubpassDescription
> *desc)
>            (desc->pDepthStencilAttachment != NULL);
>  }
>
> +static void
> +init_first_subpass_layout(struct anv_render_pass_attachment * const att,
> +                          const VkAttachmentReference att_ref)
> +{
> +   if (att->first_subpass_layout == VK_IMAGE_LAYOUT_UNDEFINED) {
> +      att->first_subpass_layout = att_ref.layout;
> +      assert(att->first_subpass_layout != VK_IMAGE_LAYOUT_UNDEFINED);
> +   }
> +}
> +
>  VkResult anv_CreateRenderPass(
>      VkDevice                                    _device,
>      const VkRenderPassCreateInfo*               pCreateInfo,
> @@ -91,6 +101,7 @@ VkResult anv_CreateRenderPass(
>        att->stencil_load_op = pCreateInfo->pAttachments[i].stencilLoadOp;
>        att->initial_layout = pCreateInfo->pAttachments[i].initialLayout;
>        att->final_layout = pCreateInfo->pAttachments[i].finalLayout;
> +      att->first_subpass_layout = VK_IMAGE_LAYOUT_UNDEFINED;
>        att->subpass_usage = subpass_usages;
>        subpass_usages += pass->subpass_count;
>     }
> @@ -119,6 +130,8 @@ VkResult anv_CreateRenderPass(
>                 pass->attachments[a].subpass_usage[i] |=
> ANV_SUBPASS_USAGE_INPUT;
>                 pass->attachments[a].last_subpass_idx = i;
>
> +               init_first_subpass_layout(&pass->attachments[a],
> +                                         desc->pInputAttachments[j]);
>                 if (desc->pDepthStencilAttachment &&
>                     a == desc->pDepthStencilAttachment->attachment)
>                    subpass->has_ds_self_dep = true;
> @@ -138,6 +151,9 @@ VkResult anv_CreateRenderPass(
>                 pass->attachments[a].usage |= VK_IMAGE_USAGE_COLOR_
> ATTACHMENT_BIT;
>                 pass->attachments[a].subpass_usage[i] |=
> ANV_SUBPASS_USAGE_DRAW;
>                 pass->attachments[a].last_subpass_idx = i;
> +
> +               init_first_subpass_layout(&pass->attachments[a],
> +                                         desc->pColorAttachments[j]);
>              }
>           }
>        }
> @@ -162,6 +178,9 @@ VkResult anv_CreateRenderPass(
>                 pass->attachments[a].subpass_usage[i] |=
>                    ANV_SUBPASS_USAGE_RESOLVE_DST;
>                 pass->attachments[a].last_subpass_idx = i;
> +
> +               init_first_subpass_layout(&pass->attachments[a],
> +                                         desc->pResolveAttachments[j]);
>              }
>           }
>        }
> @@ -176,6 +195,9 @@ VkResult anv_CreateRenderPass(
>                 VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT;
>              pass->attachments[a].subpass_usage[i] |=
> ANV_SUBPASS_USAGE_DRAW;
>              pass->attachments[a].last_subpass_idx = i;
> +
> +            init_first_subpass_layout(&pass->attachments[a],
> +                                      *desc->pDepthStencilAttachment);
>           }
>        } else {
>           subpass->depth_stencil_attachment.attachment =
> VK_ATTACHMENT_UNUSED;
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index a95188ac30..c5a2ba0888 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1518,6 +1518,7 @@ struct anv_attachment_state {
>     bool                                         fast_clear;
>     VkClearValue                                 clear_value;
>     bool                                         clear_color_is_zero_one;
> +   bool                                         clear_color_is_zero;
>  };
>
>  /** State required while building cmd buffer */
> @@ -2336,6 +2337,7 @@ struct anv_render_pass_attachment {
>     VkAttachmentLoadOp                           stencil_load_op;
>     VkImageLayout                                initial_layout;
>     VkImageLayout                                final_layout;
> +   VkImageLayout                                first_subpass_layout;
>
>     /* An array, indexed by subpass id, of how the attachment will be
> used. */
>     enum anv_subpass_usage *                     subpass_usage;
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 15927d32ad..253e68cd1f 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -253,7 +253,12 @@ color_attachment_compute_aux_usage(struct anv_device
> * device,
>
>     assert(iview->image->aux_surface.isl.usage & ISL_SURF_USAGE_CCS_BIT);
>
> -   att_state->clear_color_is_zero_one =
> +   att_state->clear_color_is_zero =
> +      att_state->clear_value.color.uint32[0] == 0 &&
> +      att_state->clear_value.color.uint32[1] == 0 &&
> +      att_state->clear_value.color.uint32[2] == 0 &&
> +      att_state->clear_value.color.uint32[3] == 0;
> +   att_state->clear_color_is_zero_one = att_state->clear_color_is_zero ||
>

I doubt the || actually gains us anything here.  function inlining, CSE,
and the processor's normal pipelineing will most likely make all of the
theoretical performance gain go away.


>        color_is_zero_one(att_state->clear_value.color, iview->isl.format);
>
>     if (att_state->pending_clear_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> @@ -299,6 +304,16 @@ color_attachment_compute_aux_usage(struct anv_device
> * device,
>           }
>        }
>
> +      /* We only allow fast clears in the GENERAL layout if the auxiliary
> +       * buffer is always enabled and the fast-clear value is all 0's. See
> +       * add_fast_clear_state_buffer() for more information.
> +       */
> +      if (cmd_state->pass->attachments[att].first_subpass_layout ==
> +          VK_IMAGE_LAYOUT_GENERAL && (!att_state->clear_color_is_zero ||
> +          iview->image->aux_usage == ISL_AUX_USAGE_NONE)) {
>

Mind line-breaking this differently?

+      if (cmd_state->pass->attachments[att].first_subpass_layout ==
+          VK_IMAGE_LAYOUT_GENERAL &&
+          (!att_state->clear_color_is_zero ||
+           iview->image->aux_usage == ISL_AUX_USAGE_NONE)) {

That way it's a bit clearer where the boolean logic is.

--Jason

+         att_state->fast_clear = false;
> +      }
> +
>        if (att_state->fast_clear) {
>           memcpy(fast_clear_color->u32, att_state->clear_value.color.
> uint32,
>                  sizeof(fast_clear_color->u32));
> --
> 2.13.1
>
> _______________________________________________
> 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/20170710/050029f9/attachment-0001.html>


More information about the mesa-dev mailing list