SF: don't use minFramePeriod if it is same as vsync

Bug: 328140524
Test: presubmit
Change-Id: Ie1d8fd1af508e567fb1340727e37d89c99ec7c6b
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 58457d8..3fc9a07 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -45,6 +45,15 @@
 
 static auto constexpr kMaxPercent = 100u;
 
+namespace {
+int numVsyncsPerFrame(const ftl::NonNull<DisplayModePtr>& displayModePtr) {
+    const auto idealPeakRefreshPeriod = displayModePtr->getPeakFps().getPeriodNsecs();
+    const auto idealRefreshPeriod = displayModePtr->getVsyncRate().getPeriodNsecs();
+    return static_cast<int>(std::round(static_cast<float>(idealPeakRefreshPeriod) /
+                                       static_cast<float>(idealRefreshPeriod)));
+}
+} // namespace
+
 VSyncPredictor::~VSyncPredictor() = default;
 
 VSyncPredictor::VSyncPredictor(std::unique_ptr<Clock> clock, ftl::NonNull<DisplayModePtr> modePtr,
@@ -56,7 +65,8 @@
         kHistorySize(historySize),
         kMinimumSamplesForPrediction(minimumSamplesForPrediction),
         kOutlierTolerancePercent(std::min(outlierTolerancePercent, kMaxPercent)),
-        mDisplayModePtr(modePtr) {
+        mDisplayModePtr(modePtr),
+        mNumVsyncsForFrame(numVsyncsPerFrame(mDisplayModePtr)) {
     resetModel();
 }
 
@@ -120,11 +130,8 @@
 }
 
 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())));
     const auto slope = mRateMap.find(idealPeriod())->second.slope;
-    return Period::fromNs(slope * numPeriods);
+    return Period::fromNs(slope * mNumVsyncsForFrame);
 }
 
 bool VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) {
@@ -302,9 +309,15 @@
 
     const auto model = getVSyncPredictionModelLocked();
     const auto threshold = model.slope / 2;
+    std::optional<Period> minFramePeriodOpt;
+
+    if (mNumVsyncsForFrame > 1) {
+        minFramePeriodOpt = minFramePeriodLocked();
+    }
+
     std::optional<TimePoint> vsyncOpt;
     for (auto& timeline : mTimelines) {
-        vsyncOpt = timeline.nextAnticipatedVSyncTimeFrom(model, minFramePeriodLocked(),
+        vsyncOpt = timeline.nextAnticipatedVSyncTimeFrom(model, minFramePeriodOpt,
                                                          snapToVsync(timePoint), mMissedVsync,
                                                          lastVsyncOpt ? snapToVsync(*lastVsyncOpt -
                                                                                     threshold)
@@ -390,6 +403,7 @@
     std::lock_guard lock(mMutex);
 
     mDisplayModePtr = modePtr;
+    mNumVsyncsForFrame = numVsyncsPerFrame(mDisplayModePtr);
     traceInt64("VSP-setPeriod", modePtr->getVsyncRate().getPeriodNsecs());
 
     static constexpr size_t kSizeLimit = 30;
@@ -407,6 +421,11 @@
 Duration VSyncPredictor::ensureMinFrameDurationIsKept(TimePoint expectedPresentTime,
                                                       TimePoint lastConfirmedPresentTime) {
     ATRACE_CALL();
+
+    if (mNumVsyncsForFrame <= 1) {
+        return 0ns;
+    }
+
     const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope;
     const auto threshold = currentPeriod / 2;
     const auto minFramePeriod = minFramePeriodLocked().ns();
@@ -596,8 +615,8 @@
 }
 
 std::optional<TimePoint> VSyncPredictor::VsyncTimeline::nextAnticipatedVSyncTimeFrom(
-        Model model, Period minFramePeriod, nsecs_t vsync, MissedVsync missedVsync,
-        std::optional<nsecs_t> lastVsyncOpt) {
+        Model model, std::optional<Period> minFramePeriodOpt, nsecs_t vsync,
+        MissedVsync missedVsync, std::optional<nsecs_t> lastVsyncOpt) {
     ATRACE_FORMAT("renderRate %s", mRenderRateOpt ? to_string(*mRenderRateOpt).c_str() : "NA");
 
     nsecs_t vsyncTime = snapToVsyncAlignedWithRenderRate(model, vsync);
@@ -611,7 +630,7 @@
         // fixup. There is no need to to shift the vsync timeline again.
         vsyncTime += missedVsync.fixup.ns();
         ATRACE_FORMAT_INSTANT("lastFrameMissed");
-    } else {
+    } else if (minFramePeriodOpt) {
         if (FlagManager::getInstance().vrr_config() && lastVsyncOpt) {
             // lastVsyncOpt is based on the old timeline before we shifted it. we should correct it
             // first before trying to use it.
@@ -619,9 +638,10 @@
                 lastVsyncOpt = snapToVsyncAlignedWithRenderRate(model, *lastVsyncOpt);
             }
             const auto vsyncDiff = vsyncTime - *lastVsyncOpt;
-            if (vsyncDiff <= minFramePeriod.ns() - threshold) {
-                vsyncFixupTime = *lastVsyncOpt + minFramePeriod.ns() - vsyncTime;
-                ATRACE_FORMAT_INSTANT("minFramePeriod violation. next in %.2f which is %.2f from "
+            if (vsyncDiff <= minFramePeriodOpt->ns() - threshold) {
+                vsyncFixupTime = *lastVsyncOpt + minFramePeriodOpt->ns() - vsyncTime;
+                ATRACE_FORMAT_INSTANT("minFramePeriod violation. next in %.2f which is %.2f "
+                                      "from "
                                       "prev. "
                                       "adjust by %.2f",
                                       static_cast<float>(vsyncTime - TimePoint::now().ns()) / 1e6f,
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 21039f1..c840cbd 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -91,8 +91,8 @@
     public:
         VsyncTimeline(TimePoint knownVsync, Period idealPeriod, std::optional<Fps> renderRateOpt);
         std::optional<TimePoint> nextAnticipatedVSyncTimeFrom(
-                Model model, Period minFramePeriod, nsecs_t vsyncTime, MissedVsync lastMissedVsync,
-                std::optional<nsecs_t> lastVsyncOpt = {});
+                Model model, std::optional<Period> minFramePeriodOpt, nsecs_t vsyncTime,
+                MissedVsync lastMissedVsync, std::optional<nsecs_t> lastVsyncOpt = {});
         void freeze(TimePoint lastVsync);
         std::optional<TimePoint> validUntil() const { return mValidUntil; }
         bool isVSyncInPhase(Model, nsecs_t vsync, Fps frameRate);
@@ -144,6 +144,7 @@
     std::vector<nsecs_t> mTimestamps GUARDED_BY(mMutex);
 
     ftl::NonNull<DisplayModePtr> mDisplayModePtr GUARDED_BY(mMutex);
+    int mNumVsyncsForFrame GUARDED_BY(mMutex);
 
     std::deque<TimePoint> mPastExpectedPresentTimes GUARDED_BY(mMutex);
 
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 6559fa5..daea28e 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -714,6 +714,33 @@
     EXPECT_EQ(20000, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
 }
 
+TEST_F(VSyncPredictorTest, minFramePeriodDoesntApplyWhenSameWithRefreshRate) {
+    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(1000);
+    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));
+
+    // Assume that the last vsync is wrong due to a vsync drift. It shouldn't matter.
+    EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000, 1700));
+}
+
 TEST_F(VSyncPredictorTest, setRenderRateExplicitAppliedImmediately) {
     SET_FLAG_FOR_TEST(flags::vrr_config, true);
 
@@ -853,21 +880,27 @@
 
     vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false);
     vrrTracker.addVsyncTimestamp(0);
+
+    // App runs ahead
     EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2700));
-
     EXPECT_EQ(4000, vrrTracker.nextAnticipatedVSyncTimeFrom(3000, 3000));
-
     EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
+
+    // SF starts to catch up
     EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2700));
     vrrTracker.onFrameBegin(TimePoint::fromNs(3000), TimePoint::fromNs(0));
 
+    // SF misses last frame (3000) and observes that when committing (4000)
     EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
     EXPECT_EQ(4000, vrrTracker.nextAnticipatedVSyncTimeFrom(3700));
     vrrTracker.onFrameMissed(TimePoint::fromNs(4000));
 
+    // SF wakes up again instead of the (4000) missed frame
     EXPECT_EQ(4500, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
     vrrTracker.onFrameBegin(TimePoint::fromNs(4500), TimePoint::fromNs(4500));
 
+    // Timeline shifted. The app needs to get the next frame at (7500) as its last frame (6500) will
+    // be presented at (7500)
     EXPECT_EQ(7500, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
     EXPECT_EQ(5500, vrrTracker.nextAnticipatedVSyncTimeFrom(4500, 4500));
     vrrTracker.onFrameBegin(TimePoint::fromNs(5500), TimePoint::fromNs(4500));