SF: allow more than one client to use eEarly[Start|End] flags
Bug: 191969790
Test: SF unit tests
Change-Id: I0a21600eba986857e0231e02cb4d59fb108e9bd3
diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.cpp b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
index 194d808..245db0f 100644
--- a/services/surfaceflinger/Scheduler/VsyncModulator.cpp
+++ b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
@@ -46,27 +46,36 @@
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 @@
return std::nullopt;
}
mTransactionSchedule = schedule;
- return updateVsyncConfig();
+ return updateVsyncConfigLocked();
}
VsyncModulator::VsyncConfigOpt VsyncModulator::onTransactionCommit() {
@@ -128,8 +137,8 @@
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 @@
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 9410768..b2b0451 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 @@
};
// 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 @@
[[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 @@
[[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 @@
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 5e17828..730ebb1 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3134,7 +3134,7 @@
hal::PowerMode::OFF);
mVsyncConfiguration = getFactory().createVsyncConfiguration(currRefreshRate);
- mVsyncModulator.emplace(mVsyncConfiguration->getCurrentConfigs());
+ mVsyncModulator = sp<VsyncModulator>::make(mVsyncConfiguration->getCurrentConfigs());
// start the EventThread
mScheduler = getFactory().createScheduler(display->holdRefreshRateConfigs(), *this);
@@ -3412,9 +3412,10 @@
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;
}
@@ -3634,7 +3635,7 @@
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 9142978..b9aefc3 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -847,7 +847,7 @@
// 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(
@@ -1400,7 +1400,7 @@
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::RefreshRateStats> mRefreshRateStats;
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 393387b..8528376 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -227,7 +227,8 @@
mRefreshRateConfigs = std::make_shared<scheduler::RefreshRateConfigs>(modes, currMode);
const auto currFps = mRefreshRateConfigs->getCurrentRefreshRate().getFps();
mFlinger->mVsyncConfiguration = mFactory.createVsyncConfiguration(currFps);
- mFlinger->mVsyncModulator.emplace(mFlinger->mVsyncConfiguration->getCurrentConfigs());
+ mFlinger->mVsyncModulator = sp<scheduler::VsyncModulator>::make(
+ mFlinger->mVsyncConfiguration->getCurrentConfigs());
mFlinger->mRefreshRateStats =
std::make_unique<scheduler::RefreshRateStats>(*mFlinger->mTimeStats, currFps,
/*powerMode=*/hal::PowerMode::OFF);
diff --git a/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp b/services/surfaceflinger/tests/unittests/VsyncModulatorTest.cpp
index 60952bf..b519582 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 @@
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, 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, 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, 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, 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