commit 09bf763bbda713b6ee69fb625c596e279d443309
author Dan Stoza <stoza@google.com> Wed Jun 10 14:28:50 2020 -0700
committer Dan Stoza <stoza@google.com> Wed Jun 10 15:20:50 2020 -0700
tree d36f2af2551299be7f5fefb618c76576478cceba
parent e0d9770df147aa47680e8a5373c35d4c8b69e764

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

diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index 16d102c..c743de0 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -278,7 +278,9 @@
         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 = {};
     }

services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index ccbd17f..a972562 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -672,15 +672,30 @@
                                     kPendingLimit, true /* supportKernelIdleTimer */);
 
     bool periodFlushed = true;
-    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(2);
+    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(_)).Times(5);
     idleReactor.setIgnorePresentFences(true);
 
-    nsecs_t const newPeriod = 5000;
-    idleReactor.setPeriod(newPeriod);
-
+    // 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);
-    EXPECT_FALSE(idleReactor.addResyncSample(newPeriod, 0, &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);
+    // Incorrect timestamp delta and period
+    EXPECT_TRUE(idleReactor.addResyncSample(20000, 10000, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+    // Incorrect timestamp delta but correct period
+    EXPECT_FALSE(idleReactor.addResyncSample(20000, 5000, &periodFlushed));
     EXPECT_TRUE(periodFlushed);
 
     EXPECT_TRUE(idleReactor.addPresentFence(generateSignalledFenceWithTime(0)));