SF: VSyncReactor change offsets at correct time
The VRR/MRR timing loop (currently flagged off) was changing
offsets incorrectly on the first hwvsync signal after initiating
a rate change. With HWC2.3 and prior, the correct strategy for
DispSync to employ is to enable hwvsync, and await the first observed
signal at the new rate. This patch makes the new system apply the
updated offsets at the correct time.
Fixes: b/146455831
Test: 2 new, 3 fixed unit tests
Test: boot on coral with integrated patches, fiddle with rate changes.
Change-Id: Iafae2e5112a5015441405055159538feb6c23a4b
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index c471e49..a652053 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -154,7 +154,7 @@
mTracker->addVsyncTimestamp(signalTime);
}
- return false; // TODO(b/144707443): add policy for turning on HWVsync.
+ return mMoreSamplesNeeded;
}
void VSyncReactor::setIgnorePresentFences(bool ignoration) {
@@ -176,14 +176,9 @@
}
void VSyncReactor::setPeriod(nsecs_t period) {
- mTracker->setPeriod(period);
- {
- std::lock_guard<std::mutex> lk(mMutex);
- mPeriodChangeInProgress = true;
- for (auto& entry : mCallbacks) {
- entry.second->setPeriod(period);
- }
- }
+ std::lock_guard lk(mMutex);
+ mLastHwVsync.reset();
+ mPeriodTransitioningTo = period;
}
nsecs_t VSyncReactor::getPeriod() {
@@ -194,15 +189,40 @@
void VSyncReactor::endResync() {}
+bool VSyncReactor::periodChangeDetected(nsecs_t vsync_timestamp) {
+ if (!mLastHwVsync || !mPeriodTransitioningTo) {
+ return false;
+ }
+ auto const distance = vsync_timestamp - *mLastHwVsync;
+ return std::abs(distance - *mPeriodTransitioningTo) < std::abs(distance - getPeriod());
+}
+
bool VSyncReactor::addResyncSample(nsecs_t timestamp, bool* periodFlushed) {
assert(periodFlushed);
- mTracker->addVsyncTimestamp(timestamp);
- {
- std::lock_guard<std::mutex> lk(mMutex);
- *periodFlushed = mPeriodChangeInProgress;
- mPeriodChangeInProgress = false;
+
+ std::lock_guard<std::mutex> lk(mMutex);
+ if (periodChangeDetected(timestamp)) {
+ mMoreSamplesNeeded = false;
+ *periodFlushed = true;
+
+ mTracker->setPeriod(*mPeriodTransitioningTo);
+ for (auto& entry : mCallbacks) {
+ entry.second->setPeriod(*mPeriodTransitioningTo);
+ }
+
+ mPeriodTransitioningTo.reset();
+ mLastHwVsync.reset();
+ } else if (mPeriodTransitioningTo) {
+ mLastHwVsync = timestamp;
+ mMoreSamplesNeeded = true;
+ *periodFlushed = false;
+ } else {
+ mMoreSamplesNeeded = false;
+ *periodFlushed = false;
}
- return false;
+
+ mTracker->addVsyncTimestamp(timestamp);
+ return mMoreSamplesNeeded;
}
status_t VSyncReactor::addEventListener(const char* name, nsecs_t phase,
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.h b/services/surfaceflinger/Scheduler/VSyncReactor.h
index 29a0a11..73a5e37 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.h
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.h
@@ -61,6 +61,8 @@
void reset() final;
private:
+ bool periodChangeDetected(nsecs_t vsync_timestamp) REQUIRES(mMutex);
+
std::unique_ptr<Clock> const mClock;
std::unique_ptr<VSyncTracker> const mTracker;
std::unique_ptr<VSyncDispatch> const mDispatch;
@@ -69,7 +71,11 @@
std::mutex mMutex;
bool mIgnorePresentFences GUARDED_BY(mMutex) = false;
std::vector<std::shared_ptr<FenceTime>> mUnfiredFences GUARDED_BY(mMutex);
- bool mPeriodChangeInProgress GUARDED_BY(mMutex) = false;
+
+ bool mMoreSamplesNeeded GUARDED_BY(mMutex) = false;
+ std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);
+ std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);
+
std::unordered_map<DispSync::Callback*, std::unique_ptr<CallbackRepeater>> mCallbacks
GUARDED_BY(mMutex);
};
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 537cc80..72f8bb9 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -159,7 +159,6 @@
static constexpr nsecs_t mPhase = 3000;
static constexpr nsecs_t mAnotherPhase = 5200;
static constexpr nsecs_t period = 10000;
- static constexpr nsecs_t mAnotherPeriod = 23333;
static constexpr nsecs_t mFakeCbTime = 2093;
static constexpr nsecs_t mFakeNow = 2214;
static constexpr const char mName[] = "callbacky";
@@ -274,10 +273,38 @@
EXPECT_THAT(mReactor.getPeriod(), Eq(fakePeriod));
}
-TEST_F(VSyncReactorTest, setPeriod) {
- nsecs_t const fakePeriod = 4098;
- EXPECT_CALL(*mMockTracker, setPeriod(fakePeriod));
- mReactor.setPeriod(fakePeriod);
+TEST_F(VSyncReactorTest, setPeriodCalledOnceConfirmedChange) {
+ nsecs_t const newPeriod = 5000;
+ EXPECT_CALL(*mMockTracker, setPeriod(_)).Times(0);
+ mReactor.setPeriod(newPeriod);
+
+ bool periodFlushed = true;
+ EXPECT_TRUE(mReactor.addResyncSample(10000, &periodFlushed));
+ EXPECT_FALSE(periodFlushed);
+
+ EXPECT_TRUE(mReactor.addResyncSample(20000, &periodFlushed));
+ EXPECT_FALSE(periodFlushed);
+
+ Mock::VerifyAndClearExpectations(mMockTracker.get());
+ EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1);
+
+ EXPECT_FALSE(mReactor.addResyncSample(25000, &periodFlushed));
+ EXPECT_TRUE(periodFlushed);
+}
+
+TEST_F(VSyncReactorTest, setPeriodCalledFirstTwoEventsNewPeriod) {
+ nsecs_t const newPeriod = 5000;
+ EXPECT_CALL(*mMockTracker, setPeriod(_)).Times(0);
+ mReactor.setPeriod(newPeriod);
+
+ bool periodFlushed = true;
+ EXPECT_TRUE(mReactor.addResyncSample(5000, &periodFlushed));
+ EXPECT_FALSE(periodFlushed);
+ Mock::VerifyAndClearExpectations(mMockTracker.get());
+
+ EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1);
+ EXPECT_FALSE(mReactor.addResyncSample(10000, &periodFlushed));
+ EXPECT_TRUE(periodFlushed);
}
TEST_F(VSyncReactorTest, addResyncSampleTypical) {
@@ -291,12 +318,43 @@
TEST_F(VSyncReactorTest, addResyncSamplePeriodChanges) {
bool periodFlushed = false;
- nsecs_t const fakeTimestamp = 4398;
- nsecs_t const newPeriod = 3490;
- EXPECT_CALL(*mMockTracker, addVsyncTimestamp(fakeTimestamp));
+ nsecs_t const newPeriod = 4000;
+
mReactor.setPeriod(newPeriod);
- EXPECT_FALSE(mReactor.addResyncSample(fakeTimestamp, &periodFlushed));
+
+ auto time = 0;
+ auto constexpr numTimestampSubmissions = 10;
+ for (auto i = 0; i < numTimestampSubmissions; i++) {
+ time += period;
+ EXPECT_TRUE(mReactor.addResyncSample(time, &periodFlushed));
+ EXPECT_FALSE(periodFlushed);
+ }
+
+ time += newPeriod;
+ EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed));
EXPECT_TRUE(periodFlushed);
+
+ for (auto i = 0; i < numTimestampSubmissions; i++) {
+ time += newPeriod;
+ EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed));
+ EXPECT_FALSE(periodFlushed);
+ }
+}
+
+TEST_F(VSyncReactorTest, addPresentFenceWhileAwaitingPeriodConfirmationRequestsHwVsync) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod = 4000;
+ mReactor.setPeriod(newPeriod);
+
+ time += period;
+ mReactor.addResyncSample(time, &periodFlushed);
+ EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
+
+ time += newPeriod;
+ mReactor.addResyncSample(time, &periodFlushed);
+
+ EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
}
static nsecs_t computeWorkload(nsecs_t period, nsecs_t phase) {
@@ -399,20 +457,26 @@
mReactor.addEventListener(mName, mAnotherPhase, &outerCb, lastCallbackTime);
}
-TEST_F(VSyncReactorTest, changingPeriodChangesOfsetsOnNextCb) {
+TEST_F(VSyncReactorTest, changingPeriodChangesOffsetsOnNextCb) {
+ static constexpr nsecs_t anotherPeriod = 23333;
Sequence seq;
EXPECT_CALL(*mMockDispatch, registerCallback(_, std::string(mName)))
.InSequence(seq)
.WillOnce(Return(mFakeToken));
EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeNow))
.InSequence(seq);
- EXPECT_CALL(*mMockTracker, setPeriod(mAnotherPeriod));
+ EXPECT_CALL(*mMockTracker, setPeriod(anotherPeriod));
EXPECT_CALL(*mMockDispatch,
- schedule(mFakeToken, computeWorkload(mAnotherPeriod, mPhase), mFakeNow))
+ schedule(mFakeToken, computeWorkload(anotherPeriod, mPhase), mFakeNow))
.InSequence(seq);
mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
- mReactor.setPeriod(mAnotherPeriod);
+
+ bool periodFlushed = false;
+ mReactor.setPeriod(anotherPeriod);
+ EXPECT_TRUE(mReactor.addResyncSample(anotherPeriod, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(anotherPeriod * 2, &periodFlushed));
+
mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
}