[Mesa-dev] [PATCH 1/2] anv: fix push descriptors with set > 0

[Lionel Landwerlin]

When writing to set > 0, we were just wrongly writing to set 0. This
commit fixes this by lazily allocating each set as we write to them.

We didn't go for having them directly into the command buffer as this
would require an additional ~45Kb per command buffer.

v2: Allocate push descriptors from system memory rather than in BO
    streams. (Lionel)

Cc: "17.2 17.1" <mesa-stable at lists.freedesktop.org>
Fixes: 9f60ed98e501 ("anv: add VK_KHR_push_descriptor support")
Reported-by: Daniel Ribeiro Maciel <daniel.maciel at gmail.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
---
 src/intel/vulkan/anv_cmd_buffer.c | 59 +++++++++++++++++++++++++++++++--------
 src/intel/vulkan/anv_private.h    |  3 +-
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c
index 3b59af8f6f4..ec4ebf7c6fc 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -120,11 +120,13 @@ anv_cmd_state_reset(struct anv_cmd_buffer *cmd_buffer)
    cmd_buffer->batch.status = VK_SUCCESS;
 
    memset(&state->descriptors, 0, sizeof(state->descriptors));
+   for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++) {
+      vk_free(&cmd_buffer->pool->alloc, state->push_descriptors[i]);
+      state->push_descriptors[i] = NULL;
+   }
    for (uint32_t i = 0; i < MESA_SHADER_STAGES; i++) {
-      if (state->push_constants[i] != NULL) {
-         vk_free(&cmd_buffer->pool->alloc, state->push_constants[i]);
-         state->push_constants[i] = NULL;
-      }
+      vk_free(&cmd_buffer->pool->alloc, state->push_constants[i]);
+      state->push_constants[i] = NULL;
    }
    memset(state->binding_tables, 0, sizeof(state->binding_tables));
    memset(state->samplers, 0, sizeof(state->samplers));
@@ -216,8 +218,8 @@ static VkResult anv_create_cmd_buffer(
    anv_state_stream_init(&cmd_buffer->dynamic_state_stream,
                          &device->dynamic_state_pool, 16384);
 
-   memset(&cmd_buffer->state.push_descriptor, 0,
-          sizeof(cmd_buffer->state.push_descriptor));
+   memset(cmd_buffer->state.push_descriptors, 0,
+          sizeof(cmd_buffer->state.push_descriptors));
 
    if (pool) {
       list_addtail(&cmd_buffer->pool_link, &pool->cmd_buffers);
@@ -269,6 +271,8 @@ VkResult anv_AllocateCommandBuffers(
 static void
 anv_cmd_buffer_destroy(struct anv_cmd_buffer *cmd_buffer)
 {
+   struct anv_cmd_state *state = &cmd_buffer->state;
+
    list_del(&cmd_buffer->pool_link);
 
    anv_cmd_buffer_fini_batch_bo_chain(cmd_buffer);
@@ -276,7 +280,10 @@ anv_cmd_buffer_destroy(struct anv_cmd_buffer *cmd_buffer)
    anv_state_stream_finish(&cmd_buffer->surface_state_stream);
    anv_state_stream_finish(&cmd_buffer->dynamic_state_stream);
 
-   vk_free(&cmd_buffer->pool->alloc, cmd_buffer->state.attachments);
+   for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++)
+      vk_free(&cmd_buffer->pool->alloc, state->push_descriptors[i]);
+
+   vk_free(&cmd_buffer->pool->alloc, state->attachments);
    vk_free(&cmd_buffer->pool->alloc, cmd_buffer);
 }
 
@@ -834,6 +841,26 @@ anv_cmd_buffer_get_depth_stencil_view(const struct anv_cmd_buffer *cmd_buffer)
    return iview;
 }
 
+static VkResult
+anv_cmd_buffer_ensure_push_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
+                                          uint32_t set)
+{
+   struct anv_push_descriptor_set **push_set =
+      &cmd_buffer->state.push_descriptors[set];
+
+   if (*push_set == NULL) {
+      *push_set = vk_alloc(&cmd_buffer->pool->alloc,
+                           sizeof(struct anv_push_descriptor_set), 8,
+                           VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+      if (*push_set == NULL) {
+         anv_batch_set_error(&cmd_buffer->batch, VK_ERROR_OUT_OF_HOST_MEMORY);
+         return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
+      }
+   }
+
+   return VK_SUCCESS;
+}
+
 void anv_CmdPushDescriptorSetKHR(
     VkCommandBuffer commandBuffer,
     VkPipelineBindPoint pipelineBindPoint,
@@ -851,12 +878,17 @@ void anv_CmdPushDescriptorSetKHR(
 
    const struct anv_descriptor_set_layout *set_layout =
       layout->set[_set].layout;
-   struct anv_descriptor_set *set = &cmd_buffer->state.push_descriptor.set;
+
+   if (anv_cmd_buffer_ensure_push_descriptor_set(cmd_buffer, _set) != VK_SUCCESS)
+      return;
+   struct anv_push_descriptor_set *push_set =
+      cmd_buffer->state.push_descriptors[_set];
+   struct anv_descriptor_set *set = &push_set->set;
 
    set->layout = set_layout;
    set->size = anv_descriptor_set_layout_size(set_layout);
    set->buffer_count = set_layout->buffer_count;
-   set->buffer_views = cmd_buffer->state.push_descriptor.buffer_views;
+   set->buffer_views = push_set->buffer_views;
 
    /* Go through the user supplied descriptors. */
    for (uint32_t i = 0; i < descriptorWriteCount; i++) {
@@ -937,12 +969,17 @@ void anv_CmdPushDescriptorSetWithTemplateKHR(
 
    const struct anv_descriptor_set_layout *set_layout =
       layout->set[_set].layout;
-   struct anv_descriptor_set *set = &cmd_buffer->state.push_descriptor.set;
+
+   if (anv_cmd_buffer_ensure_push_descriptor_set(cmd_buffer, _set) != VK_SUCCESS)
+      return;
+   struct anv_push_descriptor_set *push_set =
+      cmd_buffer->state.push_descriptors[_set];
+   struct anv_descriptor_set *set = &push_set->set;
 
    set->layout = set_layout;
    set->size = anv_descriptor_set_layout_size(set_layout);
    set->buffer_count = set_layout->buffer_count;
-   set->buffer_views = cmd_buffer->state.push_descriptor.buffer_views;
+   set->buffer_views = push_set->buffer_views;
 
    anv_descriptor_set_write_template(set,
                                      cmd_buffer->device,
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index b58c803530c..0134387a70d 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -1268,7 +1268,6 @@ struct anv_push_descriptor_set {
    /* Put this field right behind anv_descriptor_set so it fills up the
     * descriptors[0] field. */
    struct anv_descriptor descriptors[MAX_PUSH_DESCRIPTORS];
-
    struct anv_buffer_view buffer_views[MAX_PUSH_DESCRIPTORS];
 };
 
@@ -1679,7 +1678,7 @@ struct anv_cmd_state {
    struct anv_dynamic_state                     dynamic;
    bool                                         need_query_wa;
 
-   struct anv_push_descriptor_set               push_descriptor;
+   struct anv_push_descriptor_set *             push_descriptors[MAX_SETS];
 
    /**
     * Whether or not the gen8 PMA fix is enabled.  We ensure that, at the top
-- 
2.14.2