From 09bf763bbda713b6ee69fb625c596e279d443309 Mon Sep 17 00:00:00 2001
From: Dan Stoza <stoza@google.com>
Date: Wed, 10 Jun 2020 14:28:50 -0700
Subject: VSR: Only ignore Composer when rate isn't changing

This refines the logic added in
I79c66faeaed262acd9c5925fe2202a9fb3f10b7b, which started ignoring
Composer-provided refresh rates on vsync callbacks when the kernel idle
timer is enabled.

This behavior is still correct when SF doesn't think that that refresh
rate has changed, but when we initiate a change, such as when the only
active layer is infrequently updating, then it's okay to rely on the
Composer-provided rate.

Bug: 158141901
Test: atest libsurfaceflinger_unittest, systrace
Change-Id: I16c87005f5cd1ca1810014211d73b4662e7c8c86
---
 services/surfaceflinger/Scheduler/VSyncReactor.cpp |  4 +++-
 .../tests/unittests/VSyncReactorTest.cpp           | 23 ++++++++++++++++++----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index 16d102cac1..c743de07ae 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -278,7 +278,9 @@ bool VSyncReactor::periodConfirmed(nsecs_t vsync_timestamp, std::optional<nsecs_
         return false;
     }
 
-    if (mSupportKernelIdleTimer) {
+    const bool periodIsChanging =
+            mPeriodTransitioningTo && (*mPeriodTransitioningTo != getPeriod());
+    if (mSupportKernelIdleTimer && !periodIsChanging) {
         // Clear out the Composer-provided period and use the allowance logic below
         HwcVsyncPeriod = {};
     }
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index ccbd17fa36..a97256203d 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -672,15 +672,30 @@ TEST_F(VSyncReactorTest, periodIsMeasuredIfIgnoringComposer) {
                                     kPendingLimit, true /* supportKernelIdleTimer */);
 
     bool periodFlushed = true;
-    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
+    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(5);
     idleReactor.setIgnorePresentFences(true);
 
+    // First, set the same period, which should only be confirmed when we receive two
+    // matching callbacks
+    idleReactor.setPeriod(10000);
+    EXPECT_TRUE(idleReactor.addResyncSample(0, 0, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+    // Correct period but incorrect timestamp delta
+    EXPECT_TRUE(idleReactor.addResyncSample(0, 10000, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+    // Correct period and correct timestamp delta
+    EXPECT_FALSE(idleReactor.addResyncSample(10000, 10000, &periodFlushed));
+    EXPECT_TRUE(periodFlushed);
+
+    // Then, set a new period, which should be confirmed as soon as we receive a callback
+    // reporting the new period
     nsecs_t const newPeriod = 5000;
     idleReactor.setPeriod(newPeriod);
-
-    EXPECT_TRUE(idleReactor.addResyncSample(0, 0, &periodFlushed));
+    // Incorrect timestamp delta and period
+    EXPECT_TRUE(idleReactor.addResyncSample(20000, 10000, &periodFlushed));
     EXPECT_FALSE(periodFlushed);
-    EXPECT_FALSE(idleReactor.addResyncSample(newPeriod, 0, &periodFlushed));
+    // Incorrect timestamp delta but correct period
+    EXPECT_FALSE(idleReactor.addResyncSample(20000, 5000, &periodFlushed));
     EXPECT_TRUE(periodFlushed);
 
     EXPECT_TRUE(idleReactor.addPresentFence(generateSignalledFenceWithTime(0)));
-- 
cgit v1.2.3-59-g8ed1b