commit 55fa7272747ba308525e2837489637ec09eb9cdf
author Ady Abraham <adyabr@google.com> Wed Sep 30 19:19:27 2020 -0700
committer Ady Abraham <adyabr@google.com> Mon Oct 05 11:20:47 2020 -0700
tree 830d6630139907c5ad37a44bd92ca36c2a9f56b2
parent 97651d23e882aa8cb24d1a6b45808a549474bcb6

SurfaceFlinger: decouple EventThread from SF wakeup

Today we have two instances of EventThread:
1. 'app' - used to wake up Choreographer clients
2. 'sf' - used to wake up SF mian thead *and*
          Choreographer clients that uses sf instance

Now this creates an ambiguity when trying to reason about the expected
vsync time and deadline of 'sf' EventThread:
 - SF wakes up sfWorkDuration before a vsync and targets that vsync
 - Choreographer users wakes up with SF main thread but targets the
   vsync that happens after the next SF wakeup.

To resolve this ambiguity we are decoupling SF wakeup from 'sf'
EventThread. This means that Choreographer clients that uses 'sf'
instance will keep using the EventThread but SF will be waking up
directly by a callback with VSyncDispatch. This allows us to correct
the expected vsync and deadline for both.

Test: Interacting with the device and observe systraces
Test: new unit test added to SF suite
Bug: 166302754

Change-Id: I76d154029b4bc1902198074c33d38ff030c4601b

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 152d872..1b15164 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1414,8 +1414,8 @@
         Mutex::Autolock lock(mStateLock);
 
         if (const auto handle = mScheduler->enableVSyncInjection(enable)) {
-            mEventQueue->setEventConnection(
-                    mScheduler->getEventConnection(enable ? handle : mSfConnectionHandle));
+            mEventQueue->setEventConnection(enable ? mScheduler->getEventConnection(handle)
+                                                   : nullptr);
         }
     }).wait();
 
@@ -2867,19 +2867,23 @@
     // start the EventThread
     mScheduler = getFactory().createScheduler(*mRefreshRateConfigs, *this);
     const auto configs = mVsyncConfiguration->getCurrentConfigs();
+    const nsecs_t vsyncPeriod =
+            mRefreshRateConfigs->getRefreshRateFromConfigId(currentConfig).getVsyncPeriod();
     mAppConnectionHandle =
             mScheduler->createConnection("app", mFrameTimeline->getTokenManager(),
                                          /*workDuration=*/configs.late.appWorkDuration,
                                          /*readyDuration=*/configs.late.sfWorkDuration,
                                          impl::EventThread::InterceptVSyncsCallback());
     mSfConnectionHandle =
-            mScheduler->createConnection("sf", mFrameTimeline->getTokenManager(),
-                                         /*workDuration=*/configs.late.sfWorkDuration,
-                                         /*readyDuration=*/0ns, [this](nsecs_t timestamp) {
+            mScheduler->createConnection("appSf", mFrameTimeline->getTokenManager(),
+                                         /*workDuration=*/std::chrono::nanoseconds(vsyncPeriod),
+                                         /*readyDuration=*/configs.late.sfWorkDuration,
+                                         [this](nsecs_t timestamp) {
                                              mInterceptor->saveVSyncEvent(timestamp);
                                          });
 
-    mEventQueue->setEventConnection(mScheduler->getEventConnection(mSfConnectionHandle));
+    mEventQueue->initVsync(mScheduler->getVsyncDispatch(), *mFrameTimeline->getTokenManager(),
+                           configs.late.sfWorkDuration);
 
     mRegionSamplingThread =
             new RegionSamplingThread(*this, *mScheduler,
@@ -2891,8 +2895,6 @@
     // This is a bit hacky, but this avoids a back-pointer into the main SF
     // classes from EventThread, and there should be no run-time binder cost
     // anyway since there are no connected apps at this point.
-    const nsecs_t vsyncPeriod =
-            mRefreshRateConfigs->getRefreshRateFromConfigId(currentConfig).getVsyncPeriod();
     mScheduler->onPrimaryDisplayConfigChanged(mAppConnectionHandle, primaryDisplayId, currentConfig,
                                               vsyncPeriod);
     static auto ignorePresentFences =
@@ -2904,16 +2906,19 @@
 
 void SurfaceFlinger::updatePhaseConfiguration(const RefreshRate& refreshRate) {
     mVsyncConfiguration->setRefreshRateFps(refreshRate.getFps());
-    setVsyncConfig(mVsyncModulator->setVsyncConfigSet(mVsyncConfiguration->getCurrentConfigs()));
+    setVsyncConfig(mVsyncModulator->setVsyncConfigSet(mVsyncConfiguration->getCurrentConfigs()),
+                   refreshRate.getVsyncPeriod());
 }
 
-void SurfaceFlinger::setVsyncConfig(const VsyncModulator::VsyncConfig& config) {
+void SurfaceFlinger::setVsyncConfig(const VsyncModulator::VsyncConfig& config,
+                                    nsecs_t vsyncPeriod) {
     mScheduler->setDuration(mAppConnectionHandle,
                             /*workDuration=*/config.appWorkDuration,
                             /*readyDuration=*/config.sfWorkDuration);
     mScheduler->setDuration(mSfConnectionHandle,
-                            /*workDuration=*/config.sfWorkDuration,
-                            /*readyDuration=*/0ns);
+                            /*workDuration=*/std::chrono::nanoseconds(vsyncPeriod),
+                            /*readyDuration=*/config.sfWorkDuration);
+    mEventQueue->setDuration(config.sfWorkDuration);
 }
 
 void SurfaceFlinger::commitTransaction() {