Merge "SF: recover from sub-frame jank V2" into main

9 files changed, 194 insertions, 33 deletions

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 29b1d62956..6437b4eb94 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -223,6 +223,19 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId,
         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 c13c330d86..7379a4605d 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -114,6 +114,10 @@ Period VSyncPredictor::minFramePeriod() const {
     }
 
     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 @@ nsecs_t VSyncPredictor::nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const {
 
     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 @@ void VSyncPredictor::setDisplayModePtr(ftl::NonNull<DisplayModePtr> modePtr) {
     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 6cb5a678c7..72a343112a 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 @@ public:
 
     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 @@ private:
     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 @@ private:
     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 aa18897f32..1ed863cf7d 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -107,6 +107,11 @@ public:
      */
     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 cd4465c831..6e6229ac4c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2476,6 +2476,10 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId,
 
     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 2fca4cb41a..fa307e9bb4 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h
@@ -110,6 +110,10 @@ public:
 
     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 ca2f92a375..6a5635305a 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -34,39 +34,48 @@ constexpr nsecs_t toNs(std::chrono::duration<Rep, Per> const& tp) {
     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 nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final {
-        auto const floor = timePoint % mPeriod;
-        if (floor == 0) {
-            return timePoint;
-        }
-        return timePoint - floor + mPeriod;
+    nsecs_t currentPeriod() const final {
+        std::lock_guard lock(mMutex);
+        return 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 onFrameBegin(TimePoint, TimePoint) final {}
+    void onFrameMissed(TimePoint) final {}
     void dump(std::string&) const final {}
 
-private:
-    nsecs_t const mPeriod;
+protected:
+    std::mutex mutable mMutex;
+    nsecs_t mPeriod;
 };
 
-class VRRStubTracker : public VSyncTracker {
+class FixedRateIdealStubTracker : public StubTracker {
 public:
-    VRRStubTracker(nsecs_t period) : mPeriod{period} {}
+    FixedRateIdealStubTracker() : StubTracker{toNs(3ms)} {}
 
-    bool addVsyncTimestamp(nsecs_t) final { return true; }
+    nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const final {
+        auto const floor = timePoint % mPeriod;
+        if (floor == 0) {
+            return timePoint;
+        }
+        return timePoint - floor + mPeriod;
+    }
+};
+
+class VRRStubTracker : public StubTracker {
+public:
+    VRRStubTracker(nsecs_t period) : StubTracker(period) {}
 
     nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t time_point) const final {
         std::lock_guard lock(mMutex);
@@ -84,23 +93,7 @@ public:
         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 16aa249317..7a498c9d29 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -44,6 +44,7 @@ using NotifyExpectedPresentConfig =
         ::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, vsyncTrackerCallback) {
     }
 }
 
+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 0858c28c5e..e588bb9a3f 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -36,6 +36,8 @@ public:
     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));
 };