From 95dc34cd97a9cf909267a1c0fd625c8b5dc0a5ba Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Thu, 11 Apr 2024 15:34:24 -0700 Subject: [PATCH] anv/sparse: replace device->using_sparse with device->num_sparse_resources The device->using_sparse variable is only used at cmd_buffer_barrier() to decide if we need to apply the heavier-weight flushes that are only applicable to sparse resources. The big problem here is that we need to apply the flushes to the non-image and non-buffer memory barriers, so we were trying to limit those only to applications that ever submit a sparse resource to the sparse queue. The reason why we were applying this only to devices that ever submitted sparse resources is that dxvk games have this thing where during startup they create and then delete tiny sparse resources, so switching device->using_sparse to true at resource creation would make basically every dxvk game start applying the heavier-weight workaround. The problem with all that is that even if an application creates a sparse resource but doesn't ever bind them, the resource should still behave as an unbound resource (because they are bound with a NULL bind), so the flushes affecting them should happen. This case is exercised by vkd3d-proton/test_buffer_feedback_instructions_sm51. In order to satisfy all the above cases and only really apply the heavier-weight flushes to applications actually using sparse resources, let's just count the number of sparse resources that currently exist and then apply the workaround only if it's not zero. That covers the dxvk case since dxvk deletes the resources as soon as they create, so num_sparse_resources goes back to 0. Testcase: vkd3d-proton/test_buffer_feedback_instructions_sm51 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10960 Fixes: 6368c1445f44 ("anv/sparse: add the initial code for Sparse Resources") Reviewed-by: Lionel Landwerlin Signed-off-by: Paulo Zanoni Part-of: --- src/intel/vulkan/anv_batch_chain.c | 2 -- src/intel/vulkan/anv_private.h | 12 ++++++------ src/intel/vulkan/anv_sparse.c | 3 +++ src/intel/vulkan/genX_cmd_buffer.c | 3 ++- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 4117481006c..bb986847a08 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1394,8 +1394,6 @@ anv_queue_submit_sparse_bind_locked(struct anv_queue *queue, return vk_queue_set_lost(&queue->vk, "Sparse binding not supported"); } - device->using_sparse = true; - assert(submit->command_buffer_count == 0); if (INTEL_DEBUG(DEBUG_SPARSE)) { diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 641330f11c4..049c2edfd3c 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1968,13 +1968,13 @@ struct anv_device { struct list_head in_flight_batches; } trtt; - /* This is true if the user ever bound a sparse resource to memory. This - * is used for a workaround that makes every memoryBarrier flush more - * things than it should. Many applications request for the sparse - * featuers to be enabled but don't use them, and some create sparse - * resources but never use them. + /* Number of sparse resources that currently exist. This is used for a + * workaround that makes every memoryBarrier flush more things than it + * should. Some workloads create and then immediately destroy sparse + * resources when they start, so just counting if a sparse resource was + * ever created is not enough. */ - bool using_sparse; + uint32_t num_sparse_resources; struct anv_device_astc_emu astc_emu; diff --git a/src/intel/vulkan/anv_sparse.c b/src/intel/vulkan/anv_sparse.c index 1bd403cee0c..a4fa29462f4 100644 --- a/src/intel/vulkan/anv_sparse.c +++ b/src/intel/vulkan/anv_sparse.c @@ -654,6 +654,7 @@ anv_init_sparse_bindings(struct anv_device *device, return res; } + p_atomic_inc(&device->num_sparse_resources); return VK_SUCCESS; } @@ -667,6 +668,8 @@ anv_free_sparse_bindings(struct anv_device *device, sparse_debug("%s: address:0x%016"PRIx64" size:0x%08"PRIx64"\n", __func__, sparse->address, sparse->size); + p_atomic_dec(&device->num_sparse_resources); + struct anv_vm_bind unbind = { .bo = 0, .address = sparse->address, diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index ca5ad262a30..eed57513cf9 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -3939,7 +3939,8 @@ cmd_buffer_barrier(struct anv_cmd_buffer *cmd_buffer, /* There's no way of knowing if this memory barrier is related to sparse * buffers! This is pretty horrible. */ - if (device->using_sparse && mask_is_write(src_flags)) + if (mask_is_write(src_flags) && + p_atomic_read(&device->num_sparse_resources) > 0) apply_sparse_flushes = true; }