SF: Tie VSyncState lifetime to display hotplug
This CL creates/destroys EventThread::VSyncState when the display is
connected/disconnected.
Bug: 74619554
Test: libsurfaceflinger_unittest
Test: dumpsys SurfaceFlinger --vsync
Change-Id: I46dad4857222b6d2932ab568c4d66325edc3371b
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 668dfd0..52abe9c 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -264,24 +264,29 @@
void EventThread::onScreenReleased() {
std::lock_guard<std::mutex> lock(mMutex);
- if (!mVSyncState.synthetic) {
- mVSyncState.synthetic = true;
- mCondition.notify_all();
+ if (!mVSyncState || mVSyncState->synthetic) {
+ return;
}
+
+ mVSyncState->synthetic = true;
+ mCondition.notify_all();
}
void EventThread::onScreenAcquired() {
std::lock_guard<std::mutex> lock(mMutex);
- if (mVSyncState.synthetic) {
- mVSyncState.synthetic = false;
- mCondition.notify_all();
+ if (!mVSyncState || !mVSyncState->synthetic) {
+ return;
}
+
+ mVSyncState->synthetic = false;
+ mCondition.notify_all();
}
void EventThread::onVSyncEvent(nsecs_t timestamp) {
std::lock_guard<std::mutex> lock(mMutex);
- mPendingEvents.push_back(makeVSync(mVSyncState.displayId, timestamp, ++mVSyncState.count));
+ LOG_FATAL_IF(!mVSyncState);
+ mPendingEvents.push_back(makeVSync(mVSyncState->displayId, timestamp, ++mVSyncState->count));
mCondition.notify_all();
}
@@ -304,9 +309,21 @@
event = mPendingEvents.front();
mPendingEvents.pop_front();
- if (mInterceptVSyncsCallback &&
- event->header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
- mInterceptVSyncsCallback(event->header.timestamp);
+ switch (event->header.type) {
+ case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+ if (event->hotplug.connected && !mVSyncState) {
+ mVSyncState.emplace(event->header.id);
+ } else if (!event->hotplug.connected && mVSyncState &&
+ mVSyncState->displayId == event->header.id) {
+ mVSyncState.reset();
+ }
+ break;
+
+ case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+ if (mInterceptVSyncsCallback) {
+ mInterceptVSyncsCallback(event->header.timestamp);
+ }
+ break;
}
}
@@ -334,9 +351,10 @@
}
State nextState;
- if (vsyncRequested) {
- nextState = mVSyncState.synthetic ? State::SyntheticVSync : State::VSync;
+ if (mVSyncState && vsyncRequested) {
+ nextState = mVSyncState->synthetic ? State::SyntheticVSync : State::VSync;
} else {
+ ALOGW_IF(!mVSyncState, "Ignoring VSYNC request while display is disconnected");
nextState = State::Idle;
}
@@ -364,9 +382,10 @@
if (mCondition.wait_for(lock, timeout) == std::cv_status::timeout) {
ALOGW_IF(mState == State::VSync, "Faking VSYNC due to driver stall");
- mPendingEvents.push_back(makeVSync(mVSyncState.displayId,
+ LOG_FATAL_IF(!mVSyncState);
+ mPendingEvents.push_back(makeVSync(mVSyncState->displayId,
systemTime(SYSTEM_TIME_MONOTONIC),
- ++mVSyncState.count));
+ ++mVSyncState->count));
}
}
}
@@ -419,9 +438,13 @@
void EventThread::dump(std::string& result) const {
std::lock_guard<std::mutex> lock(mMutex);
- StringAppendF(&result, "%s: state=%s", mThreadName, toCString(mState));
- StringAppendF(&result, " VSyncState{displayId=%u, count=%u%s}\n", mVSyncState.displayId,
- mVSyncState.count, mVSyncState.synthetic ? ", synthetic" : "");
+ StringAppendF(&result, "%s: state=%s VSyncState=", mThreadName, toCString(mState));
+ if (mVSyncState) {
+ StringAppendF(&result, "{displayId=%u, count=%u%s}\n", mVSyncState->displayId,
+ mVSyncState->count, mVSyncState->synthetic ? ", synthetic" : "");
+ } else {
+ StringAppendF(&result, "none\n");
+ }
StringAppendF(&result, " pending events (count=%zu):\n", mPendingEvents.size());
for (const auto& event : mPendingEvents) {
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index f093e47..89b799e 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -22,6 +22,7 @@
#include <cstdint>
#include <deque>
#include <mutex>
+#include <optional>
#include <thread>
#include <vector>
@@ -198,7 +199,9 @@
// VSYNC state of connected display.
struct VSyncState {
- uint32_t displayId = 0;
+ explicit VSyncState(uint32_t displayId) : displayId(displayId) {}
+
+ const uint32_t displayId;
// Number of VSYNC events since display was connected.
uint32_t count = 0;
@@ -207,7 +210,9 @@
bool synthetic = false;
};
- VSyncState mVSyncState GUARDED_BY(mMutex);
+ // TODO(b/74619554): Create per-display threads waiting on respective VSYNC signals,
+ // and support headless mode by injecting a fake display with synthetic VSYNC.
+ std::optional<VSyncState> mVSyncState GUARDED_BY(mMutex);
// State machine for event loop.
enum class State {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 14feb43..acf0013 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2564,6 +2564,17 @@
mPendingHotplugEvents.clear();
}
+void SurfaceFlinger::dispatchDisplayHotplugEvent(EventThread::DisplayType displayType,
+ bool connected) {
+ if (mUseScheduler) {
+ mScheduler->hotplugReceived(mAppConnectionHandle, displayType, connected);
+ mScheduler->hotplugReceived(mSfConnectionHandle, displayType, connected);
+ } else {
+ mEventThread->onHotplugReceived(displayType, connected);
+ mSFEventThread->onHotplugReceived(displayType, connected);
+ }
+}
+
sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
const wp<IBinder>& displayToken, const std::optional<DisplayId>& displayId,
const DisplayDeviceState& state, const sp<compositionengine::DisplaySurface>& dispSurface,
@@ -2668,19 +2679,9 @@
display->disconnect();
}
if (internalDisplayId && internalDisplayId == draw[i].displayId) {
- if (mUseScheduler) {
- mScheduler->hotplugReceived(mAppConnectionHandle,
- EventThread::DisplayType::Primary, false);
- } else {
- mEventThread->onHotplugReceived(EventThread::DisplayType::Primary, false);
- }
+ dispatchDisplayHotplugEvent(EventThread::DisplayType::Primary, false);
} else if (externalDisplayId && externalDisplayId == draw[i].displayId) {
- if (mUseScheduler) {
- mScheduler->hotplugReceived(mAppConnectionHandle,
- EventThread::DisplayType::External, false);
- } else {
- mEventThread->onHotplugReceived(EventThread::DisplayType::External, false);
- }
+ dispatchDisplayHotplugEvent(EventThread::DisplayType::External, false);
}
mDisplays.erase(draw.keyAt(i));
} else {
@@ -2786,23 +2787,9 @@
LOG_ALWAYS_FATAL_IF(!displayId);
if (displayId == getInternalDisplayId()) {
- if (mUseScheduler) {
- mScheduler->hotplugReceived(mAppConnectionHandle,
- EventThread::DisplayType::Primary,
- true);
- } else {
- mEventThread->onHotplugReceived(EventThread::DisplayType::Primary,
- true);
- }
+ dispatchDisplayHotplugEvent(EventThread::DisplayType::Primary, true);
} else if (displayId == getExternalDisplayId()) {
- if (mUseScheduler) {
- mScheduler->hotplugReceived(mAppConnectionHandle,
- EventThread::DisplayType::External,
- true);
- } else {
- mEventThread->onHotplugReceived(EventThread::DisplayType::External,
- true);
- }
+ dispatchDisplayHotplugEvent(EventThread::DisplayType::External, true);
}
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c62f852..4d84144 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -294,10 +294,6 @@
// starts SurfaceFlinger main loop in the current thread
void run() ANDROID_API;
- enum {
- EVENT_VSYNC = HWC_EVENT_VSYNC
- };
-
// post an asynchronous message to the main thread
status_t postMessageAsync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0);
@@ -754,6 +750,8 @@
void processDisplayChangesLocked();
void processDisplayHotplugEventsLocked();
+ void dispatchDisplayHotplugEvent(EventThread::DisplayType displayType, bool connected);
+
/* ------------------------------------------------------------------------
* VSync
*/
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 8201704..2c1833b 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -118,6 +118,7 @@
TestableSurfaceFlinger mFlinger;
mock::EventThread* mEventThread = new mock::EventThread();
+ mock::EventThread* mSFEventThread = new mock::EventThread();
mock::EventControlThread* mEventControlThread = new mock::EventControlThread();
sp<mock::NativeWindow> mNativeWindow = new mock::NativeWindow();
sp<GraphicBuffer> mBuffer = new GraphicBuffer();
@@ -161,6 +162,7 @@
mFlinger.mutableEventControlThread().reset(mEventControlThread);
mFlinger.mutableEventThread().reset(mEventThread);
+ mFlinger.mutableSFEventThread().reset(mSFEventThread);
mFlinger.mutableEventQueue().reset(mMessageQueue);
mFlinger.setupRenderEngine(std::unique_ptr<renderengine::RenderEngine>(mRenderEngine));
mFlinger.mutableInterceptor().reset(mSurfaceInterceptor);
@@ -319,6 +321,11 @@
// Whether the display is primary
static constexpr Primary PRIMARY = primary;
+ static constexpr auto displayType() {
+ return static_cast<bool>(PRIMARY) ? EventThread::DisplayType::Primary
+ : EventThread::DisplayType::External;
+ }
+
static auto makeFakeExistingDisplayInjector(DisplayTransactionTest* test) {
auto injector =
FakeDisplayDeviceInjector(test->mFlinger, DISPLAY_ID::get(),
@@ -1404,23 +1411,17 @@
Case::PerFrameMetadataSupport::setupComposerCallExpectations(this);
EXPECT_CALL(*mSurfaceInterceptor, saveDisplayCreation(_)).Times(1);
- EXPECT_CALL(*mEventThread,
- onHotplugReceived(static_cast<bool>(Case::Display::PRIMARY)
- ? EventThread::DisplayType::Primary
- : EventThread::DisplayType::External,
- true))
- .Times(1);
+
+ EXPECT_CALL(*mEventThread, onHotplugReceived(Case::Display::displayType(), true)).Times(1);
+ EXPECT_CALL(*mSFEventThread, onHotplugReceived(Case::Display::displayType(), true)).Times(1);
}
template <typename Case>
void HandleTransactionLockedTest::setupCommonCallExpectationsForDisconnectProcessing() {
EXPECT_CALL(*mSurfaceInterceptor, saveDisplayDeletion(_)).Times(1);
- EXPECT_CALL(*mEventThread,
- onHotplugReceived(static_cast<bool>(Case::Display::PRIMARY)
- ? EventThread::DisplayType::Primary
- : EventThread::DisplayType::External,
- false))
- .Times(1);
+
+ EXPECT_CALL(*mEventThread, onHotplugReceived(Case::Display::displayType(), false)).Times(1);
+ EXPECT_CALL(*mSFEventThread, onHotplugReceived(Case::Display::displayType(), false)).Times(1);
}
template <typename Case>
diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
index e6cdd02..79fb034 100644
--- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
+++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
@@ -104,6 +104,10 @@
createThread();
mConnection = createConnection(mConnectionEventCallRecorder);
+
+ // A display must be connected for VSYNC events to be delivered.
+ mThread->onHotplugReceived(EventThread::DisplayType::Primary, true);
+ expectHotplugEventReceivedByConnection(EventThread::DisplayType::Primary, true);
}
EventThreadTest::~EventThreadTest() {
@@ -207,6 +211,23 @@
EXPECT_FALSE(mConnectionEventCallRecorder.waitForCall(0us).has_value());
}
+TEST_F(EventThreadTest, vsyncRequestIsIgnoredIfDisplayIsDisconnected) {
+ mThread->onHotplugReceived(EventThread::DisplayType::Primary, false);
+ expectHotplugEventReceivedByConnection(EventThread::DisplayType::Primary, false);
+
+ // Signal that we want the next vsync event to be posted to the connection.
+ mThread->requestNextVsync(mConnection, false);
+
+ // EventThread should not enable vsync callbacks.
+ EXPECT_FALSE(mVSyncSetEnabledCallRecorder.waitForUnexpectedCall().has_value());
+
+ // Use the received callback to signal a vsync event.
+ // The event should not be received by the interceptor nor the connection.
+ mCallback->onVSyncEvent(123);
+ EXPECT_FALSE(mInterceptVSyncCallRecorder.waitForCall(0us).has_value());
+ EXPECT_FALSE(mConnectionEventCallRecorder.waitForCall(0us).has_value());
+}
+
TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) {
// Signal that we want the next vsync event to be posted to the connection
mThread->requestNextVsync(mConnection, false);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 2a8dda6..9c5c967 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -290,6 +290,7 @@
auto& mutableEventControlThread() { return mFlinger->mEventControlThread; }
auto& mutableEventQueue() { return mFlinger->mEventQueue; }
auto& mutableEventThread() { return mFlinger->mEventThread; }
+ auto& mutableSFEventThread() { return mFlinger->mSFEventThread; }
auto& mutableGeometryInvalid() { return mFlinger->mGeometryInvalid; }
auto& mutableHWVsyncAvailable() { return mFlinger->mHWVsyncAvailable; }
auto& mutableInterceptor() { return mFlinger->mInterceptor; }
@@ -317,6 +318,7 @@
mutableEventControlThread().reset();
mutableEventQueue().reset();
mutableEventThread().reset();
+ mutableSFEventThread().reset();
mutableInterceptor().reset();
mutablePrimaryDispSync().reset();
mFlinger->mCompositionEngine->setHwComposer(std::unique_ptr<HWComposer>());