On Wed, Dec 12, 2018 at 7:15 PM Rafael Antognolli < [email protected]> wrote:
> On Mon, Dec 10, 2018 at 01:49:43PM -0600, Jason Ekstrand wrote: > > On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli < > [email protected]> > > 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. > > > > So, in the case of softpin, we need to pass the surface relocations at > some point, because the BO dependency is stored there. Unless we store > it somewhere else. > > If we use: > > anv_execbuf_add_bo(execbuf, bo, NULL, 0, &cmd_buffer->device->alloc); > > for the surface state pool, it seems like we end up missing some BOs in > the execbuf. > You're right. We need to at least handle the BO set in the reloc data structure. I'm not sure what the best way to do that is. I guess we could just process it on the first one or something like that. processing the BO set does involve a qsort so we probably don't want to do it too many extra times. --Jason > > > > + 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 > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > _______________________________________________ > > mesa-dev mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
