[Mesa-dev] [PATCH 7/9] anv: use a separate pool for binding tables when soft pinning
Jason Ekstrand
jason at jlekstrand.net
Fri May 4 03:41:33 UTC 2018
On Wed, May 2, 2018 at 9:01 AM, Scott D Phillips <scott.d.phillips at intel.com
> wrote:
> Soft pinning lets us satisfy the binding table address
> requirements without using both sides of a growing state_pool.
>
> If you do use both sides of a state pool, then you need to read
> the state pool's center_bo_offset (with the device mutex held) to
> know the final offset of relocations that target the state pool
> bo.
>
> By having a separate pool for binding tables that only grows in
> the forward direction, the center_bo_offset is always 0 and
> relocations don't need an update pass to adjust relocations with
> the mutex held.
> ---
> src/intel/vulkan/anv_batch_chain.c | 23 +++++++++++++++--------
> src/intel/vulkan/anv_device.c | 28 +++++++++++++++++++++++++++-
> src/intel/vulkan/anv_private.h | 36 ++++++++++++++++++++++++++++++
> ++++++
> 3 files changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> index 09514c7b84a..52f69045519 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -452,7 +452,7 @@ anv_cmd_buffer_surface_base_address(struct
> anv_cmd_buffer *cmd_buffer)
> {
> struct anv_state *bt_block = u_vector_head(&cmd_buffer->bt_
> block_states);
> return (struct anv_address) {
> - .bo = &cmd_buffer->device->surface_state_pool.block_pool.bo,
> + .bo = anv_binding_table_pool_bo(cmd_buffer->device),
> .offset = bt_block->offset,
> };
> }
> @@ -619,7 +619,8 @@ struct anv_state
> anv_cmd_buffer_alloc_binding_table(struct anv_cmd_buffer *cmd_buffer,
> uint32_t entries, uint32_t
> *state_offset)
> {
> - struct anv_state_pool *state_pool = &cmd_buffer->device->surface_
> state_pool;
> + struct anv_device *device = cmd_buffer->device;
> + struct anv_state_pool *state_pool = &device->surface_state_pool;
> struct anv_state *bt_block = u_vector_head(&cmd_buffer->bt_
> block_states);
> struct anv_state state;
>
> @@ -629,12 +630,18 @@ anv_cmd_buffer_alloc_binding_table(struct
> anv_cmd_buffer *cmd_buffer,
> return (struct anv_state) { 0 };
>
> state.offset = cmd_buffer->bt_next;
> - state.map = state_pool->block_pool.map + bt_block->offset +
> state.offset;
> + state.map = anv_binding_table_pool_map(device) + bt_block->offset +
> + state.offset;
>
> cmd_buffer->bt_next += state.alloc_size;
>
> - assert(bt_block->offset < 0);
> - *state_offset = -bt_block->offset;
> + if (device->use_separate_binding_table_pool) {
> + *state_offset = device->surface_state_pool.block_pool.offset -
> + device->binding_table_pool.block_pool.offset - bt_block->offset;
> + } else {
> + assert(bt_block->offset < 0);
> + *state_offset = -bt_block->offset;
> + }
>
> return state;
> }
> @@ -666,7 +673,7 @@ anv_cmd_buffer_new_binding_table_block(struct
> anv_cmd_buffer *cmd_buffer)
> return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> }
>
> - *bt_block = anv_state_pool_alloc_back(state_pool);
> + *bt_block = anv_binding_table_pool_alloc(cmd_buffer->device);
> cmd_buffer->bt_next = 0;
>
> return VK_SUCCESS;
> @@ -740,7 +747,7 @@ anv_cmd_buffer_fini_batch_bo_chain(struct
> anv_cmd_buffer *cmd_buffer)
> {
> struct anv_state *bt_block;
> u_vector_foreach(bt_block, &cmd_buffer->bt_block_states)
> - anv_state_pool_free(&cmd_buffer->device->surface_state_pool,
> *bt_block);
> + anv_binding_table_pool_free(cmd_buffer->device, *bt_block);
> u_vector_finish(&cmd_buffer->bt_block_states);
>
> anv_reloc_list_finish(&cmd_buffer->surface_relocs,
> &cmd_buffer->pool->alloc);
> @@ -772,7 +779,7 @@ anv_cmd_buffer_reset_batch_bo_chain(struct
> anv_cmd_buffer *cmd_buffer)
>
> while (u_vector_length(&cmd_buffer->bt_block_states) > 1) {
> struct anv_state *bt_block = u_vector_remove(&cmd_buffer->
> bt_block_states);
> - anv_state_pool_free(&cmd_buffer->device->surface_state_pool,
> *bt_block);
> + anv_binding_table_pool_free(cmd_buffer->device, *bt_block);
> }
> assert(u_vector_length(&cmd_buffer->bt_block_states) == 1);
> cmd_buffer->bt_next = 0;
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 2837d2f83ca..d1f57081b77 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -1637,9 +1637,32 @@ VkResult anv_CreateDevice(
> if (result != VK_SUCCESS)
> goto fail_instruction_state_pool;
>
> + device->use_separate_binding_table_pool = physical_device->has_exec_
> softpin;
> +
> + if (device->use_separate_binding_table_pool) {
> + result = anv_state_pool_init(&device->binding_table_pool, device,
> 4096,
> + bo_flags);
> + if (result != VK_SUCCESS)
> + goto fail_surface_state_pool;
> +
> + if (device->surface_state_pool.block_pool.offset <
> + device->binding_table_pool.block_pool.offset) {
> +
> + uint64_t tmp;
> + tmp = device->surface_state_pool.block_pool.offset;
> + device->surface_state_pool.block_pool.offset =
> + device->binding_table_pool.block_pool.offset;
> + device->binding_table_pool.block_pool.offset = tmp;
> + tmp = device->surface_state_pool.block_pool.bo.offset;
> + device->surface_state_pool.block_pool.bo.offset =
> + device->binding_table_pool.block_pool.bo.offset;
> + device->binding_table_pool.block_pool.bo.offset = tmp;
>
If we hard-code the memory layout, this should go away, I think.
> + }
> + }
> +
> result = anv_bo_init_new(&device->workaround_bo, device, 1024);
> if (result != VK_SUCCESS)
> - goto fail_surface_state_pool;
> + goto fail_binding_table_pool;
>
> anv_device_init_trivial_batch(device);
>
> @@ -1690,6 +1713,9 @@ VkResult anv_CreateDevice(
> anv_scratch_pool_finish(device, &device->scratch_pool);
> anv_gem_munmap(device->workaround_bo.map, device->workaround_bo.size);
> anv_gem_close(device, device->workaround_bo.gem_handle);
> + fail_binding_table_pool:
> + if (device->use_separate_binding_table_pool)
> + anv_state_pool_finish(&device->binding_table_pool);
> fail_surface_state_pool:
> anv_state_pool_finish(&device->surface_state_pool);
> fail_instruction_state_pool:
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 23527eebaab..81d50b3e770 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -916,6 +916,9 @@ struct anv_device {
> struct anv_state_pool instruction_state_pool;
> struct anv_state_pool surface_state_pool;
>
> + bool
> use_separate_binding_table_pool;
>
Do we need a separate bool? I suspect not.
> + struct anv_state_pool binding_table_pool;
> +
> struct anv_bo workaround_bo;
> struct anv_bo trivial_batch_bo;
> struct anv_bo hiz_clear_bo;
> @@ -936,6 +939,39 @@ struct anv_device {
> bool lost;
> };
>
> +static inline struct anv_bo*
> +anv_binding_table_pool_bo(struct anv_device *device) {
> + if (device->use_separate_binding_table_pool)
> + return &device->binding_table_pool.block_pool.bo;
> + else
> + return &device->surface_state_pool.block_pool.bo;
> +}
> +
> +static inline void*
> +anv_binding_table_pool_map(struct anv_device *device) {
> + if (device->use_separate_binding_table_pool)
> + return device->binding_table_pool.block_pool.map;
> + else
> + return device->surface_state_pool.block_pool.map;
> +}
> +
> +static inline struct anv_state
> +anv_binding_table_pool_alloc(struct anv_device *device) {
> + if (device->use_separate_binding_table_pool)
> + return anv_state_pool_alloc(&device->binding_table_pool,
> + device->binding_table_pool.block_size,
> 0);
> + else
> + return anv_state_pool_alloc_back(&device->surface_state_pool);
> +}
> +
> +static inline void
> +anv_binding_table_pool_free(struct anv_device *device, struct anv_state
> state) {
> + if (device->use_separate_binding_table_pool)
> + anv_state_pool_free(&device->binding_table_pool, state);
> + else
> + anv_state_pool_free(&device->surface_state_pool, state);
> +}
>
Three of these four would be a lot simpler if we had a fifth
static inline anv_state_pool *
anv_binding_table_pool(struct anv_device *device)
{
if (device->use_separate_binding_table_pool)
return &device->binding_table_pool;
else
return &device->surface_state_pool;
}
For that matter, the first two might not be needed at all.
> +
> static void inline
> anv_state_flush(struct anv_device *device, struct anv_state state)
> {
> --
> 2.14.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180503/208e14bb/attachment-0001.html>
More information about the mesa-dev
mailing list