diff options
author | Ady Abraham <adyabr@google.com> | 2021-07-14 16:32:56 -0700 |
---|---|---|
committer | Ady Abraham <adyabr@google.com> | 2021-07-19 19:42:54 +0000 |
commit | 51e888ba667d826245f132afdca33970d961bcd6 (patch) | |
tree | bc481ce85e903a05dd4c719f0586d0cbff6ce8fa | |
parent | 6a409e17600f320134dea722247c15770030d7b2 (diff) | |
download | native-51e888ba667d826245f132afdca33970d961bcd6.tar.gz |
SF: allow more than one client to use eEarly[Start|End] flags
Bug: 191969790
Test: SF unit tests
Change-Id: I0a21600eba986857e0231e02cb4d59fb108e9bd3
Merged-In: I0a21600eba986857e0231e02cb4d59fb108e9bd3
6 files changed, 126 insertions, 45 deletions
diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.cpp b/services/surfaceflinger/Scheduler/VsyncModulator.cpp index 194d808836..245db0f5c7 100644 --- a/services/surfaceflinger/Scheduler/VsyncModulator.cpp +++ b/services/surfaceflinger/Scheduler/VsyncModulator.cpp @@ -46,27 +46,36 @@ VsyncModulator::VsyncConfig VsyncModulator::setVsyncConfigSet(const VsyncConfigS return updateVsyncConfigLocked(); } -VsyncModulator::VsyncConfigOpt VsyncModulator::setTransactionSchedule( - TransactionSchedule schedule) { +VsyncModulator::VsyncConfigOpt VsyncModulator::setTransactionSchedule(TransactionSchedule schedule, + const sp<IBinder>& token) { + std::lock_guard<std::mutex> lock(mMutex); switch (schedule) { case Schedule::EarlyStart: - ALOGW_IF(mEarlyWakeup, "%s: Duplicate EarlyStart", __FUNCTION__); - mEarlyWakeup = true; + if (token) { + mEarlyWakeupRequests.emplace(token); + token->linkToDeath(this); + } else { + ALOGW("%s: EarlyStart requested without a valid token", __func__); + } break; - case Schedule::EarlyEnd: - ALOGW_IF(!mEarlyWakeup, "%s: Unexpected EarlyEnd", __FUNCTION__); - mEarlyWakeup = false; + case Schedule::EarlyEnd: { + if (token && mEarlyWakeupRequests.erase(token) > 0) { + token->unlinkToDeath(this); + } else { + ALOGW("%s: Unexpected EarlyEnd", __func__); + } break; + } case Schedule::Late: // No change to mEarlyWakeup for non-explicit states. break; } if (mTraceDetailedInfo) { - ATRACE_INT("mEarlyWakeup", mEarlyWakeup); + ATRACE_INT("mEarlyWakeup", static_cast<int>(mEarlyWakeupRequests.size())); } - if (!mEarlyWakeup && schedule == Schedule::EarlyEnd) { + if (mEarlyWakeupRequests.empty() && schedule == Schedule::EarlyEnd) { mEarlyTransactionFrames = MIN_EARLY_TRANSACTION_FRAMES; mEarlyTransactionStartTime = mNow(); } @@ -76,7 +85,7 @@ VsyncModulator::VsyncConfigOpt VsyncModulator::setTransactionSchedule( return std::nullopt; } mTransactionSchedule = schedule; - return updateVsyncConfig(); + return updateVsyncConfigLocked(); } VsyncModulator::VsyncConfigOpt VsyncModulator::onTransactionCommit() { @@ -128,8 +137,8 @@ VsyncModulator::VsyncConfig VsyncModulator::getVsyncConfig() const { const VsyncModulator::VsyncConfig& VsyncModulator::getNextVsyncConfig() const { // Early offsets are used if we're in the middle of a refresh rate // change, or if we recently begin a transaction. - if (mEarlyWakeup || mTransactionSchedule == Schedule::EarlyEnd || mEarlyTransactionFrames > 0 || - mRefreshRateChangePending) { + if (!mEarlyWakeupRequests.empty() || mTransactionSchedule == Schedule::EarlyEnd || + mEarlyTransactionFrames > 0 || mRefreshRateChangePending) { return mVsyncConfigSet.early; } else if (mEarlyGpuFrames > 0) { return mVsyncConfigSet.earlyGpu; @@ -160,4 +169,11 @@ VsyncModulator::VsyncConfig VsyncModulator::updateVsyncConfigLocked() { return offsets; } +void VsyncModulator::binderDied(const wp<IBinder>& who) { + std::lock_guard<std::mutex> lock(mMutex); + mEarlyWakeupRequests.erase(who); + + static_cast<void>(updateVsyncConfigLocked()); +} + } // namespace android::scheduler diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.h b/services/surfaceflinger/Scheduler/VsyncModulator.h index 9410768b2d..b2b0451666 100644 --- a/services/surfaceflinger/Scheduler/VsyncModulator.h +++ b/services/surfaceflinger/Scheduler/VsyncModulator.h @@ -19,8 +19,10 @@ #include <chrono> #include <mutex> #include <optional> +#include <unordered_set> #include <android-base/thread_annotations.h> +#include <binder/IBinder.h> #include <utils/Timers.h> namespace android::scheduler { @@ -35,7 +37,7 @@ enum class TransactionSchedule { }; // Modulates VSYNC phase depending on transaction schedule and refresh rate changes. -class VsyncModulator { +class VsyncModulator : public IBinder::DeathRecipient { public: // Number of frames to keep early offsets after an early transaction or GPU composition. // This acts as a low-pass filter in case subsequent transactions are delayed, or if the @@ -91,7 +93,8 @@ public: [[nodiscard]] VsyncConfig setVsyncConfigSet(const VsyncConfigSet&) EXCLUDES(mMutex); // Changes offsets in response to transaction flags or commit. - [[nodiscard]] VsyncConfigOpt setTransactionSchedule(TransactionSchedule); + [[nodiscard]] VsyncConfigOpt setTransactionSchedule(TransactionSchedule, + const sp<IBinder>& = {}) EXCLUDES(mMutex); [[nodiscard]] VsyncConfigOpt onTransactionCommit(); // Called when we send a refresh rate change to hardware composer, so that @@ -104,6 +107,10 @@ public: [[nodiscard]] VsyncConfigOpt onDisplayRefresh(bool usedGpuComposition); +protected: + // Called from unit tests as well + void binderDied(const wp<IBinder>&) override EXCLUDES(mMutex); + private: const VsyncConfig& getNextVsyncConfig() const REQUIRES(mMutex); [[nodiscard]] VsyncConfig updateVsyncConfig() EXCLUDES(mMutex); @@ -116,8 +123,14 @@ private: using Schedule = TransactionSchedule; std::atomic<Schedule> mTransactionSchedule = Schedule::Late; - std::atomic<bool> mEarlyWakeup = false; + struct WpHash { + size_t operator()(const wp<IBinder>& p) const { + return std::hash<IBinder*>()(p.unsafe_get()); + } + }; + + std::unordered_set<wp<IBinder>, WpHash> mEarlyWakeupRequests GUARDED_BY(mMutex); std::atomic<bool> mRefreshRateChangePending = false; std::atomic<int> mEarlyTransactionFrames = 0; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 72700162ba..0b369a5d17 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3157,7 +3157,7 @@ void SurfaceFlinger::initScheduler(const DisplayDeviceState& displayState) { hal::PowerMode::OFF); mVsyncConfiguration = getFactory().createVsyncConfiguration(currRefreshRate); - mVsyncModulator.emplace(mVsyncConfiguration->getCurrentConfigs()); + mVsyncModulator = sp<VsyncModulator>::make(mVsyncConfiguration->getCurrentConfigs()); // start the EventThread mScheduler = getFactory().createScheduler(*mRefreshRateConfigs, *this); @@ -3437,9 +3437,10 @@ uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags) { return setTransactionFlags(flags, TransactionSchedule::Late); } -uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags, TransactionSchedule schedule) { +uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags, TransactionSchedule schedule, + const sp<IBinder>& token) { uint32_t old = mTransactionFlags.fetch_or(flags); - modulateVsync(&VsyncModulator::setTransactionSchedule, schedule); + modulateVsync(&VsyncModulator::setTransactionSchedule, schedule, token); if ((old & flags) == 0) signalTransaction(); return old; } @@ -3659,7 +3660,7 @@ void SurfaceFlinger::queueTransaction(TransactionState& state) { return TransactionSchedule::Late; }(state.flags); - setTransactionFlags(eTransactionFlushNeeded, schedule); + setTransactionFlags(eTransactionFlushNeeded, schedule, state.applyToken); } void SurfaceFlinger::waitForSynchronousTransaction( diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b9b26db260..b1fd208725 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -851,7 +851,7 @@ private: // but there is no need to try and wake up immediately to do it. Rather we rely on // onFrameAvailable or another layer update to wake us up. void setTraversalNeeded(); - uint32_t setTransactionFlags(uint32_t flags, TransactionSchedule); + uint32_t setTransactionFlags(uint32_t flags, TransactionSchedule, const sp<IBinder>& = {}); void commitTransaction() REQUIRES(mStateLock); void commitOffscreenLayers(); bool transactionIsReadyToBeApplied( @@ -1389,7 +1389,7 @@ private: std::unique_ptr<scheduler::VsyncConfiguration> mVsyncConfiguration; // Optional to defer construction until PhaseConfiguration is created. - std::optional<scheduler::VsyncModulator> mVsyncModulator; + sp<VsyncModulator> mVsyncModulator; std::unique_ptr<scheduler::RefreshRateConfigs> mRefreshRateConfigs; std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats; diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 7f6e05ec9f..cf67593174 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -230,7 +230,8 @@ public: std::make_unique<scheduler::RefreshRateStats>(*mFlinger->mTimeStats, currFps, /*powerMode=*/hal::PowerMode::OFF); mFlinger->mVsyncConfiguration = mFactory.createVsyncConfiguration(currFps); - mFlinger->mVsyncModulator.emplace(mFlinger->mVsyncConfiguration->getCurrentConfigs()); + mFlinger->mVsyncModulator = sp<scheduler::VsyncModulator>::make( + mFlinger->mVsyncConfiguration->getCurrentConfigs()); mScheduler = new TestableScheduler(std::move(vsyncController), std::move(vsyncTracker), *mFlinger->mRefreshRateConfigs, *(callback ?: this)); diff --git a/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp b/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp index 60952bfa2f..b5195826b1 100644 --- a/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include <binder/Binder.h> #include <gmock/gmock.h> #include <gtest/gtest.h> @@ -21,6 +22,13 @@ namespace android::scheduler { +class TestableVsyncModulator : public VsyncModulator { +public: + TestableVsyncModulator(const VsyncConfigSet& config, Now now) : VsyncModulator(config, now) {} + + void binderDied(const wp<IBinder>& token) { VsyncModulator::binderDied(token); } +}; + class VsyncModulatorTest : public testing::Test { enum { SF_OFFSET_LATE, @@ -60,30 +68,31 @@ protected: const VsyncModulator::VsyncConfigSet mOffsets = {kEarly, kEarlyGpu, kLate, nanos(HWC_MIN_WORK_DURATION)}; - VsyncModulator mVsyncModulator{mOffsets, Now}; + sp<TestableVsyncModulator> mVsyncModulator = sp<TestableVsyncModulator>::make(mOffsets, Now); - void SetUp() override { EXPECT_EQ(kLate, mVsyncModulator.setVsyncConfigSet(mOffsets)); } + void SetUp() override { EXPECT_EQ(kLate, mVsyncModulator->setVsyncConfigSet(mOffsets)); } }; -#define CHECK_COMMIT(result, configs) \ - EXPECT_EQ(result, mVsyncModulator.onTransactionCommit()); \ - EXPECT_EQ(configs, mVsyncModulator.getVsyncConfig()); +#define CHECK_COMMIT(result, configs) \ + EXPECT_EQ(result, mVsyncModulator->onTransactionCommit()); \ + EXPECT_EQ(configs, mVsyncModulator->getVsyncConfig()); -#define CHECK_REFRESH(N, result, configs) \ - for (int i = 0; i < N; i++) { \ - EXPECT_EQ(result, mVsyncModulator.onDisplayRefresh(false)); \ - EXPECT_EQ(configs, mVsyncModulator.getVsyncConfig()); \ +#define CHECK_REFRESH(N, result, configs) \ + for (int i = 0; i < N; i++) { \ + EXPECT_EQ(result, mVsyncModulator->onDisplayRefresh(false)); \ + EXPECT_EQ(configs, mVsyncModulator->getVsyncConfig()); \ } TEST_F(VsyncModulatorTest, Late) { - EXPECT_FALSE(mVsyncModulator.setTransactionSchedule(Schedule::Late)); + EXPECT_FALSE(mVsyncModulator->setTransactionSchedule(Schedule::Late)); CHECK_COMMIT(std::nullopt, kLate); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kLate); } TEST_F(VsyncModulatorTest, EarlyEnd) { - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); @@ -91,12 +100,13 @@ TEST_F(VsyncModulatorTest, EarlyEnd) { } TEST_F(VsyncModulatorTest, EarlyStart) { - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyStart)); + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); @@ -104,16 +114,17 @@ TEST_F(VsyncModulatorTest, EarlyStart) { } TEST_F(VsyncModulatorTest, EarlyStartWithMoreTransactions) { - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyStart)); + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token)); CHECK_COMMIT(kEarly, kEarly); for (int i = 0; i < 5 * MIN_EARLY_TRANSACTION_FRAMES; i++) { - EXPECT_FALSE(mVsyncModulator.setTransactionSchedule(Schedule::Late)); + EXPECT_FALSE(mVsyncModulator->setTransactionSchedule(Schedule::Late)); CHECK_REFRESH(1, std::nullopt, kEarly); } - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); @@ -121,18 +132,19 @@ TEST_F(VsyncModulatorTest, EarlyStartWithMoreTransactions) { } TEST_F(VsyncModulatorTest, EarlyStartAfterEarlyEnd) { - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyStart)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(1, kEarly, kEarly); CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); @@ -140,26 +152,64 @@ TEST_F(VsyncModulatorTest, EarlyStartAfterEarlyEnd) { } TEST_F(VsyncModulatorTest, EarlyStartAfterEarlyEndWithMoreTransactions) { - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyStart)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(1, kEarly, kEarly); for (int i = 0; i < 5 * MIN_EARLY_TRANSACTION_FRAMES; i++) { - EXPECT_FALSE(mVsyncModulator.setTransactionSchedule(Schedule::Late)); + EXPECT_FALSE(mVsyncModulator->setTransactionSchedule(Schedule::Late)); CHECK_REFRESH(1, std::nullopt, kEarly); } - EXPECT_EQ(kEarly, mVsyncModulator.setTransactionSchedule(Schedule::EarlyEnd)); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token)); CHECK_COMMIT(kEarly, kEarly); CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); CHECK_REFRESH(1, kLate, kLate); } +TEST_F(VsyncModulatorTest, EarlyStartDifferentClients) { + const auto token1 = sp<BBinder>::make(); + const auto token2 = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token1)); + + CHECK_COMMIT(kEarly, kEarly); + CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); + + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token2)); + + CHECK_COMMIT(kEarly, kEarly); + CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); + + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token1)); + + CHECK_COMMIT(kEarly, kEarly); + CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); + + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyEnd, token2)); + + CHECK_COMMIT(kEarly, kEarly); + CHECK_REFRESH(MIN_EARLY_TRANSACTION_FRAMES - 1, kEarly, kEarly); + CHECK_REFRESH(1, kLate, kLate); +} + +TEST_F(VsyncModulatorTest, EarlyStartWithBinderDeath) { + const auto token = sp<BBinder>::make(); + EXPECT_EQ(kEarly, mVsyncModulator->setTransactionSchedule(Schedule::EarlyStart, token)); + + CHECK_COMMIT(kEarly, kEarly); + CHECK_REFRESH(5 * MIN_EARLY_TRANSACTION_FRAMES, std::nullopt, kEarly); + + mVsyncModulator->binderDied(token); + + CHECK_COMMIT(std::nullopt, kLate); +} + } // namespace android::scheduler |