From cadcfe4b395735f5b1972ce5272f20c2beb6c732 Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Sun, 30 Mar 2025 16:52:58 +0000 Subject: [PATCH] [idd] driver: fix deadlock caused by command queue completion callback The callback runs in a random thread, we can't call directx methods safely from it, so move reset so it's called automatically when a free copy list is obtained. --- idd/LGIdd/CD3D12CommandQueue.cpp | 41 ++++++++++++++++++++----------- idd/LGIdd/CD3D12CommandQueue.h | 26 +++++++++++++------- idd/LGIdd/CD3D12Device.cpp | 10 +++++--- idd/LGIdd/CSwapChainProcessor.cpp | 33 ++++++++++++------------- idd/LGIdd/CSwapChainProcessor.h | 2 ++ 5 files changed, 69 insertions(+), 43 deletions(-) diff --git a/idd/LGIdd/CD3D12CommandQueue.cpp b/idd/LGIdd/CD3D12CommandQueue.cpp index 073defd9..d19e7842 100644 --- a/idd/LGIdd/CD3D12CommandQueue.cpp +++ b/idd/LGIdd/CD3D12CommandQueue.cpp @@ -1,7 +1,7 @@ #include "CD3D12CommandQueue.h" #include "CDebug.h" -bool CD3D12CommandQueue::Init(ID3D12Device3 * device, D3D12_COMMAND_LIST_TYPE type, const WCHAR* name) +bool CD3D12CommandQueue::Init(ID3D12Device3 * device, D3D12_COMMAND_LIST_TYPE type, const WCHAR* name, CallbackMode callbackMode) { HRESULT hr; D3D12_COMMAND_QUEUE_DESC queueDesc = {}; @@ -54,16 +54,22 @@ bool CD3D12CommandQueue::Init(ID3D12Device3 * device, D3D12_COMMAND_LIST_TYPE ty return false; } - RegisterWaitForSingleObject( - &m_waitHandle, - m_event.Get(), - [](PVOID param, BOOLEAN timeout){ - if (!timeout) - ((CD3D12CommandQueue*)param)->OnCompletion(); - }, - this, - INFINITE, - WT_EXECUTEINPERSISTENTTHREAD); + if (callbackMode != DISABLED) + { + ULONG flags = (callbackMode == FAST) ? + WT_EXECUTEINWAITTHREAD : WT_EXECUTEINPERSISTENTTHREAD; + + RegisterWaitForSingleObject( + &m_waitHandle, + m_event.Get(), + [](PVOID param, BOOLEAN timeout){ + if (!timeout) + ((CD3D12CommandQueue*)param)->OnCompletion(); + }, + this, + INFINITE, + flags); + } m_name = name; m_fenceValue = 0; @@ -92,15 +98,18 @@ bool CD3D12CommandQueue::Execute() ID3D12CommandList * lists[] = { m_cmdList.Get() }; m_queue->ExecuteCommandLists(1, lists); - m_queue->Signal(m_fence.Get(), ++m_fenceValue); + m_pending = true; + m_needsReset = true; + ++m_fenceValue; + + m_fence->SetEventOnCompletion(m_fenceValue, m_event.Get()); + m_queue->Signal(m_fence.Get(), m_fenceValue); if (FAILED(hr)) { DEBUG_ERROR_HR(hr, "Failed to set the fence signal (%ls)", m_name); return false; } - m_fence->SetEventOnCompletion(m_fenceValue, m_event.Get()); - m_pending = true; return true; } @@ -121,6 +130,9 @@ void CD3D12CommandQueue::Wait() bool CD3D12CommandQueue::Reset() { + if (!m_needsReset) + return true; + HRESULT hr; hr = m_allocator->Reset(); @@ -137,6 +149,7 @@ bool CD3D12CommandQueue::Reset() return false; } + m_needsReset = false; return true; } diff --git a/idd/LGIdd/CD3D12CommandQueue.h b/idd/LGIdd/CD3D12CommandQueue.h index bfa35c4d..11fae150 100644 --- a/idd/LGIdd/CD3D12CommandQueue.h +++ b/idd/LGIdd/CD3D12CommandQueue.h @@ -4,7 +4,6 @@ #include #include #include -#include using namespace Microsoft::WRL; using namespace Microsoft::WRL::Wrappers; @@ -25,27 +24,35 @@ class CD3D12CommandQueue HandleT m_event; HANDLE m_waitHandle = INVALID_HANDLE_VALUE; UINT64 m_fenceValue = 0; + bool m_needsReset = false; - typedef std::function CompletionFunction; - CompletionFunction m_completionCallback = nullptr; - void * m_completionParams[2]; + typedef void (*CompletionFunction)(CD3D12CommandQueue * queue, void * param1, void * param2); + CompletionFunction m_completionCallback = nullptr; + void * m_completionParams[2]; - bool Reset(); void OnCompletion() { - Reset(); - m_pending = false; if (m_completionCallback) m_completionCallback( this, m_completionParams[0], m_completionParams[1]); + m_pending = false; } public: ~CD3D12CommandQueue() { DeInit(); } - bool Init(ID3D12Device3 * device, D3D12_COMMAND_LIST_TYPE type, const WCHAR * name); + enum CallbackMode + { + DISABLED, // no callbacks + FAST, // callback is expected to return almost immediately + NORMAL // normal callback + }; + + bool Init(ID3D12Device3 * device, D3D12_COMMAND_LIST_TYPE type, const WCHAR * name, + CallbackMode callbackMode = DISABLED); + void DeInit(); void SetCompletionCallback(CompletionFunction fn, void * param1, void * param2) @@ -55,10 +62,11 @@ class CD3D12CommandQueue m_completionParams[1] = param2; } + bool Reset(); bool Execute(); //void Wait(); - bool IsReady() { return !m_pending; } + bool IsReady () { return !m_pending; } HANDLE GetEvent() { return m_event.Get(); } ComPtr GetCmdQueue() { return m_queue; } diff --git a/idd/LGIdd/CD3D12Device.cpp b/idd/LGIdd/CD3D12Device.cpp index 46804dab..6ea84d91 100644 --- a/idd/LGIdd/CD3D12Device.cpp +++ b/idd/LGIdd/CD3D12Device.cpp @@ -108,7 +108,8 @@ CD3D12Device::InitResult CD3D12Device::Init(CIVSHMEM &ivshmem, UINT64 &alignSize } for(int i = 0; i < ARRAYSIZE(m_copyQueue); ++i) - if (!m_copyQueue[i].Init(m_device.Get(), D3D12_COMMAND_LIST_TYPE_COPY, L"Copy")) + if (!m_copyQueue[i].Init(m_device.Get(), D3D12_COMMAND_LIST_TYPE_COPY, L"Copy", + m_indirectCopy ? CD3D12CommandQueue::NORMAL : CD3D12CommandQueue::FAST)) return InitResult::FAILURE; //if (!m_computeQueue.Init(m_device.Get(), D3D12_COMMAND_LIST_TYPE_COMPUTE, L"Compute")) @@ -176,8 +177,11 @@ CD3D12CommandQueue * CD3D12Device::GetCopyQueue() for (int i = 0; i < ARRAYSIZE(m_copyQueue); ++i) { auto& queue = m_copyQueue[i]; - if (queue.IsReady()) - return &queue; + if (!queue.IsReady()) + continue; + + queue.Reset(); + return &queue; } Sleep(1); } diff --git a/idd/LGIdd/CSwapChainProcessor.cpp b/idd/LGIdd/CSwapChainProcessor.cpp index 79a48a4b..40600375 100644 --- a/idd/LGIdd/CSwapChainProcessor.cpp +++ b/idd/LGIdd/CSwapChainProcessor.cpp @@ -145,6 +145,21 @@ void CSwapChainProcessor::SwapChainThreadCore() } } +void CSwapChainProcessor::CompletionFunction(CD3D12CommandQueue * queue, void * param1, void * param2) +{ + UNREFERENCED_PARAMETER(queue); + + auto sc = (CSwapChainProcessor *)param1; + auto fbRes = (CFrameBufferResource*)param2; + + if (sc->m_dx12Device->IsIndirectCopy()) + sc->m_devContext->WriteFrameBuffer( + fbRes->GetFrameIndex(), + fbRes->GetMap(), 0, fbRes->GetFrameSize(), true); + else + sc->m_devContext->FinalizeFrameBuffer(fbRes->GetFrameIndex()); +} + bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer) { ComPtr texture; @@ -212,23 +227,7 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer return false; } - copyQueue->SetCompletionCallback( - [](CD3D12CommandQueue * copyQueue, void * param1, void * param2) - { - UNREFERENCED_PARAMETER(copyQueue); - - auto sc = (CSwapChainProcessor *)param1; - auto fbRes = (CFrameBufferResource *)param2; - - if (sc->m_dx12Device->IsIndirectCopy()) - sc->m_devContext->WriteFrameBuffer( - fbRes->GetFrameIndex(), - fbRes->GetMap(), 0, fbRes->GetFrameSize(), true); - else - sc->m_devContext->FinalizeFrameBuffer(fbRes->GetFrameIndex()); - }, - this, - fbRes); + copyQueue->SetCompletionCallback(&CompletionFunction, this, fbRes); srcRes->Sync(*copyQueue); copyQueue->GetGfxList()->CopyTextureRegion( diff --git a/idd/LGIdd/CSwapChainProcessor.h b/idd/LGIdd/CSwapChainProcessor.h index f7d51177..2844659a 100644 --- a/idd/LGIdd/CSwapChainProcessor.h +++ b/idd/LGIdd/CSwapChainProcessor.h @@ -54,6 +54,8 @@ private: void SwapChainThread(); void SwapChainThreadCore(); + + static void CompletionFunction(CD3D12CommandQueue * queue, void * param1, void * param2); bool SwapChainNewFrame(ComPtr acquiredBuffer); public: