SF: hwvsync on for more samples on recalibration
Decrease the amount of time that the VSyncPredictor is using the
idealPeriod (ie, the hwc-reported period) by leaving hwVsync on until
there's enough info (4 additional pulses) to infer device's
measured/actual period.
This will benefit frames that come intermittently, as the vsync's
cumulative error over the larger time gap will be smaller, giving the
timer a better chance of landing closer to the vsync signal when those
one-off frames come in.
Test: 3 new unit tests, 2 other tests modified in substance.
Test: dogfooding
Test: uibench
Fixes: 159896971
Change-Id: Ic3190822e38c5a24d3445ea89ef6d7e34a590077
Merged-In: Ic3190822e38c5a24d3445ea89ef6d7e34a590077
(cherry picked from commit b818bfaf7b5bb2f62ed8af0a3ede300a8eb65e2f)
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index be49ef3..c2a7752 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -51,6 +51,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
+ bool needsMoreSamples() const final { return false; }
void dump(std::string&) const final {}
private:
@@ -86,6 +87,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
+ bool needsMoreSamples() const final { return false; }
void dump(std::string&) const final {}
private:
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index d940dc5..65c391d 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -47,6 +47,7 @@
MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
+ MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD1(dump, void(std::string&));
nsecs_t nextVSyncTime(nsecs_t timePoint) const {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index fc39235..d4cd11d 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -73,27 +73,27 @@
TEST_F(VSyncPredictorTest, reportsSamplesNeededWhenHasNoDataPoints) {
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
- EXPECT_TRUE(tracker.needsMoreSamples(mNow += mPeriod));
- tracker.addVsyncTimestamp(mNow);
+ EXPECT_TRUE(tracker.needsMoreSamples());
+ tracker.addVsyncTimestamp(mNow += mPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
}
TEST_F(VSyncPredictorTest, reportsSamplesNeededAfterExplicitRateChange) {
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
tracker.addVsyncTimestamp(mNow += mPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
auto const changedPeriod = mPeriod * 2;
tracker.setPeriod(changedPeriod);
- EXPECT_TRUE(tracker.needsMoreSamples(mNow));
+ EXPECT_TRUE(tracker.needsMoreSamples());
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
- EXPECT_TRUE(tracker.needsMoreSamples(mNow += changedPeriod));
- tracker.addVsyncTimestamp(mNow);
+ EXPECT_TRUE(tracker.needsMoreSamples());
+ tracker.addVsyncTimestamp(mNow += changedPeriod);
}
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
+ EXPECT_FALSE(tracker.needsMoreSamples());
}
TEST_F(VSyncPredictorTest, transitionsToModelledPointsAfterSynthetic) {
@@ -320,20 +320,6 @@
EXPECT_THAT(intercept, Eq(0));
}
-TEST_F(VSyncPredictorTest, willBecomeInaccurateAfterA_longTimeWithNoSamples) {
- auto const simulatedVsyncs = generateVsyncTimestamps(kMinimumSamplesForPrediction, mPeriod, 0);
-
- for (auto const& timestamp : simulatedVsyncs) {
- tracker.addVsyncTimestamp(timestamp);
- }
- auto const mNow = *simulatedVsyncs.rbegin();
- EXPECT_FALSE(tracker.needsMoreSamples(mNow));
-
- // TODO: would be better to decay this as a result of the variance of the samples
- static auto constexpr aLongTimeOut = 1000000000;
- EXPECT_TRUE(tracker.needsMoreSamples(mNow + aLongTimeOut));
-}
-
TEST_F(VSyncPredictorTest, idealModelPredictionsBeforeRegressionModelIsBuilt) {
auto const simulatedVsyncs =
generateVsyncTimestamps(kMinimumSamplesForPrediction + 1, mPeriod, 0);
@@ -443,7 +429,7 @@
// When VsyncPredictor returns the period it means that it doesn't know how to predict and
// it needs to get more samples
if (slope == mPeriod && intercept == 0) {
- EXPECT_TRUE(tracker.needsMoreSamples(now));
+ EXPECT_TRUE(tracker.needsMoreSamples());
}
}
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index a972562..6856612 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -41,6 +41,7 @@
MOCK_CONST_METHOD0(currentPeriod, nsecs_t());
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
+ MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD1(dump, void(std::string&));
};
@@ -57,6 +58,7 @@
nsecs_t currentPeriod() const final { return mTracker->currentPeriod(); }
void setPeriod(nsecs_t period) final { mTracker->setPeriod(period); }
void resetModel() final { mTracker->resetModel(); }
+ bool needsMoreSamples() const final { return mTracker->needsMoreSamples(); }
void dump(std::string& result) const final { mTracker->dump(result); }
private:
@@ -455,6 +457,83 @@
EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));
}
+TEST_F(VSyncReactorTest, hwVsyncIsRequestedForTracker) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod = 4000;
+ mReactor.setPeriod(newPeriod);
+
+ static auto constexpr numSamplesWithNewPeriod = 4;
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(numSamplesWithNewPeriod - 2)
+ .InSequence(seq)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(numSamplesWithNewPeriod);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ // confirmed period, but predictor wants numRequest samples. This one and prior are valid.
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+}
+
+TEST_F(VSyncReactorTest, hwVsyncturnsOffOnConfirmationWhenTrackerDoesntRequest) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod = 4000;
+ mReactor.setPeriod(newPeriod);
+
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod, std::nullopt, &periodFlushed));
+}
+
+TEST_F(VSyncReactorTest, hwVsyncIsRequestedForTrackerMultiplePeriodChanges) {
+ auto time = 0;
+ bool periodFlushed = false;
+ nsecs_t const newPeriod1 = 4000;
+ nsecs_t const newPeriod2 = 7000;
+
+ mReactor.setPeriod(newPeriod1);
+
+ Sequence seq;
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(4)
+ .InSequence(seq)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mMockTracker, needsMoreSamples())
+ .Times(1)
+ .InSequence(seq)
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(7);
+
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += period, std::nullopt, &periodFlushed));
+ // confirmed period, but predictor wants numRequest samples. This one and prior are valid.
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+
+ mReactor.setPeriod(newPeriod2);
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod1, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+ EXPECT_TRUE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+ EXPECT_FALSE(mReactor.addResyncSample(time += newPeriod2, std::nullopt, &periodFlushed));
+}
+
static nsecs_t computeWorkload(nsecs_t period, nsecs_t phase) {
return period - phase;
}
@@ -648,7 +727,7 @@
TEST_F(VSyncReactorTest, periodChangeWithGivenVsyncPeriod) {
bool periodFlushed = true;
- EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(3);
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
mReactor.setIgnorePresentFences(true);
nsecs_t const newPeriod = 5000;
@@ -672,7 +751,7 @@
kPendingLimit, true /* supportKernelIdleTimer */);
bool periodFlushed = true;
- EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(5);
+ EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(4);
idleReactor.setIgnorePresentFences(true);
// First, set the same period, which should only be confirmed when we receive two