SF: Avoid adjusting Vsync Timeline when minFramePeriod isn't violated
BUG: 350738983
Test: atest VSyncPredictorTest
Flag: EXEMPT bugfix
Change-Id: Ie83ff9cb515c0c17b1f72fd31a92c28ad961d0c1
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index e79eac0..82a638d 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -451,7 +451,7 @@
const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope;
const auto threshold = currentPeriod / 2;
- const auto minFramePeriod = minFramePeriodLocked().ns();
+ const auto minFramePeriod = minFramePeriodLocked();
auto prev = lastConfirmedPresentTime.ns();
for (auto& current : mPastExpectedPresentTimes) {
@@ -462,10 +462,10 @@
1e6f);
}
- const auto minPeriodViolation = current.ns() - prev + threshold < minFramePeriod;
+ const auto minPeriodViolation = current.ns() - prev + threshold < minFramePeriod.ns();
if (minPeriodViolation) {
SFTRACE_NAME("minPeriodViolation");
- current = TimePoint::fromNs(prev + minFramePeriod);
+ current = TimePoint::fromNs(prev + minFramePeriod.ns());
prev = current.ns();
} else {
break;
@@ -476,7 +476,7 @@
const auto phase = Duration(mPastExpectedPresentTimes.back() - expectedPresentTime);
if (phase > 0ns) {
for (auto& timeline : mTimelines) {
- timeline.shiftVsyncSequence(phase);
+ timeline.shiftVsyncSequence(phase, minFramePeriod);
}
mPastExpectedPresentTimes.clear();
return phase;
@@ -777,8 +777,15 @@
return vsyncSequence.seq % divisor == 0;
}
-void VSyncPredictor::VsyncTimeline::shiftVsyncSequence(Duration phase) {
+void VSyncPredictor::VsyncTimeline::shiftVsyncSequence(Duration phase, Period minFramePeriod) {
if (mLastVsyncSequence) {
+ const auto renderRate = mRenderRateOpt.value_or(Fps::fromPeriodNsecs(mIdealPeriod.ns()));
+ const auto threshold = mIdealPeriod.ns() / 2;
+ if (renderRate.getPeriodNsecs() - phase.ns() + threshold >= minFramePeriod.ns()) {
+ SFTRACE_FORMAT_INSTANT("Not-Adjusting vsync by %.2f",
+ static_cast<float>(phase.ns()) / 1e6f);
+ return;
+ }
SFTRACE_FORMAT_INSTANT("adjusting vsync by %.2f", static_cast<float>(phase.ns()) / 1e6f);
mLastVsyncSequence->vsyncTime += phase.ns();
}
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index db5f455..9e93d87 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -104,7 +104,7 @@
void freeze(TimePoint lastVsync);
std::optional<TimePoint> validUntil() const { return mValidUntil; }
bool isVSyncInPhase(Model, nsecs_t vsync, Fps frameRate);
- void shiftVsyncSequence(Duration phase);
+ void shiftVsyncSequence(Duration phase, Period minFramePeriod);
void setRenderRate(std::optional<Fps> renderRateOpt) { mRenderRateOpt = renderRateOpt; }
enum class VsyncOnTimeline {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 8690dba..7c678bd 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -1055,6 +1055,36 @@
EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1400, 1000));
EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2000, 1000));
}
+
+TEST_F(VSyncPredictorTest, adjustsOnlyMinFrameViolatingVrrTimeline) {
+ const auto refreshRate = Fps::fromPeriodNsecs(500);
+ auto minFrameRate = Fps::fromPeriodNsecs(1000);
+ hal::VrrConfig vrrConfig{.minFrameIntervalNs =
+ static_cast<int32_t>(minFrameRate.getPeriodNsecs())};
+ ftl::NonNull<DisplayModePtr> mode =
+ ftl::as_non_null(createVrrDisplayMode(DisplayModeId(0), refreshRate, vrrConfig));
+ VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), mode, kHistorySize,
+ kMinimumSamplesForPrediction, kOutlierTolerancePercent};
+ vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false);
+ vrrTracker.addVsyncTimestamp(0);
+
+ EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700));
+ EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000));
+ auto lastConfirmedSignalTime = TimePoint::fromNs(1500);
+ auto lastConfirmedExpectedPresentTime = TimePoint::fromNs(1000);
+ vrrTracker.onFrameBegin(TimePoint::fromNs(2000),
+ {lastConfirmedSignalTime, lastConfirmedExpectedPresentTime});
+ EXPECT_EQ(3500, vrrTracker.nextAnticipatedVSyncTimeFrom(3000, 1500));
+
+ minFrameRate = Fps::fromPeriodNsecs(2000);
+ vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false);
+ lastConfirmedSignalTime = TimePoint::fromNs(2500);
+ lastConfirmedExpectedPresentTime = TimePoint::fromNs(2500);
+ vrrTracker.onFrameBegin(TimePoint::fromNs(3000),
+ {lastConfirmedSignalTime, lastConfirmedExpectedPresentTime});
+ // Enough time without adjusting vsync to present with new rate on time, no need of adjustment
+ EXPECT_EQ(5500, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 3500));
+}
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues