Merge "SF: allow more than one client to use eEarly[Start|End] flags" into sc-dev

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