Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 029cc12b authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Toggle VSYNC in EventThread with state machine 

This CL simplifies the logic to enable and disable VSYNC in the
EventThread loop.

Bug: 74619554
Test: libsurfaceflinger_unittest
Test: dumpsys SurfaceFlinger --vsync

Change-Id: I7637fcf1982d60cfced5dae68c74556f0ee67a0f
parent bf9a49a2

services/surfaceflinger/Scheduler/EventThread.cpp

+44 −47
Original line number Diff line number Diff line
@@ -163,6 +163,7 @@ EventThread::EventThread(VSyncSource* src, std::unique_ptr<VSyncSource> uniqueSr 
    if (src == nullptr) {

        mVSyncSource = mVSyncSourceUnique.get();

    }

    mVSyncSource->setCallback(this);



    mThread = std::thread([this]() NO_THREAD_SAFETY_ANALYSIS {

        std::unique_lock<std::mutex> lock(mMutex);
@@ -185,9 +186,11 @@ EventThread::EventThread(VSyncSource* src, std::unique_ptr<VSyncSource> uniqueSr 
}



EventThread::~EventThread() {

    mVSyncSource->setCallback(nullptr);



    {

        std::lock_guard<std::mutex> lock(mMutex);

        mKeepRunning = false;

        mState = State::Quit;

        mCondition.notify_all();

    }

    mThread.join();
@@ -293,21 +296,19 @@ void EventThread::onHotplugReceived(DisplayType displayType, bool connected) { 
void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {

    DisplayEventConsumers consumers;



    while (mKeepRunning) {

    while (mState != State::Quit) {

        std::optional<DisplayEventReceiver::Event> event;



        // Determine next event to dispatch.

        if (!mPendingEvents.empty()) {

            event = mPendingEvents.front();

            mPendingEvents.pop_front();

        }



        const bool vsyncPending =

                event && event->header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC;



        if (mInterceptVSyncsCallback && vsyncPending) {

            if (mInterceptVSyncsCallback &&

                event->header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {

                mInterceptVSyncsCallback(event->header.timestamp);

            }

        }



        bool vsyncRequested = false;
@@ -332,19 +333,21 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { 
            consumers.clear();

        }



        // Here we figure out if we need to enable or disable vsyncs

        if (vsyncPending && !vsyncRequested) {

            // we received a VSYNC but we have no clients

            // don't report it, and disable VSYNC events

            disableVSyncLocked();

        } 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

            // at the vsync rate, e.g. 60fps.  If we can accurately

            // track the current state we could avoid making this call

            // so often.)

            enableVSyncLocked();

        State nextState;

        if (vsyncRequested) {

            nextState = mVSyncState.synthetic ? State::SyntheticVSync : State::VSync;

        } else {

            nextState = State::Idle;

        }



        if (mState != nextState) {

            if (mState == State::VSync) {

                mVSyncSource->setVSyncEnabled(false);

            } else if (nextState == State::VSync) {

                mVSyncSource->setVSyncEnabled(true);

            }



            mState = nextState;

        }



        if (event) {
@@ -352,19 +355,19 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { 
        }



        // Wait for event or client registration/request.

        if (vsyncRequested) {

        if (mState == State::Idle) {

            mCondition.wait(lock);

        } else {

            // 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 auto timeout = mVSyncState.synthetic ? 16ms : 1000ms;

            const auto timeout = mState == State::SyntheticVSync ? 16ms : 1000ms;

            if (mCondition.wait_for(lock, timeout) == std::cv_status::timeout) {

                ALOGW_IF(!mVSyncState.synthetic, "Faking VSYNC due to driver stall");

                ALOGW_IF(mState == State::VSync, "Faking VSYNC due to driver stall");



                mPendingEvents.push_back(makeVSync(mVSyncState.displayId,

                                                   systemTime(SYSTEM_TIME_MONOTONIC),

                                                   ++mVSyncState.count));

            }

        } else {

            mCondition.wait(lock);

        }

    }

}
@@ -413,29 +416,10 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, 
    }

}



void EventThread::enableVSyncLocked() {

    if (!mVSyncState.synthetic) {

        if (!mVsyncEnabled) {

            mVsyncEnabled = true;

            mVSyncSource->setCallback(this);

            mVSyncSource->setVSyncEnabled(true);

        }

    }

    mDebugVsyncEnabled = true;

}



void EventThread::disableVSyncLocked() {

    if (mVsyncEnabled) {

        mVsyncEnabled = false;

        mVSyncSource->setVSyncEnabled(false);

        mDebugVsyncEnabled = false;

    }

}



void EventThread::dump(std::string& result) const {

    std::lock_guard<std::mutex> lock(mMutex);



    StringAppendF(&result, "%s: VSYNC %s\n", mThreadName, mDebugVsyncEnabled ? "on" : "off");

    StringAppendF(&result, "%s: state=%s", mThreadName, toCString(mState));

    StringAppendF(&result, " VSyncState{displayId=%u, count=%u%s}\n", mVSyncState.displayId,

                  mVSyncState.count, mVSyncState.synthetic ? ", synthetic" : "");
@@ -452,5 +436,18 @@ void EventThread::dump(std::string& result) const { 
    }

}



const char* EventThread::toCString(State state) {

    switch (state) {

        case State::Idle:

            return "Idle";

        case State::Quit:

            return "Quit";

        case State::SyntheticVSync:

            return "SyntheticVSync";

        case State::VSync:

            return "VSync";

    }

}



} // namespace impl

} // namespace android

services/surfaceflinger/Scheduler/EventThread.h

+10 −7
Original line number Diff line number Diff line
@@ -176,9 +176,6 @@ private: 
    void removeDisplayEventConnectionLocked(const wp<EventThreadConnection>& connection)

            REQUIRES(mMutex);



    void enableVSyncLocked() REQUIRES(mMutex);

    void disableVSyncLocked() REQUIRES(mMutex);



    // Implements VSyncSource::Callback

    void onVSyncEvent(nsecs_t timestamp) override;
@@ -212,11 +209,17 @@ private: 


    VSyncState mVSyncState GUARDED_BY(mMutex);



    bool mVsyncEnabled GUARDED_BY(mMutex) = false;

    bool mKeepRunning GUARDED_BY(mMutex) = true;

    // State machine for event loop.

    enum class State {

        Idle,

        Quit,

        SyntheticVSync,

        VSync,

    };



    State mState GUARDED_BY(mMutex) = State::Idle;



    // for debugging

    bool mDebugVsyncEnabled GUARDED_BY(mMutex) = false;

    static const char* toCString(State);



    // Callback that resets the idle timer when the next vsync is received.

    ResetIdleTimerCallback mResetIdleTimer;

services/surfaceflinger/tests/unittests/EventThreadTest.cpp

+30 −43
Original line number Diff line number Diff line
@@ -83,6 +83,7 @@ protected: 
    ConnectionEventRecorder mConnectionEventCallRecorder{0};



    MockVSyncSource mVSyncSource;

    VSyncSource::Callback* mCallback = nullptr;

    std::unique_ptr<android::impl::EventThread> mThread;

    sp<MockEventThreadConnection> mConnection;

};
@@ -109,6 +110,9 @@ EventThreadTest::~EventThreadTest() { 
    const ::testing::TestInfo* const test_info =

            ::testing::UnitTest::GetInstance()->current_test_info();

    ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());



    // EventThread should unregister itself as VSyncSource callback.

    EXPECT_FALSE(expectVSyncSetCallbackCallReceived());

}



void EventThreadTest::createThread() {
@@ -116,6 +120,10 @@ void EventThreadTest::createThread() { 
            std::make_unique<android::impl::EventThread>(&mVSyncSource,

                                                         mInterceptVSyncCallRecorder.getInvocable(),

                                                         "unit-test-event-thread");



    // EventThread should register itself as VSyncSource callback.

    mCallback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(mCallback);

}



sp<EventThreadTest::MockEventThreadConnection> EventThreadTest::createConnection(
@@ -206,22 +214,19 @@ TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) { 
    // EventThread should immediately request a resync.

    EXPECT_TRUE(mResyncCallRecorder.waitForCall().has_value());



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // Use the received callback to signal a first vsync event.

    // The interceptor should receive the event, as well as the connection.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    expectVsyncEventReceivedByConnection(123, 1u);



    // Use the received callback to signal a second vsync event.

    // The interceptor should receive the event, but the the connection should

    // not as it was only interested in the first.

    callback->onVSyncEvent(456);

    mCallback->onVSyncEvent(456);

    expectInterceptCallReceived(456);

    EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value());
@@ -246,16 +251,13 @@ TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { 
            createConnection(secondConnectionEventRecorder);

    mThread->setVsyncRate(1, secondConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // Send a vsync event. EventThread should then make a call to the

    // interceptor, and the second connection. The first connection should not

    // get the event.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    EXPECT_FALSE(firstConnectionEventRecorder.waitForUnexpectedCall().has_value());

    expectVsyncEventReceivedByConnection("secondConnection", secondConnectionEventRecorder, 123,
@@ -265,25 +267,22 @@ TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { 
TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) {

    mThread->setVsyncRate(1, mConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // Send a vsync event. EventThread should then make a call to the

    // interceptor, and the connection.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    expectVsyncEventReceivedByConnection(123, 1u);



    // A second event should go to the same places.

    callback->onVSyncEvent(456);

    mCallback->onVSyncEvent(456);

    expectInterceptCallReceived(456);

    expectVsyncEventReceivedByConnection(456, 2u);



    // A third event should go to the same places.

    callback->onVSyncEvent(789);

    mCallback->onVSyncEvent(789);

    expectInterceptCallReceived(789);

    expectVsyncEventReceivedByConnection(789, 3u);

}
@@ -291,29 +290,26 @@ TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) { 
TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) {

    mThread->setVsyncRate(2, mConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // The first event will be seen by the interceptor, and not the connection.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value());



    // The second event will be seen by the interceptor and the connection.

    callback->onVSyncEvent(456);

    mCallback->onVSyncEvent(456);

    expectInterceptCallReceived(456);

    expectVsyncEventReceivedByConnection(456, 2u);



    // The third event will be seen by the interceptor, and not the connection.

    callback->onVSyncEvent(789);

    mCallback->onVSyncEvent(789);

    expectInterceptCallReceived(789);

    EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value());



    // The fourth event will be seen by the interceptor and the connection.

    callback->onVSyncEvent(101112);

    mCallback->onVSyncEvent(101112);

    expectInterceptCallReceived(101112);

    expectVsyncEventReceivedByConnection(101112, 4u);

}
@@ -321,17 +317,14 @@ TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) { 
TEST_F(EventThreadTest, connectionsRemovedIfInstanceDestroyed) {

    mThread->setVsyncRate(1, mConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // Destroy the only (strong) reference to the connection.

    mConnection = nullptr;



    // The first event will be seen by the interceptor, and not the connection.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value());
@@ -344,21 +337,18 @@ TEST_F(EventThreadTest, connectionsRemovedIfEventDeliveryError) { 
    sp<MockEventThreadConnection> errorConnection = createConnection(errorConnectionEventRecorder);

    mThread->setVsyncRate(1, errorConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // The first event will be seen by the interceptor, and by the connection,

    // which then returns an error.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u);



    // A subsequent event will be seen by the interceptor and not by the

    // connection.

    callback->onVSyncEvent(456);

    mCallback->onVSyncEvent(456);

    expectInterceptCallReceived(456);

    EXPECT_FALSE(errorConnectionEventRecorder.waitForUnexpectedCall().has_value());
@@ -371,21 +361,18 @@ TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) { 
    sp<MockEventThreadConnection> errorConnection = createConnection(errorConnectionEventRecorder);

    mThread->setVsyncRate(1, errorConnection);



    // EventThread should enable vsync callbacks, and set a callback interface

    // pointer to use them with the VSync source.

    // EventThread should enable vsync callbacks.

    expectVSyncSetEnabledCallReceived(true);

    auto callback = expectVSyncSetCallbackCallReceived();

    ASSERT_TRUE(callback);



    // The first event will be seen by the interceptor, and by the connection,

    // which then returns an non-fatal error.

    callback->onVSyncEvent(123);

    mCallback->onVSyncEvent(123);

    expectInterceptCallReceived(123);

    expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u);



    // A subsequent event will be seen by the interceptor, and by the connection,

    // which still then returns an non-fatal error.

    callback->onVSyncEvent(456);

    mCallback->onVSyncEvent(456);

    expectInterceptCallReceived(456);

    expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 456, 2u);