From 9a9e06c425ea0a93f1553407d65fef2ab2d4f12b Mon Sep 17 00:00:00 2001 From: Juston Li Date: Tue, 13 Aug 2024 20:45:14 +0000 Subject: [Vulkan] Query global priority support for queue creation Setting up a device queue with a global queue priority above the system default requires sufficient privileges (SYS_NICE) otherwise queue creation will fail. SurfaceFlinger and system_server have SYS_NICE and will set priority to REALTIME. SysUI and Launcher take that as an indication they should request HIGH for better preemption strategy. However for upstream DRM drivers, we currently lack a way to grant them granular access to HIGH but not RT. For long term, the right way to do this is via drm specific cgroup for scheduling controls however this is currently being worked on upstream. In the meantime, instead of fatally crashing, we can query the global priorities supported before queue creation and if its not supported, drop the priority request and log a warning. Note this requires vkGetPhysicalDeviceQueueFamilyProperties2 which is provided by Vulkan 1.1 while the global priority query can be safely added to the pNext chain and will be ignored if VK_EXT_global_priority_query isn't supported. Bug: 343986434 Flag: com.android.graphics.hwui.flags.query_global_priority Test: UI comes up debug.renderengine.backend=skiavkthreaded Change-Id: I662c9ce3f3724e87690e25d260ee010340451c53 --- libs/hwui/Properties.cpp | 6 +++ libs/hwui/Properties.h | 1 + libs/hwui/aconfig/hwui_flags.aconfig | 10 ++++ libs/hwui/renderthread/VulkanManager.cpp | 85 ++++++++++++++++++++++++++------ libs/hwui/renderthread/VulkanManager.h | 2 +- 5 files changed, 88 insertions(+), 16 deletions(-) diff --git a/libs/hwui/Properties.cpp b/libs/hwui/Properties.cpp index b6476c9d466f..ae46a99f09c8 100644 --- a/libs/hwui/Properties.cpp +++ b/libs/hwui/Properties.cpp @@ -50,6 +50,10 @@ constexpr bool skip_eglmanager_telemetry() { constexpr bool resample_gainmap_regions() { return false; } + +constexpr bool query_global_priority() { + return false; +} } // namespace hwui_flags #endif @@ -110,6 +114,7 @@ bool Properties::clipSurfaceViews = false; bool Properties::hdr10bitPlus = false; bool Properties::skipTelemetry = false; bool Properties::resampleGainmapRegions = false; +bool Properties::queryGlobalPriority = false; int Properties::timeoutMultiplier = 1; @@ -187,6 +192,7 @@ bool Properties::load() { hdr10bitPlus = hwui_flags::hdr_10bit_plus(); resampleGainmapRegions = base::GetBoolProperty("debug.hwui.resample_gainmap_regions", hwui_flags::resample_gainmap_regions()); + queryGlobalPriority = hwui_flags::query_global_priority(); timeoutMultiplier = android::base::GetIntProperty("ro.hw_timeout_multiplier", 1); skipTelemetry = base::GetBoolProperty(PROPERTY_SKIP_EGLMANAGER_TELEMETRY, diff --git a/libs/hwui/Properties.h b/libs/hwui/Properties.h index db471527b861..6f84796fb11e 100644 --- a/libs/hwui/Properties.h +++ b/libs/hwui/Properties.h @@ -346,6 +346,7 @@ public: static bool hdr10bitPlus; static bool skipTelemetry; static bool resampleGainmapRegions; + static bool queryGlobalPriority; static int timeoutMultiplier; diff --git a/libs/hwui/aconfig/hwui_flags.aconfig b/libs/hwui/aconfig/hwui_flags.aconfig index ab052b902e02..93df47853769 100644 --- a/libs/hwui/aconfig/hwui_flags.aconfig +++ b/libs/hwui/aconfig/hwui_flags.aconfig @@ -129,3 +129,13 @@ flag { description: "APIs that expose gainmap metadata corresponding to those defined in ISO 21496-1" bug: "349357636" } + +flag { + name: "query_global_priority" + namespace: "core_graphics" + description: "Attempt to query whether the vulkan driver supports the requested global priority before queue creation." + bug: "343986434" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp index e3023937964e..6571d92aeafa 100644 --- a/libs/hwui/renderthread/VulkanManager.cpp +++ b/libs/hwui/renderthread/VulkanManager.cpp @@ -44,7 +44,7 @@ namespace uirenderer { namespace renderthread { // Not all of these are strictly required, but are all enabled if present. -static std::array sEnableExtensions{ +static std::array sEnableExtensions{ VK_KHR_BIND_MEMORY_2_EXTENSION_NAME, VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME, VK_KHR_EXTERNAL_MEMORY_CAPABILITIES_EXTENSION_NAME, @@ -65,6 +65,8 @@ static std::array sEnableExtensions{ VK_KHR_EXTERNAL_SEMAPHORE_FD_EXTENSION_NAME, VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, VK_EXT_GLOBAL_PRIORITY_EXTENSION_NAME, + VK_EXT_GLOBAL_PRIORITY_QUERY_EXTENSION_NAME, + VK_KHR_GLOBAL_PRIORITY_EXTENSION_NAME, VK_EXT_DEVICE_FAULT_EXTENSION_NAME, }; @@ -206,7 +208,7 @@ void VulkanManager::setupDevice(skgpu::VulkanExtensions& grExtensions, GET_INST_PROC(GetPhysicalDeviceFeatures2); GET_INST_PROC(GetPhysicalDeviceImageFormatProperties2); GET_INST_PROC(GetPhysicalDeviceProperties); - GET_INST_PROC(GetPhysicalDeviceQueueFamilyProperties); + GET_INST_PROC(GetPhysicalDeviceQueueFamilyProperties2); uint32_t gpuCount; LOG_ALWAYS_FATAL_IF(mEnumeratePhysicalDevices(mInstance, &gpuCount, nullptr)); @@ -225,21 +227,30 @@ void VulkanManager::setupDevice(skgpu::VulkanExtensions& grExtensions, // query to get the initial queue props size uint32_t queueCount = 0; - mGetPhysicalDeviceQueueFamilyProperties(mPhysicalDevice, &queueCount, nullptr); + mGetPhysicalDeviceQueueFamilyProperties2(mPhysicalDevice, &queueCount, nullptr); LOG_ALWAYS_FATAL_IF(!queueCount); // now get the actual queue props - std::unique_ptr queueProps(new VkQueueFamilyProperties[queueCount]); - mGetPhysicalDeviceQueueFamilyProperties(mPhysicalDevice, &queueCount, queueProps.get()); + std::unique_ptr + queueProps(new VkQueueFamilyProperties2[queueCount]); + // query the global priority, this ignored if VK_EXT_global_priority isn't supported + std::vector queuePriorityProps(queueCount); + for (uint32_t i = 0; i < queueCount; i++) { + queuePriorityProps[i].sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_GLOBAL_PRIORITY_PROPERTIES_EXT; + queuePriorityProps[i].pNext = nullptr; + queueProps[i].pNext = &queuePriorityProps[i]; + } + mGetPhysicalDeviceQueueFamilyProperties2(mPhysicalDevice, &queueCount, queueProps.get()); constexpr auto kRequestedQueueCount = 2; // iterate to find the graphics queue mGraphicsQueueIndex = queueCount; for (uint32_t i = 0; i < queueCount; i++) { - if (queueProps[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) { + if (queueProps[i].queueFamilyProperties.queueFlags & VK_QUEUE_GRAPHICS_BIT) { mGraphicsQueueIndex = i; - LOG_ALWAYS_FATAL_IF(queueProps[i].queueCount < kRequestedQueueCount); + LOG_ALWAYS_FATAL_IF( + queueProps[i].queueFamilyProperties.queueCount < kRequestedQueueCount); break; } } @@ -327,6 +338,15 @@ void VulkanManager::setupDevice(skgpu::VulkanExtensions& grExtensions, tailPNext = &formatFeatures->pNext; } + VkPhysicalDeviceGlobalPriorityQueryFeaturesEXT* globalPriorityQueryFeatures = + new VkPhysicalDeviceGlobalPriorityQueryFeaturesEXT; + globalPriorityQueryFeatures->sType = + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_GLOBAL_PRIORITY_QUERY_FEATURES_EXT; + globalPriorityQueryFeatures->pNext = nullptr; + globalPriorityQueryFeatures->globalPriorityQuery = false; + *tailPNext = globalPriorityQueryFeatures; + tailPNext = &globalPriorityQueryFeatures->pNext; + // query to get the physical device features mGetPhysicalDeviceFeatures2(mPhysicalDevice, &features); // this looks like it would slow things down, @@ -341,24 +361,59 @@ void VulkanManager::setupDevice(skgpu::VulkanExtensions& grExtensions, if (Properties::contextPriority != 0 && grExtensions.hasExtension(VK_EXT_GLOBAL_PRIORITY_EXTENSION_NAME, 2)) { - memset(&queuePriorityCreateInfo, 0, sizeof(VkDeviceQueueGlobalPriorityCreateInfoEXT)); - queuePriorityCreateInfo.sType = - VK_STRUCTURE_TYPE_DEVICE_QUEUE_GLOBAL_PRIORITY_CREATE_INFO_EXT; - queuePriorityCreateInfo.pNext = nullptr; + VkQueueGlobalPriorityEXT globalPriority; switch (Properties::contextPriority) { case EGL_CONTEXT_PRIORITY_LOW_IMG: - queuePriorityCreateInfo.globalPriority = VK_QUEUE_GLOBAL_PRIORITY_LOW_EXT; + globalPriority = VK_QUEUE_GLOBAL_PRIORITY_LOW_EXT; break; case EGL_CONTEXT_PRIORITY_MEDIUM_IMG: - queuePriorityCreateInfo.globalPriority = VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT; + globalPriority = VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_EXT; break; case EGL_CONTEXT_PRIORITY_HIGH_IMG: - queuePriorityCreateInfo.globalPriority = VK_QUEUE_GLOBAL_PRIORITY_HIGH_EXT; + globalPriority = VK_QUEUE_GLOBAL_PRIORITY_HIGH_EXT; break; default: LOG_ALWAYS_FATAL("Unsupported context priority"); } - queueNextPtr = &queuePriorityCreateInfo; + + // check if the requested priority is reported by the query + bool attachGlobalPriority = false; + if (uirenderer::Properties::queryGlobalPriority && + globalPriorityQueryFeatures->globalPriorityQuery) { + for (uint32_t i = 0; i < queuePriorityProps[mGraphicsQueueIndex].priorityCount; i++) { + if (queuePriorityProps[mGraphicsQueueIndex].priorities[i] == globalPriority) { + attachGlobalPriority = true; + break; + } + } + } else { + // Querying is not supported so attempt queue creation with requested priority anyways + // If the priority turns out not to be supported, the driver *may* fail with + // VK_ERROR_NOT_PERMITTED_KHR + attachGlobalPriority = true; + } + + if (attachGlobalPriority) { + memset(&queuePriorityCreateInfo, 0, sizeof(VkDeviceQueueGlobalPriorityCreateInfoEXT)); + queuePriorityCreateInfo.sType = + VK_STRUCTURE_TYPE_DEVICE_QUEUE_GLOBAL_PRIORITY_CREATE_INFO_EXT; + queuePriorityCreateInfo.pNext = nullptr; + queuePriorityCreateInfo.globalPriority = globalPriority; + queueNextPtr = &queuePriorityCreateInfo; + } else { + // If globalPriorityQuery is enabled, attempting queue creation with an unsupported + // priority will return VK_ERROR_INITIALIZATION_FAILED. + // + // SysUI and Launcher will request HIGH when SF has RT but it is a known issue that + // upstream drm drivers currently lack a way to grant them the granular privileges + // they need for HIGH (but not RT) so they will fail queue creation. + // For now, drop the unsupported global priority request so that queue creation + // succeeds. + // + // Once that is fixed, this should probably be a fatal error indicating an improper + // request or an app needs to get the correct privileges. + ALOGW("Requested context priority is not supported by the queue"); + } } const VkDeviceQueueCreateInfo queueInfo = { diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h index f0425719ea89..a593ec6f8351 100644 --- a/libs/hwui/renderthread/VulkanManager.h +++ b/libs/hwui/renderthread/VulkanManager.h @@ -152,7 +152,7 @@ private: VkPtr mDestroyInstance; VkPtr mEnumeratePhysicalDevices; VkPtr mGetPhysicalDeviceProperties; - VkPtr mGetPhysicalDeviceQueueFamilyProperties; + VkPtr mGetPhysicalDeviceQueueFamilyProperties2; VkPtr mGetPhysicalDeviceFeatures2; VkPtr mGetPhysicalDeviceImageFormatProperties2; VkPtr mCreateDevice; -- cgit v1.2.3-59-g8ed1b