SurfaceFlinger: make sure VsyncPredictor always maintains a valid slope
- Add a safety check around the predicated vsync period in case
the timestamps we got from present fences are incorrect.
We suspect this happens when the device is in AOD.
- Ignore present fences when device is in AOD.
Change-Id: Ib02287cb0b8e693b1f5f384d754c2c3d978024dc
Test: adb shell /data/nativetest64/libsurfaceflinger_unittest/libsurfaceflinger_unittest
Bug: 149299560
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index b467f24..399da19 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -53,15 +53,15 @@
}
inline size_t VSyncPredictor::next(int i) const {
- return (i + 1) % timestamps.size();
+ return (i + 1) % mTimestamps.size();
}
bool VSyncPredictor::validate(nsecs_t timestamp) const {
- if (lastTimestampIndex < 0 || timestamps.empty()) {
+ if (mLastTimestampIndex < 0 || mTimestamps.empty()) {
return true;
}
- auto const aValidTimestamp = timestamps[lastTimestampIndex];
+ auto const aValidTimestamp = mTimestamps[mLastTimestampIndex];
auto const percent = (timestamp - aValidTimestamp) % mIdealPeriod * kMaxPercent / mIdealPeriod;
return percent < kOutlierTolerancePercent || percent > (kMaxPercent - kOutlierTolerancePercent);
}
@@ -79,15 +79,15 @@
return false;
}
- if (timestamps.size() != kHistorySize) {
- timestamps.push_back(timestamp);
- lastTimestampIndex = next(lastTimestampIndex);
+ if (mTimestamps.size() != kHistorySize) {
+ mTimestamps.push_back(timestamp);
+ mLastTimestampIndex = next(mLastTimestampIndex);
} else {
- lastTimestampIndex = next(lastTimestampIndex);
- timestamps[lastTimestampIndex] = timestamp;
+ mLastTimestampIndex = next(mLastTimestampIndex);
+ mTimestamps[mLastTimestampIndex] = timestamp;
}
- if (timestamps.size() < kMinimumSamplesForPrediction) {
+ if (mTimestamps.size() < kMinimumSamplesForPrediction) {
mRateMap[mIdealPeriod] = {mIdealPeriod, 0};
return true;
}
@@ -107,11 +107,11 @@
//
// intercept = mean(Y) - slope * mean(X)
//
- std::vector<nsecs_t> vsyncTS(timestamps.size());
- std::vector<nsecs_t> ordinals(timestamps.size());
+ std::vector<nsecs_t> vsyncTS(mTimestamps.size());
+ std::vector<nsecs_t> ordinals(mTimestamps.size());
// normalizing to the oldest timestamp cuts down on error in calculating the intercept.
- auto const oldest_ts = *std::min_element(timestamps.begin(), timestamps.end());
+ auto const oldest_ts = *std::min_element(mTimestamps.begin(), mTimestamps.end());
auto it = mRateMap.find(mIdealPeriod);
auto const currentPeriod = std::get<0>(it->second);
// TODO (b/144707443): its important that there's some precision in the mean of the ordinals
@@ -120,10 +120,10 @@
// scheduler::utils::calculate_mean to have a fixed point fractional part.
static constexpr int kScalingFactor = 10;
- for (auto i = 0u; i < timestamps.size(); i++) {
- traceInt64If("VSP-ts", timestamps[i]);
+ for (auto i = 0u; i < mTimestamps.size(); i++) {
+ traceInt64If("VSP-ts", mTimestamps[i]);
- vsyncTS[i] = timestamps[i] - oldest_ts;
+ vsyncTS[i] = mTimestamps[i] - oldest_ts;
ordinals[i] = ((vsyncTS[i] + (currentPeriod / 2)) / currentPeriod) * kScalingFactor;
}
@@ -143,12 +143,20 @@
if (CC_UNLIKELY(bottom == 0)) {
it->second = {mIdealPeriod, 0};
+ clearTimestamps();
return false;
}
nsecs_t const anticipatedPeriod = top / bottom * kScalingFactor;
nsecs_t const intercept = meanTS - (anticipatedPeriod * meanOrdinal / kScalingFactor);
+ auto const percent = std::abs(anticipatedPeriod - mIdealPeriod) * kMaxPercent / mIdealPeriod;
+ if (percent >= kOutlierTolerancePercent) {
+ it->second = {mIdealPeriod, 0};
+ clearTimestamps();
+ return false;
+ }
+
traceInt64If("VSP-period", anticipatedPeriod);
traceInt64If("VSP-intercept", intercept);
@@ -164,14 +172,14 @@
auto const [slope, intercept] = getVSyncPredictionModel(lk);
- if (timestamps.empty()) {
+ if (mTimestamps.empty()) {
traceInt64If("VSP-mode", 1);
auto const knownTimestamp = mKnownTimestamp ? *mKnownTimestamp : timePoint;
auto const numPeriodsOut = ((timePoint - knownTimestamp) / mIdealPeriod) + 1;
return knownTimestamp + numPeriodsOut * mIdealPeriod;
}
- auto const oldest = *std::min_element(timestamps.begin(), timestamps.end());
+ auto const oldest = *std::min_element(mTimestamps.begin(), mTimestamps.end());
// See b/145667109, the ordinal calculation must take into account the intercept.
auto const zeroPoint = oldest + intercept;
@@ -225,10 +233,10 @@
}
void VSyncPredictor::clearTimestamps() {
- if (!timestamps.empty()) {
- mKnownTimestamp = *std::max_element(timestamps.begin(), timestamps.end());
- timestamps.clear();
- lastTimestampIndex = 0;
+ if (!mTimestamps.empty()) {
+ mKnownTimestamp = *std::max_element(mTimestamps.begin(), mTimestamps.end());
+ mTimestamps.clear();
+ mLastTimestampIndex = 0;
}
}
@@ -236,11 +244,11 @@
using namespace std::literals::chrono_literals;
std::lock_guard<std::mutex> lk(mMutex);
bool needsMoreSamples = true;
- if (timestamps.size() >= kMinimumSamplesForPrediction) {
+ if (mTimestamps.size() >= kMinimumSamplesForPrediction) {
nsecs_t constexpr aLongTime =
std::chrono::duration_cast<std::chrono::nanoseconds>(500ms).count();
- if (!(lastTimestampIndex < 0 || timestamps.empty())) {
- auto const lastTimestamp = timestamps[lastTimestampIndex];
+ if (!(mLastTimestampIndex < 0 || mTimestamps.empty())) {
+ auto const lastTimestamp = mTimestamps[mLastTimestampIndex];
needsMoreSamples = !((lastTimestamp + aLongTime) > now);
}
}
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 532fe9e..ef1d88a 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -83,8 +83,8 @@
std::unordered_map<nsecs_t, std::tuple<nsecs_t, nsecs_t>> mutable mRateMap GUARDED_BY(mMutex);
- int lastTimestampIndex GUARDED_BY(mMutex) = 0;
- std::vector<nsecs_t> timestamps GUARDED_BY(mMutex);
+ int mLastTimestampIndex GUARDED_BY(mMutex) = 0;
+ std::vector<nsecs_t> mTimestamps GUARDED_BY(mMutex);
};
} // namespace android::scheduler
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f81179a..edf9b42 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2081,7 +2081,8 @@
}
});
- if (presentFenceTime->isValid()) {
+ if (displayDevice && displayDevice->isPrimary() &&
+ displayDevice->getPowerMode() == HWC_POWER_MODE_NORMAL && presentFenceTime->isValid()) {
mScheduler->addPresentFence(presentFenceTime);
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 6ec3844..f834af8 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -36,7 +36,7 @@
namespace android::scheduler {
MATCHER_P2(IsCloseTo, value, tolerance, "is within tolerance") {
- return arg <= value + tolerance && value >= value - tolerance;
+ return arg <= value + tolerance && arg >= value - tolerance;
}
std::vector<nsecs_t> generateVsyncTimestamps(size_t count, nsecs_t period, nsecs_t bias) {
@@ -370,6 +370,26 @@
IsCloseTo(idealPeriod, mMaxRoundingError));
}
+TEST_F(VSyncPredictorTest, slopeAlwaysValid) {
+ constexpr auto kNumVsyncs = 100;
+ auto invalidPeriod = mPeriod;
+ auto now = 0;
+ for (int i = 0; i < kNumVsyncs; i++) {
+ tracker.addVsyncTimestamp(now);
+ now += invalidPeriod;
+ invalidPeriod *= 0.9f;
+
+ auto [slope, intercept] = tracker.getVSyncPredictionModel();
+ EXPECT_THAT(slope, IsCloseTo(mPeriod, mPeriod * kOutlierTolerancePercent / 100.f));
+
+ // 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));
+ }
+ }
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues