diff options
8 files changed, 112 insertions, 29 deletions
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp index ac326334f4..e6884159c3 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -71,12 +71,12 @@ nsecs_t VSyncPredictor::currentPeriod() const { 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 @@ void VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) { 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 @@ void VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) { if (CC_UNLIKELY(bottom == 0)) { it->second = {mIdealPeriod, 0}; - return; + return false; } nsecs_t const anticipatedPeriod = top / bottom * kScalingFactor; @@ -156,6 +156,7 @@ void VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) { 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 e3665553c7..532fe9e928 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.h +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h @@ -38,7 +38,7 @@ public: 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 53fa212465..70e4760676 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp @@ -130,10 +130,11 @@ bool VSyncReactor::addPresentFence(const std::shared_ptr<FenceTime>& fence) { } 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 @@ bool VSyncReactor::addPresentFence(const std::shared_ptr<FenceTime>& fence) { } 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 @@ bool VSyncReactor::addPresentFence(const std::shared_ptr<FenceTime>& fence) { } 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 @@ bool VSyncReactor::addPresentFence(const std::shared_ptr<FenceTime>& fence) { 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 @@ nsecs_t VSyncReactor::expectedPresentTime() { } void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) { + mPeriodConfirmationInProgress = true; mPeriodTransitioningTo = newPeriod; mMoreSamplesNeeded = true; + setIgnorePresentFencesInternal(true); } void VSyncReactor::endPeriodTransition() { + setIgnorePresentFencesInternal(false); + mMoreSamplesNeeded = false; mPeriodTransitioningTo.reset(); + mPeriodConfirmationInProgress = false; mLastHwVsync.reset(); - mMoreSamplesNeeded = false; } void VSyncReactor::setPeriod(nsecs_t period) { @@ -208,27 +229,33 @@ void VSyncReactor::beginResync() { 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 f318dcb720..5b79f35c23 100644 --- a/services/surfaceflinger/Scheduler/VSyncReactor.h +++ b/services/surfaceflinger/Scheduler/VSyncReactor.h @@ -61,9 +61,11 @@ public: 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 @@ private: 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 2b278848b3..a25b8a98de 100644 --- a/services/surfaceflinger/Scheduler/VSyncTracker.h +++ b/services/surfaceflinger/Scheduler/VSyncTracker.h @@ -33,8 +33,10 @@ public: * 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 acf852daa1..caac61da29 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp @@ -37,7 +37,7 @@ class FixedRateIdealStubTracker : public VSyncTracker { 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 @@ class VRRStubTracker : public VSyncTracker { 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 70c92254b2..3ab38e40ef 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp @@ -39,9 +39,10 @@ public: 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 ce1fafedc9..1de72b9599 100644 --- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp @@ -35,7 +35,8 @@ namespace android::scheduler { 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 @@ class VSyncTrackerWrapper : public VSyncTracker { 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 @@ TEST_F(VSyncReactorTest, ignoresPresentFencesWhenToldTo) { 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 @@ TEST_F(VSyncReactorTest, changingToAThirdPeriodWillWaitForLastPeriod) { 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); |