From 7fb31361f443b8a968ec3f6a9fd2fa59f3b3ceb7 Mon Sep 17 00:00:00 2001 From: Jason Macnak Date: Thu, 5 Sep 2024 14:08:08 -0700 Subject: [PATCH] Handle external fences in vkGetFenceStatus() The vkGetFenceStatus() call can not be sent to the host for fences that have imported an external payload (sync fd) because the sync fd does not exist on the host. A fence used as part of a swapchain present may be created in the unsignaled state. Then, during vkQueuePresentKHR() on Android, vkQueueSignalReleaseImage() is used to import a sync fd payload into the present fence. Prior to this change, if the user (ANGLE) does vkGetFenceStatus() on this fence, it would never appear as signaled because the sync fd fence is not actuallly connected to the fence on the host and the host would just always return the VK_NOT_READY from the fence's initial unsignaled state. This change also updates VkFence_Info to use a std::optional to make it possible to distinguish if a fence has an imported already-signaled payload vs not having an imported payload. Reviewed-by: Aaron Ruby Acked-by: Yonggang Luo Acked-by: Adam Jackson Part-of: --- .../codegen/scripts/cereal/functable.py | 5 +- .../guest/vulkan_enc/ResourceTracker.cpp | 127 +++++++++++------- .../guest/vulkan_enc/ResourceTracker.h | 6 +- 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/functable.py b/src/gfxstream/codegen/scripts/cereal/functable.py index ab0462406f9..a14bc92a743 100644 --- a/src/gfxstream/codegen/scripts/cereal/functable.py +++ b/src/gfxstream/codegen/scripts/cereal/functable.py @@ -68,6 +68,7 @@ RESOURCE_TRACKER_ENTRIES = [ "vkResetFences", "vkImportFenceFdKHR", "vkGetFenceFdKHR", + "vkGetFenceStatus", "vkWaitForFences", "vkCreateDescriptorPool", "vkDestroyDescriptorPool", @@ -323,7 +324,7 @@ class VulkanFuncTable(VulkanWrapperGenerator): if retVar: retTypeName = api.getRetTypeExpr() # ex: vkCreateBuffer_VkResult_return = gfxstream_buffer ? VK_SUCCESS : VK_ERROR_OUT_OF_HOST_MEMORY; - cgen.stmt("%s = %s ? %s : %s" % + cgen.stmt("%s = %s ? %s : %s" % (retVar, paramNameToObjectName(createParam.paramName), SUCCESS_VAL[retTypeName][0], "VK_ERROR_OUT_OF_HOST_MEMORY")) return True @@ -541,7 +542,7 @@ class VulkanFuncTable(VulkanWrapperGenerator): if retVar and createdObject: cgen.beginIf("%s == %s" % (SUCCESS_VAL[retTypeName][0], retVar)) else: - cgen.beginBlock() + cgen.beginBlock() genEncoderOrResourceTrackerCall() cgen.endBlock() # Destroy gfxstream objects diff --git a/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp b/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp index 49ddc32106d..05ba9be7d2d 100644 --- a/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp +++ b/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp @@ -1180,8 +1180,8 @@ void ResourceTracker::unregister_VkFence(VkFence fence) { (void)fenceInfo; #if defined(VK_USE_PLATFORM_ANDROID_KHR) || defined(__linux__) - if (fenceInfo.syncFd >= 0) { - mSyncHelper->close(fenceInfo.syncFd); + if (fenceInfo.syncFd && *fenceInfo.syncFd >= 0) { + mSyncHelper->close(*fenceInfo.syncFd); } #endif @@ -4733,12 +4733,12 @@ VkResult ResourceTracker::on_vkResetFences(void* context, VkResult, VkDevice dev if (!info.external) continue; #if GFXSTREAM_ENABLE_GUEST_GOLDFISH - if (info.syncFd >= 0) { + if (info.syncFd && *info.syncFd >= 0) { mesa_logd("%s: resetting fence. make fd -1\n", __func__); - goldfish_sync_signal(info.syncFd); - mSyncHelper->close(info.syncFd); - info.syncFd = -1; + goldfish_sync_signal(*info.syncFd); + mSyncHelper->close(*info.syncFd); } + info.syncFd.reset(); #endif } @@ -4779,10 +4779,10 @@ VkResult ResourceTracker::on_vkImportFenceFdKHR(void* context, VkResult, VkDevic auto& info = it->second; #if GFXSTREAM_ENABLE_GUEST_GOLDFISH - if (info.syncFd >= 0) { + if (info.syncFd && *info.syncFd >= 0) { mesa_logd("%s: previous sync fd exists, close it\n", __func__); - goldfish_sync_signal(info.syncFd); - mSyncHelper->close(info.syncFd); + goldfish_sync_signal(*info.syncFd); + mSyncHelper->close(*info.syncFd); } #endif @@ -4791,7 +4791,15 @@ VkResult ResourceTracker::on_vkImportFenceFdKHR(void* context, VkResult, VkDevic info.syncFd = -1; } else { mesa_logd("%s: import actual fd, dup and close()\n", __func__); - info.syncFd = mSyncHelper->dup(pImportFenceFdInfo->fd); + + int fenceCopy = mSyncHelper->dup(pImportFenceFdInfo->fd); + if (fenceCopy < 0) { + mesa_loge("Failed to dup() import sync fd."); + return VK_ERROR_OUT_OF_HOST_MEMORY; + } + + info.syncFd = fenceCopy; + mSyncHelper->close(pImportFenceFdInfo->fd); } return VK_SUCCESS; @@ -4875,7 +4883,8 @@ VkResult ResourceTracker::on_vkGetFenceFdKHR(void* context, VkResult, VkDevice d } // relinquish ownership - info.syncFd = -1; + info.syncFd.reset(); + mesa_logd("%s: got fd: %d\n", __func__, *pFd); return VK_SUCCESS; } @@ -4885,14 +4894,42 @@ VkResult ResourceTracker::on_vkGetFenceFdKHR(void* context, VkResult, VkDevice d #endif } +VkResult ResourceTracker::on_vkGetFenceStatus(void* context, VkResult input_result, VkDevice device, + VkFence fence) { + VkEncoder* enc = (VkEncoder*)context; + +#if defined(VK_USE_PLATFORM_ANDROID_KHR) || defined(__linux__) + { + std::unique_lock lock(mLock); + + auto fenceInfoIt = info_VkFence.find(fence); + if (fenceInfoIt == info_VkFence.end()) { + mesa_loge("Failed to find VkFence:%p", fence); + return VK_NOT_READY; + } + auto& fenceInfo = fenceInfoIt->second; + + if (fenceInfo.syncFd) { + if (*fenceInfo.syncFd == -1) { + return VK_SUCCESS; + } + + int syncFdSignaled = mSyncHelper->wait(*fenceInfo.syncFd, /*timeout=*/0) == 0; + return syncFdSignaled ? VK_SUCCESS : VK_NOT_READY; + } + } +#endif + + return enc->vkGetFenceStatus(device, fence, /*doLock=*/true); +} + VkResult ResourceTracker::on_vkWaitForFences(void* context, VkResult, VkDevice device, uint32_t fenceCount, const VkFence* pFences, VkBool32 waitAll, uint64_t timeout) { VkEncoder* enc = (VkEncoder*)context; #if defined(VK_USE_PLATFORM_ANDROID_KHR) || defined(__linux__) - std::vector fencesExternal; - std::vector fencesExternalWaitFds; + std::vector fencesExternalSyncFds; std::vector fencesNonExternal; std::unique_lock lock(mLock); @@ -4901,9 +4938,10 @@ VkResult ResourceTracker::on_vkWaitForFences(void* context, VkResult, VkDevice d auto it = info_VkFence.find(pFences[i]); if (it == info_VkFence.end()) continue; const auto& info = it->second; - if (info.syncFd >= 0) { - fencesExternal.push_back(pFences[i]); - fencesExternalWaitFds.push_back(info.syncFd); + if (info.syncFd) { + if (*info.syncFd >= 0) { + fencesExternalSyncFds.push_back(*info.syncFd); + } } else { fencesNonExternal.push_back(pFences[i]); } @@ -4911,40 +4949,35 @@ VkResult ResourceTracker::on_vkWaitForFences(void* context, VkResult, VkDevice d lock.unlock(); - if (fencesExternal.empty()) { - // No need for work pool, just wait with host driver. - return enc->vkWaitForFences(device, fenceCount, pFences, waitAll, timeout, - true /* do lock */); - } else { - for (auto fd : fencesExternalWaitFds) { - mesa_logd("Waiting on sync fd: %d", fd); + for (auto fd : fencesExternalSyncFds) { + mesa_logd("Waiting on sync fd: %d", fd); - std::chrono::steady_clock::time_point begin = std::chrono::steady_clock::now(); - // syncHelper works in milliseconds - mSyncHelper->wait(fd, DIV_ROUND_UP(timeout, 1000)); - std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); + std::chrono::steady_clock::time_point begin = std::chrono::steady_clock::now(); + // syncHelper works in milliseconds + mSyncHelper->wait(fd, DIV_ROUND_UP(timeout, 1000)); + std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); - uint64_t timeTaken = - std::chrono::duration_cast(end - begin).count(); - if (timeTaken >= timeout) { - return VK_TIMEOUT; - } - - timeout -= timeTaken; - mesa_logd("Done waiting on sync fd: %d", fd); + uint64_t timeTaken = + std::chrono::duration_cast(end - begin).count(); + if (timeTaken >= timeout) { + return VK_TIMEOUT; } - if (!fencesNonExternal.empty()) { - auto hostConn = ResourceTracker::threadingCallbacks.hostConnectionGetFunc(); - auto vkEncoder = ResourceTracker::threadingCallbacks.vkEncoderGetFunc(hostConn); - mesa_logd("vkWaitForFences to host"); - return vkEncoder->vkWaitForFences(device, fencesNonExternal.size(), - fencesNonExternal.data(), waitAll, timeout, - true /* do lock */); - } - - return VK_SUCCESS; + timeout -= timeTaken; + mesa_logd("Done waiting on sync fd: %d", fd); } + + if (!fencesNonExternal.empty()) { + auto hostConn = ResourceTracker::threadingCallbacks.hostConnectionGetFunc(); + auto vkEncoder = ResourceTracker::threadingCallbacks.vkEncoderGetFunc(hostConn); + mesa_logd("vkWaitForFences to host"); + return vkEncoder->vkWaitForFences(device, fencesNonExternal.size(), + fencesNonExternal.data(), waitAll, timeout, + true /* do lock */); + } + + return VK_SUCCESS; + #else return enc->vkWaitForFences(device, fenceCount, pFences, waitAll, timeout, true /* do lock */); #endif @@ -6157,8 +6190,8 @@ VkResult ResourceTracker::on_vkQueueSubmitTemplate(void* context, VkResult input auto it = info_VkFence.find(fence); if (it != info_VkFence.end()) { const auto& info = it->second; - if (info.syncFd >= 0) { - externalFenceFdToSignal = info.syncFd; + if (info.syncFd && *info.syncFd >= 0) { + externalFenceFdToSignal = *info.syncFd; } } } diff --git a/src/gfxstream/guest/vulkan_enc/ResourceTracker.h b/src/gfxstream/guest/vulkan_enc/ResourceTracker.h index 0680b950773..5be0419e1e6 100644 --- a/src/gfxstream/guest/vulkan_enc/ResourceTracker.h +++ b/src/gfxstream/guest/vulkan_enc/ResourceTracker.h @@ -387,6 +387,9 @@ class ResourceTracker { VkResult on_vkGetFenceFdKHR(void* context, VkResult input_result, VkDevice device, const VkFenceGetFdInfoKHR* pGetFdInfo, int* pFd); + VkResult on_vkGetFenceStatus(void* context, VkResult input_result, VkDevice device, + VkFence fence); + VkResult on_vkWaitForFences(void* context, VkResult input_result, VkDevice device, uint32_t fenceCount, const VkFence* pFences, VkBool32 waitAll, uint64_t timeout); @@ -850,7 +853,8 @@ class ResourceTracker { bool external = false; VkExportFenceCreateInfo exportFenceCreateInfo; #if defined(VK_USE_PLATFORM_ANDROID_KHR) || defined(__linux__) - int syncFd = -1; + // Note: -1 means already signaled. + std::optional syncFd; #endif };