[PATCH v2 0/4] MR260: vkd3d: Prepare for buffering of command list commands.
-- v2: vkd3d: Add a pending execution count to struct d3d12_command_signature. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260
From: Conor McCarthy <cmccarthy(a)codeweavers.com> This is the simplest way to gather this information when command lists are executed later. --- libs/vkd3d/command.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 1fc6c00d..1eaf60ea 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -3186,6 +3186,20 @@ static void command_list_flush_vk_heap_updates(struct d3d12_command_list *list) } } +static void command_list_add_descriptor_heap(struct d3d12_command_list *list, struct d3d12_descriptor_heap *heap) +{ + if (!contains_heap(list->descriptor_heaps, list->descriptor_heap_count, heap)) + { + if (list->descriptor_heap_count == ARRAY_SIZE(list->descriptor_heaps)) + { + /* Descriptors can be written after binding. */ + FIXME("Flushing descriptor updates while list %p is not closed.\n", list); + command_list_flush_vk_heap_updates(list); + } + list->descriptor_heaps[list->descriptor_heap_count++] = heap; + } +} + static void d3d12_command_list_bind_descriptor_heap(struct d3d12_command_list *list, enum vkd3d_pipeline_bind_point bind_point, struct d3d12_descriptor_heap *heap) { @@ -3210,18 +3224,6 @@ static void d3d12_command_list_bind_descriptor_heap(struct d3d12_command_list *l bindings->sampler_heap_id = heap->serial_id; } - if (!contains_heap(list->descriptor_heaps, list->descriptor_heap_count, heap)) - { - if (list->descriptor_heap_count == ARRAY_SIZE(list->descriptor_heaps)) - { - /* Descriptors can be written after binding. */ - FIXME("Flushing descriptor updates while list %p is not closed.\n", list); - command_list_flush_vk_heap_updates(list); - list->descriptor_heap_count = 0; - } - list->descriptor_heaps[list->descriptor_heap_count++] = heap; - } - vkd3d_mutex_lock(&heap->vk_sets_mutex); for (set = 0; set < ARRAY_SIZE(heap->vk_descriptor_sets); ++set) @@ -4419,6 +4421,8 @@ static void d3d12_command_list_set_descriptor_table(struct d3d12_command_list *l if (bindings->descriptor_tables[index] == desc) return; + command_list_add_descriptor_heap(list, d3d12_desc_get_descriptor_heap(desc)); + bindings->descriptor_tables[index] = desc; bindings->descriptor_table_dirty_mask |= (uint64_t)1 << index; bindings->descriptor_table_active_mask |= (uint64_t)1 << index; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260
From: Conor McCarthy <cmccarthy(a)codeweavers.com> For correct function it cannot be freed until the command allocator is reset, and d3d12_command_allocator_free_command_buffer() normally does not free it. --- libs/vkd3d/command.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 1eaf60ea..caf9964d 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -1316,32 +1316,26 @@ static HRESULT d3d12_command_allocator_allocate_command_buffer(struct d3d12_comm return hr; } - allocator->current_command_list = list; - - return S_OK; -} - -static void d3d12_command_allocator_free_command_buffer(struct d3d12_command_allocator *allocator, - struct d3d12_command_list *list) -{ - struct d3d12_device *device = allocator->device; - const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - - TRACE("allocator %p, list %p.\n", allocator, list); - - if (allocator->current_command_list == list) - allocator->current_command_list = NULL; - if (!vkd3d_array_reserve((void **)&allocator->command_buffers, &allocator->command_buffers_size, allocator->command_buffer_count + 1, sizeof(*allocator->command_buffers))) { WARN("Failed to add command buffer.\n"); VK_CALL(vkFreeCommandBuffers(device->vk_device, allocator->vk_command_pool, 1, &list->vk_command_buffer)); - return; + return E_OUTOFMEMORY; } - allocator->command_buffers[allocator->command_buffer_count++] = list->vk_command_buffer; + + allocator->current_command_list = list; + + return S_OK; +} + +static void d3d12_command_allocator_remove_command_list(struct d3d12_command_allocator *allocator, + const struct d3d12_command_list *list) +{ + if (allocator->current_command_list == list) + allocator->current_command_list = NULL; } static bool d3d12_command_allocator_add_render_pass(struct d3d12_command_allocator *allocator, VkRenderPass pass) @@ -2314,7 +2308,7 @@ static ULONG STDMETHODCALLTYPE d3d12_command_list_Release(ID3D12GraphicsCommandL /* When command pool is destroyed, all command buffers are implicitly freed. */ if (list->allocator) - d3d12_command_allocator_free_command_buffer(list->allocator, list); + d3d12_command_allocator_remove_command_list(list->allocator, list); vkd3d_pipeline_bindings_cleanup(&list->pipeline_bindings[VKD3D_PIPELINE_BIND_POINT_COMPUTE]); vkd3d_pipeline_bindings_cleanup(&list->pipeline_bindings[VKD3D_PIPELINE_BIND_POINT_GRAPHICS]); @@ -2412,7 +2406,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_list_Close(ID3D12GraphicsCommandL if (list->allocator) { - d3d12_command_allocator_free_command_buffer(list->allocator, list); + d3d12_command_allocator_remove_command_list(list->allocator, list); list->allocator = NULL; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260
From: Conor McCarthy <cmccarthy(a)codeweavers.com> D3D12 docs are silent on a required lifetime and immediate release works in Windows. --- tests/d3d12.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/d3d12.c b/tests/d3d12.c index 2f1c905f..3948a491 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -24328,6 +24328,7 @@ static void test_execute_indirect(void) ID3D12CommandQueue *queue; ID3D12Resource *vb, *ib; unsigned int i; + ULONG refcount; D3D12_BOX box; HRESULT hr; @@ -24507,13 +24508,15 @@ static void test_execute_indirect(void) ID3D12GraphicsCommandList_ExecuteIndirect(command_list, command_signature, 4, argument_buffer, 0, count_buffer, 0); + refcount = ID3D12CommandSignature_Release(command_signature); + ok(!refcount, "ID3D12CommandSignature has %u references left.\n", refcount); + transition_resource_state(command_list, context.render_target, D3D12_RESOURCE_STATE_RENDER_TARGET, D3D12_RESOURCE_STATE_COPY_SOURCE); check_sub_resource_uint(context.render_target, 0, queue, command_list, 0xff00ff00, 0); reset_command_list(command_list, context.allocator); - ID3D12CommandSignature_Release(command_signature); command_signature = create_command_signature(context.device, D3D12_INDIRECT_ARGUMENT_TYPE_DISPATCH); uav = create_default_buffer(context.device, 2 * 3 * 4 * sizeof(UINT), @@ -24540,6 +24543,9 @@ static void test_execute_indirect(void) ID3D12GraphicsCommandList_ExecuteIndirect(command_list, command_signature, 1, argument_buffer, offsetof(struct argument_data, dispatch), NULL, 0); + refcount = ID3D12CommandSignature_Release(command_signature); + ok(!refcount, "ID3D12CommandSignature has %u references left.\n", refcount); + transition_sub_resource_state(command_list, uav, 0, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_SOURCE); get_buffer_readback_with_command_list(uav, DXGI_FORMAT_R32_UINT, &rb, queue, command_list); @@ -24554,7 +24560,6 @@ static void test_execute_indirect(void) transition_resource_state(command_list, context.render_target, D3D12_RESOURCE_STATE_COPY_SOURCE, D3D12_RESOURCE_STATE_RENDER_TARGET); - ID3D12CommandSignature_Release(command_signature); command_signature = create_command_signature(context.device, D3D12_INDIRECT_ARGUMENT_TYPE_DRAW_INDEXED); ID3D12GraphicsCommandList_ClearRenderTargetView(command_list, context.rtv, white, 0, NULL); @@ -24600,6 +24605,9 @@ static void test_execute_indirect(void) ARRAY_SIZE(argument_data.indexed_draws), argument_buffer, offsetof(struct argument_data, indexed_draws), count_buffer, sizeof(uint32_t)); + refcount = ID3D12CommandSignature_Release(command_signature); + ok(!refcount, "ID3D12CommandSignature has %u references left.\n", refcount); + transition_resource_state(command_list, context.render_target, D3D12_RESOURCE_STATE_RENDER_TARGET, D3D12_RESOURCE_STATE_COPY_SOURCE); check_sub_resource_uint(context.render_target, 0, queue, command_list, 0xffffff00, 0); @@ -24609,7 +24617,6 @@ static void test_execute_indirect(void) ID3D12Resource_Release(ib); ID3D12Resource_Release(uav); ID3D12Resource_Release(vb); - ID3D12CommandSignature_Release(command_signature); ID3D12Resource_Release(argument_buffer); ID3D12Resource_Release(count_buffer); destroy_test_context(&context); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Buffering of command list commands will depend upon command signatures persisting until the buffer is flushed. --- libs/vkd3d/command.c | 19 ++++++++++++++++++- libs/vkd3d/vkd3d_private.h | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index caf9964d..c4527195 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -1905,6 +1905,17 @@ HRESULT d3d12_command_allocator_create(struct d3d12_device *device, return S_OK; } +static void d3d12_command_signature_incref(struct d3d12_command_signature *signature) +{ + vkd3d_atomic_increment(&signature->internal_refcount); +} + +static void d3d12_command_signature_decref(struct d3d12_command_signature *signature) +{ + unsigned int refcount = vkd3d_atomic_decrement(&signature->internal_refcount); + assert(refcount); +} + /* ID3D12CommandList */ static inline struct d3d12_command_list *impl_from_ID3D12GraphicsCommandList2(ID3D12GraphicsCommandList2 *iface) { @@ -5713,6 +5724,8 @@ static void STDMETHODCALLTYPE d3d12_command_list_ExecuteIndirect(ID3D12GraphicsC return; } + d3d12_command_signature_incref(sig_impl); + signature_desc = &sig_impl->desc; for (i = 0; i < signature_desc->NumArgumentDescs; ++i) { @@ -5775,6 +5788,7 @@ static void STDMETHODCALLTYPE d3d12_command_list_ExecuteIndirect(ID3D12GraphicsC if (!d3d12_command_list_update_compute_state(list)) { WARN("Failed to update compute state, ignoring dispatch.\n"); + d3d12_command_signature_decref(sig_impl); return; } @@ -5787,6 +5801,8 @@ static void STDMETHODCALLTYPE d3d12_command_list_ExecuteIndirect(ID3D12GraphicsC break; } } + + d3d12_command_signature_decref(sig_impl); } static void STDMETHODCALLTYPE d3d12_command_list_AtomicCopyBufferUINT(ID3D12GraphicsCommandList2 *iface, @@ -7252,7 +7268,7 @@ static ULONG STDMETHODCALLTYPE d3d12_command_signature_Release(ID3D12CommandSign TRACE("%p decreasing refcount to %u.\n", signature, refcount); - if (!refcount) + if (!refcount && !vkd3d_atomic_decrement(&signature->internal_refcount)) { struct d3d12_device *device = signature->device; @@ -7369,6 +7385,7 @@ HRESULT d3d12_command_signature_create(struct d3d12_device *device, const D3D12_ object->ID3D12CommandSignature_iface.lpVtbl = &d3d12_command_signature_vtbl; object->refcount = 1; + object->internal_refcount = 1; object->desc = *desc; if (!(object->desc.pArgumentDescs = vkd3d_calloc(desc->NumArgumentDescs, sizeof(*desc->pArgumentDescs)))) diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index c5259420..05e0b784 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -253,6 +253,11 @@ static inline void vkd3d_cond_destroy(struct vkd3d_cond *cond) { } +static inline unsigned int vkd3d_atomic_increment(unsigned int volatile *x) +{ + return InterlockedIncrement((LONG volatile *)x); +} + static inline unsigned int vkd3d_atomic_decrement(unsigned int volatile *x) { return InterlockedDecrement((LONG volatile *)x); @@ -387,6 +392,15 @@ static inline unsigned int vkd3d_atomic_decrement(unsigned int volatile *x) } # else # error "vkd3d_atomic_decrement() not implemented for this platform" +# endif /* HAVE_SYNC_SUB_AND_FETCH */ + +# if HAVE_SYNC_ADD_AND_FETCH +static inline unsigned int vkd3d_atomic_increment(unsigned int volatile *x) +{ + return __sync_add_and_fetch(x, 1); +} +# else +# error "vkd3d_atomic_increment() not implemented for this platform" # endif /* HAVE_SYNC_ADD_AND_FETCH */ # if HAVE_SYNC_BOOL_COMPARE_AND_SWAP @@ -1575,6 +1589,7 @@ struct d3d12_command_signature { ID3D12CommandSignature ID3D12CommandSignature_iface; LONG refcount; + unsigned int internal_refcount; D3D12_COMMAND_SIGNATURE_DESC desc; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260
On Wed Jul 19 02:27:41 2023 +0000, Conor McCarthy wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/260/diffs?diff_id=58206&start_sha=602b59cbc328b56ee57b7a7ab7e4b4c2c1d4e8fc#6ce56346069f6ab6f2e9a298a46873ae68494ff2_1927_1915) Oops. I changed it to an internal refcount initialised to 1 as we do elsewhere.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260#note_39369
On Wed Jul 19 02:28:43 2023 +0000, Conor McCarthy wrote:
Oops. I changed it to an internal refcount initialised to 1 as we do elsewhere. With the current (new) code, `d3d12_command_signature_decref` can't actually free anything, relying on `d3d12_command_signature_Release` to do the freeing. The assert makes it seem like `d3d12_command_signature_decref` is never expected to consume the final refcount, but if that's the case, what's the point of the secondary refcount anyways, if you know the final release will always be through the D3D method? Assuming you do actually need the secondary refcount, `d3d12_command_signature_decref` should probably be in charge of actually freeing, and `d3d12_command_signature_Release` should `if (!refcount) d3d12_command_signature_decref(signature)`
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/260#note_39493
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Evan Tang (＠etang-cw)