SF: flush setRenderRate when changing mode
There is no point to try to syncronize changing the render rate
if the display mode changes. This would just cause more latency
to getting the next vsync.
Bug: 328140524
Test: presubmit
Change-Id: I78a82bee01ba793890d459564ab701d257851b49
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 3f91682..d92edb8 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -565,7 +565,7 @@
}));
}
-void Scheduler::setRenderRate(PhysicalDisplayId id, Fps renderFrameRate) {
+void Scheduler::setRenderRate(PhysicalDisplayId id, Fps renderFrameRate, bool applyImmediately) {
std::scoped_lock lock(mDisplayLock);
ftl::FakeGuard guard(kMainThreadContext);
@@ -586,7 +586,7 @@
ALOGV("%s %s (%s)", __func__, to_string(mode.fps).c_str(),
to_string(mode.modePtr->getVsyncRate()).c_str());
- display.schedulePtr->getTracker().setRenderRate(renderFrameRate);
+ display.schedulePtr->getTracker().setRenderRate(renderFrameRate, applyImmediately);
}
Fps Scheduler::getNextFrameInterval(PhysicalDisplayId id,
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 09f75fd..494a91b 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -188,7 +188,7 @@
const VsyncConfiguration& getVsyncConfiguration() const { return *mVsyncConfiguration; }
// Sets the render rate for the scheduler to run at.
- void setRenderRate(PhysicalDisplayId, Fps);
+ void setRenderRate(PhysicalDisplayId, Fps, bool applyImmediately);
void enableHardwareVsync(PhysicalDisplayId) REQUIRES(kMainThreadContext);
void disableHardwareVsync(PhysicalDisplayId, bool disallow) REQUIRES(kMainThreadContext);
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 156cf99..58457d8 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -296,6 +296,10 @@
const auto now = TimePoint::fromNs(mClock->now());
purgeTimelines(now);
+ if (lastVsyncOpt && *lastVsyncOpt > timePoint) {
+ timePoint = *lastVsyncOpt;
+ }
+
const auto model = getVSyncPredictionModelLocked();
const auto threshold = model.slope / 2;
std::optional<TimePoint> vsyncOpt;
@@ -352,7 +356,7 @@
return mTimelines.back().isVSyncInPhase(model, vsync, frameRate);
}
-void VSyncPredictor::setRenderRate(Fps renderRate) {
+void VSyncPredictor::setRenderRate(Fps renderRate, bool applyImmediately) {
ATRACE_FORMAT("%s %s", __func__, to_string(renderRate).c_str());
ALOGV("%s %s: RenderRate %s ", __func__, to_string(mId).c_str(), to_string(renderRate).c_str());
std::lock_guard lock(mMutex);
@@ -360,8 +364,9 @@
mRenderRateOpt = renderRate;
const auto renderPeriodDelta =
prevRenderRate ? prevRenderRate->getPeriodNsecs() - renderRate.getPeriodNsecs() : 0;
- if (renderPeriodDelta > renderRate.getPeriodNsecs() &&
- mLastCommittedVsync.ns() - mClock->now() > 2 * renderRate.getPeriodNsecs()) {
+ const bool newRenderRateIsHigher = renderPeriodDelta > renderRate.getPeriodNsecs() &&
+ mLastCommittedVsync.ns() - mClock->now() > 2 * renderRate.getPeriodNsecs();
+ if (applyImmediately || newRenderRateIsHigher) {
mTimelines.clear();
mLastCommittedVsync = TimePoint::fromNs(0);
} else {
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index a1e72d1..21039f1 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -68,7 +68,7 @@
void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final EXCLUDES(mMutex);
- void setRenderRate(Fps) final EXCLUDES(mMutex);
+ void setRenderRate(Fps, bool applyImmediately) final EXCLUDES(mMutex);
void onFrameBegin(TimePoint expectedPresentTime, TimePoint lastConfirmedPresentTime) final
EXCLUDES(mMutex);
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index 1e55a87..8787cdb 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -102,8 +102,10 @@
* when a display is running at 120Hz but the render frame rate is 60Hz.
*
* \param [in] Fps The render rate the tracker should operate at.
+ * \param [in] applyImmediately Whether to apply the new render rate immediately regardless of
+ * already committed vsyncs.
*/
- virtual void setRenderRate(Fps) = 0;
+ virtual void setRenderRate(Fps, bool applyImmediately) = 0;
virtual void onFrameBegin(TimePoint expectedPresentTime,
TimePoint lastConfirmedPresentTime) = 0;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 040bf55..da423b8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1239,8 +1239,8 @@
switch (display->setDesiredMode(std::move(desiredMode))) {
case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch:
// DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler.
- mScheduler->setRenderRate(displayId,
- display->refreshRateSelector().getActiveMode().fps);
+ mScheduler->setRenderRate(displayId, display->refreshRateSelector().getActiveMode().fps,
+ /*applyImmediately*/ true);
// Schedule a new frame to initiate the display mode switch.
scheduleComposite(FrameHint::kNone);
@@ -1261,7 +1261,7 @@
mScheduler->setModeChangePending(true);
break;
case DisplayDevice::DesiredModeAction::InitiateRenderRateSwitch:
- mScheduler->setRenderRate(displayId, mode.fps);
+ mScheduler->setRenderRate(displayId, mode.fps, /*applyImmediately*/ false);
if (displayId == mActiveDisplayId) {
mScheduler->updatePhaseConfiguration(mode.fps);
@@ -1382,7 +1382,7 @@
constexpr bool kAllowToEnable = true;
mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, std::move(activeModePtr).take());
- mScheduler->setRenderRate(displayId, renderFps);
+ mScheduler->setRenderRate(displayId, renderFps, /*applyImmediately*/ true);
if (displayId == mActiveDisplayId) {
mScheduler->updatePhaseConfiguration(renderFps);
@@ -4376,7 +4376,8 @@
// The pacesetter must be registered before EventThread creation below.
mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector());
if (FlagManager::getInstance().vrr_config()) {
- mScheduler->setRenderRate(display->getPhysicalId(), activeMode.fps);
+ mScheduler->setRenderRate(display->getPhysicalId(), activeMode.fps,
+ /*applyImmediately*/ true);
}
const auto configs = mScheduler->getVsyncConfiguration().getCurrentConfigs();
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 2d3796a..d4735c7 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -583,7 +583,7 @@
scheduler.registerDisplay(kMode->getPhysicalDisplayId(), vrrSelectorPtr, vrrTracker);
vrrSelectorPtr->setActiveMode(kMode->getId(), frameRate);
- scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate);
+ scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate, /*applyImmediately*/ false);
vrrTracker->addVsyncTimestamp(0);
// Set 1000 as vsync seq #0
vrrTracker->nextAnticipatedVSyncTimeFrom(700);
@@ -603,7 +603,7 @@
// Change render rate
frameRate = Fps::fromPeriodNsecs(2000);
vrrSelectorPtr->setActiveMode(kMode->getId(), frameRate);
- scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate);
+ scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate, /*applyImmediately*/ false);
EXPECT_EQ(Fps::fromPeriodNsecs(2000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index c22deab..d701a97 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -50,7 +50,7 @@
bool needsMoreSamples() const final { return false; }
bool isVSyncInPhase(nsecs_t, Fps) final { return false; }
void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final {}
- void setRenderRate(Fps) final {}
+ void setRenderRate(Fps, bool) final {}
void onFrameBegin(TimePoint, TimePoint) final {}
void onFrameMissed(TimePoint) final {}
void dump(std::string&) const final {}
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index b58ac66..6559fa5 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -620,7 +620,7 @@
tracker.addVsyncTimestamp(mNow);
}
- tracker.setRenderRate(Fps::fromPeriodNsecs(3 * mPeriod));
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3 * mPeriod), /*applyImmediately*/ false);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + 3 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 100), Eq(mNow + 3 * mPeriod));
@@ -640,7 +640,7 @@
tracker.addVsyncTimestamp(mNow);
}
- tracker.setRenderRate(Fps::fromPeriodNsecs(3.5f * mPeriod));
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3.5f * mPeriod), /*applyImmediately*/ false);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 100), Eq(mNow + mPeriod));
@@ -651,7 +651,7 @@
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 5100), Eq(mNow + 6 * mPeriod));
}
-TEST_F(VSyncPredictorTest, setRenderRateHighIsAppliedImmediatley) {
+TEST_F(VSyncPredictorTest, setRenderRateHighIsAppliedImmediately) {
SET_FLAG_FOR_TEST(flags::vrr_config, true);
const int32_t kGroup = 0;
@@ -669,7 +669,7 @@
VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
kMinimumSamplesForPrediction, kOutlierTolerancePercent};
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700));
EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000, 1000));
@@ -677,14 +677,14 @@
// commit to a vsync in the future
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(2000));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(2000), /*applyImmediately*/ false);
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
EXPECT_EQ(8000, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
EXPECT_EQ(12000, vrrTracker.nextAnticipatedVSyncTimeFrom(10000, 10000));
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(3500));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(3500), /*applyImmediately*/ false);
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
EXPECT_EQ(8000, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
@@ -693,7 +693,7 @@
EXPECT_EQ(15500, vrrTracker.nextAnticipatedVSyncTimeFrom(12000, 12000));
EXPECT_EQ(19000, vrrTracker.nextAnticipatedVSyncTimeFrom(15500, 15500));
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(2500));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(2500), /*applyImmediately*/ false);
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
EXPECT_EQ(8000, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
@@ -703,7 +703,7 @@
EXPECT_EQ(19000, vrrTracker.nextAnticipatedVSyncTimeFrom(15500, 15500));
EXPECT_EQ(21500, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
EXPECT_EQ(7000, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
@@ -714,6 +714,38 @@
EXPECT_EQ(20000, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
}
+TEST_F(VSyncPredictorTest, setRenderRateExplicitAppliedImmediately) {
+ SET_FLAG_FOR_TEST(flags::vrr_config, true);
+
+ const int32_t kGroup = 0;
+ const auto kResolution = ui::Size(1920, 1080);
+ const auto vsyncRate = Fps::fromPeriodNsecs(500);
+ const auto minFrameRate = Fps::fromPeriodNsecs(1000);
+ hal::VrrConfig vrrConfig;
+ vrrConfig.minFrameIntervalNs = minFrameRate.getPeriodNsecs();
+ const ftl::NonNull<DisplayModePtr> kMode =
+ ftl::as_non_null(createDisplayModeBuilder(DisplayModeId(0), vsyncRate, kGroup,
+ kResolution, DEFAULT_DISPLAY_ID)
+ .setVrrConfig(std::move(vrrConfig))
+ .build());
+
+ VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
+ kMinimumSamplesForPrediction, kOutlierTolerancePercent};
+
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
+ vrrTracker.addVsyncTimestamp(0);
+ EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700));
+ EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000, 1000));
+
+ // commit to a vsync in the future
+ EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 2000));
+
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(2000), /*applyImmediately*/ true);
+ EXPECT_EQ(4500, vrrTracker.nextAnticipatedVSyncTimeFrom(4000));
+ EXPECT_EQ(6500, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 4500));
+ EXPECT_EQ(8500, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6500));
+}
+
TEST_F(VSyncPredictorTest, selectsClosestVsyncAfterInactivity) {
SET_FLAG_FOR_TEST(flags::vrr_config, true);
@@ -732,7 +764,7 @@
VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
kMinimumSamplesForPrediction, kOutlierTolerancePercent};
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(5000));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(5000), /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4700));
EXPECT_EQ(10000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
@@ -759,7 +791,7 @@
VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
kMinimumSamplesForPrediction, kOutlierTolerancePercent};
- vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000));
+ vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1234, 1234));
}
@@ -782,7 +814,7 @@
VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
kMinimumSamplesForPrediction, kOutlierTolerancePercent};
- vrrTracker.setRenderRate(minFrameRate);
+ vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700));
EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000));
@@ -819,7 +851,7 @@
VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
kMinimumSamplesForPrediction, kOutlierTolerancePercent};
- vrrTracker.setRenderRate(minFrameRate);
+ vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2700));
@@ -852,7 +884,7 @@
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
- tracker.setRenderRate(Fps::fromPeriodNsecs(2000));
+ tracker.setRenderRate(Fps::fromPeriodNsecs(2000), /*applyImmediately*/ false);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
@@ -861,7 +893,7 @@
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(9001), Eq(11000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(10001), Eq(11000));
- tracker.setRenderRate(Fps::fromPeriodNsecs(3000));
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3000), /*applyImmediately*/ false);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
index 6d10a5c..c311901 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -36,7 +36,7 @@
MOCK_METHOD(bool, needsMoreSamples, (), (const, override));
MOCK_METHOD(bool, isVSyncInPhase, (nsecs_t, Fps), (override));
MOCK_METHOD(void, setDisplayModePtr, (ftl::NonNull<DisplayModePtr>), (override));
- MOCK_METHOD(void, setRenderRate, (Fps), (override));
+ MOCK_METHOD(void, setRenderRate, (Fps, bool), (override));
MOCK_METHOD(void, onFrameBegin, (TimePoint, TimePoint), (override));
MOCK_METHOD(void, onFrameMissed, (TimePoint), (override));
MOCK_METHOD(void, dump, (std::string&), (const, override));