Merge "SF: VSR: VSyncTracker can reorients itself"
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index ac32633..e688415 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -71,12 +71,12 @@
     return std::get<0>(mRateMap.find(mIdealPeriod)->second);
 }
 
-void VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) {
+bool VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) {
     std::lock_guard<std::mutex> lk(mMutex);
 
     if (!validate(timestamp)) {
         ALOGV("timestamp was too far off the last known timestamp");
-        return;
+        return false;
     }
 
     if (timestamps.size() != kHistorySize) {
@@ -89,7 +89,7 @@
 
     if (timestamps.size() < kMinimumSamplesForPrediction) {
         mRateMap[mIdealPeriod] = {mIdealPeriod, 0};
-        return;
+        return true;
     }
 
     // This is a 'simple linear regression' calculation of Y over X, with Y being the
@@ -143,7 +143,7 @@
 
     if (CC_UNLIKELY(bottom == 0)) {
         it->second = {mIdealPeriod, 0};
-        return;
+        return false;
     }
 
     nsecs_t const anticipatedPeriod = top / bottom * kScalingFactor;
@@ -156,6 +156,7 @@
 
     ALOGV("model update ts: %" PRId64 " slope: %" PRId64 " intercept: %" PRId64, timestamp,
           anticipatedPeriod, intercept);
+    return true;
 }
 
 nsecs_t VSyncPredictor::nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const {
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index e366555..532fe9e 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -38,7 +38,7 @@
                    uint32_t outlierTolerancePercent);
     ~VSyncPredictor();
 
-    void addVsyncTimestamp(nsecs_t timestamp) final;
+    bool addVsyncTimestamp(nsecs_t timestamp) final;
     nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final;
     nsecs_t currentPeriod() const final;
     void resetModel() final;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index 53fa212..70e4760 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -130,10 +130,11 @@
     }
 
     std::lock_guard<std::mutex> lk(mMutex);
-    if (mIgnorePresentFences) {
+    if (mExternalIgnoreFences || mInternalIgnoreFences) {
         return true;
     }
 
+    bool timestampAccepted = true;
     for (auto it = mUnfiredFences.begin(); it != mUnfiredFences.end();) {
         auto const time = (*it)->getCachedSignalTime();
         if (time == Fence::SIGNAL_TIME_PENDING) {
@@ -141,7 +142,8 @@
         } else if (time == Fence::SIGNAL_TIME_INVALID) {
             it = mUnfiredFences.erase(it);
         } else {
-            mTracker->addVsyncTimestamp(time);
+            timestampAccepted &= mTracker->addVsyncTimestamp(time);
+
             it = mUnfiredFences.erase(it);
         }
     }
@@ -152,7 +154,13 @@
         }
         mUnfiredFences.push_back(fence);
     } else {
-        mTracker->addVsyncTimestamp(signalTime);
+        timestampAccepted &= mTracker->addVsyncTimestamp(signalTime);
+    }
+
+    if (!timestampAccepted) {
+        mMoreSamplesNeeded = true;
+        setIgnorePresentFencesInternal(true);
+        mPeriodConfirmationInProgress = true;
     }
 
     return mMoreSamplesNeeded;
@@ -160,8 +168,17 @@
 
 void VSyncReactor::setIgnorePresentFences(bool ignoration) {
     std::lock_guard<std::mutex> lk(mMutex);
-    mIgnorePresentFences = ignoration;
-    if (mIgnorePresentFences == true) {
+    mExternalIgnoreFences = ignoration;
+    updateIgnorePresentFencesInternal();
+}
+
+void VSyncReactor::setIgnorePresentFencesInternal(bool ignoration) {
+    mInternalIgnoreFences = ignoration;
+    updateIgnorePresentFencesInternal();
+}
+
+void VSyncReactor::updateIgnorePresentFencesInternal() {
+    if (mExternalIgnoreFences || mInternalIgnoreFences) {
         mUnfiredFences.clear();
     }
 }
@@ -177,14 +194,18 @@
 }
 
 void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) {
+    mPeriodConfirmationInProgress = true;
     mPeriodTransitioningTo = newPeriod;
     mMoreSamplesNeeded = true;
+    setIgnorePresentFencesInternal(true);
 }
 
 void VSyncReactor::endPeriodTransition() {
-    mPeriodTransitioningTo.reset();
-    mLastHwVsync.reset();
+    setIgnorePresentFencesInternal(false);
     mMoreSamplesNeeded = false;
+    mPeriodTransitioningTo.reset();
+    mPeriodConfirmationInProgress = false;
+    mLastHwVsync.reset();
 }
 
 void VSyncReactor::setPeriod(nsecs_t period) {
@@ -208,27 +229,33 @@
 
 void VSyncReactor::endResync() {}
 
-bool VSyncReactor::periodChangeDetected(nsecs_t vsync_timestamp) {
-    if (!mLastHwVsync || !mPeriodTransitioningTo) {
+bool VSyncReactor::periodConfirmed(nsecs_t vsync_timestamp) {
+    if (!mLastHwVsync || !mPeriodConfirmationInProgress) {
         return false;
     }
+    auto const period = mPeriodTransitioningTo ? *mPeriodTransitioningTo : getPeriod();
+
+    static constexpr int allowancePercent = 10;
+    static constexpr std::ratio<allowancePercent, 100> allowancePercentRatio;
+    auto const allowance = period * allowancePercentRatio.num / allowancePercentRatio.den;
     auto const distance = vsync_timestamp - *mLastHwVsync;
-    return std::abs(distance - *mPeriodTransitioningTo) < std::abs(distance - getPeriod());
+    return std::abs(distance - period) < allowance;
 }
 
 bool VSyncReactor::addResyncSample(nsecs_t timestamp, bool* periodFlushed) {
     assert(periodFlushed);
 
     std::lock_guard<std::mutex> lk(mMutex);
-    if (periodChangeDetected(timestamp)) {
-        mTracker->setPeriod(*mPeriodTransitioningTo);
-        for (auto& entry : mCallbacks) {
-            entry.second->setPeriod(*mPeriodTransitioningTo);
+    if (periodConfirmed(timestamp)) {
+        if (mPeriodTransitioningTo) {
+            mTracker->setPeriod(*mPeriodTransitioningTo);
+            for (auto& entry : mCallbacks) {
+                entry.second->setPeriod(*mPeriodTransitioningTo);
+            }
+            *periodFlushed = true;
         }
-
         endPeriodTransition();
-        *periodFlushed = true;
-    } else if (mPeriodTransitioningTo) {
+    } else if (mPeriodConfirmationInProgress) {
         mLastHwVsync = timestamp;
         mMoreSamplesNeeded = true;
         *periodFlushed = false;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.h b/services/surfaceflinger/Scheduler/VSyncReactor.h
index f318dcb..5b79f35 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.h
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.h
@@ -61,9 +61,11 @@
     void reset() final;
 
 private:
+    void setIgnorePresentFencesInternal(bool ignoration) REQUIRES(mMutex);
+    void updateIgnorePresentFencesInternal() REQUIRES(mMutex);
     void startPeriodTransition(nsecs_t newPeriod) REQUIRES(mMutex);
     void endPeriodTransition() REQUIRES(mMutex);
-    bool periodChangeDetected(nsecs_t vsync_timestamp) REQUIRES(mMutex);
+    bool periodConfirmed(nsecs_t vsync_timestamp) REQUIRES(mMutex);
 
     std::unique_ptr<Clock> const mClock;
     std::unique_ptr<VSyncTracker> const mTracker;
@@ -71,10 +73,12 @@
     size_t const mPendingLimit;
 
     std::mutex mMutex;
-    bool mIgnorePresentFences GUARDED_BY(mMutex) = false;
+    bool mInternalIgnoreFences GUARDED_BY(mMutex) = false;
+    bool mExternalIgnoreFences GUARDED_BY(mMutex) = false;
     std::vector<std::shared_ptr<FenceTime>> mUnfiredFences GUARDED_BY(mMutex);
 
     bool mMoreSamplesNeeded GUARDED_BY(mMutex) = false;
+    bool mPeriodConfirmationInProgress GUARDED_BY(mMutex) = false;
     std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);
     std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);
 
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index 2b27884..a25b8a9 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -33,8 +33,10 @@
      * to the model.
      *
      * \param [in] timestamp    The timestamp when the vsync signal was.
+     * \return                  True if the timestamp was consistent with the internal model,
+     *                          False otherwise
      */
-    virtual void addVsyncTimestamp(nsecs_t timestamp) = 0;
+    virtual bool addVsyncTimestamp(nsecs_t timestamp) = 0;
 
     /*
      * Access the next anticipated vsync time such that the anticipated time >= timePoint.
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index acf852d..caac61d 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -37,7 +37,7 @@
 public:
     FixedRateIdealStubTracker() : mPeriod{toNs(3ms)} {}
 
-    void addVsyncTimestamp(nsecs_t) final {}
+    bool addVsyncTimestamp(nsecs_t) final { return true; }
 
     nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final {
         auto const floor = timePoint % mPeriod;
@@ -60,7 +60,7 @@
 public:
     VRRStubTracker(nsecs_t period) : mPeriod{period} {}
 
-    void addVsyncTimestamp(nsecs_t) final {}
+    bool addVsyncTimestamp(nsecs_t) final { return true; }
 
     nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t time_point) const final {
         std::lock_guard<decltype(mMutex)> lk(mMutex);
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 70c9225..3ab38e4 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -39,9 +39,10 @@
     MockVSyncTracker(nsecs_t period) : mPeriod{period} {
         ON_CALL(*this, nextAnticipatedVSyncTimeFrom(_))
                 .WillByDefault(Invoke(this, &MockVSyncTracker::nextVSyncTime));
+        ON_CALL(*this, addVsyncTimestamp(_)).WillByDefault(Return(true));
     }
 
-    MOCK_METHOD1(addVsyncTimestamp, void(nsecs_t));
+    MOCK_METHOD1(addVsyncTimestamp, bool(nsecs_t));
     MOCK_CONST_METHOD1(nextAnticipatedVSyncTimeFrom, nsecs_t(nsecs_t));
     MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
     MOCK_METHOD1(setPeriod, void(nsecs_t));
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index ce1fafe..1de72b9 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -35,7 +35,8 @@
 
 class MockVSyncTracker : public VSyncTracker {
 public:
-    MOCK_METHOD1(addVsyncTimestamp, void(nsecs_t));
+    MockVSyncTracker() { ON_CALL(*this, addVsyncTimestamp(_)).WillByDefault(Return(true)); }
+    MOCK_METHOD1(addVsyncTimestamp, bool(nsecs_t));
     MOCK_CONST_METHOD1(nextAnticipatedVSyncTimeFrom, nsecs_t(nsecs_t));
     MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
     MOCK_METHOD1(setPeriod, void(nsecs_t));
@@ -46,7 +47,9 @@
 public:
     VSyncTrackerWrapper(std::shared_ptr<VSyncTracker> const& tracker) : mTracker(tracker) {}
 
-    void addVsyncTimestamp(nsecs_t timestamp) final { mTracker->addVsyncTimestamp(timestamp); }
+    bool addVsyncTimestamp(nsecs_t timestamp) final {
+        return mTracker->addVsyncTimestamp(timestamp);
+    }
     nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final {
         return mTracker->nextAnticipatedVSyncTimeFrom(timePoint);
     }
@@ -239,6 +242,22 @@
     EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(mDummyTime)));
 }
 
+TEST_F(VSyncReactorTest, ignoresProperlyAfterAPeriodConfirmation) {
+    bool periodFlushed = true;
+    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
+    mReactor.setIgnorePresentFences(true);
+
+    nsecs_t const newPeriod = 5000;
+    mReactor.setPeriod(newPeriod);
+
+    EXPECT_TRUE(mReactor.addResyncSample(0, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+    EXPECT_FALSE(mReactor.addResyncSample(newPeriod, &periodFlushed));
+    EXPECT_TRUE(periodFlushed);
+
+    EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
+}
+
 TEST_F(VSyncReactorTest, queriesTrackerForNextRefreshNow) {
     nsecs_t const fakeTimestamp = 4839;
     EXPECT_CALL(*mMockTracker, currentPeriod()).Times(0);
@@ -330,6 +349,35 @@
     EXPECT_TRUE(periodFlushed);
 }
 
+TEST_F(VSyncReactorTest, reportedBadTimestampFromPredictorWillReactivateHwVSync) {
+    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_))
+            .WillOnce(Return(false))
+            .WillOnce(Return(true))
+            .WillOnce(Return(true));
+    EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
+    EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
+
+    nsecs_t skewyPeriod = period >> 1;
+    bool periodFlushed = false;
+    nsecs_t sampleTime = 0;
+    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += skewyPeriod, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+    EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+}
+
+TEST_F(VSyncReactorTest, reportedBadTimestampFromPredictorWillReactivateHwVSyncPendingFence) {
+    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_))
+            .Times(2)
+            .WillOnce(Return(false))
+            .WillOnce(Return(true));
+
+    auto fence = generatePendingFence();
+    EXPECT_FALSE(mReactor.addPresentFence(fence));
+    signalFenceWithTime(fence, period >> 1);
+    EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
+}
+
 TEST_F(VSyncReactorTest, presentFenceAdditionDoesNotInterruptConfirmationProcess) {
     nsecs_t const newPeriod = 5000;
     mReactor.setPeriod(newPeriod);