On Wed, Jun 28, 2017 at 2:14 PM, Nanley Chery <nanleych...@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.ch...@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@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
