From 5e8622099e35b8d18062ba79c9d1fc062cb9598a Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 21 Jun 2019 14:59:16 -0700 Subject: Vulkan: clean up android::Vector in the loader Bug: 134185757 Test: CtsDeqpTestCases Test: CtsGraphicsTestCases Test: CtsGpuToolsHostTestCases Change-Id: Ic8080e822f2b1788eee672cfffa664f9bd29b9be --- vulkan/libvulkan/driver.cpp | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 23506bad54..d33e5528e8 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -16,30 +16,29 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS +#include "driver.h" + +#include #include #include #include -#include - -#include -#include -#include -#include - -#include #include #include +#include #include #include #include +#include +#include #include #include -#include -#include "android-base/properties.h" +#include +#include +#include +#include -#include "driver.h" #include "stubhal.h" using namespace android::hardware::configstore; @@ -809,8 +808,7 @@ VkResult EnumerateInstanceExtensionProperties( const char* pLayerName, uint32_t* pPropertyCount, VkExtensionProperties* pProperties) { - - android::Vector loader_extensions; + std::vector loader_extensions; loader_extensions.push_back({ VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_SURFACE_SPEC_VERSION}); @@ -833,7 +831,7 @@ VkResult EnumerateInstanceExtensionProperties( uint32_t count = std::min( *pPropertyCount, static_cast(loader_extensions.size())); - std::copy_n(loader_extensions.begin(), count, pProperties); + std::copy_n(loader_extensions.data(), count, pProperties); if (count < loader_extensions.size()) { *pPropertyCount = count; @@ -879,8 +877,7 @@ VkResult EnumerateInstanceExtensionProperties( bool QueryPresentationProperties( VkPhysicalDevice physicalDevice, - VkPhysicalDevicePresentationPropertiesANDROID *presentation_properties) -{ + VkPhysicalDevicePresentationPropertiesANDROID *presentation_properties) { const InstanceData& data = GetData(physicalDevice); // GPDP2 must be present and enabled on the instance. @@ -920,7 +917,7 @@ VkResult EnumerateDeviceExtensionProperties( VkExtensionProperties* pProperties) { const InstanceData& data = GetData(physicalDevice); // extensions that are unconditionally exposed by the loader - android::Vector loader_extensions; + std::vector loader_extensions; loader_extensions.push_back({ VK_KHR_INCREMENTAL_PRESENT_EXTENSION_NAME, VK_KHR_INCREMENTAL_PRESENT_SPEC_VERSION}); @@ -956,7 +953,7 @@ VkResult EnumerateDeviceExtensionProperties( uint32_t count = std::min( *pPropertyCount, static_cast(loader_extensions.size())); - std::copy_n(loader_extensions.begin(), count, pProperties); + std::copy_n(loader_extensions.data(), count, pProperties); if (count < loader_extensions.size()) { *pPropertyCount = count; @@ -1245,11 +1242,10 @@ VkResult EnumeratePhysicalDeviceGroups( if (!device_count) return VK_INCOMPLETE; - android::Vector devices; - devices.resize(device_count); + std::vector devices(device_count); *pPhysicalDeviceGroupCount = device_count; - result = EnumeratePhysicalDevices(instance, &device_count, - devices.editArray()); + result = + EnumeratePhysicalDevices(instance, &device_count, devices.data()); if (result < 0) return result; -- cgit v1.2.3-59-g8ed1b From 27ab3ac610954ac01a18a1cf8559827cf7679f99 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Tue, 2 Jul 2019 18:10:55 -0700 Subject: GpuStats: move GpuStats related structs and enums away from GraphicsEnv Bug: 135210726 Test: build, flash and boot Test: adb shell dumpsys gpu Change-Id: I48c5c432aca916f923ab5674f8ec533d4f5aac0f --- libs/graphicsenv/GraphicsEnv.cpp | 38 +++++++------- libs/graphicsenv/IGpuService.cpp | 4 +- .../graphicsenv/include/graphicsenv/GpuStatsInfo.h | 39 ++++++++++++++ libs/graphicsenv/include/graphicsenv/GraphicsEnv.h | 59 +++------------------- libs/graphicsenv/include/graphicsenv/IGpuService.h | 2 +- opengl/libs/EGL/Loader.cpp | 12 ++--- services/gpuservice/GpuService.cpp | 2 +- services/gpuservice/GpuService.h | 2 +- services/gpuservice/gpustats/GpuStats.cpp | 26 +++++----- services/gpuservice/gpustats/GpuStats.h | 2 +- vulkan/libvulkan/driver.cpp | 10 ++-- 11 files changed, 94 insertions(+), 102 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index 24b6c2d6de..a411dd56f6 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -170,11 +170,11 @@ void GraphicsEnv::hintActivityLaunch() { std::lock_guard lock(mStatsLock); if (mGpuStats.glDriverToSend) { mGpuStats.glDriverToSend = false; - sendGpuStatsLocked(GraphicsEnv::Api::API_GL, true, mGpuStats.glDriverLoadingTime); + sendGpuStatsLocked(GpuStatsInfo::Api::API_GL, true, mGpuStats.glDriverLoadingTime); } if (mGpuStats.vkDriverToSend) { mGpuStats.vkDriverToSend = false; - sendGpuStatsLocked(GraphicsEnv::Api::API_VK, true, mGpuStats.vkDriverLoadingTime); + sendGpuStatsLocked(GpuStatsInfo::Api::API_VK, true, mGpuStats.vkDriverLoadingTime); } }); trySendGpuStatsThread.detach(); @@ -205,32 +205,32 @@ void GraphicsEnv::setGpuStats(const std::string& driverPackageName, mGpuStats.vulkanVersion = vulkanVersion; } -void GraphicsEnv::setDriverToLoad(GraphicsEnv::Driver driver) { +void GraphicsEnv::setDriverToLoad(GpuStatsInfo::Driver driver) { ATRACE_CALL(); std::lock_guard lock(mStatsLock); switch (driver) { - case GraphicsEnv::Driver::GL: - case GraphicsEnv::Driver::GL_UPDATED: - case GraphicsEnv::Driver::ANGLE: { - if (mGpuStats.glDriverToLoad == GraphicsEnv::Driver::NONE) { + case GpuStatsInfo::Driver::GL: + case GpuStatsInfo::Driver::GL_UPDATED: + case GpuStatsInfo::Driver::ANGLE: { + if (mGpuStats.glDriverToLoad == GpuStatsInfo::Driver::NONE) { mGpuStats.glDriverToLoad = driver; break; } - if (mGpuStats.glDriverFallback == GraphicsEnv::Driver::NONE) { + if (mGpuStats.glDriverFallback == GpuStatsInfo::Driver::NONE) { mGpuStats.glDriverFallback = driver; } break; } - case Driver::VULKAN: - case Driver::VULKAN_UPDATED: { - if (mGpuStats.vkDriverToLoad == GraphicsEnv::Driver::NONE) { + case GpuStatsInfo::Driver::VULKAN: + case GpuStatsInfo::Driver::VULKAN_UPDATED: { + if (mGpuStats.vkDriverToLoad == GpuStatsInfo::Driver::NONE) { mGpuStats.vkDriverToLoad = driver; break; } - if (mGpuStats.vkDriverFallback == GraphicsEnv::Driver::NONE) { + if (mGpuStats.vkDriverFallback == GpuStatsInfo::Driver::NONE) { mGpuStats.vkDriverFallback = driver; } break; @@ -240,13 +240,13 @@ void GraphicsEnv::setDriverToLoad(GraphicsEnv::Driver driver) { } } -void GraphicsEnv::setDriverLoaded(GraphicsEnv::Api api, bool isDriverLoaded, +void GraphicsEnv::setDriverLoaded(GpuStatsInfo::Api api, bool isDriverLoaded, int64_t driverLoadingTime) { ATRACE_CALL(); std::lock_guard lock(mStatsLock); const bool doNotSend = mGpuStats.appPackageName.empty(); - if (api == GraphicsEnv::Api::API_GL) { + if (api == GpuStatsInfo::Api::API_GL) { if (doNotSend) mGpuStats.glDriverToSend = true; mGpuStats.glDriverLoadingTime = driverLoadingTime; } else { @@ -278,7 +278,7 @@ void GraphicsEnv::setCpuVulkanInUse() { } } -void GraphicsEnv::sendGpuStatsLocked(GraphicsEnv::Api api, bool isDriverLoaded, +void GraphicsEnv::sendGpuStatsLocked(GpuStatsInfo::Api api, bool isDriverLoaded, int64_t driverLoadingTime) { ATRACE_CALL(); @@ -299,16 +299,16 @@ void GraphicsEnv::sendGpuStatsLocked(GraphicsEnv::Api api, bool isDriverLoaded, mGpuStats.driverVersionCode, mGpuStats.driverBuildTime, mGpuStats.appPackageName.c_str(), mGpuStats.vulkanVersion, static_cast(api), isDriverLoaded, driverLoadingTime); - GraphicsEnv::Driver driver = GraphicsEnv::Driver::NONE; + GpuStatsInfo::Driver driver = GpuStatsInfo::Driver::NONE; bool isIntendedDriverLoaded = false; - if (api == GraphicsEnv::Api::API_GL) { + if (api == GpuStatsInfo::Api::API_GL) { driver = mGpuStats.glDriverToLoad; isIntendedDriverLoaded = - isDriverLoaded && (mGpuStats.glDriverFallback == GraphicsEnv::Driver::NONE); + isDriverLoaded && (mGpuStats.glDriverFallback == GpuStatsInfo::Driver::NONE); } else { driver = mGpuStats.vkDriverToLoad; isIntendedDriverLoaded = - isDriverLoaded && (mGpuStats.vkDriverFallback == GraphicsEnv::Driver::NONE); + isDriverLoaded && (mGpuStats.vkDriverFallback == GpuStatsInfo::Driver::NONE); } const sp gpuService = getGpuService(); diff --git a/libs/graphicsenv/IGpuService.cpp b/libs/graphicsenv/IGpuService.cpp index 5f9624918f..30e5370650 100644 --- a/libs/graphicsenv/IGpuService.cpp +++ b/libs/graphicsenv/IGpuService.cpp @@ -30,7 +30,7 @@ public: virtual void setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, - const int32_t vulkanVersion, GraphicsEnv::Driver driver, + const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) { Parcel data, reply; data.writeInterfaceToken(IGpuService::getInterfaceDescriptor()); @@ -143,7 +143,7 @@ status_t BnGpuService::onTransact(uint32_t code, const Parcel& data, Parcel* rep if ((status = data.readInt64(&driverLoadingTime)) != OK) return status; setGpuStats(driverPackageName, driverVersionName, driverVersionCode, driverBuildTime, - appPackageName, vulkanVersion, static_cast(driver), + appPackageName, vulkanVersion, static_cast(driver), isDriverLoaded, driverLoadingTime); return OK; diff --git a/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h b/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h index edcccfea4a..c2fd10ae63 100644 --- a/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h +++ b/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h @@ -72,4 +72,43 @@ public: bool cpuVulkanInUse = false; }; +/* + * class for holding the gpu stats in GraphicsEnv before sending to GpuService. + */ +class GpuStatsInfo { +public: + enum Api { + API_GL = 0, + API_VK = 1, + }; + + enum Driver { + NONE = 0, + GL = 1, + GL_UPDATED = 2, + VULKAN = 3, + VULKAN_UPDATED = 4, + ANGLE = 5, + }; + + GpuStatsInfo() = default; + GpuStatsInfo(const GpuStatsInfo&) = default; + virtual ~GpuStatsInfo() = default; + + std::string driverPackageName = ""; + std::string driverVersionName = ""; + uint64_t driverVersionCode = 0; + int64_t driverBuildTime = 0; + std::string appPackageName = ""; + int32_t vulkanVersion = 0; + Driver glDriverToLoad = Driver::NONE; + Driver glDriverFallback = Driver::NONE; + Driver vkDriverToLoad = Driver::NONE; + Driver vkDriverFallback = Driver::NONE; + bool glDriverToSend = false; + bool vkDriverToSend = false; + int64_t glDriverLoadingTime = 0; + int64_t vkDriverLoadingTime = 0; +}; + } // namespace android diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index f5d19db493..aa14059d72 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -17,6 +17,8 @@ #ifndef ANDROID_UI_GRAPHICS_ENV_H #define ANDROID_UI_GRAPHICS_ENV_H 1 +#include + #include #include #include @@ -28,55 +30,6 @@ namespace android { struct NativeLoaderNamespace; class GraphicsEnv { -public: - enum Api { - API_GL = 0, - API_VK = 1, - }; - - enum Driver { - NONE = 0, - GL = 1, - GL_UPDATED = 2, - VULKAN = 3, - VULKAN_UPDATED = 4, - ANGLE = 5, - }; - -private: - struct GpuStats { - std::string driverPackageName; - std::string driverVersionName; - uint64_t driverVersionCode; - int64_t driverBuildTime; - std::string appPackageName; - int32_t vulkanVersion; - Driver glDriverToLoad; - Driver glDriverFallback; - Driver vkDriverToLoad; - Driver vkDriverFallback; - bool glDriverToSend; - bool vkDriverToSend; - int64_t glDriverLoadingTime; - int64_t vkDriverLoadingTime; - - GpuStats() - : driverPackageName(""), - driverVersionName(""), - driverVersionCode(0), - driverBuildTime(0), - appPackageName(""), - vulkanVersion(0), - glDriverToLoad(Driver::NONE), - glDriverFallback(Driver::NONE), - vkDriverToLoad(Driver::NONE), - vkDriverFallback(Driver::NONE), - glDriverToSend(false), - vkDriverToSend(false), - glDriverLoadingTime(0), - vkDriverLoadingTime(0) {} - }; - public: static GraphicsEnv& getInstance(); @@ -97,9 +50,9 @@ public: uint64_t versionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion); void setCpuVulkanInUse(); - void setDriverToLoad(Driver driver); - void setDriverLoaded(Api api, bool isDriverLoaded, int64_t driverLoadingTime); - void sendGpuStatsLocked(Api api, bool isDriverLoaded, int64_t driverLoadingTime); + void setDriverToLoad(GpuStatsInfo::Driver driver); + void setDriverLoaded(GpuStatsInfo::Api api, bool isDriverLoaded, int64_t driverLoadingTime); + void sendGpuStatsLocked(GpuStatsInfo::Api api, bool isDriverLoaded, int64_t driverLoadingTime); bool shouldUseAngle(std::string appName); bool shouldUseAngle(); @@ -135,7 +88,7 @@ private: std::string mDriverPath; std::string mSphalLibraries; std::mutex mStatsLock; - GpuStats mGpuStats; + GpuStatsInfo mGpuStats; std::string mAnglePath; std::string mAngleAppName; std::string mAngleDeveloperOptIn; diff --git a/libs/graphicsenv/include/graphicsenv/IGpuService.h b/libs/graphicsenv/include/graphicsenv/IGpuService.h index 34f1c7ee7e..a47bbafcc4 100644 --- a/libs/graphicsenv/include/graphicsenv/IGpuService.h +++ b/libs/graphicsenv/include/graphicsenv/IGpuService.h @@ -37,7 +37,7 @@ public: virtual void setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, - const int32_t vulkanVersion, GraphicsEnv::Driver driver, + const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) = 0; // set CPU Vulkan in use signal from GraphicsEnvironment. diff --git a/opengl/libs/EGL/Loader.cpp b/opengl/libs/EGL/Loader.cpp index 038a432337..23e11a82ac 100644 --- a/opengl/libs/EGL/Loader.cpp +++ b/opengl/libs/EGL/Loader.cpp @@ -311,7 +311,7 @@ void* Loader::open(egl_connection_t* cnx) } if (!hnd) { - android::GraphicsEnv::getInstance().setDriverLoaded(android::GraphicsEnv::Api::API_GL, + android::GraphicsEnv::getInstance().setDriverLoaded(android::GpuStatsInfo::Api::API_GL, false, systemTime() - openTime); } @@ -330,7 +330,7 @@ void* Loader::open(egl_connection_t* cnx) } if (!cnx->libEgl || !cnx->libGles2 || !cnx->libGles1) { - android::GraphicsEnv::getInstance().setDriverLoaded(android::GraphicsEnv::Api::API_GL, + android::GraphicsEnv::getInstance().setDriverLoaded(android::GpuStatsInfo::Api::API_GL, false, systemTime() - openTime); } @@ -340,7 +340,7 @@ void* Loader::open(egl_connection_t* cnx) LOG_ALWAYS_FATAL_IF(!cnx->libGles2 || !cnx->libGles1, "couldn't load system OpenGL ES wrapper libraries"); - android::GraphicsEnv::getInstance().setDriverLoaded(android::GraphicsEnv::Api::API_GL, true, + android::GraphicsEnv::getInstance().setDriverLoaded(android::GpuStatsInfo::Api::API_GL, true, systemTime() - openTime); return (void*)hnd; @@ -637,7 +637,7 @@ Loader::driver_t* Loader::attempt_to_load_angle(egl_connection_t* cnx) { return nullptr; } - android::GraphicsEnv::getInstance().setDriverToLoad(android::GraphicsEnv::Driver::ANGLE); + android::GraphicsEnv::getInstance().setDriverToLoad(android::GpuStatsInfo::Driver::ANGLE); driver_t* hnd = nullptr; // ANGLE doesn't ship with GLES library, and thus we skip GLES driver. @@ -666,7 +666,7 @@ Loader::driver_t* Loader::attempt_to_load_updated_driver(egl_connection_t* cnx) } ALOGD("Load updated gl driver."); - android::GraphicsEnv::getInstance().setDriverToLoad(android::GraphicsEnv::Driver::GL_UPDATED); + android::GraphicsEnv::getInstance().setDriverToLoad(android::GpuStatsInfo::Driver::GL_UPDATED); driver_t* hnd = nullptr; void* dso = load_updated_driver("GLES", ns); if (dso) { @@ -697,7 +697,7 @@ Loader::driver_t* Loader::attempt_to_load_updated_driver(egl_connection_t* cnx) Loader::driver_t* Loader::attempt_to_load_system_driver(egl_connection_t* cnx, const char* suffix, const bool exact) { ATRACE_CALL(); - android::GraphicsEnv::getInstance().setDriverToLoad(android::GraphicsEnv::Driver::GL); + android::GraphicsEnv::getInstance().setDriverToLoad(android::GpuStatsInfo::Driver::GL); driver_t* hnd = nullptr; void* dso = load_system_driver("GLES", suffix, exact); if (dso) { diff --git a/services/gpuservice/GpuService.cpp b/services/gpuservice/GpuService.cpp index 8accf9d450..72757dd02b 100644 --- a/services/gpuservice/GpuService.cpp +++ b/services/gpuservice/GpuService.cpp @@ -51,7 +51,7 @@ GpuService::GpuService() : mGpuStats(std::make_unique()){}; void GpuService::setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, - const int32_t vulkanVersion, GraphicsEnv::Driver driver, + const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) { ATRACE_CALL(); diff --git a/services/gpuservice/GpuService.h b/services/gpuservice/GpuService.h index 822690134a..3e0e1b5f9b 100644 --- a/services/gpuservice/GpuService.h +++ b/services/gpuservice/GpuService.h @@ -46,7 +46,7 @@ private: void setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion, - GraphicsEnv::Driver driver, bool isDriverLoaded, + GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) override; status_t getGpuStatsGlobalInfo(std::vector* outStats) const override; status_t getGpuStatsAppInfo(std::vector* outStats) const override; diff --git a/services/gpuservice/gpustats/GpuStats.cpp b/services/gpuservice/gpustats/GpuStats.cpp index 37c6abc96b..576c72cd3c 100644 --- a/services/gpuservice/gpustats/GpuStats.cpp +++ b/services/gpuservice/gpustats/GpuStats.cpp @@ -27,20 +27,20 @@ namespace android { -static void addLoadingCount(GraphicsEnv::Driver driver, bool isDriverLoaded, +static void addLoadingCount(GpuStatsInfo::Driver driver, bool isDriverLoaded, GpuStatsGlobalInfo* const outGlobalInfo) { switch (driver) { - case GraphicsEnv::Driver::GL: - case GraphicsEnv::Driver::GL_UPDATED: + case GpuStatsInfo::Driver::GL: + case GpuStatsInfo::Driver::GL_UPDATED: outGlobalInfo->glLoadingCount++; if (!isDriverLoaded) outGlobalInfo->glLoadingFailureCount++; break; - case GraphicsEnv::Driver::VULKAN: - case GraphicsEnv::Driver::VULKAN_UPDATED: + case GpuStatsInfo::Driver::VULKAN: + case GpuStatsInfo::Driver::VULKAN_UPDATED: outGlobalInfo->vkLoadingCount++; if (!isDriverLoaded) outGlobalInfo->vkLoadingFailureCount++; break; - case GraphicsEnv::Driver::ANGLE: + case GpuStatsInfo::Driver::ANGLE: outGlobalInfo->angleLoadingCount++; if (!isDriverLoaded) outGlobalInfo->angleLoadingFailureCount++; break; @@ -49,22 +49,22 @@ static void addLoadingCount(GraphicsEnv::Driver driver, bool isDriverLoaded, } } -static void addLoadingTime(GraphicsEnv::Driver driver, int64_t driverLoadingTime, +static void addLoadingTime(GpuStatsInfo::Driver driver, int64_t driverLoadingTime, GpuStatsAppInfo* const outAppInfo) { switch (driver) { - case GraphicsEnv::Driver::GL: - case GraphicsEnv::Driver::GL_UPDATED: + case GpuStatsInfo::Driver::GL: + case GpuStatsInfo::Driver::GL_UPDATED: if (outAppInfo->glDriverLoadingTime.size() < GpuStats::MAX_NUM_LOADING_TIMES) { outAppInfo->glDriverLoadingTime.emplace_back(driverLoadingTime); } break; - case GraphicsEnv::Driver::VULKAN: - case GraphicsEnv::Driver::VULKAN_UPDATED: + case GpuStatsInfo::Driver::VULKAN: + case GpuStatsInfo::Driver::VULKAN_UPDATED: if (outAppInfo->vkDriverLoadingTime.size() < GpuStats::MAX_NUM_LOADING_TIMES) { outAppInfo->vkDriverLoadingTime.emplace_back(driverLoadingTime); } break; - case GraphicsEnv::Driver::ANGLE: + case GpuStatsInfo::Driver::ANGLE: if (outAppInfo->angleDriverLoadingTime.size() < GpuStats::MAX_NUM_LOADING_TIMES) { outAppInfo->angleDriverLoadingTime.emplace_back(driverLoadingTime); } @@ -77,7 +77,7 @@ static void addLoadingTime(GraphicsEnv::Driver driver, int64_t driverLoadingTime void GpuStats::insert(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion, - GraphicsEnv::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) { + GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) { ATRACE_CALL(); std::lock_guard lock(mLock); diff --git a/services/gpuservice/gpustats/GpuStats.h b/services/gpuservice/gpustats/GpuStats.h index b293f5988d..a79b2ba208 100644 --- a/services/gpuservice/gpustats/GpuStats.h +++ b/services/gpuservice/gpustats/GpuStats.h @@ -36,7 +36,7 @@ public: void insert(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion, - GraphicsEnv::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime); + GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime); // Set CPU Vulkan in use signal into app stats. void setCpuVulkanInUse(const std::string& appPackageName, const uint64_t driverVersionCode); // dumpsys interface diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index d33e5528e8..680f94f42a 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -211,7 +211,7 @@ int LoadBuiltinDriver(const hwvulkan_module_t** module) { if (!ns) return -ENOENT; android::GraphicsEnv::getInstance().setDriverToLoad( - android::GraphicsEnv::Driver::VULKAN); + android::GpuStatsInfo::Driver::VULKAN); return LoadDriver(ns, module); } @@ -222,7 +222,7 @@ int LoadUpdatedDriver(const hwvulkan_module_t** module) { if (!ns) return -ENOENT; android::GraphicsEnv::getInstance().setDriverToLoad( - android::GraphicsEnv::Driver::VULKAN_UPDATED); + android::GpuStatsInfo::Driver::VULKAN_UPDATED); return LoadDriver(ns, module); } @@ -257,7 +257,7 @@ bool Hal::Open() { } if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( - android::GraphicsEnv::Api::API_VK, false, systemTime() - openTime); + android::GpuStatsInfo::Api::API_VK, false, systemTime() - openTime); ALOGV("unable to load Vulkan HAL, using stub HAL (result=%d)", result); return true; } @@ -271,7 +271,7 @@ bool Hal::Open() { ATRACE_END(); if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( - android::GraphicsEnv::Api::API_VK, false, systemTime() - openTime); + android::GpuStatsInfo::Api::API_VK, false, systemTime() - openTime); // Any device with a Vulkan HAL should be able to open the device. ALOGE("failed to open Vulkan HAL device: %s (%d)", strerror(-result), result); @@ -283,7 +283,7 @@ bool Hal::Open() { hal_.InitDebugReportIndex(); android::GraphicsEnv::getInstance().setDriverLoaded( - android::GraphicsEnv::Api::API_VK, true, systemTime() - openTime); + android::GpuStatsInfo::Api::API_VK, true, systemTime() - openTime); return true; } -- cgit v1.2.3-59-g8ed1b From bcba4117941a0506971654331d89961b6fbfd3c0 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 3 Jul 2019 13:39:32 -0700 Subject: GpuStats: refactor single stats pieces into a uniform way This change makes it easy for adding single stats pieces into GpuService without adding or modifying the binder interface each time. Bug: 135210726 Test: adb shell dumpsys gpu Change-Id: I2907065a55d03a6c1494737e6f0a77f6e94272eb --- libs/graphicsenv/GraphicsEnv.cpp | 6 +++--- libs/graphicsenv/IGpuService.cpp | 19 ++++++++++++++----- libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h | 4 ++++ libs/graphicsenv/include/graphicsenv/GraphicsEnv.h | 4 ++-- libs/graphicsenv/include/graphicsenv/IGpuService.h | 8 ++++---- services/gpuservice/GpuService.cpp | 16 +++------------- services/gpuservice/GpuService.h | 4 ++-- services/gpuservice/gpustats/GpuStats.cpp | 17 ++++++++++++++--- services/gpuservice/gpustats/GpuStats.h | 5 +++-- vulkan/libvulkan/driver.cpp | 3 ++- 10 files changed, 51 insertions(+), 35 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index a411dd56f6..c5d5f71800 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -267,14 +267,14 @@ static sp getGpuService() { return interface_cast(binder); } -void GraphicsEnv::setCpuVulkanInUse() { +void GraphicsEnv::setTargetStats(const GpuStatsInfo::Stats stats, const uint64_t value) { ATRACE_CALL(); - // Use the same stats lock to protect getGpuService() as well. std::lock_guard lock(mStatsLock); const sp gpuService = getGpuService(); if (gpuService) { - gpuService->setCpuVulkanInUse(mGpuStats.appPackageName, mGpuStats.driverVersionCode); + gpuService->setTargetStats(mGpuStats.appPackageName, mGpuStats.driverVersionCode, stats, + value); } } diff --git a/libs/graphicsenv/IGpuService.cpp b/libs/graphicsenv/IGpuService.cpp index 30e5370650..9f5b0ff46f 100644 --- a/libs/graphicsenv/IGpuService.cpp +++ b/libs/graphicsenv/IGpuService.cpp @@ -92,15 +92,17 @@ public: return reply.readParcelableVector(outStats); } - virtual void setCpuVulkanInUse(const std::string& appPackageName, - const uint64_t driverVersionCode) { + virtual void setTargetStats(const std::string& appPackageName, const uint64_t driverVersionCode, + const GpuStatsInfo::Stats stats, const uint64_t value) { Parcel data, reply; data.writeInterfaceToken(IGpuService::getInterfaceDescriptor()); data.writeUtf8AsUtf16(appPackageName); data.writeUint64(driverVersionCode); + data.writeInt32(static_cast(stats)); + data.writeUint64(value); - remote()->transact(BnGpuService::SET_CPU_VULKAN_IN_USE, data, &reply, IBinder::FLAG_ONEWAY); + remote()->transact(BnGpuService::SET_TARGET_STATS, data, &reply, IBinder::FLAG_ONEWAY); } }; @@ -174,7 +176,7 @@ status_t BnGpuService::onTransact(uint32_t code, const Parcel& data, Parcel* rep return OK; } - case SET_CPU_VULKAN_IN_USE: { + case SET_TARGET_STATS: { CHECK_INTERFACE(IGpuService, data, reply); std::string appPackageName; @@ -183,7 +185,14 @@ status_t BnGpuService::onTransact(uint32_t code, const Parcel& data, Parcel* rep uint64_t driverVersionCode; if ((status = data.readUint64(&driverVersionCode)) != OK) return status; - setCpuVulkanInUse(appPackageName, driverVersionCode); + int32_t stats; + if ((status = data.readInt32(&stats)) != OK) return status; + + uint64_t value; + if ((status = data.readUint64(&value)) != OK) return status; + + setTargetStats(appPackageName, driverVersionCode, + static_cast(stats), value); return OK; } diff --git a/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h b/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h index c2fd10ae63..711e8691ab 100644 --- a/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h +++ b/libs/graphicsenv/include/graphicsenv/GpuStatsInfo.h @@ -91,6 +91,10 @@ public: ANGLE = 5, }; + enum Stats { + CPU_VULKAN_IN_USE = 0, + }; + GpuStatsInfo() = default; GpuStatsInfo(const GpuStatsInfo&) = default; virtual ~GpuStatsInfo() = default; diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index d0fc580fb6..a47f468e7a 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -60,8 +60,8 @@ public: void setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t versionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion); - // Set that CPU type physical device is in use. - void setCpuVulkanInUse(); + // Set stats for target GpuStatsInfo::Stats type. + void setTargetStats(const GpuStatsInfo::Stats stats, const uint64_t value = 0); // Set which driver is intended to load. void setDriverToLoad(GpuStatsInfo::Driver driver); // Set which driver is actually loaded. diff --git a/libs/graphicsenv/include/graphicsenv/IGpuService.h b/libs/graphicsenv/include/graphicsenv/IGpuService.h index a47bbafcc4..f523d585be 100644 --- a/libs/graphicsenv/include/graphicsenv/IGpuService.h +++ b/libs/graphicsenv/include/graphicsenv/IGpuService.h @@ -40,9 +40,9 @@ public: const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) = 0; - // set CPU Vulkan in use signal from GraphicsEnvironment. - virtual void setCpuVulkanInUse(const std::string& appPackageName, - const uint64_t driverVersionCode) = 0; + // set target stats. + virtual void setTargetStats(const std::string& appPackageName, const uint64_t driverVersionCode, + const GpuStatsInfo::Stats stats, const uint64_t value = 0) = 0; // get GPU global stats from GpuStats module. virtual status_t getGpuStatsGlobalInfo(std::vector* outStats) const = 0; @@ -57,7 +57,7 @@ public: SET_GPU_STATS = IBinder::FIRST_CALL_TRANSACTION, GET_GPU_STATS_GLOBAL_INFO, GET_GPU_STATS_APP_INFO, - SET_CPU_VULKAN_IN_USE, + SET_TARGET_STATS, // Always append new enum to the end. }; diff --git a/services/gpuservice/GpuService.cpp b/services/gpuservice/GpuService.cpp index 72757dd02b..c81ab509c3 100644 --- a/services/gpuservice/GpuService.cpp +++ b/services/gpuservice/GpuService.cpp @@ -53,33 +53,23 @@ void GpuService::setGpuStats(const std::string& driverPackageName, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime) { - ATRACE_CALL(); - mGpuStats->insert(driverPackageName, driverVersionName, driverVersionCode, driverBuildTime, appPackageName, vulkanVersion, driver, isDriverLoaded, driverLoadingTime); } status_t GpuService::getGpuStatsGlobalInfo(std::vector* outStats) const { - ATRACE_CALL(); - mGpuStats->pullGlobalStats(outStats); - return OK; } status_t GpuService::getGpuStatsAppInfo(std::vector* outStats) const { - ATRACE_CALL(); - mGpuStats->pullAppStats(outStats); - return OK; } -void GpuService::setCpuVulkanInUse(const std::string& appPackageName, - const uint64_t driverVersionCode) { - ATRACE_CALL(); - - mGpuStats->setCpuVulkanInUse(appPackageName, driverVersionCode); +void GpuService::setTargetStats(const std::string& appPackageName, const uint64_t driverVersionCode, + const GpuStatsInfo::Stats stats, const uint64_t value) { + mGpuStats->insertTargetStats(appPackageName, driverVersionCode, stats, value); } status_t GpuService::shellCommand(int /*in*/, int out, int err, std::vector& args) { diff --git a/services/gpuservice/GpuService.h b/services/gpuservice/GpuService.h index 3e0e1b5f9b..525fb4fada 100644 --- a/services/gpuservice/GpuService.h +++ b/services/gpuservice/GpuService.h @@ -50,8 +50,8 @@ private: int64_t driverLoadingTime) override; status_t getGpuStatsGlobalInfo(std::vector* outStats) const override; status_t getGpuStatsAppInfo(std::vector* outStats) const override; - void setCpuVulkanInUse(const std::string& appPackageName, - const uint64_t driverVersionCode) override; + void setTargetStats(const std::string& appPackageName, const uint64_t driverVersionCode, + const GpuStatsInfo::Stats stats, const uint64_t value) override; /* * IBinder interface diff --git a/services/gpuservice/gpustats/GpuStats.cpp b/services/gpuservice/gpustats/GpuStats.cpp index 576c72cd3c..423c89f797 100644 --- a/services/gpuservice/gpustats/GpuStats.cpp +++ b/services/gpuservice/gpustats/GpuStats.cpp @@ -126,14 +126,25 @@ void GpuStats::insert(const std::string& driverPackageName, const std::string& d addLoadingTime(driver, driverLoadingTime, &mAppStats[appStatsKey]); } -void GpuStats::setCpuVulkanInUse(const std::string& appPackageName, - const uint64_t driverVersionCode) { +void GpuStats::insertTargetStats(const std::string& appPackageName, + const uint64_t driverVersionCode, const GpuStatsInfo::Stats stats, + const uint64_t /*value*/) { + ATRACE_CALL(); + const std::string appStatsKey = appPackageName + std::to_string(driverVersionCode); + + std::lock_guard lock(mLock); if (!mAppStats.count(appStatsKey)) { return; } - mAppStats[appStatsKey].cpuVulkanInUse = true; + switch (stats) { + case GpuStatsInfo::Stats::CPU_VULKAN_IN_USE: + mAppStats[appStatsKey].cpuVulkanInUse = true; + break; + default: + break; + } } void GpuStats::interceptSystemDriverStatsLocked() { diff --git a/services/gpuservice/gpustats/GpuStats.h b/services/gpuservice/gpustats/GpuStats.h index a79b2ba208..656b181464 100644 --- a/services/gpuservice/gpustats/GpuStats.h +++ b/services/gpuservice/gpustats/GpuStats.h @@ -37,8 +37,9 @@ public: uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, const int32_t vulkanVersion, GpuStatsInfo::Driver driver, bool isDriverLoaded, int64_t driverLoadingTime); - // Set CPU Vulkan in use signal into app stats. - void setCpuVulkanInUse(const std::string& appPackageName, const uint64_t driverVersionCode); + // Insert target stats into app stats or potentially global stats as well. + void insertTargetStats(const std::string& appPackageName, const uint64_t driverVersionCode, + const GpuStatsInfo::Stats stats, const uint64_t value); // dumpsys interface void dump(const Vector& args, std::string* result); // Pull gpu global stats diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 680f94f42a..f596086ccf 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -1173,7 +1173,8 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, if (properties.deviceType == VK_PHYSICAL_DEVICE_TYPE_CPU) { // Log that the app is hitting software Vulkan implementation - android::GraphicsEnv::getInstance().setCpuVulkanInUse(); + android::GraphicsEnv::getInstance().setTargetStats( + android::GpuStatsInfo::Stats::CPU_VULKAN_IN_USE); } data->driver_device = dev; -- cgit v1.2.3-59-g8ed1b From e40dd7396cab9326985339efb513c4d04fc584e4 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Mon, 5 Aug 2019 16:41:03 -0700 Subject: libvulkan: get android_get_exported_namespace from real header Bug: 134185757 Test: build, flash and boot. Change-Id: Ife2cab93ebddf898ed8c031a33e7997ae8d5d63f --- vulkan/libvulkan/Android.bp | 1 + vulkan/libvulkan/driver.cpp | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/Android.bp b/vulkan/libvulkan/Android.bp index 993b751747..85ef475680 100644 --- a/vulkan/libvulkan/Android.bp +++ b/vulkan/libvulkan/Android.bp @@ -75,6 +75,7 @@ cc_library_shared { header_libs: [ "hwvulkan_headers", + "libnativeloader-dummy-headers", "vulkan_headers", ], export_header_lib_headers: ["vulkan_headers"], diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index f596086ccf..9a670f6c4d 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -44,11 +45,6 @@ using namespace android::hardware::configstore; using namespace android::hardware::configstore::V1_0; -// TODO(b/37049319) Get this from a header once one exists -extern "C" { -android_namespace_t* android_get_exported_namespace(const char*); -} - // #define ENABLE_ALLOC_CALLSTACKS 1 #if ENABLE_ALLOC_CALLSTACKS #include -- cgit v1.2.3-59-g8ed1b From 899d1758a2bb79128c3fcf0b5736986619b02658 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Mon, 23 Sep 2019 16:05:35 -0700 Subject: libvulkan: intercept vkQueueSubmit to insert a tracepoint Bug: 141501384 Test: build, flash and boot. Run deqp Change-Id: I840270f213455250f72b2e7dc996124692bbc473 --- vulkan/libvulkan/driver.cpp | 11 +++++++++++ vulkan/libvulkan/driver.h | 1 + vulkan/libvulkan/driver_gen.cpp | 8 ++++++++ vulkan/libvulkan/driver_gen.h | 1 + vulkan/scripts/driver_generator.py | 4 ++++ 5 files changed, 25 insertions(+) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 9a670f6c4d..b413ac9375 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -1313,5 +1313,16 @@ AllocateCommandBuffers(VkDevice device, return result; } +VKAPI_ATTR VkResult QueueSubmit(VkQueue queue, + uint32_t submitCount, + const VkSubmitInfo* pSubmits, + VkFence fence) { + ATRACE_CALL(); + + const auto& data = GetData(queue); + + return data.driver.QueueSubmit(queue, submitCount, pSubmits, fence); +} + } // namespace driver } // namespace vulkan diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index 57c956d9a4..f058c47d54 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -131,6 +131,7 @@ VKAPI_ATTR VkResult EnumeratePhysicalDeviceGroups(VkInstance instance, uint32_t* VKAPI_ATTR void GetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue* pQueue); VKAPI_ATTR void GetDeviceQueue2(VkDevice device, const VkDeviceQueueInfo2* pQueueInfo, VkQueue* pQueue); VKAPI_ATTR VkResult AllocateCommandBuffers(VkDevice device, const VkCommandBufferAllocateInfo* pAllocateInfo, VkCommandBuffer* pCommandBuffers); +VKAPI_ATTR VkResult QueueSubmit(VkQueue queue, uint32_t submitCount, const VkSubmitInfo* pSubmits, VkFence fence); // clang-format on template diff --git a/vulkan/libvulkan/driver_gen.cpp b/vulkan/libvulkan/driver_gen.cpp index 4ea5a6106d..d829e41c83 100644 --- a/vulkan/libvulkan/driver_gen.cpp +++ b/vulkan/libvulkan/driver_gen.cpp @@ -443,6 +443,13 @@ const ProcHook g_proc_hooks[] = { nullptr, nullptr, }, + { + "vkQueueSubmit", + ProcHook::DEVICE, + ProcHook::EXTENSION_CORE, + reinterpret_cast(QueueSubmit), + nullptr, + }, { "vkSetHdrMetadataEXT", ProcHook::DEVICE, @@ -537,6 +544,7 @@ bool InitDriverTable(VkDevice dev, INIT_PROC(true, dev, GetDeviceProcAddr); INIT_PROC(true, dev, DestroyDevice); INIT_PROC(true, dev, GetDeviceQueue); + INIT_PROC(true, dev, QueueSubmit); INIT_PROC(true, dev, CreateImage); INIT_PROC(true, dev, DestroyImage); INIT_PROC(true, dev, AllocateCommandBuffers); diff --git a/vulkan/libvulkan/driver_gen.h b/vulkan/libvulkan/driver_gen.h index f4182c7893..fb2f257755 100644 --- a/vulkan/libvulkan/driver_gen.h +++ b/vulkan/libvulkan/driver_gen.h @@ -84,6 +84,7 @@ struct DeviceDriverTable { PFN_vkGetDeviceProcAddr GetDeviceProcAddr; PFN_vkDestroyDevice DestroyDevice; PFN_vkGetDeviceQueue GetDeviceQueue; + PFN_vkQueueSubmit QueueSubmit; PFN_vkCreateImage CreateImage; PFN_vkDestroyImage DestroyImage; PFN_vkAllocateCommandBuffers AllocateCommandBuffers; diff --git a/vulkan/scripts/driver_generator.py b/vulkan/scripts/driver_generator.py index 04d9f239e7..ef36f8cd0c 100644 --- a/vulkan/scripts/driver_generator.py +++ b/vulkan/scripts/driver_generator.py @@ -91,6 +91,8 @@ def isDriverTableEntry(functionName): 'vkGetInstanceProcAddr' : True, 'vkGetDeviceProcAddr' : True, + 'vkQueueSubmit' : True, + # VK_KHR_swapchain->VK_ANDROID_native_buffer translation 'vkCreateImage' : True, 'vkDestroyImage' : True, @@ -191,6 +193,8 @@ def isIntercepted(functionName): 'vkGetInstanceProcAddr' : True, 'vkGetDeviceProcAddr' : True, + 'vkQueueSubmit' : True, + # VK_KHR_swapchain v69 requirement 'vkBindImageMemory2' : True, 'vkBindImageMemory2KHR' : True -- cgit v1.2.3-59-g8ed1b From 7cc36a50e15cc781d9b1260d2cf418780173b2ee Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 11 Oct 2019 19:02:09 -0700 Subject: Vulkan: correctly expose Vulkan entry points This change fixes the advertisement of core Vulkan entry points as below: 1. GIPA returns a valid checked_proc for 1.1 core device APIs. 2. GDPA returns NULL for 1.1 core device APIs on a 1.0 physical device. Bug: 134185757 Bug: 142266108 Test: dEQP-VK.memory.binding on 1.1 loader and 1.0 device ICD Test: dEQP-VK.api.info.instance on 1.1 loader and 1.0 instance ICD Change-Id: I0a3e06dc04bade4f36a7e68ee2f53979c656ee4e --- vulkan/libvulkan/api.cpp | 6 +++- vulkan/libvulkan/driver.cpp | 60 +++++++++++++++++++++++++++-------- vulkan/libvulkan/driver.h | 8 ++--- vulkan/libvulkan/driver_gen.cpp | 51 ++++++++++++++++++++---------- vulkan/libvulkan/driver_gen.h | 3 +- vulkan/scripts/driver_generator.py | 65 ++++++++++++++++++++++++++++---------- vulkan/scripts/generator_common.py | 19 +++++++++++ 7 files changed, 158 insertions(+), 54 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 48f26e7e43..1578d9ffce 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -519,7 +519,11 @@ LayerChain::LayerChain(bool is_instance, get_device_proc_addr_(nullptr), driver_extensions_(nullptr), driver_extension_count_(0) { - enabled_extensions_.set(driver::ProcHook::EXTENSION_CORE); + // advertise the loader supported core Vulkan API version at vulkan::api + for (uint32_t i = driver::ProcHook::EXTENSION_CORE_1_0; + i != driver::ProcHook::EXTENSION_COUNT; ++i) { + enabled_extensions_.set(i); + } } LayerChain::~LayerChain() { diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index b413ac9375..c3c19ecdff 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -99,6 +99,7 @@ class CreateInfoWrapper { VkResult Validate(); void DowngradeApiVersion(); + void UpgradeDeviceCoreApiVersion(uint32_t api_version); const std::bitset& GetHookExtensions() const; const std::bitset& GetHalExtensions() const; @@ -328,8 +329,12 @@ CreateInfoWrapper::CreateInfoWrapper(const VkInstanceCreateInfo& create_info, physical_dev_(VK_NULL_HANDLE), instance_info_(create_info), extension_filter_() { - hook_extensions_.set(ProcHook::EXTENSION_CORE); - hal_extensions_.set(ProcHook::EXTENSION_CORE); + // instance core versions need to match the loader api version + for (uint32_t i = ProcHook::EXTENSION_CORE_1_0; + i != ProcHook::EXTENSION_COUNT; ++i) { + hook_extensions_.set(i); + hal_extensions_.set(i); + } } CreateInfoWrapper::CreateInfoWrapper(VkPhysicalDevice physical_dev, @@ -340,8 +345,9 @@ CreateInfoWrapper::CreateInfoWrapper(VkPhysicalDevice physical_dev, physical_dev_(physical_dev), dev_info_(create_info), extension_filter_() { - hook_extensions_.set(ProcHook::EXTENSION_CORE); - hal_extensions_.set(ProcHook::EXTENSION_CORE); + // initialize with baseline core API version + hook_extensions_.set(ProcHook::EXTENSION_CORE_1_0); + hal_extensions_.set(ProcHook::EXTENSION_CORE_1_0); } CreateInfoWrapper::~CreateInfoWrapper() { @@ -540,7 +546,8 @@ void CreateInfoWrapper::FilterExtension(const char* name) { case ProcHook::ANDROID_external_memory_android_hardware_buffer: case ProcHook::ANDROID_native_buffer: case ProcHook::GOOGLE_display_timing: - case ProcHook::EXTENSION_CORE: + case ProcHook::EXTENSION_CORE_1_0: + case ProcHook::EXTENSION_CORE_1_1: case ProcHook::EXTENSION_COUNT: // Device and meta extensions. If we ever get here it's a bug in // our code. But enumerating them lets us avoid having a default @@ -588,7 +595,8 @@ void CreateInfoWrapper::FilterExtension(const char* name) { case ProcHook::EXT_debug_report: case ProcHook::EXT_swapchain_colorspace: case ProcHook::ANDROID_native_buffer: - case ProcHook::EXTENSION_CORE: + case ProcHook::EXTENSION_CORE_1_0: + case ProcHook::EXTENSION_CORE_1_1: case ProcHook::EXTENSION_COUNT: // Instance and meta extensions. If we ever get here it's a bug // in our code. But enumerating them lets us avoid having a @@ -636,6 +644,31 @@ void CreateInfoWrapper::DowngradeApiVersion() { } } +void CreateInfoWrapper::UpgradeDeviceCoreApiVersion(uint32_t api_version) { + ALOG_ASSERT(!is_instance_, "Device only API called by instance wrapper."); + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wold-style-cast" + api_version ^= VK_VERSION_PATCH(api_version); +#pragma clang diagnostic pop + + // cap the API version to the loader supported highest version + if (api_version > VK_API_VERSION_1_1) + api_version = VK_API_VERSION_1_1; + + switch (api_version) { + case VK_API_VERSION_1_1: + hook_extensions_.set(ProcHook::EXTENSION_CORE_1_1); + hal_extensions_.set(ProcHook::EXTENSION_CORE_1_1); + [[clang::fallthrough]]; + case VK_API_VERSION_1_0: + break; + default: + ALOGD("Unknown upgrade API version[%u]", api_version); + break; + } +} + VKAPI_ATTR void* DefaultAllocate(void*, size_t size, size_t alignment, @@ -771,7 +804,7 @@ PFN_vkVoidFunction GetInstanceProcAddr(VkInstance instance, const char* pName) { : nullptr; break; case ProcHook::DEVICE: - proc = (hook->extension == ProcHook::EXTENSION_CORE) + proc = (hook->extension == ProcHook::EXTENSION_CORE_1_0) ? hook->proc : hook->checked_proc; break; @@ -1117,6 +1150,13 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, if (!data) return VK_ERROR_OUT_OF_HOST_MEMORY; + VkPhysicalDeviceProperties properties; + ATRACE_BEGIN("driver.GetPhysicalDeviceProperties"); + instance_data.driver.GetPhysicalDeviceProperties(physicalDevice, + &properties); + ATRACE_END(); + + wrapper.UpgradeDeviceCoreApiVersion(properties.apiVersion); data->hook_extensions |= wrapper.GetHookExtensions(); // call into the driver @@ -1161,12 +1201,6 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, return VK_ERROR_INCOMPATIBLE_DRIVER; } - VkPhysicalDeviceProperties properties; - ATRACE_BEGIN("driver.GetPhysicalDeviceProperties"); - instance_data.driver.GetPhysicalDeviceProperties(physicalDevice, - &properties); - ATRACE_END(); - if (properties.deviceType == VK_PHYSICAL_DEVICE_TYPE_CPU) { // Log that the app is hitting software Vulkan implementation android::GraphicsEnv::getInstance().setTargetStats( diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index f058c47d54..61e1818f28 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -67,9 +67,7 @@ struct InstanceData { : opaque_api_data(), allocator(alloc), driver(), - get_device_proc_addr(nullptr) { - hook_extensions.set(ProcHook::EXTENSION_CORE); - } + get_device_proc_addr(nullptr) {} api::InstanceData opaque_api_data; @@ -89,9 +87,7 @@ struct DeviceData { : opaque_api_data(), allocator(alloc), debug_report_callbacks(debug_report_callbacks_), - driver() { - hook_extensions.set(ProcHook::EXTENSION_CORE); - } + driver() {} api::DeviceData opaque_api_data; diff --git a/vulkan/libvulkan/driver_gen.cpp b/vulkan/libvulkan/driver_gen.cpp index f676573de9..52205e9e79 100644 --- a/vulkan/libvulkan/driver_gen.cpp +++ b/vulkan/libvulkan/driver_gen.cpp @@ -74,6 +74,15 @@ VKAPI_ATTR VkResult checkedQueuePresentKHR(VkQueue queue, const VkPresentInfoKHR } } +VKAPI_ATTR VkResult checkedBindImageMemory2(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { + if (GetData(device).hook_extensions[ProcHook::EXTENSION_CORE_1_1]) { + return BindImageMemory2(device, bindInfoCount, pBindInfos); + } else { + Logger(device).Err(device, "VK_VERSION_1_1 not enabled. vkBindImageMemory2 not executed."); + return VK_SUCCESS; + } +} + VKAPI_ATTR VkResult checkedBindImageMemory2KHR(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { if (GetData(device).hook_extensions[ProcHook::KHR_bind_memory2]) { return BindImageMemory2KHR(device, bindInfoCount, pBindInfos); @@ -145,6 +154,14 @@ VKAPI_ATTR VkResult checkedGetPastPresentationTimingGOOGLE(VkDevice device, VkSw } } +VKAPI_ATTR void checkedGetDeviceQueue2(VkDevice device, const VkDeviceQueueInfo2* pQueueInfo, VkQueue* pQueue) { + if (GetData(device).hook_extensions[ProcHook::EXTENSION_CORE_1_1]) { + GetDeviceQueue2(device, pQueueInfo, pQueue); + } else { + Logger(device).Err(device, "VK_VERSION_1_1 not enabled. vkGetDeviceQueue2 not executed."); + } +} + // clang-format on const ProcHook g_proc_hooks[] = { @@ -173,16 +190,16 @@ const ProcHook g_proc_hooks[] = { { "vkAllocateCommandBuffers", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(AllocateCommandBuffers), nullptr, }, { "vkBindImageMemory2", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(BindImageMemory2), - nullptr, + reinterpret_cast(checkedBindImageMemory2), }, { "vkBindImageMemory2KHR", @@ -208,14 +225,14 @@ const ProcHook g_proc_hooks[] = { { "vkCreateDevice", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(CreateDevice), nullptr, }, { "vkCreateInstance", ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(CreateInstance), nullptr, }, @@ -243,14 +260,14 @@ const ProcHook g_proc_hooks[] = { { "vkDestroyDevice", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(DestroyDevice), nullptr, }, { "vkDestroyInstance", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(DestroyInstance), nullptr, }, @@ -271,28 +288,28 @@ const ProcHook g_proc_hooks[] = { { "vkEnumerateDeviceExtensionProperties", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumerateDeviceExtensionProperties), nullptr, }, { "vkEnumerateInstanceExtensionProperties", ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumerateInstanceExtensionProperties), nullptr, }, { "vkEnumeratePhysicalDeviceGroups", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(EnumeratePhysicalDeviceGroups), nullptr, }, { "vkEnumeratePhysicalDevices", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumeratePhysicalDevices), nullptr, }, @@ -313,28 +330,28 @@ const ProcHook g_proc_hooks[] = { { "vkGetDeviceProcAddr", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetDeviceProcAddr), nullptr, }, { "vkGetDeviceQueue", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetDeviceQueue), nullptr, }, { "vkGetDeviceQueue2", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(GetDeviceQueue2), - nullptr, + reinterpret_cast(checkedGetDeviceQueue2), }, { "vkGetInstanceProcAddr", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetInstanceProcAddr), nullptr, }, @@ -446,7 +463,7 @@ const ProcHook g_proc_hooks[] = { { "vkQueueSubmit", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(QueueSubmit), nullptr, }, diff --git a/vulkan/libvulkan/driver_gen.h b/vulkan/libvulkan/driver_gen.h index cd7d8f82e0..43c4d1469e 100644 --- a/vulkan/libvulkan/driver_gen.h +++ b/vulkan/libvulkan/driver_gen.h @@ -49,7 +49,8 @@ struct ProcHook { KHR_bind_memory2, KHR_get_physical_device_properties2, - EXTENSION_CORE, // valid bit + EXTENSION_CORE_1_0, + EXTENSION_CORE_1_1, EXTENSION_COUNT, EXTENSION_UNKNOWN, }; diff --git a/vulkan/scripts/driver_generator.py b/vulkan/scripts/driver_generator.py index 0f3d760d5f..a64a7026db 100644 --- a/vulkan/scripts/driver_generator.py +++ b/vulkan/scripts/driver_generator.py @@ -177,8 +177,12 @@ struct ProcHook { for ext in _KNOWN_EXTENSIONS: f.write(gencom.indent(2) + gencom.base_ext_name(ext) + ',\n') - f.write(""" - EXTENSION_CORE, // valid bit + f.write('\n') + for version in gencom.version_code_list: + f.write(gencom.indent(2) + 'EXTENSION_CORE_' + version + ',\n') + + # EXTENSION_COUNT must be the next enum after the highest API version. + f.write("""\ EXTENSION_COUNT, EXTENSION_UNKNOWN, }; @@ -249,6 +253,18 @@ def _is_intercepted(cmd): return False +def _get_proc_hook_enum(cmd): + """Returns the ProcHook enumeration for the corresponding core function. + + Args: + cmd: Vulkan function name. + """ + assert cmd in gencom.version_dict + for version in gencom.version_code_list: + if gencom.version_dict[cmd] == 'VK_VERSION_' + version: + return 'ProcHook::EXTENSION_CORE_' + version + + def _need_proc_hook_stub(cmd): """Returns true if a function needs a ProcHook stub. @@ -259,6 +275,8 @@ def _need_proc_hook_stub(cmd): if cmd in gencom.extension_dict: if not gencom.is_extension_internal(gencom.extension_dict[cmd]): return True + elif gencom.version_dict[cmd] != 'VK_VERSION_1_0': + return True return False @@ -271,8 +289,16 @@ def _define_proc_hook_stub(cmd, f): """ if _need_proc_hook_stub(cmd): return_type = gencom.return_type_dict[cmd] - ext_name = gencom.extension_dict[cmd] - ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + + ext_name = '' + ext_hook = '' + if cmd in gencom.extension_dict: + ext_name = gencom.extension_dict[cmd] + ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + else: + ext_name = gencom.version_dict[cmd] + ext_hook = _get_proc_hook_enum(cmd) + handle = gencom.param_dict[cmd][0][1] param_types = ', '.join([''.join(i) for i in gencom.param_dict[cmd]]) param_names = ', '.join([''.join(i[1]) for i in gencom.param_dict[cmd]]) @@ -306,12 +332,12 @@ def _define_global_proc_hook(cmd, f): f.write(gencom.indent(1) + '{\n') f.write(gencom.indent(2) + '\"' + cmd + '\",\n') - f.write("""\ - ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, - reinterpret_cast(""" + gencom.base_name(cmd) + """), - nullptr, - },\n""") + f.write(gencom.indent(2) + 'ProcHook::GLOBAL,\n') + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') + f.write(gencom.indent(2) + 'reinterpret_cast(' + + gencom.base_name(cmd) + '),\n') + f.write(gencom.indent(2) + 'nullptr,\n') + f.write(gencom.indent(1) + '},\n') def _define_instance_proc_hook(cmd, f): @@ -339,8 +365,8 @@ def _define_instance_proc_hook(cmd, f): reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") else: + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') f.write("""\ - ProcHook::EXTENSION_CORE, reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") @@ -358,10 +384,17 @@ def _define_device_proc_hook(cmd, f): f.write(gencom.indent(2) + '\"' + cmd + '\",\n') f.write(gencom.indent(2) + 'ProcHook::DEVICE,\n') - if cmd in gencom.extension_dict: - ext_name = gencom.extension_dict[cmd] - f.write(gencom.indent(2) + 'ProcHook::' + - gencom.base_ext_name(ext_name) + ',\n') + if (cmd in gencom.extension_dict or + gencom.version_dict[cmd] != 'VK_VERSION_1_0'): + ext_name = '' + ext_hook = '' + if cmd in gencom.extension_dict: + ext_name = gencom.extension_dict[cmd] + ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + else: + ext_name = gencom.version_dict[cmd] + ext_hook = _get_proc_hook_enum(cmd) + f.write(gencom.indent(2) + ext_hook + ',\n') if gencom.is_extension_internal(ext_name): f.write("""\ @@ -374,8 +407,8 @@ def _define_device_proc_hook(cmd, f): gencom.base_name(cmd) + '),\n') else: + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') f.write("""\ - ProcHook::EXTENSION_CORE, reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") diff --git a/vulkan/scripts/generator_common.py b/vulkan/scripts/generator_common.py index 670ba66d1b..cf370fafe5 100644 --- a/vulkan/scripts/generator_common.py +++ b/vulkan/scripts/generator_common.py @@ -91,6 +91,9 @@ param_dict = {} # Dict for mapping a function to its return type. return_type_dict = {} +# List of the sorted Vulkan version codes. e.g. '1_0', '1_1'. +version_code_list = [] + # Dict for mapping a function to the core Vulkan API version. version_dict = {} @@ -171,6 +174,15 @@ def base_ext_name(ext): return ext[3:] +def version_code(version): + """Returns the version code from a version string. + + Args: + version: Vulkan version string. + """ + return version[11:] + + def is_function_supported(cmd): """Returns true if a function is core or from a supportable extension. @@ -313,6 +325,7 @@ def parse_vulkan_registry(): extension_dict param_dict return_type_dict + version_code_list version_dict """ registry = os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', @@ -385,3 +398,9 @@ def parse_vulkan_registry(): cmd_name = command.get('name') if cmd_name in command_list: version_dict[cmd_name] = apiversion + + version_code_set = set() + for version in version_dict.values(): + version_code_set.add(version_code(version)) + for code in sorted(version_code_set): + version_code_list.append(code) -- cgit v1.2.3-59-g8ed1b From 7c123cc15041254190da53fe828fb44677fc879e Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Mon, 21 Oct 2019 13:52:41 -0700 Subject: [frameworks][native][vulkan] fix -Walloca Alloca cannot be checked for failure. Replace alloca with dynamic memory allocations. Bug: 139945549 Bug: 142475221 Test: mm Change-Id: I1a154b479c8da66e25d20dced6bf9ae269b906f8 Signed-off-by: Nick Desaulniers --- vulkan/libvulkan/driver.cpp | 19 +++++++------- vulkan/libvulkan/layers_extensions.cpp | 45 ++++++++++------------------------ 2 files changed, 23 insertions(+), 41 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index c3c19ecdff..2f6747dd70 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -37,7 +37,10 @@ #include #include +#include #include +#include +#include #include #include "stubhal.h" @@ -148,15 +151,12 @@ class CreateInfoWrapper { Hal Hal::hal_; void* LoadLibrary(const android_dlextinfo& dlextinfo, - const char* subname, - int subname_len) { + const std::string_view subname) { ATRACE_CALL(); - const char kLibFormat[] = "vulkan.%*s.so"; - char* name = static_cast( - alloca(sizeof(kLibFormat) + static_cast(subname_len))); - sprintf(name, kLibFormat, subname_len, subname); - return android_dlopen_ext(name, RTLD_LOCAL | RTLD_NOW, &dlextinfo); + std::stringstream ss; + ss << "vulkan." << subname << ".so"; + return android_dlopen_ext(ss.str().c_str(), RTLD_LOCAL | RTLD_NOW, &dlextinfo); } const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ @@ -176,8 +176,9 @@ int LoadDriver(android_namespace_t* library_namespace, char prop[PROPERTY_VALUE_MAX]; for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { int prop_len = property_get(key, prop, nullptr); - if (prop_len > 0) { - so = LoadLibrary(dlextinfo, prop, prop_len); + if (prop_len > 0 && prop_len <= UINT_MAX) { + std::string_view lib_name(prop, static_cast(prop_len)); + so = LoadLibrary(dlextinfo, lib_name); if (so) break; } diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 5679412732..dd917393d1 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -101,9 +102,7 @@ class LayerLibrary { bool EnumerateLayers(size_t library_idx, std::vector& instance_layers) const; - void* GetGPA(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) const; + void* GetGPA(const Layer& layer, const std::string_view gpa_name) const; const std::string GetFilename() { return filename_; } @@ -226,17 +225,10 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, } // get layer properties - VkLayerProperties* properties = static_cast(alloca( - (num_instance_layers + num_device_layers) * sizeof(VkLayerProperties))); - result = enumerate_instance_layers(&num_instance_layers, properties); - if (result != VK_SUCCESS) { - ALOGE("vkEnumerateInstanceLayerProperties failed for library '%s': %d", - path_.c_str(), result); - return false; - } + auto properties = std::make_unique(num_instance_layers + num_device_layers); if (num_device_layers > 0) { result = enumerate_device_layers(VK_NULL_HANDLE, &num_device_layers, - properties + num_instance_layers); + properties.get() + num_instance_layers); if (result != VK_SUCCESS) { ALOGE( "vkEnumerateDeviceLayerProperties failed for library '%s': %d", @@ -321,21 +313,11 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, return true; } -void* LayerLibrary::GetGPA(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) const { - void* gpa; - size_t layer_name_len = - std::max(size_t{2}, strlen(layer.properties.layerName)); - char* name = static_cast(alloca(layer_name_len + gpa_name_len + 1)); - strcpy(name, layer.properties.layerName); - strcpy(name + layer_name_len, gpa_name); - if (!(gpa = GetTrampoline(name))) { - strcpy(name, "vk"); - strcpy(name + 2, gpa_name); - gpa = GetTrampoline(name); - } - return gpa; +void* LayerLibrary::GetGPA(const Layer& layer, const std::string_view gpa_name) const { + std::string layer_name { layer.properties.layerName }; + if (void* gpa = GetTrampoline((layer_name.append(gpa_name).c_str()))) + return gpa; + return GetTrampoline((std::string {"vk"}.append(gpa_name)).c_str()); } // ---------------------------------------------------------------------------- @@ -470,10 +452,9 @@ const VkExtensionProperties* FindExtension( } void* GetLayerGetProcAddr(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) { + const std::string_view gpa_name) { const LayerLibrary& library = g_layer_libraries[layer.library_idx]; - return library.GetGPA(layer, gpa_name, gpa_name_len); + return library.GetGPA(layer, gpa_name); } } // anonymous namespace @@ -556,13 +537,13 @@ LayerRef::LayerRef(LayerRef&& other) noexcept : layer_(other.layer_) { PFN_vkGetInstanceProcAddr LayerRef::GetGetInstanceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr", 19)) + GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr")) : nullptr; } PFN_vkGetDeviceProcAddr LayerRef::GetGetDeviceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr", 17)) + GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr")) : nullptr; } -- cgit v1.2.3-59-g8ed1b From f8f506c5ad5c51ae95e0675c52017a11be145274 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Thu, 24 Oct 2019 11:04:58 -0700 Subject: Vulkan: remove some redundant codes Bug: 134185757 Test: build Change-Id: I843f361e31d098bc1f0ab2de67aac1e1161aa351 --- vulkan/libvulkan/driver.cpp | 1 - vulkan/libvulkan/driver.h | 1 - 2 files changed, 2 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index c3c19ecdff..4aa7d55e12 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -1208,7 +1208,6 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, } data->driver_device = dev; - data->driver_version = properties.driverVersion; *pDevice = dev; diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index 61e1818f28..7edadea09d 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -98,7 +98,6 @@ struct DeviceData { VkDevice driver_device; DeviceDriverTable driver; - uint32_t driver_version; }; bool Debuggable(); -- cgit v1.2.3-59-g8ed1b From a885c06c07e9a0838b185daed58bc7626c2b961e Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Thu, 24 Oct 2019 12:07:57 -0700 Subject: Vulkan: convert all the TODOs into actual issues to chase against Bug: 134185757 Test: build Change-Id: Ib9a27ba8b8da53707337d3d48e6da502f38670e5 --- vulkan/libvulkan/api.cpp | 4 ++-- vulkan/libvulkan/debug_report.h | 2 +- vulkan/libvulkan/driver.cpp | 2 +- vulkan/libvulkan/layers_extensions.cpp | 8 +------- vulkan/libvulkan/swapchain.cpp | 8 ++++---- 5 files changed, 9 insertions(+), 15 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 1578d9ffce..24039b1fee 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -1279,7 +1279,7 @@ VkResult EnumerateInstanceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO how about extensions from implicitly enabled layers? + // TODO(b/143293104): expose extensions from implicitly enabled layers return vulkan::driver::EnumerateInstanceExtensionProperties( nullptr, pPropertyCount, pProperties); } @@ -1333,7 +1333,7 @@ VkResult EnumerateDeviceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO how about extensions from implicitly enabled layers? + // TODO(b/143293104): expose extensions from implicitly enabled layers const InstanceData& data = GetData(physicalDevice); return data.dispatch.EnumerateDeviceExtensionProperties( physicalDevice, nullptr, pPropertyCount, pProperties); diff --git a/vulkan/libvulkan/debug_report.h b/vulkan/libvulkan/debug_report.h index 3d8bd50c28..e5b1587b4f 100644 --- a/vulkan/libvulkan/debug_report.h +++ b/vulkan/libvulkan/debug_report.h @@ -78,7 +78,7 @@ class DebugReportCallbackList { VkDebugReportCallbackEXT driver_handle; }; - // TODO(jessehall): replace with std::shared_mutex when available in libc++ + // TODO(b/143295577): use std::shared_mutex when available in libc++ mutable std::shared_timed_mutex rwmutex_; Node head_; }; diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 4aa7d55e12..733e823bed 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -692,7 +692,7 @@ VKAPI_ATTR void* DefaultReallocate(void*, return nullptr; } - // TODO(jessehall): Right now we never shrink allocations; if the new + // TODO(b/143295633): Right now we never shrink allocations; if the new // request is smaller than the existing chunk, we just continue using it. // Right now the loader never reallocs, so this doesn't matter. If that // changes, or if this code is copied into some other project, this should diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 5679412732..2f33fee8ac 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -38,13 +38,7 @@ #include #include -// TODO(jessehall): The whole way we deal with extensions is pretty hokey, and -// not a good long-term solution. Having a hard-coded enum of extensions is -// bad, of course. Representing sets of extensions (requested, supported, etc.) -// as a bitset isn't necessarily bad, if the mapping from extension to bit were -// dynamic. Need to rethink this completely when there's a little more time. - -// TODO(jessehall): This file currently builds up global data structures as it +// TODO(b/143296676): This file currently builds up global data structures as it // loads, and never cleans them up. This means we're doing heap allocations // without going through an app-provided allocator, but worse, we'll leak those // allocations if the loader is unloaded. diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index bbf50a1529..d234e177d9 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -694,8 +694,8 @@ VkResult GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice pdev, const InstanceData& instance_data = GetData(pdev); - // TODO(jessehall): Fill out the set of supported formats. Longer term, add - // a new gralloc method to query whether a (format, usage) pair is + // TODO(b/143296550): Fill out the set of supported formats. Longer term, + // add a new gralloc method to query whether a (format, usage) pair is // supported, and check that for each gralloc format that corresponds to a // Vulkan format. Shorter term, just add a few more formats to the ones // hardcoded below. @@ -953,7 +953,7 @@ VkResult GetPhysicalDevicePresentRectanglesKHR(VkPhysicalDevice, strerror(-err), err); } - // TODO: Return something better than "whole window" + // TODO(b/143294545): Return something better than "whole window" pRects[0].offset.x = 0; pRects[0].offset.y = 0; pRects[0].extent = VkExtent2D{static_cast(width), @@ -1812,7 +1812,7 @@ VkResult GetSwapchainStatusKHR( return VK_ERROR_OUT_OF_DATE_KHR; } - // TODO(chrisforbes): Implement this function properly + // TODO(b/143296009): Implement this function properly return result; } -- cgit v1.2.3-59-g8ed1b From 03b11cfca168df1e6b37790a7672a27ae280c9d1 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Fri, 25 Oct 2019 11:23:08 -0700 Subject: Revert "[frameworks][native][vulkan] fix -Walloca" This reverts commit a70447192bd04c77f4380a37f5a56a94d41488b5. Test: atest CtsGpuToolsHostTestCases:android.gputools.cts.CtsRootlessGpuDebugHostTest#testDebugLayerLoadExternalVulkan Bug: 139945549 Bug: 142475221 Bug: 143156243 Change-Id: Ie8b3fbddf63c8a4505f7fb196082c58a530d4993 Signed-off-by: Nick Desaulniers --- vulkan/libvulkan/driver.cpp | 16 ++++++------ vulkan/libvulkan/layers_extensions.cpp | 45 ++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 20 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index d92f35ac23..a544bc5ff8 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -151,12 +151,15 @@ class CreateInfoWrapper { Hal Hal::hal_; void* LoadLibrary(const android_dlextinfo& dlextinfo, - const std::string_view subname) { + const char* subname, + int subname_len) { ATRACE_CALL(); - std::stringstream ss; - ss << "vulkan." << subname << ".so"; - return android_dlopen_ext(ss.str().c_str(), RTLD_LOCAL | RTLD_NOW, &dlextinfo); + const char kLibFormat[] = "vulkan.%*s.so"; + char* name = static_cast( + alloca(sizeof(kLibFormat) + static_cast(subname_len))); + sprintf(name, kLibFormat, subname_len, subname); + return android_dlopen_ext(name, RTLD_LOCAL | RTLD_NOW, &dlextinfo); } const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ @@ -176,9 +179,8 @@ int LoadDriver(android_namespace_t* library_namespace, char prop[PROPERTY_VALUE_MAX]; for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { int prop_len = property_get(key, prop, nullptr); - if (prop_len > 0 && prop_len <= UINT_MAX) { - std::string_view lib_name(prop, static_cast(prop_len)); - so = LoadLibrary(dlextinfo, lib_name); + if (prop_len > 0) { + so = LoadLibrary(dlextinfo, prop, prop_len); if (so) break; } diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 758ab25885..2f33fee8ac 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -24,7 +24,6 @@ #include #include -#include #include #include #include @@ -96,7 +95,9 @@ class LayerLibrary { bool EnumerateLayers(size_t library_idx, std::vector& instance_layers) const; - void* GetGPA(const Layer& layer, const std::string_view gpa_name) const; + void* GetGPA(const Layer& layer, + const char* gpa_name, + size_t gpa_name_len) const; const std::string GetFilename() { return filename_; } @@ -219,10 +220,17 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, } // get layer properties - auto properties = std::make_unique(num_instance_layers + num_device_layers); + VkLayerProperties* properties = static_cast(alloca( + (num_instance_layers + num_device_layers) * sizeof(VkLayerProperties))); + result = enumerate_instance_layers(&num_instance_layers, properties); + if (result != VK_SUCCESS) { + ALOGE("vkEnumerateInstanceLayerProperties failed for library '%s': %d", + path_.c_str(), result); + return false; + } if (num_device_layers > 0) { result = enumerate_device_layers(VK_NULL_HANDLE, &num_device_layers, - properties.get() + num_instance_layers); + properties + num_instance_layers); if (result != VK_SUCCESS) { ALOGE( "vkEnumerateDeviceLayerProperties failed for library '%s': %d", @@ -307,11 +315,21 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, return true; } -void* LayerLibrary::GetGPA(const Layer& layer, const std::string_view gpa_name) const { - std::string layer_name { layer.properties.layerName }; - if (void* gpa = GetTrampoline((layer_name.append(gpa_name).c_str()))) - return gpa; - return GetTrampoline((std::string {"vk"}.append(gpa_name)).c_str()); +void* LayerLibrary::GetGPA(const Layer& layer, + const char* gpa_name, + size_t gpa_name_len) const { + void* gpa; + size_t layer_name_len = + std::max(size_t{2}, strlen(layer.properties.layerName)); + char* name = static_cast(alloca(layer_name_len + gpa_name_len + 1)); + strcpy(name, layer.properties.layerName); + strcpy(name + layer_name_len, gpa_name); + if (!(gpa = GetTrampoline(name))) { + strcpy(name, "vk"); + strcpy(name + 2, gpa_name); + gpa = GetTrampoline(name); + } + return gpa; } // ---------------------------------------------------------------------------- @@ -446,9 +464,10 @@ const VkExtensionProperties* FindExtension( } void* GetLayerGetProcAddr(const Layer& layer, - const std::string_view gpa_name) { + const char* gpa_name, + size_t gpa_name_len) { const LayerLibrary& library = g_layer_libraries[layer.library_idx]; - return library.GetGPA(layer, gpa_name); + return library.GetGPA(layer, gpa_name, gpa_name_len); } } // anonymous namespace @@ -531,13 +550,13 @@ LayerRef::LayerRef(LayerRef&& other) noexcept : layer_(other.layer_) { PFN_vkGetInstanceProcAddr LayerRef::GetGetInstanceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr")) + GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr", 19)) : nullptr; } PFN_vkGetDeviceProcAddr LayerRef::GetGetDeviceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr")) + GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr", 17)) : nullptr; } -- cgit v1.2.3-59-g8ed1b From 60307ce84d311beef63e1fe770ba49e3fde7133a Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Fri, 25 Oct 2019 13:34:21 -0700 Subject: [frameworks][native][vulkan] fix -Walloca Alloca cannot be checked for failure. Replace alloca with dynamic memory allocations. Reapply a commit that was previously reverted. The original commit removed a call to enumerate_instance_layers() that it should not have. Prefer std::vector to std::unique_ptr of an array. Bug: 139945549 Bug: 142475221 Bug: 143156243 Test: mm && adb shell stop && adb sync && adb shell start && atest -it \ CtsGpuToolsHostTestCases:android.gputools.cts.CtsRootlessGpuDebugHostTest#testDebugLayerLoadExternalVulkan,testMultipleExternalApps,testSystemPropertyIgnoreVulkan,testDebugLayerLoadVulkan,testSystemPropertyEnableVulkan Signed-off-by: Nick Desaulniers Change-Id: Ifdf747fdabc41ee6da8cd83bda5e3f030649030f --- vulkan/libvulkan/driver.cpp | 16 ++++++-------- vulkan/libvulkan/layers_extensions.cpp | 40 +++++++++++----------------------- 2 files changed, 20 insertions(+), 36 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index a544bc5ff8..d92f35ac23 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -151,15 +151,12 @@ class CreateInfoWrapper { Hal Hal::hal_; void* LoadLibrary(const android_dlextinfo& dlextinfo, - const char* subname, - int subname_len) { + const std::string_view subname) { ATRACE_CALL(); - const char kLibFormat[] = "vulkan.%*s.so"; - char* name = static_cast( - alloca(sizeof(kLibFormat) + static_cast(subname_len))); - sprintf(name, kLibFormat, subname_len, subname); - return android_dlopen_ext(name, RTLD_LOCAL | RTLD_NOW, &dlextinfo); + std::stringstream ss; + ss << "vulkan." << subname << ".so"; + return android_dlopen_ext(ss.str().c_str(), RTLD_LOCAL | RTLD_NOW, &dlextinfo); } const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ @@ -179,8 +176,9 @@ int LoadDriver(android_namespace_t* library_namespace, char prop[PROPERTY_VALUE_MAX]; for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { int prop_len = property_get(key, prop, nullptr); - if (prop_len > 0) { - so = LoadLibrary(dlextinfo, prop, prop_len); + if (prop_len > 0 && prop_len <= UINT_MAX) { + std::string_view lib_name(prop, static_cast(prop_len)); + so = LoadLibrary(dlextinfo, lib_name); if (so) break; } diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 2f33fee8ac..22f92d619d 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -95,9 +95,7 @@ class LayerLibrary { bool EnumerateLayers(size_t library_idx, std::vector& instance_layers) const; - void* GetGPA(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) const; + void* GetGPA(const Layer& layer, const std::string_view gpa_name) const; const std::string GetFilename() { return filename_; } @@ -220,9 +218,8 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, } // get layer properties - VkLayerProperties* properties = static_cast(alloca( - (num_instance_layers + num_device_layers) * sizeof(VkLayerProperties))); - result = enumerate_instance_layers(&num_instance_layers, properties); + std::vector properties(num_instance_layers + num_device_layers); + result = enumerate_instance_layers(&num_instance_layers, properties.data()); if (result != VK_SUCCESS) { ALOGE("vkEnumerateInstanceLayerProperties failed for library '%s': %d", path_.c_str(), result); @@ -230,7 +227,7 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, } if (num_device_layers > 0) { result = enumerate_device_layers(VK_NULL_HANDLE, &num_device_layers, - properties + num_instance_layers); + &properties[num_instance_layers]); if (result != VK_SUCCESS) { ALOGE( "vkEnumerateDeviceLayerProperties failed for library '%s': %d", @@ -315,21 +312,11 @@ bool LayerLibrary::EnumerateLayers(size_t library_idx, return true; } -void* LayerLibrary::GetGPA(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) const { - void* gpa; - size_t layer_name_len = - std::max(size_t{2}, strlen(layer.properties.layerName)); - char* name = static_cast(alloca(layer_name_len + gpa_name_len + 1)); - strcpy(name, layer.properties.layerName); - strcpy(name + layer_name_len, gpa_name); - if (!(gpa = GetTrampoline(name))) { - strcpy(name, "vk"); - strcpy(name + 2, gpa_name); - gpa = GetTrampoline(name); - } - return gpa; +void* LayerLibrary::GetGPA(const Layer& layer, const std::string_view gpa_name) const { + std::string layer_name { layer.properties.layerName }; + if (void* gpa = GetTrampoline((layer_name.append(gpa_name).c_str()))) + return gpa; + return GetTrampoline((std::string {"vk"}.append(gpa_name)).c_str()); } // ---------------------------------------------------------------------------- @@ -464,10 +451,9 @@ const VkExtensionProperties* FindExtension( } void* GetLayerGetProcAddr(const Layer& layer, - const char* gpa_name, - size_t gpa_name_len) { + const std::string_view gpa_name) { const LayerLibrary& library = g_layer_libraries[layer.library_idx]; - return library.GetGPA(layer, gpa_name, gpa_name_len); + return library.GetGPA(layer, gpa_name); } } // anonymous namespace @@ -550,13 +536,13 @@ LayerRef::LayerRef(LayerRef&& other) noexcept : layer_(other.layer_) { PFN_vkGetInstanceProcAddr LayerRef::GetGetInstanceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr", 19)) + GetLayerGetProcAddr(*layer_, "GetInstanceProcAddr")) : nullptr; } PFN_vkGetDeviceProcAddr LayerRef::GetGetDeviceProcAddr() const { return layer_ ? reinterpret_cast( - GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr", 17)) + GetLayerGetProcAddr(*layer_, "GetDeviceProcAddr")) : nullptr; } -- cgit v1.2.3-59-g8ed1b From 0f9d717a7463d833cd460610c97b5854ff18096a Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 8 Nov 2019 10:49:01 -0800 Subject: Fix the behavior of vulkan::driver::Debuggable() function Bug: 144169750 Test: Vulkan app on user build is unable to load implicit layers by default Change-Id: Ief47c6daaa7d5722920e0fd75183d35ae2362adb --- vulkan/libvulkan/driver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index a544bc5ff8..0b686f189a 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -756,7 +756,7 @@ void FreeDeviceData(DeviceData* data, const VkAllocationCallbacks& allocator) { } // anonymous namespace bool Debuggable() { - return (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) >= 0); + return prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) > 0; } bool OpenHAL() { -- cgit v1.2.3-59-g8ed1b From 6a674c9e105bdc5d736c06a4500dcdac1c6c4006 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 8 Nov 2019 11:55:36 -0800 Subject: GraphicsEnv: refactor to unify the debuggable logic By default, PR_SET_DUMPABLE is 0 for zygote spawned apps, except in the following circumstances: 1. ro.debuggable=1 (global debuggable enabled, i.e., userdebug or eng builds). 2. android:debuggable="true" in the manifest for an individual application. 3. An app which explicitly calls prctl(PR_SET_DUMPABLE, 1). 4. GraphicsEnv calls prctl(PR_SET_DUMPABLE, 1) in the presence of in the application manifest. So checking both ro.debuggable=1 and PR_GET_DUMPABLE is redundant. Bug: 144186877 Test: CtsAngleIntegrationHostTestCases Test: CtsRootlessGpuDebugHostTest Change-Id: I934f64315b67db77ee2c2a9dff50fb23bc0a546a --- libs/graphicsenv/GraphicsEnv.cpp | 8 ++------ libs/graphicsenv/include/graphicsenv/GraphicsEnv.h | 12 ++++++++++-- opengl/libs/EGL/egl_layers.cpp | 2 +- vulkan/libvulkan/api.cpp | 6 ++++-- vulkan/libvulkan/driver.cpp | 4 ---- vulkan/libvulkan/driver.h | 1 - vulkan/libvulkan/layers_extensions.cpp | 3 +-- 7 files changed, 18 insertions(+), 18 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index 28591110f0..354703b9e4 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -124,12 +124,8 @@ static const std::string getSystemNativeLibraries(NativeLibrary type) { return env; } -int GraphicsEnv::getCanLoadSystemLibraries() { - if (property_get_bool("ro.debuggable", false) && prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { - // Return an integer value since this crosses library boundaries - return 1; - } - return 0; +bool GraphicsEnv::isDebuggable() { + return prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) > 0; } void GraphicsEnv::setDriverPathAndSphalLibraries(const std::string path, diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index 83448d4ce7..c6dc1f831a 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -33,8 +33,16 @@ class GraphicsEnv { public: static GraphicsEnv& getInstance(); - // Check if device is debuggable. - int getCanLoadSystemLibraries(); + // Check if the process is debuggable. It returns false except in any of the + // following circumstances: + // 1. ro.debuggable=1 (global debuggable enabled). + // 2. android:debuggable="true" in the manifest for an individual app. + // 3. An app which explicitly calls prctl(PR_SET_DUMPABLE, 1). + // 4. GraphicsEnv calls prctl(PR_SET_DUMPABLE, 1) in the presence of + // + // in the application manifest. + bool isDebuggable(); /* * Apis for updatable driver diff --git a/opengl/libs/EGL/egl_layers.cpp b/opengl/libs/EGL/egl_layers.cpp index ac01dc8f96..ba7cacdbf2 100644 --- a/opengl/libs/EGL/egl_layers.cpp +++ b/opengl/libs/EGL/egl_layers.cpp @@ -337,7 +337,7 @@ void LayerLoader::LoadLayers() { // Only enable the system search path for non-user builds std::string system_path; - if (property_get_bool("ro.debuggable", false) && prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { + if (android::GraphicsEnv::getInstance().isDebuggable()) { system_path = kSystemLayerLibraryDir; } diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 24039b1fee..a1207d324f 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -124,7 +124,8 @@ class OverrideLayerNames { }; void AddImplicitLayers() { - if (!is_instance_ || !driver::Debuggable()) + if (!is_instance_ || + !android::GraphicsEnv::getInstance().isDebuggable()) return; GetLayersFromSettings(); @@ -370,7 +371,8 @@ class OverrideExtensionNames { private: bool EnableDebugCallback() const { - return (is_instance_ && driver::Debuggable() && + return (is_instance_ && + android::GraphicsEnv::getInstance().isDebuggable() && property_get_bool("debug.vulkan.enable_callback", false)); } diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 0b686f189a..c1994f9976 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -755,10 +755,6 @@ void FreeDeviceData(DeviceData* data, const VkAllocationCallbacks& allocator) { } // anonymous namespace -bool Debuggable() { - return prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) > 0; -} - bool OpenHAL() { return Hal::Open(); } diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index 7edadea09d..23c717cf5d 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -100,7 +100,6 @@ struct DeviceData { DeviceDriverTable driver; }; -bool Debuggable(); bool OpenHAL(); const VkAllocationCallbacks& GetDefaultAllocator(); diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 2f33fee8ac..3ebfd7b629 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -475,8 +475,7 @@ void* GetLayerGetProcAddr(const Layer& layer, void DiscoverLayers() { ATRACE_CALL(); - if (property_get_bool("ro.debuggable", false) && - prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { + if (android::GraphicsEnv::getInstance().isDebuggable()) { DiscoverLayersInPathList(kSystemLayerLibraryDir); } if (!android::GraphicsEnv::getInstance().getLayerPaths().empty()) -- cgit v1.2.3-59-g8ed1b From efa0cbd8218113a0ab3b606af7c54acc1e16194f Mon Sep 17 00:00:00 2001 From: Peiyong Lin Date: Wed, 29 Jan 2020 20:51:50 -0800 Subject: Don't fall back to system driver when driver apk fails. Previously when driver apk fails to load, the loader falls back to load system driver. However, this provides no indication of driver apk failure and hence users that intend to use driver apk may end up working against the system driver. BUG: b/147459984 Test: Verified by forcing to use a dummy apk Change-Id: I81f3be5710d1daaba7476f4ccb17d532049a1e68 --- vulkan/libvulkan/driver.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 90a73e2d32..a7ec4aed1d 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -164,6 +164,11 @@ const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ "ro.board.platform", }}; +// LoadDriver returns: +// * 0 when succeed, or +// * -ENOENT when fail to open binary libraries, or +// * -EINVAL when fail to find HAL_MODULE_INFO_SYM_AS_STR or +// HWVULKAN_HARDWARE_MODULE_ID in the library. int LoadDriver(android_namespace_t* library_namespace, const hwvulkan_module_t** module) { ATRACE_CALL(); @@ -221,7 +226,13 @@ int LoadUpdatedDriver(const hwvulkan_module_t** module) { return -ENOENT; android::GraphicsEnv::getInstance().setDriverToLoad( android::GpuStatsInfo::Driver::VULKAN_UPDATED); - return LoadDriver(ns, module); + int result = LoadDriver(ns, module); + if (result != 0) { + LOG_ALWAYS_FATAL( + "couldn't find an updated Vulkan implementation from %s", + android::GraphicsEnv::getInstance().getDriverPath().c_str()); + } + return result; } bool Hal::Open() { -- cgit v1.2.3-59-g8ed1b From bc37dd56361b17e3e9815bc6d43b114c659b5a41 Mon Sep 17 00:00:00 2001 From: Sundong Ahn Date: Thu, 23 Apr 2020 21:21:00 +0900 Subject: Change to using sysprop for libvulkan The configstore service was deprecated. So change to use sysprop instead of configstore Bug: 124531214 Test: m -j Change-Id: I08a02f1f3ea8d86c45aff3cf3d72930a3d0b7898 --- vulkan/libvulkan/Android.bp | 2 ++ vulkan/libvulkan/driver.cpp | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/Android.bp b/vulkan/libvulkan/Android.bp index 018f2004cd..f69de1f324 100644 --- a/vulkan/libvulkan/Android.bp +++ b/vulkan/libvulkan/Android.bp @@ -79,6 +79,7 @@ cc_library_shared { "hwvulkan_headers", "libnativeloader-headers", "vulkan_headers", + "libsurfaceflinger_headers", ], export_header_lib_headers: ["vulkan_headers"], shared_libs: [ @@ -100,6 +101,7 @@ cc_library_shared { "libnativeloader_lazy", "libnativewindow", "android.hardware.graphics.common@1.0", + "libSurfaceFlingerProp", ], static_libs: ["libgrallocusage"], } diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index a7ec4aed1d..a5f0c9f803 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -23,9 +23,10 @@ #include #include +#include +#include #include #include -#include #include #include #include @@ -959,9 +960,7 @@ VkResult EnumerateDeviceExtensionProperties( VK_KHR_INCREMENTAL_PRESENT_EXTENSION_NAME, VK_KHR_INCREMENTAL_PRESENT_SPEC_VERSION}); - bool hdrBoardConfig = - getBool( - false); + bool hdrBoardConfig = android::sysprop::has_HDR_display(false); if (hdrBoardConfig) { loader_extensions.push_back({VK_EXT_HDR_METADATA_EXTENSION_NAME, VK_EXT_HDR_METADATA_SPEC_VERSION}); -- cgit v1.2.3-59-g8ed1b From 4264917141fe1f4a38da5f0698c5cc87f7720f80 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Thu, 7 May 2020 12:48:54 -0700 Subject: Vulkan: remove the fallback path to load Vulkan driver This fallback loading path was for backwards compatibility with devices being upgraded from pre-Oreo (i.e. pre-Treble) to Oreo or later. Those devices weren't required to follow a lot of the Treble rules, so linker namespaces (or binaries compatible with them) couldn't be enforced. This change removes it so that we can discover issues around loading Vulkan driver from sphal namespace. Bug: 156021362 Test: build, flash and boot Change-Id: I505bdbdb40a06e1d837f1d0b75822b0c60de96c9 --- vulkan/libvulkan/driver.cpp | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'vulkan/libvulkan/driver.cpp') diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index a5f0c9f803..7bcb2c1441 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -252,18 +252,6 @@ bool Hal::Open() { result = LoadUpdatedDriver(&module); if (result == -ENOENT) { result = LoadBuiltinDriver(&module); - if (result != 0) { - // -ENOENT means the sphal namespace doesn't exist, not that there - // is a problem with the driver. - ALOGW_IF( - result != -ENOENT, - "Failed to load Vulkan driver into sphal namespace. This " - "usually means the driver has forbidden library dependencies." - "Please fix, this will soon stop working."); - result = - hw_get_module(HWVULKAN_HARDWARE_MODULE_ID, - reinterpret_cast(&module)); - } } if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( -- cgit v1.2.3-59-g8ed1b