From 0802bbd486415026ce98bbba97e2be012637db97 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 27 Aug 2024 16:06:28 -0700 Subject: [PATCH] anv/trtt: don't submit empty batches when there are no binds to do The application can submit bind operations where it simply resets state that is already in our page tables, so there's nothing to do. Before commit 7da5b1caef21 ("anv: move trtt submissions over to the anv_async_submit") we would simply return and not submit any batches when this happened, but the commit reorganized things in a way where we started submitting empty batches instead. Fix this by simply jumping out when we detect this case. Because of this, rename the "error" labels to "out" as they can now happen on a happy case. It should be noted that an alternative to this implementation would be to move all the handling of 'submit' to after the n_lX_binds check, but this would put all the initialization inside the trtt->mutex, creating extra contention even when we have stuff to bind. Since the "there's nothing to bind" check is now rare (after we stopped doing NULL binds during resource creation), it is probably better to reduce lock contention in the common case at the expense of a little more CPU in the rare case. Reviewed-by: Lionel Landwerlin Signed-off-by: Paulo Zanoni Part-of: --- src/intel/vulkan/anv_sparse.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/intel/vulkan/anv_sparse.c b/src/intel/vulkan/anv_sparse.c index cd85721e4e7..1ac5684ae03 100644 --- a/src/intel/vulkan/anv_sparse.c +++ b/src/intel/vulkan/anv_sparse.c @@ -736,7 +736,7 @@ anv_sparse_bind_trtt(struct anv_device *device, &device->batch_bo_pool, false, false); if (result != VK_SUCCESS) - goto error_async; + goto out_async; simple_mtx_lock(&trtt->mutex); @@ -787,7 +787,7 @@ anv_sparse_bind_trtt(struct anv_device *device, l3l2_binds, &n_l3l2_binds, l1_binds, &n_l1_binds); if (result != VK_SUCCESS) - goto error_stack_arrays; + goto out_stack_arrays; } } @@ -800,10 +800,13 @@ anv_sparse_bind_trtt(struct anv_device *device, sparse_debug("trtt_binds: num_vm_binds:%02d l3l2:%04d l1:%04d\n", sparse_submit->binds_len, n_l3l2_binds, n_l1_binds); - if (n_l3l2_binds || n_l1_binds) { - anv_genX(device->info, write_trtt_entries)( - &submit->base, l3l2_binds, n_l3l2_binds, l1_binds, n_l1_binds); - } + /* This is not an error, the application is simply trying to reset state + * that was already there. */ + if (n_l3l2_binds == 0 && n_l1_binds == 0) + goto out_stack_arrays; + + anv_genX(device->info, write_trtt_entries)( + &submit->base, l3l2_binds, n_l3l2_binds, l1_binds, n_l1_binds); STACK_ARRAY_FINISH(l1_binds); STACK_ARRAY_FINISH(l3l2_binds); @@ -812,7 +815,7 @@ anv_sparse_bind_trtt(struct anv_device *device, if (submit->base.batch.status != VK_SUCCESS) { result = submit->base.batch.status; - goto error_add_bind; + goto out_add_bind; } /* Add all the BOs backing TRTT page tables to the reloc list. @@ -825,7 +828,7 @@ anv_sparse_bind_trtt(struct anv_device *device, result = anv_reloc_list_add_bo(&submit->base.relocs, trtt->page_table_bos[i]); if (result != VK_SUCCESS) - goto error_add_bind; + goto out_add_bind; } } @@ -836,7 +839,7 @@ anv_sparse_bind_trtt(struct anv_device *device, sparse_submit->signal_count, sparse_submit->signals); if (result != VK_SUCCESS) - goto error_add_bind; + goto out_add_bind; list_addtail(&submit->link, &trtt->in_flight_batches); @@ -847,13 +850,13 @@ anv_sparse_bind_trtt(struct anv_device *device, return VK_SUCCESS; - error_stack_arrays: + out_stack_arrays: STACK_ARRAY_FINISH(l1_binds); STACK_ARRAY_FINISH(l3l2_binds); - error_add_bind: + out_add_bind: simple_mtx_unlock(&trtt->mutex); anv_async_submit_fini(&submit->base); - error_async: + out_async: vk_free(&device->vk.alloc, submit); return result; }