<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 2, 2018 at 9:01 AM, Scott D Phillips <<a href="mailto:scott.d.phillips@intel.com" target="_blank">scott.d.phillips@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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-><a href="http://surface_state_pool.block_pool.bo" rel="noreferrer" target="_blank">surface_state_pool.block_pool.bo</a>, + .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; </blockquote><div> </div><div>If we hard-code the memory layout, this should go away, I think. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } + } + 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; </blockquote><div> </div><div>Do we need a separate bool? I suspect not. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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-><a href="http://binding_table_pool.block_pool.bo" rel="noreferrer" target="_blank">binding_table_pool.block_pool.bo</a>; + else + return &device-><a href="http://surface_state_pool.block_pool.bo" rel="noreferrer" target="_blank">surface_state_pool.block_pool.bo</a>; +} + +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); +} </blockquote><div> </div><div>Three of these four would be a lot simpler if we had a fifth </div><div>static inline anv_state_pool * </div><div>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; } </div><div>For that matter, the first two might not be needed at all. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + static void inline anv_state_flush(struct anv_device *device, struct anv_state state) { -- 2.14.3 </blockquote></div> </div></div>