anv: Allow PMA optimization to enabled in secondary command buffers

Submitted by Alex Smith on Jan. 5, 2018, 11:20 a.m.

Details

Message ID 20180105112054.10469-1-asmith@feralinteractive.com
State New
Headers show
Series "anv: Allow PMA optimization to enabled in secondary command buffers" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Alex Smith Jan. 5, 2018, 11:20 a.m.
This was never enabled in secondary buffers because hiz_enabled was
never set to true for those.

If the app provides a framebuffer in the inheritance info when beginning
a secondary buffer, we can determine if HiZ is enabled and therefore
allow the PMA optimization to be enabled within the command buffer.

This improves performance by ~13% on an internal benchmark on Skylake.

Signed-off-by: Alex Smith <asmith@feralinteractive.com>
---
 src/intel/vulkan/genX_cmd_buffer.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
index 0bd3874db7..2036151249 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -977,11 +977,31 @@  genX(BeginCommandBuffer)(
          anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
       cmd_buffer->state.subpass =
          &cmd_buffer->state.pass->subpasses[pBeginInfo->pInheritanceInfo->subpass];
-      cmd_buffer->state.framebuffer = NULL;
+
+      /* This is optional in the inheritance info. */
+      cmd_buffer->state.framebuffer =
+         anv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
 
       result = genX(cmd_buffer_setup_attachments)(cmd_buffer,
                                                   cmd_buffer->state.pass, NULL);
 
+      /* Record that HiZ is enabled if we can. */
+      if (cmd_buffer->state.framebuffer) {
+         const VkAttachmentReference * const ds =
+            &cmd_buffer->state.subpass->depth_stencil_attachment;
+
+         if (ds->attachment != VK_ATTACHMENT_UNUSED) {
+            const struct anv_image_view * const iview =
+               cmd_buffer->state.framebuffer->attachments[ds->attachment];
+            const struct anv_image * const image = iview->image;
+            enum isl_aux_usage aux_usage =
+               anv_layout_to_aux_usage(&cmd_buffer->device->info, image,
+                                       VK_IMAGE_ASPECT_DEPTH_BIT, ds->layout);
+
+            cmd_buffer->state.hiz_enabled = aux_usage == ISL_AUX_USAGE_HIZ;
+         }
+      }
+
       cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
    }
 

Comments

This makes sense to me, it would be good to have Jason's opinion.
I have a suggestion below.

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Thanks!

On 05/01/18 11:20, Alex Smith wrote:
> This was never enabled in secondary buffers because hiz_enabled was
> never set to true for those.
>
> If the app provides a framebuffer in the inheritance info when beginning
> a secondary buffer, we can determine if HiZ is enabled and therefore
> allow the PMA optimization to be enabled within the command buffer.
>
> This improves performance by ~13% on an internal benchmark on Skylake.
>
> Signed-off-by: Alex Smith <asmith@feralinteractive.com>
> ---
>   src/intel/vulkan/genX_cmd_buffer.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
> index 0bd3874db7..2036151249 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -977,11 +977,31 @@ genX(BeginCommandBuffer)(
>            anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
>         cmd_buffer->state.subpass =
>            &cmd_buffer->state.pass->subpasses[pBeginInfo->pInheritanceInfo->subpass];
> -      cmd_buffer->state.framebuffer = NULL;
> +
> +      /* This is optional in the inheritance info. */
> +      cmd_buffer->state.framebuffer =
> +         anv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
>   
>         result = genX(cmd_buffer_setup_attachments)(cmd_buffer,
>                                                     cmd_buffer->state.pass, NULL);
>   
> +      /* Record that HiZ is enabled if we can. */
> +      if (cmd_buffer->state.framebuffer) {

You might be able to knock a few lines below by reusing 
anv_cmd_buffer_get_depth_stencil_view().

> +         const VkAttachmentReference * const ds =
> +            &cmd_buffer->state.subpass->depth_stencil_attachment;
> +
> +         if (ds->attachment != VK_ATTACHMENT_UNUSED) {
> +            const struct anv_image_view * const iview =
> +               cmd_buffer->state.framebuffer->attachments[ds->attachment];
> +            const struct anv_image * const image = iview->image;
> +            enum isl_aux_usage aux_usage =
> +               anv_layout_to_aux_usage(&cmd_buffer->device->info, image,
> +                                       VK_IMAGE_ASPECT_DEPTH_BIT, ds->layout);
> +
> +            cmd_buffer->state.hiz_enabled = aux_usage == ISL_AUX_USAGE_HIZ;
> +         }
> +      }
> +
>         cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
>      }
>
Thanks, that change makes it look a bit tidier - I'll send a v2 and wait to
see what Jason thinks.

On 5 January 2018 at 16:06, Lionel Landwerlin <lionel.g.landwerlin@intel.com
> wrote:

> This makes sense to me, it would be good to have Jason's opinion.
> I have a suggestion below.
>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>
> Thanks!
>
>
> On 05/01/18 11:20, Alex Smith wrote:
>
>> This was never enabled in secondary buffers because hiz_enabled was
>> never set to true for those.
>>
>> If the app provides a framebuffer in the inheritance info when beginning
>> a secondary buffer, we can determine if HiZ is enabled and therefore
>> allow the PMA optimization to be enabled within the command buffer.
>>
>> This improves performance by ~13% on an internal benchmark on Skylake.
>>
>> Signed-off-by: Alex Smith <asmith@feralinteractive.com>
>> ---
>>   src/intel/vulkan/genX_cmd_buffer.c | 22 +++++++++++++++++++++-
>>   1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
>> b/src/intel/vulkan/genX_cmd_buffer.c
>> index 0bd3874db7..2036151249 100644
>> --- a/src/intel/vulkan/genX_cmd_buffer.c
>> +++ b/src/intel/vulkan/genX_cmd_buffer.c
>> @@ -977,11 +977,31 @@ genX(BeginCommandBuffer)(
>>            anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->r
>> enderPass);
>>         cmd_buffer->state.subpass =
>>            &cmd_buffer->state.pass->subpasses[pBeginInfo->pInheritance
>> Info->subpass];
>> -      cmd_buffer->state.framebuffer = NULL;
>> +
>> +      /* This is optional in the inheritance info. */
>> +      cmd_buffer->state.framebuffer =
>> +         anv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->f
>> ramebuffer);
>>           result = genX(cmd_buffer_setup_attachments)(cmd_buffer,
>>
>> cmd_buffer->state.pass, NULL);
>>   +      /* Record that HiZ is enabled if we can. */
>> +      if (cmd_buffer->state.framebuffer) {
>>
>
> You might be able to knock a few lines below by reusing
> anv_cmd_buffer_get_depth_stencil_view().
>
>
> +         const VkAttachmentReference * const ds =
>> +            &cmd_buffer->state.subpass->depth_stencil_attachment;
>> +
>> +         if (ds->attachment != VK_ATTACHMENT_UNUSED) {
>> +            const struct anv_image_view * const iview =
>> +               cmd_buffer->state.framebuffer->attachments[ds->
>> attachment];
>> +            const struct anv_image * const image = iview->image;
>> +            enum isl_aux_usage aux_usage =
>> +               anv_layout_to_aux_usage(&cmd_buffer->device->info, image,
>> +                                       VK_IMAGE_ASPECT_DEPTH_BIT,
>> ds->layout);
>> +
>> +            cmd_buffer->state.hiz_enabled = aux_usage ==
>> ISL_AUX_USAGE_HIZ;
>> +         }
>> +      }
>> +
>>         cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
>>      }
>>
>>
>
>
>