From: Tobin Ehlis Date: Wed, 20 Jan 2016 23:23:37 +0000 (-0700) Subject: layers: MR153, Correctly track inFlight cmdBuffers per queue X-Git-Tag: upstream/1.1.92~4048 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d02e9df295c8d183b5c1dd83e4c49dce4f3f11f3;p=platform%2Fupstream%2FVulkan-Tools.git layers: MR153, Correctly track inFlight cmdBuffers per queue There were per-queue vectors of the cmdBuffers in flight, but they were not being consistently used/updated. This change keeps two sets of inFlight cmdBuffers: one globally for all cmdBuffers on the device, and one for each queue. Ideally we could just track per-queue, but secondary command buffers are an exception that's considered "in-flight" from the time they're recorded in a primary command buffer with the vkCmdExecuteCommands() call so having a global list provides a way to account for that. Command buffers are added into both the global inFlight set and the individual queue set at QueueSubmit time. When cleaning up command buffers based on fences or Idle waits, correctly remove the cmdBuffers from the appropriate queue inFlight set. Also, if the cmdBuffer is not inFlight on any other queues, remove it from the the global inFlight set. Removed the deviceMap as it was only being used to hold a vector of queues. Since layer_data struct is already per-device, just moved vector of queues directly into layer_data struct. --- diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index 7a513d2..de2a164 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -95,6 +95,7 @@ struct layer_data { VkLayerDispatchTable* device_dispatch_table; VkLayerInstanceDispatchTable* instance_dispatch_table; devExts device_extensions; + vector queues; // all queues under given device unordered_set inFlightCmdBuffers; // Layer specific data unordered_map> sampleMap; @@ -111,7 +112,6 @@ struct layer_data { unordered_map memImageMap; unordered_map fenceMap; unordered_map queueMap; - unordered_map deviceMap; unordered_map eventMap; unordered_map queryToStateMap; unordered_map semaphoreSignaledMap; @@ -3202,10 +3202,13 @@ void trackCommandBuffers(layer_data* my_data, VkQueue queue, uint32_t cmdBufferC } if (queue_data != my_data->queueMap.end()) { for (uint32_t i = 0; i < cmdBufferCount; ++i) { + // Add cmdBuffers to both the global set and queue set for (auto secondaryCmdBuffer : my_data->commandBufferMap[pCmdBuffers[i]]->secondaryCommandBuffers) { my_data->inFlightCmdBuffers.insert(secondaryCmdBuffer); + my_data->queueMap[queue].inFlightCmdBuffers.insert(secondaryCmdBuffer); } my_data->inFlightCmdBuffers.insert(pCmdBuffers[i]); + my_data->queueMap[queue].inFlightCmdBuffers.insert(pCmdBuffers[i]); } } } @@ -3288,6 +3291,20 @@ VkBool32 cleanInFlightCmdBuffer(layer_data* my_data, VkCommandBuffer cmdBuffer) } return skip_call; } +// Remove given cmd_buffer from inFlight set for given queue +// If cmd_buffer is not inFlight on any other queues, also remove it from global inFlight set +static inline void removeInFlightCmdBuffer(layer_data* dev_data, VkQueue queue, VkCommandBuffer cmd_buffer) +{ + dev_data->queueMap[queue].inFlightCmdBuffers.erase(cmd_buffer); + // Pull it off of global list initially, but if we find it in any other queue list, add it back in + dev_data->inFlightCmdBuffers.erase(cmd_buffer); + for (auto q : dev_data->queues) { + if ((q != queue) && (dev_data->queueMap[q].inFlightCmdBuffers.find(cmd_buffer) != dev_data->queueMap[q].inFlightCmdBuffers.end())) { + dev_data->inFlightCmdBuffers.insert(cmd_buffer); + break; + } + } +} VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkWaitForFences(VkDevice device, uint32_t fenceCount, const VkFence* pFences, VkBool32 waitAll, uint64_t timeout) { @@ -3296,9 +3313,10 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkWaitForFences(VkDevice device, VkBool32 skip_call = VK_FALSE; if ((waitAll || fenceCount == 1) && result == VK_SUCCESS) { for (uint32_t i = 0; i < fenceCount; ++i) { + VkQueue fence_queue = dev_data->fenceMap[pFences[i]].queue; for (auto cmdBuffer : dev_data->fenceMap[pFences[i]].cmdBuffers) { - dev_data->inFlightCmdBuffers.erase(cmdBuffer); skip_call |= cleanInFlightCmdBuffer(dev_data, cmdBuffer); + removeInFlightCmdBuffer(dev_data, fence_queue, cmdBuffer); } } decrementResources(dev_data, fenceCount, pFences); @@ -3313,13 +3331,17 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkGetFenceStatus(VkDevice device, { layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); VkResult result = dev_data->device_dispatch_table->GetFenceStatus(device, fence); + VkBool32 skip_call = VK_FALSE; if (result == VK_SUCCESS) { + auto fence_queue = dev_data->fenceMap[fence].queue; for (auto cmdBuffer : dev_data->fenceMap[fence].cmdBuffers) { - dev_data->inFlightCmdBuffers.erase(cmdBuffer); - cleanInFlightCmdBuffer(dev_data, cmdBuffer); + skip_call |= cleanInFlightCmdBuffer(dev_data, cmdBuffer); + removeInFlightCmdBuffer(dev_data, fence_queue, cmdBuffer); } decrementResources(dev_data, 1, &fence); } + if (VK_FALSE != skip_call) + return VK_ERROR_VALIDATION_FAILED_EXT; return result; } @@ -3327,7 +3349,7 @@ VK_LAYER_EXPORT VKAPI_ATTR void VKAPI_CALL vkGetDeviceQueue(VkDevice device, uin { layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); dev_data->device_dispatch_table->GetDeviceQueue(device, queueFamilyIndex, queueIndex, pQueue); - dev_data->deviceMap[device].queues.push_back(*pQueue); + dev_data->queues.push_back(*pQueue); dev_data->queueMap[*pQueue].device = device; } @@ -3335,22 +3357,26 @@ VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueueWaitIdle(VkQueue queue) { layer_data* dev_data = get_my_data_ptr(get_dispatch_key(queue), layer_data_map); decrementResources(dev_data, queue); - // TODO : Is this a bug? Can't clear all device CmdBuffers on QueueWaitIdle - for (auto cmdBuffer : dev_data->inFlightCmdBuffers) { - cleanInFlightCmdBuffer(dev_data, cmdBuffer); + VkBool32 skip_call = VK_FALSE; + // Iterate over local set since we erase set members as we go in for loop + auto local_cb_set = dev_data->queueMap[queue].inFlightCmdBuffers; + for (auto cmdBuffer : local_cb_set) { + skip_call |= cleanInFlightCmdBuffer(dev_data, cmdBuffer); + removeInFlightCmdBuffer(dev_data, queue, cmdBuffer); } - dev_data->inFlightCmdBuffers.clear(); + dev_data->queueMap[queue].inFlightCmdBuffers.clear(); + if (VK_FALSE != skip_call) + return VK_ERROR_VALIDATION_FAILED_EXT; return dev_data->device_dispatch_table->QueueWaitIdle(queue); } VK_LAYER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkDeviceWaitIdle(VkDevice device) { layer_data* dev_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map); - auto device_data = dev_data->deviceMap.find(device); - if (device_data != dev_data->deviceMap.end()) { - for (auto queue : device_data->second.queues) { - decrementResources(dev_data, queue); - } + for (auto queue : dev_data->queues) { + decrementResources(dev_data, queue); + // Clear all of the queue inFlightCmdBuffers (global set cleared below) + dev_data->queueMap[queue].inFlightCmdBuffers.clear(); } for (auto cmdBuffer : dev_data->inFlightCmdBuffers) { cleanInFlightCmdBuffer(dev_data, cmdBuffer); diff --git a/layers/draw_state.h b/layers/draw_state.h index 9f17bb9..45e8955 100755 --- a/layers/draw_state.h +++ b/layers/draw_state.h @@ -255,6 +255,7 @@ struct RENDER_PASS_NODE { class FENCE_NODE : public BASE_NODE { public: using BASE_NODE::in_use; + VkQueue queue; vector cmdBuffers; bool needsSignaled; VkFence priorFence; @@ -274,11 +275,6 @@ class QUEUE_NODE { unordered_set inFlightCmdBuffers; }; -class DEVICE_NODE { - public: - vector queues; -}; - // Descriptor Data structures // Layout Node has the core layout data typedef struct _LAYOUT_NODE {