SF: recover from sub-frame jank V2
Respect the VrrConfig::minFrameInterval and adjust the vsync timeline
when a frame miss causes a violation of the minFrameInterval
with the next frame scheduled frame(s).
Bug: 296635687
Test: presubmit
Test: adb root && adb shell service call SurfaceFlinger 1045 f 0.9
Change-Id: Ice2128e291ca4890c7be3b24b9938e6faa383a82
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 29b1d62..6437b4e 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -223,6 +223,19 @@
mPacesetterFrameDurationFractionToSkip = 0.f;
}
+ if (FlagManager::getInstance().vrr_config()) {
+ const auto minFramePeriod = pacesetterOpt->get().schedulePtr->minFramePeriod();
+ const auto presentFenceForPastVsync =
+ pacesetterTargeter.target().presentFenceForPastVsync(minFramePeriod);
+ const auto lastConfirmedPresentTime = presentFenceForPastVsync->getSignalTime();
+ if (lastConfirmedPresentTime != Fence::SIGNAL_TIME_PENDING &&
+ lastConfirmedPresentTime != Fence::SIGNAL_TIME_INVALID) {
+ pacesetterOpt->get()
+ .schedulePtr->getTracker()
+ .onFrameBegin(expectedVsyncTime, TimePoint::fromNs(lastConfirmedPresentTime));
+ }
+ }
+
const auto resultsPerDisplay = compositor.composite(pacesetterId, targeters);
compositor.sample();
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index c13c330..7379a46 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -114,6 +114,10 @@
}
std::lock_guard lock(mMutex);
+ return minFramePeriodLocked();
+}
+
+Period VSyncPredictor::minFramePeriodLocked() const {
const auto idealPeakRefreshPeriod = mDisplayModePtr->getPeakFps().getPeriodNsecs();
const auto numPeriods = static_cast<int>(std::round(static_cast<float>(idealPeakRefreshPeriod) /
static_cast<float>(idealPeriod())));
@@ -297,15 +301,20 @@
const auto renderRatePhase = [&]() REQUIRES(mMutex) -> int {
if (!mRenderRateOpt) return 0;
-
const auto divisor =
RefreshRateSelector::getFrameRateDivisor(Fps::fromPeriodNsecs(idealPeriod()),
*mRenderRateOpt);
if (divisor <= 1) return 0;
- const int mod = mLastVsyncSequence->seq % divisor;
+ int mod = mLastVsyncSequence->seq % divisor;
if (mod == 0) return 0;
+ // This is actually a bug fix, but guarded with vrr_config since we found it with this
+ // config
+ if (FlagManager::getInstance().vrr_config()) {
+ if (mod < 0) mod += divisor;
+ }
+
return divisor - mod;
}();
@@ -406,6 +415,95 @@
clearTimestamps();
}
+void VSyncPredictor::ensureMinFrameDurationIsKept(TimePoint expectedPresentTime,
+ TimePoint lastConfirmedPresentTime) {
+ const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope;
+ const auto threshold = currentPeriod / 2;
+ const auto minFramePeriod = minFramePeriodLocked().ns();
+
+ auto prev = lastConfirmedPresentTime.ns();
+ for (auto& current : mPastExpectedPresentTimes) {
+ if (CC_UNLIKELY(mTraceOn)) {
+ ATRACE_FORMAT_INSTANT("current %.2f past last signaled fence",
+ static_cast<float>(current.ns() - lastConfirmedPresentTime.ns()) /
+ 1e6f);
+ }
+
+ const auto minPeriodViolation = current.ns() - prev + threshold < minFramePeriod;
+ if (minPeriodViolation) {
+ ATRACE_NAME("minPeriodViolation");
+ current = TimePoint::fromNs(prev + minFramePeriod);
+ prev = current.ns();
+ } else {
+ break;
+ }
+ }
+
+ if (!mPastExpectedPresentTimes.empty()) {
+ const auto phase = Duration(mPastExpectedPresentTimes.back() - expectedPresentTime);
+ if (phase > 0ns) {
+ if (mLastVsyncSequence) {
+ mLastVsyncSequence->vsyncTime += phase.ns();
+ }
+ }
+ }
+}
+
+void VSyncPredictor::onFrameBegin(TimePoint expectedPresentTime,
+ TimePoint lastConfirmedPresentTime) {
+ ATRACE_CALL();
+ std::lock_guard lock(mMutex);
+
+ if (!mDisplayModePtr->getVrrConfig()) return;
+
+ if (CC_UNLIKELY(mTraceOn)) {
+ ATRACE_FORMAT_INSTANT("vsync is %.2f past last signaled fence",
+ static_cast<float>(expectedPresentTime.ns() -
+ lastConfirmedPresentTime.ns()) /
+ 1e6f);
+ }
+ mPastExpectedPresentTimes.push_back(expectedPresentTime);
+
+ const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope;
+ const auto threshold = currentPeriod / 2;
+
+ const auto minFramePeriod = minFramePeriodLocked().ns();
+ while (!mPastExpectedPresentTimes.empty()) {
+ const auto front = mPastExpectedPresentTimes.front().ns();
+ const bool frontIsLastConfirmed =
+ std::abs(front - lastConfirmedPresentTime.ns()) < threshold;
+ const bool frontIsBeforeConfirmed =
+ front < lastConfirmedPresentTime.ns() - minFramePeriod + threshold;
+ if (frontIsLastConfirmed || frontIsBeforeConfirmed) {
+ if (CC_UNLIKELY(mTraceOn)) {
+ ATRACE_FORMAT_INSTANT("Discarding old vsync - %.2f before last signaled fence",
+ static_cast<float>(lastConfirmedPresentTime.ns() -
+ mPastExpectedPresentTimes.front().ns()) /
+ 1e6f);
+ }
+ mPastExpectedPresentTimes.pop_front();
+ } else {
+ break;
+ }
+ }
+
+ ensureMinFrameDurationIsKept(expectedPresentTime, lastConfirmedPresentTime);
+}
+
+void VSyncPredictor::onFrameMissed(TimePoint expectedPresentTime) {
+ ATRACE_CALL();
+
+ std::lock_guard lock(mMutex);
+ if (!mDisplayModePtr->getVrrConfig()) return;
+
+ // We don't know when the frame is going to be presented, so we assume it missed one vsync
+ const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope;
+ const auto lastConfirmedPresentTime =
+ TimePoint::fromNs(expectedPresentTime.ns() + currentPeriod);
+
+ ensureMinFrameDurationIsKept(expectedPresentTime, lastConfirmedPresentTime);
+}
+
VSyncPredictor::Model VSyncPredictor::getVSyncPredictionModel() const {
std::lock_guard lock(mMutex);
const auto model = VSyncPredictor::getVSyncPredictionModelLocked();
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 6cb5a67..72a3431 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -16,6 +16,7 @@
#pragma once
+#include <deque>
#include <mutex>
#include <unordered_map>
#include <vector>
@@ -67,6 +68,10 @@
void setRenderRate(Fps) final EXCLUDES(mMutex);
+ void onFrameBegin(TimePoint expectedPresentTime, TimePoint lastConfirmedPresentTime) final
+ EXCLUDES(mMutex);
+ void onFrameMissed(TimePoint expectedPresentTime) final EXCLUDES(mMutex);
+
void dump(std::string& result) const final EXCLUDES(mMutex);
private:
@@ -84,6 +89,8 @@
Model getVSyncPredictionModelLocked() const REQUIRES(mMutex);
nsecs_t nextAnticipatedVSyncTimeFromLocked(nsecs_t timePoint) const REQUIRES(mMutex);
bool isVSyncInPhaseLocked(nsecs_t timePoint, unsigned divisor) const REQUIRES(mMutex);
+ Period minFramePeriodLocked() const REQUIRES(mMutex);
+ void ensureMinFrameDurationIsKept(TimePoint, TimePoint) REQUIRES(mMutex);
struct VsyncSequence {
nsecs_t vsyncTime;
@@ -111,6 +118,8 @@
std::optional<Fps> mRenderRateOpt GUARDED_BY(mMutex);
mutable std::optional<VsyncSequence> mLastVsyncSequence GUARDED_BY(mMutex);
+
+ std::deque<TimePoint> mPastExpectedPresentTimes GUARDED_BY(mMutex);
};
} // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index aa18897..1ed863c 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -107,6 +107,11 @@
*/
virtual void setRenderRate(Fps) = 0;
+ virtual void onFrameBegin(TimePoint expectedPresentTime,
+ TimePoint lastConfirmedPresentTime) = 0;
+
+ virtual void onFrameMissed(TimePoint expectedPresentTime) = 0;
+
virtual void dump(std::string& result) const = 0;
protected:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 89486a3..61501e0 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2476,6 +2476,10 @@
if (pacesetterFrameTarget.isFramePending()) {
if (mBackpressureGpuComposition || pacesetterFrameTarget.didMissHwcFrame()) {
+ if (FlagManager::getInstance().vrr_config()) {
+ mScheduler->getVsyncSchedule()->getTracker().onFrameMissed(
+ pacesetterFrameTarget.expectedPresentTime());
+ }
scheduleCommit(FrameHint::kNone);
return false;
}
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h
index 2fca4cb..fa307e9 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h
@@ -110,6 +110,10 @@
void setRenderRate(Fps) override {}
+ void onFrameBegin(TimePoint, TimePoint) override {}
+
+ void onFrameMissed(TimePoint) override {}
+
void dump(std::string& /* result */) const override {}
protected:
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index ca2f92a..6a56353 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -34,12 +34,36 @@
return std::chrono::duration_cast<std::chrono::nanoseconds>(tp).count();
}
-class FixedRateIdealStubTracker : public VSyncTracker {
+class StubTracker : public VSyncTracker {
public:
- FixedRateIdealStubTracker() : mPeriod{toNs(3ms)} {}
+ StubTracker(nsecs_t period) : mPeriod(period) {}
bool addVsyncTimestamp(nsecs_t) final { return true; }
+ nsecs_t currentPeriod() const final {
+ std::lock_guard lock(mMutex);
+ return mPeriod;
+ }
+
+ Period minFramePeriod() const final { return Period::fromNs(currentPeriod()); }
+ void resetModel() final {}
+ bool needsMoreSamples() const final { return false; }
+ bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
+ void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final {}
+ void setRenderRate(Fps) final {}
+ void onFrameBegin(TimePoint, TimePoint) final {}
+ void onFrameMissed(TimePoint) final {}
+ void dump(std::string&) const final {}
+
+protected:
+ std::mutex mutable mMutex;
+ nsecs_t mPeriod;
+};
+
+class FixedRateIdealStubTracker : public StubTracker {
+public:
+ FixedRateIdealStubTracker() : StubTracker{toNs(3ms)} {}
+
nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final {
auto const floor = timePoint % mPeriod;
if (floor == 0) {
@@ -47,26 +71,11 @@
}
return timePoint - floor + mPeriod;
}
-
- nsecs_t currentPeriod() const final { return mPeriod; }
- Period minFramePeriod() const final { return Period::fromNs(currentPeriod()); }
-
- void resetModel() final {}
- bool needsMoreSamples() const final { return false; }
- bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
- void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final {}
- void setRenderRate(Fps) final {}
- void dump(std::string&) const final {}
-
-private:
- nsecs_t const mPeriod;
};
-class VRRStubTracker : public VSyncTracker {
+class VRRStubTracker : public StubTracker {
public:
- VRRStubTracker(nsecs_t period) : mPeriod{period} {}
-
- bool addVsyncTimestamp(nsecs_t) final { return true; }
+ VRRStubTracker(nsecs_t period) : StubTracker(period) {}
nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t time_point) const final {
std::lock_guard lock(mMutex);
@@ -84,23 +93,7 @@
mBase = last_known;
}
- nsecs_t currentPeriod() const final {
- std::lock_guard lock(mMutex);
- return mPeriod;
- }
-
- Period minFramePeriod() const final { return Period::fromNs(currentPeriod()); }
-
- void resetModel() final {}
- bool needsMoreSamples() const final { return false; }
- bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
- void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final {}
- void setRenderRate(Fps) final {}
- void dump(std::string&) const final {}
-
private:
- std::mutex mutable mMutex;
- nsecs_t mPeriod;
nsecs_t mBase = 0;
};
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 16aa249..7a498c9 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -44,6 +44,7 @@
::aidl::android::hardware::graphics::composer3::VrrConfig::NotifyExpectedPresentConfig;
using android::mock::createDisplayMode;
+using android::mock::createDisplayModeBuilder;
using android::mock::createVrrDisplayMode;
namespace android::scheduler {
@@ -698,6 +699,38 @@
}
}
+TEST_F(VSyncPredictorTest, adjustsVrrTimeline) {
+ SET_FLAG_FOR_TEST(flags::vrr_config, true);
+
+ const int32_t kGroup = 0;
+ const auto kResolution = ui::Size(1920, 1080);
+ const auto refreshRate = 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), refreshRate, kGroup,
+ kResolution, DEFAULT_DISPLAY_ID)
+ .setVrrConfig(std::move(vrrConfig))
+ .build());
+
+ VSyncPredictor vrrTracker{kMode, kHistorySize, kMinimumSamplesForPrediction,
+ kOutlierTolerancePercent, mVsyncTrackerCallback};
+
+ vrrTracker.setRenderRate(minFrameRate);
+ vrrTracker.addVsyncTimestamp(0);
+ EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700));
+ EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1300));
+
+ vrrTracker.onFrameBegin(TimePoint::fromNs(2000), TimePoint::fromNs(1500));
+ EXPECT_EQ(1500, vrrTracker.nextAnticipatedVSyncTimeFrom(1300));
+ EXPECT_EQ(2500, vrrTracker.nextAnticipatedVSyncTimeFrom(2300));
+
+ vrrTracker.onFrameMissed(TimePoint::fromNs(2500));
+ EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2300));
+ EXPECT_EQ(4000, vrrTracker.nextAnticipatedVSyncTimeFrom(3300));
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
index 0858c28..e588bb9 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -36,6 +36,8 @@
MOCK_METHOD(bool, isVSyncInPhase, (nsecs_t, Fps), (const, override));
MOCK_METHOD(void, setDisplayModePtr, (ftl::NonNull<DisplayModePtr>), (override));
MOCK_METHOD(void, setRenderRate, (Fps), (override));
+ MOCK_METHOD(void, onFrameBegin, (TimePoint, TimePoint), (override));
+ MOCK_METHOD(void, onFrameMissed, (TimePoint), (override));
MOCK_METHOD(void, dump, (std::string&), (const, override));
};