SF: Refactor event dispatch

This CL refactors the EventThread loop to simplify control flow, remove
redundant state, avoid gratuitous allocation, and improve readability.

It also adds a dumpsys flag for VSYNC information.

Bug: 74619554
Test: libsurfaceflinger_unittest
Test: dumpsys SurfaceFlinger --vsync
Change-Id: I2d875349031ae2869fb370a8a090c71b0e181482

4 files changed, 226 insertions, 202 deletions

diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 7b25adb73f..01aedf45d7 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -19,8 +19,11 @@
 #include <pthread.h>
 #include <sched.h>
 #include <sys/types.h>
+
 #include <chrono>
 #include <cstdint>
+#include <optional>
+#include <type_traits>
 
 #include <android-base/stringprintf.h>
 
@@ -35,18 +38,52 @@
 #include "EventThread.h"
 
 using namespace std::chrono_literals;
-using android::base::StringAppendF;
-
-// ---------------------------------------------------------------------------
 
 namespace android {
 
-// ---------------------------------------------------------------------------
+using base::StringAppendF;
+using base::StringPrintf;
+
+namespace {
+
+auto vsyncPeriod(VSyncRequest request) {
+    return static_cast<std::underlying_type_t<VSyncRequest>>(request);
+}
+
+std::string toString(VSyncRequest request) {
+    switch (request) {
+        case VSyncRequest::None:
+            return "VSyncRequest::None";
+        case VSyncRequest::Single:
+            return "VSyncRequest::Single";
+        default:
+            return StringPrintf("VSyncRequest::Periodic{period=%d}", vsyncPeriod(request));
+    }
+}
+
+std::string toString(const EventThreadConnection& connection) {
+    return StringPrintf("Connection{%p, %s}", &connection,
+                        toString(connection.vsyncRequest).c_str());
+}
+
+std::string toString(const DisplayEventReceiver::Event& event) {
+    switch (event.header.type) {
+        case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+            return StringPrintf("Hotplug{displayId=%u, %s}", event.header.id,
+                                event.hotplug.connected ? "connected" : "disconnected");
+        case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+            return StringPrintf("VSync{displayId=%u, count=%u}", event.header.id,
+                                event.vsync.count);
+        default:
+            return "Event{}";
+    }
+}
+
+} // namespace
 
 EventThreadConnection::EventThreadConnection(EventThread* eventThread,
                                              ResyncCallback resyncCallback)
       : resyncCallback(std::move(resyncCallback)),
-        count(-1),
         mEventThread(eventThread),
         mChannel(gui::BitTube::DefaultSize) {}
 
@@ -65,8 +102,8 @@ status_t EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) {
     return NO_ERROR;
 }
 
-status_t EventThreadConnection::setVsyncRate(uint32_t count) {
-    mEventThread->setVsyncRate(count, this);
+status_t EventThreadConnection::setVsyncRate(uint32_t rate) {
+    mEventThread->setVsyncRate(rate, this);
     return NO_ERROR;
 }
 
@@ -107,7 +144,8 @@ EventThread::EventThread(VSyncSource* src, std::unique_ptr<VSyncSource> uniqueSr
                          InterceptVSyncsCallback interceptVSyncsCallback, const char* threadName)
       : mVSyncSource(src),
         mVSyncSourceUnique(std::move(uniqueSrc)),
-        mInterceptVSyncsCallback(interceptVSyncsCallback) {
+        mInterceptVSyncsCallback(interceptVSyncsCallback),
+        mThreadName(threadName) {
     if (src == nullptr) {
         mVSyncSource = mVSyncSourceUnique.get();
     }
@@ -118,7 +156,10 @@ EventThread::EventThread(VSyncSource* src, std::unique_ptr<VSyncSource> uniqueSr
         event.vsync.count = 0;
     }
 
-    mThread = std::thread(&EventThread::threadMain, this);
+    mThread = std::thread([this]() NO_THREAD_SAFETY_ANALYSIS {
+        std::unique_lock<std::mutex> lock(mMutex);
+        threadMain(lock);
+    });
 
     pthread_setname_np(mThread.native_handle(), threadName);
 
@@ -178,14 +219,17 @@ void EventThread::removeDisplayEventConnectionLocked(const wp<EventThreadConnect
     }
 }
 
-void EventThread::setVsyncRate(uint32_t count, const sp<EventThreadConnection>& connection) {
-    if (int32_t(count) >= 0) { // server must protect against bad params
-        std::lock_guard<std::mutex> lock(mMutex);
-        const int32_t new_count = (count == 0) ? -1 : count;
-        if (connection->count != new_count) {
-            connection->count = new_count;
-            mCondition.notify_all();
-        }
+void EventThread::setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) {
+    if (static_cast<std::underlying_type_t<VSyncRequest>>(rate) < 0) {
+        return;
+    }
+
+    std::lock_guard<std::mutex> lock(mMutex);
+
+    const auto request = rate == 0 ? VSyncRequest::None : static_cast<VSyncRequest>(rate);
+    if (connection->vsyncRequest != request) {
+        connection->vsyncRequest = request;
+        mCondition.notify_all();
     }
 }
 
@@ -201,8 +245,8 @@ void EventThread::requestNextVsync(const sp<EventThreadConnection>& connection,
 
     std::lock_guard<std::mutex> lock(mMutex);
 
-    if (connection->count < 0) {
-        connection->count = 0;
+    if (connection->vsyncRequest == VSyncRequest::None) {
+        connection->vsyncRequest = VSyncRequest::Single;
         mCondition.notify_all();
     }
 }
@@ -243,122 +287,66 @@ void EventThread::onHotplugReceived(DisplayType displayType, bool connected) {
     event.header.timestamp = systemTime();
     event.hotplug.connected = connected;
 
-    mPendingEvents.push(event);
+    mPendingEvents.push_back(event);
     mCondition.notify_all();
 }
 
-void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS {
-    std::unique_lock<std::mutex> lock(mMutex);
-    while (mKeepRunning) {
-        DisplayEventReceiver::Event event;
-        std::vector<sp<EventThreadConnection>> signalConnections;
-        signalConnections = waitForEventLocked(&lock, &event);
-
-        // dispatch events to listeners...
-        for (const sp<EventThreadConnection>& conn : signalConnections) {
-            // now see if we still need to report this event
-            status_t err = conn->postEvent(event);
-            if (err == -EAGAIN || err == -EWOULDBLOCK) {
-                // The destination doesn't accept events anymore, it's probably
-                // full. For now, we just drop the events on the floor.
-                // FIXME: Note that some events cannot be dropped and would have
-                // to be re-sent later.
-                // Right-now we don't have the ability to do this.
-                ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type,
-                      conn.get());
-            } else if (err < 0) {
-                // handle any other error on the pipe as fatal. the only
-                // reasonable thing to do is to clean-up this connection.
-                // The most common error we'll get here is -EPIPE.
-                removeDisplayEventConnectionLocked(conn);
-            }
-        }
-    }
-}
+void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {
+    DisplayEventConsumers consumers;
 
-// This will return when (1) a vsync event has been received, and (2) there was
-// at least one connection interested in receiving it when we started waiting.
-std::vector<sp<EventThreadConnection>> EventThread::waitForEventLocked(
-        std::unique_lock<std::mutex>* lock, DisplayEventReceiver::Event* outEvent) {
-    std::vector<sp<EventThreadConnection>> signalConnections;
-
-    while (signalConnections.empty() && mKeepRunning) {
-        bool eventPending = false;
-        bool waitForVSync = false;
-
-        size_t vsyncCount = 0;
-        nsecs_t timestamp = 0;
-        for (auto& event : mVSyncEvent) {
-            timestamp = event.header.timestamp;
-            if (timestamp) {
-                // we have a vsync event to dispatch
-                if (mInterceptVSyncsCallback) {
-                    mInterceptVSyncsCallback(timestamp);
+    while (mKeepRunning) {
+        std::optional<DisplayEventReceiver::Event> event;
+
+        // Determine next event to dispatch.
+        if (!mPendingEvents.empty()) {
+            event = mPendingEvents.front();
+            mPendingEvents.pop_front();
+        } else {
+            for (auto& vsyncEvent : mVSyncEvent) {
+                if (vsyncEvent.header.timestamp) {
+                    event = vsyncEvent;
+                    vsyncEvent.header.timestamp = 0;
+
+                    if (mInterceptVSyncsCallback) {
+                        mInterceptVSyncsCallback(event->header.timestamp);
+                    }
+                    break;
                 }
-                *outEvent = event;
-                event.header.timestamp = 0;
-                vsyncCount = event.vsync.count;
-                break;
             }
         }
 
-        if (!timestamp) {
-            // no vsync event, see if there are some other event
-            eventPending = !mPendingEvents.empty();
-            if (eventPending) {
-                // we have some other event to dispatch
-                *outEvent = mPendingEvents.front();
-                mPendingEvents.pop();
-            }
-        }
+        bool vsyncRequested = false;
 
-        // find out connections waiting for events
+        // Find connections that should consume this event.
         auto it = mDisplayEventConnections.begin();
         while (it != mDisplayEventConnections.end()) {
-            sp<EventThreadConnection> connection(it->promote());
-            if (connection != nullptr) {
-                bool added = false;
-                if (connection->count >= 0) {
-                    // we need vsync events because at least
-                    // one connection is waiting for it
-                    waitForVSync = true;
-                    if (timestamp) {
-                        // we consume the event only if it's time
-                        // (ie: we received a vsync event)
-                        if (connection->count == 0) {
-                            // fired this time around
-                            connection->count = -1;
-                            signalConnections.push_back(connection);
-                            added = true;
-                        } else if (connection->count == 1 ||
-                                   (vsyncCount % connection->count) == 0) {
-                            // continuous event, and time to report it
-                            signalConnections.push_back(connection);
-                            added = true;
-                        }
-                    }
-                }
+            if (const auto connection = it->promote()) {
+                vsyncRequested |= connection->vsyncRequest != VSyncRequest::None;
 
-                if (eventPending && !timestamp && !added) {
-                    // we don't have a vsync event to process
-                    // (timestamp==0), but we have some pending
-                    // messages.
-                    signalConnections.push_back(connection);
+                if (event && shouldConsumeEvent(*event, connection)) {
+                    consumers.push_back(connection);
                 }
+
                 ++it;
             } else {
-                // we couldn't promote this reference, the connection has
-                // died, so clean-up!
                 it = mDisplayEventConnections.erase(it);
             }
         }
 
+        if (!consumers.empty()) {
+            dispatchEvent(*event, consumers);
+            consumers.clear();
+        }
+
+        const bool vsyncPending =
+                event && event->header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+
         // Here we figure out if we need to enable or disable vsyncs
-        if (timestamp && !waitForVSync) {
+        if (vsyncPending && !vsyncRequested) {
             // we received a VSYNC but we have no clients
             // don't report it, and disable VSYNC events
             disableVSyncLocked();
-        } else if (!timestamp && waitForVSync) {
+        } else if (!vsyncPending && vsyncRequested) {
             // we have at least one client, so we want vsync enabled
             // (TODO: this function is called right after we finish
             // notifying clients of a vsync, so this call will be made
@@ -368,48 +356,72 @@ std::vector<sp<EventThreadConnection>> EventThread::waitForEventLocked(
             enableVSyncLocked();
         }
 
-        // note: !timestamp implies signalConnections.isEmpty(), because we
-        // don't populate signalConnections if there's no vsync pending
-        if (!timestamp && !eventPending) {
-            // wait for something to happen
-            if (waitForVSync) {
-                // This is where we spend most of our time, waiting
-                // for vsync events and new client registrations.
-                //
-                // If the screen is off, we can't use h/w vsync, so we
-                // use a 16ms timeout instead.  It doesn't need to be
-                // precise, we just need to keep feeding our clients.
-                //
-                // We don't want to stall if there's a driver bug, so we
-                // use a (long) timeout when waiting for h/w vsync, and
-                // generate fake events when necessary.
-                bool softwareSync = mUseSoftwareVSync;
-                auto timeout = softwareSync ? 16ms : 1000ms;
-                if (mCondition.wait_for(*lock, timeout) == std::cv_status::timeout) {
-                    if (!softwareSync) {
-                        ALOGW("Timed out waiting for hw vsync; faking it");
-                    }
-                    // FIXME: how do we decide which display id the fake
-                    // vsync came from ?
-                    mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
-                    mVSyncEvent[0].header.id = 0;
-                    mVSyncEvent[0].header.timestamp = systemTime(SYSTEM_TIME_MONOTONIC);
-                    mVSyncEvent[0].vsync.count++;
-                }
-            } else {
-                // Nobody is interested in vsync, so we just want to sleep.
-                // h/w vsync should be disabled, so this will wait until we
-                // get a new connection, or an existing connection becomes
-                // interested in receiving vsync again.
-                mCondition.wait(*lock);
+        if (event) {
+            continue;
+        }
+
+        // Wait for event or client registration/request.
+        if (vsyncRequested) {
+            // Generate a fake VSYNC after a long timeout in case the driver stalls. When the
+            // display is off, keep feeding clients at 60 Hz.
+            const bool softwareSync = mUseSoftwareVSync;
+            const auto timeout = softwareSync ? 16ms : 1000ms;
+            if (mCondition.wait_for(lock, timeout) == std::cv_status::timeout) {
+                ALOGW_IF(!softwareSync, "Faking VSYNC due to driver stall");
+
+                mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+                mVSyncEvent[0].header.id = 0;
+                mVSyncEvent[0].header.timestamp = systemTime(SYSTEM_TIME_MONOTONIC);
+                mVSyncEvent[0].vsync.count++;
             }
+        } else {
+            mCondition.wait(lock);
         }
     }
+}
+
+bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event,
+                                     const sp<EventThreadConnection>& connection) const {
+    switch (event.header.type) {
+        case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+            return true;
+
+        case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+            switch (connection->vsyncRequest) {
+                case VSyncRequest::None:
+                    return false;
+                case VSyncRequest::Single:
+                    connection->vsyncRequest = VSyncRequest::None;
+                    return true;
+                case VSyncRequest::Periodic:
+                    return true;
+                default:
+                    return event.vsync.count % vsyncPeriod(connection->vsyncRequest) == 0;
+            }
+
+        default:
+            return false;
+    }
+}
+
+void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
+                                const DisplayEventConsumers& consumers) {
+    for (const auto& consumer : consumers) {
+        switch (consumer->postEvent(event)) {
+            case NO_ERROR:
+                break;
+
+            case -EAGAIN:
+                // TODO: Try again if pipe is full.
+                ALOGW("Failed dispatching %s for %s", toString(event).c_str(),
+                      toString(*consumer).c_str());
+                break;
 
-    // here we're guaranteed to have a timestamp and some connections to signal
-    // (The connections might have dropped out of mDisplayEventConnections
-    // while we were asleep, but we'll still have strong references to them.)
-    return signalConnections;
+            default:
+                // Treat EPIPE and other errors as fatal.
+                removeDisplayEventConnectionLocked(consumer);
+        }
+    }
 }
 
 void EventThread::enableVSyncLocked() {
@@ -434,16 +446,22 @@ void EventThread::disableVSyncLocked() {
 
 void EventThread::dump(std::string& result) const {
     std::lock_guard<std::mutex> lock(mMutex);
-    StringAppendF(&result, "VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled");
-    StringAppendF(&result, "  soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled");
-    StringAppendF(&result, "  numListeners=%zu,\n  events-delivered: %u\n",
-                  mDisplayEventConnections.size(), mVSyncEvent[0].vsync.count);
-    for (const wp<EventThreadConnection>& weak : mDisplayEventConnections) {
-        sp<EventThreadConnection> connection = weak.promote();
-        StringAppendF(&result, "    %p: count=%d\n", connection.get(),
-                      connection != nullptr ? connection->count : 0);
+
+    StringAppendF(&result, "%s: VSYNC %s\n", mThreadName, mDebugVsyncEnabled ? "on" : "off");
+    StringAppendF(&result, "  software VSYNC: %s\n", mUseSoftwareVSync ? "on" : "off");
+    StringAppendF(&result, "  VSYNC count: %u\n", mVSyncEvent[0].vsync.count);
+
+    StringAppendF(&result, "  pending events (count=%zu):\n", mPendingEvents.size());
+    for (const auto& event : mPendingEvents) {
+        StringAppendF(&result, "    %s\n", toString(event).c_str());
+    }
+
+    StringAppendF(&result, "  connections (count=%zu):\n", mDisplayEventConnections.size());
+    for (const auto& ptr : mDisplayEventConnections) {
+        if (const auto connection = ptr.promote()) {
+            StringAppendF(&result, "    %s\n", toString(*connection).c_str());
+        }
     }
-    StringAppendF(&result, "  other-events-pending: %zu\n", mPendingEvents.size());
 }
 
 } // namespace impl
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index a41188569c..b622e7abc5 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -21,8 +21,8 @@
 #include <array>
 #include <condition_variable>
 #include <cstdint>
+#include <deque>
 #include <mutex>
-#include <queue>
 #include <thread>
 #include <vector>
 
@@ -46,6 +46,13 @@ class SurfaceFlinger;
 
 using ResyncCallback = std::function<void()>;
 
+enum class VSyncRequest {
+    None = -1,
+    Single = 0,
+    Periodic = 1,
+    // Subsequent values are periods.
+};
+
 class VSyncSource {
 public:
     class Callback {
@@ -68,7 +75,7 @@ public:
     virtual status_t postEvent(const DisplayEventReceiver::Event& event);
 
     status_t stealReceiveChannel(gui::BitTube* outChannel) override;
-    status_t setVsyncRate(uint32_t count) override;
+    status_t setVsyncRate(uint32_t rate) override;
     void requestNextVsync() override; // asynchronous
     // Requesting Vsync for HWC does not reset the idle timer, since HWC requires a refresh
     // in order to update the configs.
@@ -77,10 +84,7 @@ public:
     // Called in response to requestNextVsync.
     const ResyncCallback resyncCallback;
 
-    // count >= 1 : continuous event. count is the vsync rate
-    // count == 0 : one-shot event that has not fired
-    // count ==-1 : one-shot event that fired this round / disabled
-    int32_t count;
+    VSyncRequest vsyncRequest = VSyncRequest::None;
 
 private:
     virtual void onFirstRef();
@@ -113,7 +117,7 @@ public:
 
     virtual status_t registerDisplayEventConnection(
             const sp<EventThreadConnection>& connection) = 0;
-    virtual void setVsyncRate(uint32_t count, const sp<EventThreadConnection>& connection) = 0;
+    virtual void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) = 0;
     // Requests the next vsync. If resetIdleTimer is set to true, it resets the idle timer.
     virtual void requestNextVsync(const sp<EventThreadConnection>& connection,
                                   bool resetIdleTimer) = 0;
@@ -137,7 +141,7 @@ public:
     sp<EventThreadConnection> createEventConnection(ResyncCallback resyncCallback) const override;
 
     status_t registerDisplayEventConnection(const sp<EventThreadConnection>& connection) override;
-    void setVsyncRate(uint32_t count, const sp<EventThreadConnection>& connection) override;
+    void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) override;
     void requestNextVsync(const sp<EventThreadConnection>& connection,
                           bool resetIdleTimer) override;
 
@@ -157,18 +161,22 @@ public:
 private:
     friend EventThreadTest;
 
+    using DisplayEventConsumers = std::vector<sp<EventThreadConnection>>;
+
     // TODO(b/113612090): Once the Scheduler is complete this constructor will become obsolete.
     EventThread(VSyncSource* src, std::unique_ptr<VSyncSource> uniqueSrc,
                 InterceptVSyncsCallback interceptVSyncsCallback, const char* threadName);
 
+    void threadMain(std::unique_lock<std::mutex>& lock) REQUIRES(mMutex);
 
-    void threadMain();
-    std::vector<sp<EventThreadConnection>> waitForEventLocked(std::unique_lock<std::mutex>* lock,
-                                                              DisplayEventReceiver::Event* event)
-            REQUIRES(mMutex);
+    bool shouldConsumeEvent(const DisplayEventReceiver::Event& event,
+                            const sp<EventThreadConnection>& connection) const REQUIRES(mMutex);
+    void dispatchEvent(const DisplayEventReceiver::Event& event,
+                       const DisplayEventConsumers& consumers) REQUIRES(mMutex);
 
     void removeDisplayEventConnectionLocked(const wp<EventThreadConnection>& connection)
             REQUIRES(mMutex);
+
     void enableVSyncLocked() REQUIRES(mMutex);
     void disableVSyncLocked() REQUIRES(mMutex);
 
@@ -181,16 +189,16 @@ private:
     // TODO(b/113612090): Once the Scheduler is complete this pointer will become obsolete.
     VSyncSource* mVSyncSource GUARDED_BY(mMutex) = nullptr;
     std::unique_ptr<VSyncSource> mVSyncSourceUnique GUARDED_BY(mMutex) = nullptr;
-    // constants
+
     const InterceptVSyncsCallback mInterceptVSyncsCallback;
+    const char* const mThreadName;
 
     std::thread mThread;
     mutable std::mutex mMutex;
     mutable std::condition_variable mCondition;
 
-    // protected by mLock
     std::vector<wp<EventThreadConnection>> mDisplayEventConnections GUARDED_BY(mMutex);
-    std::queue<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex);
+    std::deque<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex);
     std::array<DisplayEventReceiver::Event, 2> mVSyncEvent GUARDED_BY(mMutex);
     bool mUseSoftwareVSync GUARDED_BY(mMutex) = false;
     bool mVsyncEnabled GUARDED_BY(mMutex) = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1cb38c80b4..3531296830 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4386,6 +4386,7 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args,
                 {"--list"s, dumper(&SurfaceFlinger::listLayersLocked)},
                 {"--static-screen"s, dumper(&SurfaceFlinger::dumpStaticScreenStats)},
                 {"--timestats"s, protoDumper(&SurfaceFlinger::dumpTimeStats)},
+                {"--vsync"s, dumper(&SurfaceFlinger::dumpVSync)},
                 {"--wide-color"s, dumper(&SurfaceFlinger::dumpWideColorInfo)},
         };
 
@@ -4469,6 +4470,26 @@ void SurfaceFlinger::appendSfConfigString(std::string& result) const {
     result.append("]");
 }
 
+void SurfaceFlinger::dumpVSync(std::string& result) const {
+    const auto [sfEarlyOffset, appEarlyOffset] = mVsyncModulator.getEarlyOffsets();
+    const auto [sfEarlyGlOffset, appEarlyGlOffset] = mVsyncModulator.getEarlyGlOffsets();
+    StringAppendF(&result,
+                  "         app phase: %9" PRId64 " ns\t         SF phase: %9" PRId64 " ns\n"
+                  "   early app phase: %9" PRId64 " ns\t   early SF phase: %9" PRId64 " ns\n"
+                  "GL early app phase: %9" PRId64 " ns\tGL early SF phase: %9" PRId64 " ns\n"
+                  "    present offset: %9" PRId64 " ns\t     VSYNC period: %9" PRId64 " ns\n\n",
+                  vsyncPhaseOffsetNs, sfVsyncPhaseOffsetNs, appEarlyOffset, sfEarlyOffset,
+                  appEarlyGlOffset, sfEarlyGlOffset, dispSyncPresentTimeOffset, getVsyncPeriod());
+
+    StringAppendF(&result, "Scheduler: %s\n\n", mUseScheduler ? "enabled" : "disabled");
+
+    if (mUseScheduler) {
+        mScheduler->dump(mAppConnectionHandle, result);
+    } else {
+        mEventThread->dump(result);
+    }
+}
+
 void SurfaceFlinger::dumpStaticScreenStats(std::string& result) const {
     result.append("Static screen stats:\n");
     for (size_t b = 0; b < SurfaceFlingerBE::NUM_BUCKETS - 1; ++b) {
@@ -4703,27 +4724,14 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co
     result.append("Sync configuration: ");
     colorizer.reset(result);
     result.append(SyncFeatures::getInstance().toString());
-    result.append("\n");
+    result.append("\n\n");
 
     colorizer.bold(result);
-    result.append("DispSync configuration:\n");
+    result.append("VSYNC configuration:\n");
     colorizer.reset(result);
-
-    const auto [sfEarlyOffset, appEarlyOffset] = mVsyncModulator.getEarlyOffsets();
-    const auto [sfEarlyGlOffset, appEarlyGlOffset] = mVsyncModulator.getEarlyGlOffsets();
-    StringAppendF(&result,
-                  "app phase %" PRId64 " ns, "
-                  "sf phase %" PRId64 " ns, "
-                  "early app phase %" PRId64 " ns, "
-                  "early sf phase %" PRId64 " ns, "
-                  "early app gl phase %" PRId64 " ns, "
-                  "early sf gl phase %" PRId64 " ns, "
-                  "present offset %" PRId64 " ns (refresh %" PRId64 " ns)\n",
-                  vsyncPhaseOffsetNs, sfVsyncPhaseOffsetNs, appEarlyOffset, sfEarlyOffset,
-                  appEarlyGlOffset, sfEarlyGlOffset, dispSyncPresentTimeOffset, getVsyncPeriod());
-
-    // Dump static screen stats
+    dumpVSync(result);
     result.append("\n");
+
     dumpStaticScreenStats(result);
     result.append("\n");
 
@@ -4797,17 +4805,6 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co
 
     StringAppendF(&result, "  transaction time: %f us\n", inTransactionDuration / 1000.0);
 
-    StringAppendF(&result, "  use Scheduler: %s\n", mUseScheduler ? "true" : "false");
-    /*
-     * VSYNC state
-     */
-    if (mUseScheduler) {
-        mScheduler->dump(mAppConnectionHandle, result);
-    } else {
-        mEventThread->dump(result);
-    }
-    result.append("\n");
-
     /*
      * Tracing state
      */
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1a54f6c8fc..d401a157c6 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -863,6 +863,7 @@ private:
     void dumpTimeStats(const DumpArgs& args, bool asProto, std::string& result) const;
     void logFrameStats();
 
+    void dumpVSync(std::string& result) const REQUIRES(mStateLock);
     void dumpStaticScreenStats(std::string& result) const;
     // Not const because each Layer needs to query Fences and cache timestamps.
     void dumpFrameEventsLocked(std::string& result);