Merge "SF: add a work duration slack when missing a frame" into main

13 files changed, 42 insertions, 37 deletions

diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp
index 6a67ac5d42..2e1f938126 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.cpp
+++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp
@@ -188,12 +188,13 @@ void MessageQueue::scheduleConfigure() {
     postMessage(sp<ConfigureHandler>::make(mCompositor));
 }
 
-void MessageQueue::scheduleFrame() {
+void MessageQueue::scheduleFrame(Duration workDurationSlack) {
     SFTRACE_CALL();
 
     std::lock_guard lock(mVsync.mutex);
+    const auto workDuration = Duration(mVsync.workDuration.get() - workDurationSlack);
     mVsync.scheduledFrameTimeOpt =
-            mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(),
+            mVsync.registration->schedule({.workDuration = workDuration.ns(),
                                            .readyDuration = 0,
                                            .lastVsync = mVsync.lastCallbackTime.ns()});
 }
diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h
index c5fc371d3a..ba1efbe58f 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.h
+++ b/services/surfaceflinger/Scheduler/MessageQueue.h
@@ -74,7 +74,7 @@ public:
     virtual void postMessage(sp<MessageHandler>&&) = 0;
     virtual void postMessageDelayed(sp<MessageHandler>&&, nsecs_t uptimeDelay) = 0;
     virtual void scheduleConfigure() = 0;
-    virtual void scheduleFrame() = 0;
+    virtual void scheduleFrame(Duration workDurationSlack = Duration::fromNs(0)) = 0;
 
     virtual std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const = 0;
 };
@@ -149,7 +149,7 @@ public:
     void postMessageDelayed(sp<MessageHandler>&&, nsecs_t uptimeDelay) override;
 
     void scheduleConfigure() override;
-    void scheduleFrame() override;
+    void scheduleFrame(Duration workDurationSlack = Duration::fromNs(0)) override;
 
     std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const override;
 };
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e7582b43d6..299f70bf97 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2161,12 +2161,12 @@ sp<IDisplayEventConnection> SurfaceFlinger::createDisplayEventConnection(
     return mScheduler->createDisplayEventConnection(cycle, eventRegistration, layerHandle);
 }
 
-void SurfaceFlinger::scheduleCommit(FrameHint hint) {
+void SurfaceFlinger::scheduleCommit(FrameHint hint, Duration workDurationSlack) {
     if (hint == FrameHint::kActive) {
         mScheduler->resetIdleTimer();
     }
     mPowerAdvisor->notifyDisplayUpdateImminentAndCpuReset();
-    mScheduler->scheduleFrame();
+    mScheduler->scheduleFrame(workDurationSlack);
 }
 
 void SurfaceFlinger::scheduleComposite(FrameHint hint) {
@@ -2626,7 +2626,10 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId,
                 mScheduler->getVsyncSchedule()->getTracker().onFrameMissed(
                         pacesetterFrameTarget.expectedPresentTime());
             }
-            scheduleCommit(FrameHint::kNone);
+            const Duration slack = FlagManager::getInstance().allow_n_vsyncs_in_targeter()
+                    ? TimePoint::now() - pacesetterFrameTarget.frameBeginTime()
+                    : Duration::fromNs(0);
+            scheduleCommit(FrameHint::kNone, slack);
             return false;
         }
     }
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 89ade4ec76..157b72244f 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -273,7 +273,7 @@ public:
     enum class FrameHint { kNone, kActive };
 
     // Schedule commit of transactions on the main thread ahead of the next VSYNC.
-    void scheduleCommit(FrameHint);
+    void scheduleCommit(FrameHint, Duration workDurationSlack = Duration::fromNs(0));
     // As above, but also force composite regardless if transactions were committed.
     void scheduleComposite(FrameHint);
     // As above, but also force dirty geometry to repaint.
diff --git a/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp b/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp
index 5c742d7360..866eb08913 100644
--- a/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp
@@ -28,6 +28,7 @@
 
 namespace android {
 
+using testing::_;
 using testing::DoAll;
 using testing::Mock;
 using testing::SetArgPointee;
@@ -91,7 +92,7 @@ INSTANTIATE_TEST_SUITE_P(PerLayerType, FrameRateSelectionStrategyTest,
                          PrintToStringParamName);
 
 TEST_P(FrameRateSelectionStrategyTest, SetAndGet) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
     auto layer = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
@@ -104,7 +105,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetAndGet) {
 }
 
 TEST_P(FrameRateSelectionStrategyTest, SetChildOverrideChildren) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
     auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
@@ -128,7 +129,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetChildOverrideChildren) {
 }
 
 TEST_P(FrameRateSelectionStrategyTest, SetParentOverrideChildren) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
     auto layer1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
@@ -169,7 +170,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetParentOverrideChildren) {
 }
 
 TEST_P(FrameRateSelectionStrategyTest, OverrideChildrenAndSelf) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
     auto layer1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
index 9899d4290b..4705dd1cc7 100644
--- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
@@ -35,7 +35,7 @@
 #include "mock/MockVsyncController.h"
 
 namespace android {
-
+using testing::_;
 using testing::DoAll;
 using testing::Mock;
 using testing::SetArgPointee;
@@ -93,7 +93,7 @@ void SetFrameRateTest::commitTransaction() {
 namespace {
 
 TEST_P(SetFrameRateTest, SetAndGet) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -104,7 +104,7 @@ TEST_P(SetFrameRateTest, SetAndGet) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetParent) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -129,7 +129,7 @@ TEST_P(SetFrameRateTest, SetAndGetParent) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetParentAllVote) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -168,7 +168,7 @@ TEST_P(SetFrameRateTest, SetAndGetParentAllVote) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetChild) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -193,7 +193,7 @@ TEST_P(SetFrameRateTest, SetAndGetChild) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetChildAllVote) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -232,7 +232,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAllVote) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -262,7 +262,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -293,7 +293,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) {
 }
 
 TEST_P(SetFrameRateTest, SetAndGetParentNotInTree) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
@@ -352,7 +352,7 @@ TEST_P(SetFrameRateTest, SetOnParentActivatesTree) {
 }
 
 TEST_P(SetFrameRateTest, addChildForParentWithTreeVote) {
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     const auto& layerFactory = GetParam();
 
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
index e5f2a9138d..2d3ebb47bd 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
@@ -97,7 +97,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
     // Cleanup conditions
 
     // Creating the display commits a display transaction.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 }
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
@@ -129,7 +129,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
     // Cleanup conditions
 
     // Creating the display commits a display transaction.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 }
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
@@ -159,7 +159,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
     // Cleanup conditions
 
     // Creating the display commits a display transaction.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 }
 
 // Requesting 0 tells SF not to do anything, i.e., default to refresh as physical displays
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
index f8ad8e1e1b..df8f68f2b6 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
@@ -38,7 +38,7 @@ TEST_F(DestroyDisplayTest, destroyDisplayClearsCurrentStateForDisplay) {
     // Call Expectations
 
     // Destroying the display commits a display transaction.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     // --------------------------------------------------------------------
     // Invocation
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
index 897f9a0319..aef467ab9d 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
@@ -48,7 +48,7 @@ TEST_F(HotplugTest, schedulesConfigureToProcessHotplugEvents) {
 
 TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) {
     EXPECT_CALL(*mFlinger.scheduler(), scheduleConfigure()).Times(1);
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     constexpr HWDisplayId displayId1 = 456;
     mFlinger.onComposerHalHotplugEvent(displayId1, DisplayHotplugEvent::DISCONNECTED);
@@ -73,7 +73,7 @@ TEST_F(HotplugTest, ignoresDuplicateDisconnection) {
             .WillOnce(Return(Error::NONE));
 
     // A single commit should be scheduled for both configure calls.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
     mFlinger.configure();
@@ -116,7 +116,7 @@ TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) {
                 setVsyncEnabled(ExternalDisplay::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
             .WillOnce(Return(Error::NONE));
 
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
     mFlinger.configure();
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
index eaf468432c..5231965277 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
@@ -28,7 +28,7 @@ struct InitializeDisplaysTest : DualDisplayTransactionTest<hal::PowerMode::OFF,
 
 TEST_F(InitializeDisplaysTest, initializesDisplays) {
     // Scheduled by the display transaction, and by powering on each display.
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(3);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(3);
 
     EXPECT_CALL(static_cast<mock::VSyncTracker&>(
                         mFlinger.scheduler()->getVsyncSchedule()->getTracker()),
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index 83e2f980ce..fed7b2e767 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -271,7 +271,7 @@ struct DisplayPowerCase {
     }
 
     static void setupRepaintEverythingCallExpectations(DisplayTransactionTest* test) {
-        EXPECT_CALL(*test->mFlinger.scheduler(), scheduleFrame()).Times(1);
+        EXPECT_CALL(*test->mFlinger.scheduler(), scheduleFrame(_)).Times(1);
     }
 
     static void setupComposerCallExpectations(DisplayTransactionTest* test, PowerMode mode) {
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index f0638094c7..0814e3d6fe 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -62,7 +62,7 @@ public:
     }
 
     MOCK_METHOD(void, scheduleConfigure, (), (override));
-    MOCK_METHOD(void, scheduleFrame, (), (override));
+    MOCK_METHOD(void, scheduleFrame, (Duration), (override));
     MOCK_METHOD(void, postMessage, (sp<MessageHandler>&&), (override));
 
     void doFrameSignal(ICompositor& compositor, VsyncId vsyncId) {
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index e13fe4958c..fab1f6d451 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -105,7 +105,7 @@ public:
 
     void NotPlacedOnTransactionQueue(uint32_t flags) {
         ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty());
-        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
         TransactionInfo transaction;
         setupSingle(transaction, flags,
                     /*desiredPresentTime*/ systemTime(), /*isAutoTimestamp*/ true,
@@ -129,7 +129,7 @@ public:
 
     void PlaceOnTransactionQueue(uint32_t flags) {
         ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty());
-        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
         // first check will see desired present time has not passed,
         // but afterwards it will look like the desired present time has passed
@@ -155,7 +155,7 @@ public:
     void BlockedByPriorTransaction(uint32_t flags) {
         ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty());
         nsecs_t time = systemTime();
-        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(2);
+        EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(2);
 
         // transaction that should go on the pending thread
         TransactionInfo transactionA;
@@ -217,7 +217,7 @@ public:
 
 TEST_F(TransactionApplicationTest, AddToPendingQueue) {
     ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty());
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     TransactionInfo transactionA; // transaction to go on pending queue
     setupSingle(transactionA, /*flags*/ 0, /*desiredPresentTime*/ s2ns(1), false,
@@ -238,7 +238,7 @@ TEST_F(TransactionApplicationTest, AddToPendingQueue) {
 
 TEST_F(TransactionApplicationTest, Flush_RemovesFromQueue) {
     ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty());
-    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+    EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
 
     TransactionInfo transactionA; // transaction to go on pending queue
     setupSingle(transactionA, /*flags*/ 0, /*desiredPresentTime*/ s2ns(1), false,