[Mesa-dev] [PATCH 8/9] anv: elide relocations to pinned target bos

[Jason Ekstrand]

If I'm honest, I don't really like the way this patch worked out.  It has
the virtue of being fairly simple and a nice incremental change.  However,
it seems to me like we should be able to do better.  That said, I don't
really know how off-hand and this looks correct and gets us going.

On Wed, May 2, 2018 at 9:01 AM, Scott D Phillips <scott.d.phillips at intel.com
> wrote:

> References to pinned bos won't need relocated, so just write the
> final value of the reference into the bo. Add a `set` to the
> relocation lists for tracking dependencies that were previously
> tracked by relocations.
> ---
>  src/intel/vulkan/anv_batch_chain.c | 36 ++++++++++++++++++++++++++++++
> ++++++
>  src/intel/vulkan/anv_private.h     |  3 +++
>  2 files changed, 39 insertions(+)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> index 52f69045519..a4856376d8d 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -75,11 +75,24 @@ anv_reloc_list_init_clone(struct anv_reloc_list *list,
>        return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
>     }
>
> +   list->deps = _mesa_set_create(NULL, _mesa_hash_pointer,
> +                                 _mesa_key_pointer_equal);
> +
> +   if (!list->deps) {
> +      vk_free(alloc, list->relocs);
> +      vk_free(alloc, list->reloc_bos);
> +      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> +   }
> +
>     if (other_list) {
>        memcpy(list->relocs, other_list->relocs,
>               list->array_length * sizeof(*list->relocs));
>        memcpy(list->reloc_bos, other_list->reloc_bos,
>               list->array_length * sizeof(*list->reloc_bos));
> +      struct set_entry *entry;
> +      set_foreach(other_list->deps, entry) {
> +         _mesa_set_add_pre_hashed(list->deps, entry->hash, entry->key);
> +      }
>     }
>
>     return VK_SUCCESS;
> @@ -98,6 +111,7 @@ anv_reloc_list_finish(struct anv_reloc_list *list,
>  {
>     vk_free(alloc, list->relocs);
>     vk_free(alloc, list->reloc_bos);
> +   _mesa_set_destroy(list->deps, NULL);
>  }
>
>  static VkResult
> @@ -148,6 +162,11 @@ anv_reloc_list_add(struct anv_reloc_list *list,
>     struct drm_i915_gem_relocation_entry *entry;
>     int index;
>
> +   if (target_bo->flags & EXEC_OBJECT_PINNED) {
> +      _mesa_set_add(list->deps, target_bo);
> +      return VK_SUCCESS;
> +   }
> +
>     VkResult result = anv_reloc_list_grow(list, alloc, 1);
>     if (result != VK_SUCCESS)
>        return result;
> @@ -185,6 +204,12 @@ anv_reloc_list_append(struct anv_reloc_list *list,
>        list->relocs[i + list->num_relocs].offset += offset;
>
>     list->num_relocs += other->num_relocs;
> +
> +   struct set_entry *entry;
> +   set_foreach(other->deps, entry) {
> +      _mesa_set_add_pre_hashed(list->deps, entry->hash, entry->key);
> +   }
> +
>     return VK_SUCCESS;
>  }
>
> @@ -338,6 +363,7 @@ anv_batch_bo_start(struct anv_batch_bo *bbo, struct
> anv_batch *batch,
>     batch->end = bbo->bo.map + bbo->bo.size - batch_padding;
>     batch->relocs = &bbo->relocs;
>     bbo->relocs.num_relocs = 0;
> +   _mesa_set_clear(bbo->relocs.deps, NULL);
>  }
>
>  static void
> @@ -785,6 +811,7 @@ anv_cmd_buffer_reset_batch_bo_chain(struct
> anv_cmd_buffer *cmd_buffer)
>     cmd_buffer->bt_next = 0;
>
>     cmd_buffer->surface_relocs.num_relocs = 0;
> +   _mesa_set_clear(cmd_buffer->surface_relocs.deps, NULL);
>     cmd_buffer->last_ss_pool_center = 0;
>
>     /* Reset the list of seen buffers */
> @@ -1070,6 +1097,15 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
>           if (result != VK_SUCCESS)
>              return result;
>        }
> +
> +      struct set_entry *entry;
> +      set_foreach(relocs->deps, entry) {
> +         VkResult result = anv_execbuf_add_bo(exec, entry->key, NULL,
> +                                              extra_flags, alloc);
>

Usually, when walking hash sets, it's good to sort them by something (such
as GEM handle) so we get some amount of determinism.  I'd like to think it
doesn't matter but I know better. :-)


> +
> +         if (result != VK_SUCCESS)
> +            return result;
> +      }
>     }
>
>     return VK_SUCCESS;
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 81d50b3e770..3a448e41bae 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -46,7 +46,9 @@
>  #include "blorp/blorp.h"
>  #include "compiler/brw_compiler.h"
>  #include "util/macros.h"
> +#include "util/hash_table.h"
>
 #include "util/list.h"
> +#include "util/set.h"
>  #include "util/u_atomic.h"
>  #include "util/u_vector.h"
>  #include "util/vma.h"
> @@ -1047,6 +1049,7 @@ struct anv_reloc_list {
>     uint32_t                                     array_length;
>     struct drm_i915_gem_relocation_entry *       relocs;
>     struct anv_bo **                             reloc_bos;
> +   struct set *                                 deps;
>  };
>
>  VkResult anv_reloc_list_init(struct anv_reloc_list *list,
> --
> 2.14.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180503/e789c32e/attachment.html>