From ca4744c6030b441531d6640424dbdfe49e082b44 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Tue, 29 Jun 2021 05:12:16 +0530 Subject: [PATCH] Improve `ENUM_STRING` + Fix Host1X Syncpoints `ENUM_STRING` now has a unified implementation in with a documented format and can be used throughout the codebase. A major performance regression was added in the Host1X Syncpoint revamp as it did a syscall if there were any waiters during `Increment` even if they would just be woken up and go back to sleep as the threshold wasn't hit. It has now been optimized to only do a wake if any waiting thread needs to be awoken. There was also a bug concerning increment where it would perform actions corresponding to the previous increment rather than the current one which has also been fixed. --- app/src/main/cpp/skyline/common/macros.h | 25 +++++ .../hosbinder/GraphicBufferProducer.h | 24 ++--- .../services/hosbinder/android_types.h | 69 +++++-------- .../services/hosbinder/native_window.h | 99 ++++++++----------- .../main/cpp/skyline/soc/host1x/syncpoint.cpp | 24 ++++- .../main/cpp/skyline/soc/host1x/syncpoint.h | 4 +- 6 files changed, 118 insertions(+), 127 deletions(-) create mode 100644 app/src/main/cpp/skyline/common/macros.h diff --git a/app/src/main/cpp/skyline/common/macros.h b/app/src/main/cpp/skyline/common/macros.h new file mode 100644 index 00000000..4a6c4dd0 --- /dev/null +++ b/app/src/main/cpp/skyline/common/macros.h @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright © 2021 Skyline Team and Contributors (https://github.com/skyline-emu/) + +#pragma once + +/** + * @brief A case statement for an enumerant value to use alongside ENUM_STRING + */ +#define ENUM_CASE(key) \ + case ENUM_TYPE::key: \ + return #key + +/** + * @brief Creates a function to convert an enumerant to its string representation at runtime + * @example ENUM_STRING(Example, { ENUM_CASE(A); ENUM_CASE(B); }) + */ +#define ENUM_STRING(name, cases) \ + constexpr const char *ToString(name value) { \ + using ENUM_TYPE = name; \ + switch (value) { \ + cases \ + default: \ + return "Unknown"; \ + }; \ + }; diff --git a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.h b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.h index d2eba053..f4f5276c 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.h +++ b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.h @@ -14,20 +14,6 @@ namespace skyline::gpu { class Texture; } -#define ENUM_CASE(key) \ - case ENUM_TYPE::key: \ - return #key - -#define ENUM_STRING(name, cases) \ - constexpr const char *ToString(name value) { \ - using ENUM_TYPE = name; \ - switch (value) { \ - cases \ - default: \ - return "Unknown"; \ - }; \ - }; - namespace skyline::service::hosbinder { /** * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/include/gui/BufferSlot.h;l=52-91 @@ -39,8 +25,12 @@ namespace skyline::service::hosbinder { Acquired, }; - ENUM_STRING(BufferState, ENUM_CASE(Free);ENUM_CASE(Dequeued);ENUM_CASE(Queued);ENUM_CASE(Acquired); - ); + ENUM_STRING(BufferState, { + ENUM_CASE(Free); + ENUM_CASE(Dequeued); + ENUM_CASE(Queued); + ENUM_CASE(Acquired); + }) /** * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/include/gui/BufferSlot.h;l=32-138 @@ -203,5 +193,3 @@ namespace skyline::service::hosbinder { extern std::weak_ptr producer; //!< A globally shared instance of the GraphicsBufferProducer } - -#undef ENUM_CASE diff --git a/app/src/main/cpp/skyline/services/hosbinder/android_types.h b/app/src/main/cpp/skyline/services/hosbinder/android_types.h index c37855bc..3416e46d 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/android_types.h +++ b/app/src/main/cpp/skyline/services/hosbinder/android_types.h @@ -6,13 +6,10 @@ #pragma once #include +#include #include #include -#define ENUM_CASE(name, key) \ - case name::key: \ - return #key - namespace skyline::service::hosbinder { /** * @brief An enumeration of all status codes for Android including Binder IPC @@ -113,26 +110,22 @@ namespace skyline::service::hosbinder { sRGBX8888 = 13, //! 4x8-bit sRGB + 0 }; - constexpr const char *ToString(AndroidPixelFormat format) { - switch (format) { - ENUM_CASE(AndroidPixelFormat, None); - ENUM_CASE(AndroidPixelFormat, Custom); - ENUM_CASE(AndroidPixelFormat, Translucent); - ENUM_CASE(AndroidPixelFormat, Transparent); - ENUM_CASE(AndroidPixelFormat, Opaque); - ENUM_CASE(AndroidPixelFormat, RGBA8888); - ENUM_CASE(AndroidPixelFormat, RGBX8888); - ENUM_CASE(AndroidPixelFormat, RGB888); - ENUM_CASE(AndroidPixelFormat, RGB565); - ENUM_CASE(AndroidPixelFormat, BGRA8888); - ENUM_CASE(AndroidPixelFormat, RGBA5551); - ENUM_CASE(AndroidPixelFormat, RGBA4444); - ENUM_CASE(AndroidPixelFormat, sRGBA8888); - ENUM_CASE(AndroidPixelFormat, sRGBX8888); - default: - return "Unknown"; - } - } + ENUM_STRING(AndroidPixelFormat, { + ENUM_CASE(None); + ENUM_CASE(Custom); + ENUM_CASE(Translucent); + ENUM_CASE(Transparent); + ENUM_CASE(Opaque); + ENUM_CASE(RGBA8888); + ENUM_CASE(RGBX8888); + ENUM_CASE(RGB888); + ENUM_CASE(RGB565); + ENUM_CASE(BGRA8888); + ENUM_CASE(RGBA5551); + ENUM_CASE(RGBA4444); + ENUM_CASE(sRGBA8888); + ENUM_CASE(sRGBX8888); + }) /** * @brief The layout of the surface's pixels in GPU memory @@ -143,15 +136,11 @@ namespace skyline::service::hosbinder { Blocklinear = 0x3, //!< A generic block layout which is further defined by it's kind }; - constexpr const char *ToString(NvSurfaceLayout layout) { - switch (layout) { - ENUM_CASE(NvSurfaceLayout, Pitch); - ENUM_CASE(NvSurfaceLayout, Tiled); - ENUM_CASE(NvSurfaceLayout, Blocklinear); - default: - return "Unknown"; - } - } + ENUM_STRING(NvSurfaceLayout, { + ENUM_CASE(Pitch); + ENUM_CASE(Tiled); + ENUM_CASE(Blocklinear); + }) /** * @brief The kind of tiling used to arrange pixels in a blocklinear surface @@ -170,14 +159,10 @@ namespace skyline::service::hosbinder { Interlaced, //!< Odd and even rows are updated in an alternating pattern }; - constexpr const char *ToString(NvDisplayScanFormat format) { - switch (format) { - ENUM_CASE(NvDisplayScanFormat, Progressive); - ENUM_CASE(NvDisplayScanFormat, Interlaced); - default: - return "Unknown"; - } - } + ENUM_STRING(NvDisplayScanFormat, { + ENUM_CASE(Progressive); + ENUM_CASE(Interlaced); + }) #pragma pack(push, 1) @@ -246,5 +231,3 @@ namespace skyline::service::hosbinder { #pragma pack(pop) } - -#undef ENUM_CASE diff --git a/app/src/main/cpp/skyline/services/hosbinder/native_window.h b/app/src/main/cpp/skyline/services/hosbinder/native_window.h index 29a00960..a5fa9fb1 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/native_window.h +++ b/app/src/main/cpp/skyline/services/hosbinder/native_window.h @@ -4,9 +4,7 @@ #pragma once -#define ENUM_CASE(name, key) \ - case name::key: \ - return #key +#include namespace skyline::service::hosbinder { /** @@ -20,17 +18,13 @@ namespace skyline::service::hosbinder { Camera = 4, }; - constexpr const char *ToString(NativeWindowApi api) { - switch (api) { - ENUM_CASE(NativeWindowApi, None); - ENUM_CASE(NativeWindowApi, EGL); - ENUM_CASE(NativeWindowApi, CPU); - ENUM_CASE(NativeWindowApi, Media); - ENUM_CASE(NativeWindowApi, Camera); - default: - return "Unknown"; - } - } + ENUM_STRING(NativeWindowApi, { + ENUM_CASE(None); + ENUM_CASE(EGL); + ENUM_CASE(CPU); + ENUM_CASE(Media); + ENUM_CASE(Camera); + }); /** * @note A few combinations of transforms that are not in the NATIVE_WINDOW_TRANSFORM enum were added to assist with conversion to/from Vulkan transforms @@ -48,21 +42,17 @@ namespace skyline::service::hosbinder { InvertDisplay = 0b1000, }; - constexpr const char *ToString(NativeWindowTransform transform) { - switch (transform) { - ENUM_CASE(NativeWindowTransform, Identity); - ENUM_CASE(NativeWindowTransform, MirrorHorizontal); - ENUM_CASE(NativeWindowTransform, MirrorVertical); - ENUM_CASE(NativeWindowTransform, Rotate90); - ENUM_CASE(NativeWindowTransform, Rotate180); - ENUM_CASE(NativeWindowTransform, Rotate270); - ENUM_CASE(NativeWindowTransform, MirrorHorizontalRotate90); - ENUM_CASE(NativeWindowTransform, MirrorVerticalRotate90); - ENUM_CASE(NativeWindowTransform, InvertDisplay); - default: - return "Unknown"; - } - } + ENUM_STRING(NativeWindowTransform, { + ENUM_CASE(Identity); + ENUM_CASE(MirrorHorizontal); + ENUM_CASE(MirrorVertical); + ENUM_CASE(Rotate90); + ENUM_CASE(Rotate180); + ENUM_CASE(Rotate270); + ENUM_CASE(MirrorHorizontalRotate90); + ENUM_CASE(MirrorVerticalRotate90); + ENUM_CASE(InvertDisplay); + }); /** * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:system/core/include/system/window.h;l=338-354 @@ -74,16 +64,12 @@ namespace skyline::service::hosbinder { NoScaleCrop = 3, }; - constexpr const char *ToString(NativeWindowScalingMode scalingMode) { - switch (scalingMode) { - ENUM_CASE(NativeWindowScalingMode, Freeze); - ENUM_CASE(NativeWindowScalingMode, ScaleToWindow); - ENUM_CASE(NativeWindowScalingMode, ScaleCrop); - ENUM_CASE(NativeWindowScalingMode, NoScaleCrop); - default: - return "Unknown"; - } - } + ENUM_STRING(NativeWindowScalingMode, { + ENUM_CASE(Freeze); + ENUM_CASE(ScaleToWindow); + ENUM_CASE(ScaleCrop); + ENUM_CASE(NoScaleCrop); + }); /** * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:system/core/include/system/window.h;l=127-265 @@ -104,24 +90,19 @@ namespace skyline::service::hosbinder { MaxBufferCount = 12, //!< A custom query for HOS which returns the maximum number of buffers that can be allocated at once }; - constexpr const char *ToString(NativeWindowQuery query) { - switch (query) { - ENUM_CASE(NativeWindowQuery, Width); - ENUM_CASE(NativeWindowQuery, Height); - ENUM_CASE(NativeWindowQuery, Format); - ENUM_CASE(NativeWindowQuery, MinUndequeuedBuffers); - ENUM_CASE(NativeWindowQuery, QueuesToWindowComposer); - ENUM_CASE(NativeWindowQuery, ConcreteType); - ENUM_CASE(NativeWindowQuery, DefaultWidth); - ENUM_CASE(NativeWindowQuery, DefaultHeight); - ENUM_CASE(NativeWindowQuery, TransformHint); - ENUM_CASE(NativeWindowQuery, ConsumerRunningBehind); - ENUM_CASE(NativeWindowQuery, ConsumerUsageBits); - ENUM_CASE(NativeWindowQuery, StickyTransform); - default: - return "Unknown"; - } - } + ENUM_STRING(NativeWindowQuery, { + ENUM_CASE(Width); + ENUM_CASE(Height); + ENUM_CASE(Format); + ENUM_CASE(MinUndequeuedBuffers); + ENUM_CASE(QueuesToWindowComposer); + ENUM_CASE(ConcreteType); + ENUM_CASE(DefaultWidth); + ENUM_CASE(DefaultHeight); + ENUM_CASE(TransformHint); + ENUM_CASE(ConsumerRunningBehind); + ENUM_CASE(ConsumerUsageBits); + ENUM_CASE(StickyTransform); + ENUM_CASE(MaxBufferCount); + }); } - -#undef ENUM_CASE diff --git a/app/src/main/cpp/skyline/soc/host1x/syncpoint.cpp b/app/src/main/cpp/skyline/soc/host1x/syncpoint.cpp index b6990925..9995c118 100644 --- a/app/src/main/cpp/skyline/soc/host1x/syncpoint.cpp +++ b/app/src/main/cpp/skyline/soc/host1x/syncpoint.cpp @@ -35,15 +35,24 @@ namespace skyline::soc::host1x { } u32 Syncpoint::Increment() { - auto readValue{value.fetch_add(1, std::memory_order_acq_rel)}; // We don't want to constantly do redundant atomic loads + auto readValue{value.fetch_add(1, std::memory_order_acq_rel) + 1}; // We don't want to constantly do redundant atomic loads - std::lock_guard lock(mutex); + std::scoped_lock lock(mutex); + bool signalCondition{}; auto it{waiters.begin()}; - while (it != waiters.end() && readValue >= it->threshold) - it++->callback(); + while (it != waiters.end() && readValue >= it->threshold) { + auto &waiter{*it}; + if (waiter.callback) + waiter.callback(); + else + signalCondition = true; + it++; + } + waiters.erase(waiters.begin(), it); - incrementCondition.notify_all(); + if (signalCondition) + incrementCondition.notify_all(); return readValue; } @@ -54,6 +63,11 @@ namespace skyline::soc::host1x { return {}; std::unique_lock lock(mutex); + auto it{waiters.begin()}; + while (it != waiters.end() && threshold >= it->threshold) + it++; + waiters.emplace(it, threshold, nullptr); + if (timeout == std::chrono::steady_clock::duration::max()) { incrementCondition.wait(lock, [&] { return value.load(std::memory_order_relaxed) >= threshold; }); return true; diff --git a/app/src/main/cpp/skyline/soc/host1x/syncpoint.h b/app/src/main/cpp/skyline/soc/host1x/syncpoint.h index 3f48738f..66de18b8 100644 --- a/app/src/main/cpp/skyline/soc/host1x/syncpoint.h +++ b/app/src/main/cpp/skyline/soc/host1x/syncpoint.h @@ -17,11 +17,11 @@ namespace skyline::soc::host1x { std::atomic value{}; //!< An atomically-incrementing counter at the core of a syncpoint std::mutex mutex; //!< Synchronizes insertions and deletions of waiters alongside locking the increment condition - std::condition_variable incrementCondition; //!< Signalled on every increment to the syncpoint + std::condition_variable incrementCondition; //!< Signalled on thresholds for waiters which are tied to Wait(...) struct Waiter { u32 threshold; //!< The syncpoint value to wait on to be reached - std::function callback; //!< The callback to do after the wait has ended + std::function callback; //!< The callback to do after the wait has ended, refers to cvar signal when nullptr Waiter(u32 threshold, std::function callback) : threshold(threshold), callback(std::move(callback)) {} };