[Mesa-dev] [RFC PATCH 09/14] anv: Validate the list of BOs from the block pool.

[Jason Ekstrand]

On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli <
rafael.antognolli at intel.com> wrote:

> We now have multiple BOs in the block pool, but sometimes we still
> reference only the first one in some instructions, and use relative
> offsets in others. So we must be sure to add all the BOs from the block
> pool to the validation list when submitting commands.
> ---
>  src/intel/vulkan/anv_batch_chain.c | 47 ++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> index bec4d647b7e..65df28ccb91 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -1356,6 +1356,36 @@ relocate_cmd_buffer(struct anv_cmd_buffer
> *cmd_buffer,
>     return true;
>  }
>
> +static void
> +anv_reloc_list_add_dep(struct anv_cmd_buffer *cmd_buffer,
> +                       struct anv_bo_list *bo_list)
> +{
> +   struct anv_bo_list *iter;
> +   struct anv_bo *bo;
> +   struct anv_reloc_list *relocs = cmd_buffer->batch.relocs;
> +
> +   anv_block_pool_foreach_bo(bo_list, iter, bo) {
> +      _mesa_set_add(relocs->deps, bo);
> +   }
> +}
> +
> +static void
> +anv_batch_bos_add(struct anv_cmd_buffer *cmd_buffer)
> +{
> +   struct anv_bo_list *bo_list;
> +
> +   bo_list = cmd_buffer->device->dynamic_state_pool.block_pool.bos;
> +   anv_reloc_list_add_dep(cmd_buffer, bo_list);
> +
> +   bo_list = cmd_buffer->device->instruction_state_pool.block_pool.bos;
> +   anv_reloc_list_add_dep(cmd_buffer, bo_list);
> +
> +   if (cmd_buffer->device->instance->physicalDevice.use_softpin) {
> +      bo_list = cmd_buffer->device->binding_table_pool.block_pool.bos;
> +      anv_reloc_list_add_dep(cmd_buffer, bo_list);
>

I don't think want to add things to the command buffer's dependency set
this late (at submit time).  Instead, I think we want to just do
anv_execbuf_add_bo for each of them directly at the top of
setup_execbuf_for_cmd_buffer.


> +   }
> +}
> +
>  static VkResult
>  setup_execbuf_for_cmd_buffer(struct anv_execbuf *execbuf,
>                               struct anv_cmd_buffer *cmd_buffer)
> @@ -1364,13 +1394,20 @@ setup_execbuf_for_cmd_buffer(struct anv_execbuf
> *execbuf,
>     struct anv_state_pool *ss_pool =
>        &cmd_buffer->device->surface_state_pool;
>
> +   anv_batch_bos_add(cmd_buffer);
> +
>     adjust_relocations_from_state_pool(ss_pool,
> &cmd_buffer->surface_relocs,
>                                        cmd_buffer->last_ss_pool_center);
> -   VkResult result = anv_execbuf_add_bo(execbuf, ss_pool->block_pool.bo,
> -                                        &cmd_buffer->surface_relocs, 0,
> -                                        &cmd_buffer->device->alloc);
> -   if (result != VK_SUCCESS)
> -      return result;
> +   VkResult result;
> +   struct anv_bo *bo;
> +   struct anv_bo_list *iter;
> +   anv_block_pool_foreach_bo(ss_pool->block_pool.bos, iter, bo) {
> +      result = anv_execbuf_add_bo(execbuf, bo,
> +                                  &cmd_buffer->surface_relocs, 0,
> +                                  &cmd_buffer->device->alloc);
>

I don't think we want to pass the relocation list on every BO.  Instead, we
should have a softpin case where we walk the list and don't provide any
relocations and a non-softpin case where we assert that there is only one
BO and do provide the relocations.


> +      if (result != VK_SUCCESS)
> +         return result;
> +   }
>
>     /* First, we walk over all of the bos we've seen and add them and their
>      * relocations to the validate list.
> --
> 2.17.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/20181210/bb9e622b/attachment.html>