Move hotplug processing to the main thread
Queue up all hotplug events for processing in the main thread, as part
of a display transaction (eDisplayTransactionNeeded).
This is needed so that everything done for each individual hotplug disconnect or
connect is done at the same time, such as creating and destroying the
corresponding DisplayDevice.
This fixes an issue with a hotplug disconnect event followed by an
immediate connect event for the same display not being handled properly.
Bug: 38464421
Test: Immediate disconnect/connect handled correctly.
Change-Id: I96266db8b02ffd6ad9eb4897d6c8510657775991
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a180606..1e0b67d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -618,6 +618,10 @@
"Starting with vr flinger active is not currently supported.");
getBE().mHwc.reset(new HWComposer(getBE().mHwcServiceName));
getBE().mHwc->registerCallback(this, getBE().mComposerSequenceId);
+ // Process any initial hotplug and resulting display changes.
+ processDisplayHotplugEventsLocked();
+ LOG_ALWAYS_FATAL_IF(!getBE().mHwc->isConnected(HWC_DISPLAY_PRIMARY),
+ "Registered composer callback but didn't create the default primary display");
if (useVrFlinger) {
auto vrFlingerRequestDisplayCallback = [this] (bool requestDisplay) {
@@ -1321,39 +1325,20 @@
"connected" : "disconnected",
primaryDisplay ? "primary" : "external");
+ // Ignore events that do not have the right sequenceId.
+ if (sequenceId != getBE().mComposerSequenceId) {
+ return;
+ }
+
// Only lock if we're not on the main thread. This function is normally
// called on a hwbinder thread, but for the primary display it's called on
// the main thread with the state lock already held, so don't attempt to
// acquire it here.
- ConditionalLock lock(mStateLock,
- std::this_thread::get_id() != mMainThreadId);
+ ConditionalLock lock(mStateLock, std::this_thread::get_id() != mMainThreadId);
- if (primaryDisplay) {
- getBE().mHwc->onHotplug(display, connection);
- if (!mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY].get()) {
- createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);
- }
- createDefaultDisplayDevice();
- } else {
- if (sequenceId != getBE().mComposerSequenceId) {
- return;
- }
- if (getBE().mHwc->isUsingVrComposer()) {
- ALOGE("External displays are not supported by the vr hardware composer.");
- return;
- }
- getBE().mHwc->onHotplug(display, connection);
- auto type = DisplayDevice::DISPLAY_EXTERNAL;
- if (connection == HWC2::Connection::Connected) {
- createBuiltinDisplayLocked(type);
- } else {
- mCurrentState.displays.removeItem(mBuiltinDisplays[type]);
- mBuiltinDisplays[type].clear();
- }
- setTransactionFlags(eDisplayTransactionNeeded);
+ mPendingHotplugEvents.emplace_back(HotplugEvent{display, connection, primaryDisplay});
- // Defer EventThread notification until SF has updated mDisplays.
- }
+ setTransactionFlags(eDisplayTransactionNeeded);
}
void SurfaceFlinger::onRefreshReceived(int sequenceId,
@@ -2112,6 +2097,35 @@
// here the transaction has been committed
}
+void SurfaceFlinger::processDisplayHotplugEventsLocked() {
+ for (const auto& event : mPendingHotplugEvents) {
+ DisplayDevice::DisplayType displayType = event.isPrimaryDisplay ?
+ DisplayDevice::DISPLAY_PRIMARY : DisplayDevice::DISPLAY_EXTERNAL;
+
+ if (getBE().mHwc->isUsingVrComposer() && displayType == DisplayDevice::DISPLAY_EXTERNAL) {
+ ALOGE("External displays are not supported by the vr hardware composer.");
+ continue;
+ }
+
+ getBE().mHwc->onHotplug(event.display, event.connection);
+
+ if (event.connection == HWC2::Connection::Connected) {
+ createBuiltinDisplayLocked(displayType);
+ } else {
+ mCurrentState.displays.removeItem(mBuiltinDisplays[displayType]);
+ mBuiltinDisplays[displayType].clear();
+ }
+
+ if (displayType == DisplayDevice::DISPLAY_PRIMARY) {
+ createDefaultDisplayDevice();
+ }
+
+ processDisplayChangesLocked();
+ }
+
+ mPendingHotplugEvents.clear();
+}
+
void SurfaceFlinger::processDisplayChangesLocked() {
// here we take advantage of Vector's copy-on-write semantics to
// improve performance by skipping the transaction entirely when
@@ -2253,6 +2267,8 @@
}
}
}
+
+ mDrawingState.displays = mCurrentState.displays;
}
void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
@@ -2284,6 +2300,7 @@
if (transactionFlags & eDisplayTransactionNeeded) {
processDisplayChangesLocked();
+ processDisplayHotplugEventsLocked();
}
if (transactionFlags & (eTraversalNeeded|eDisplayTransactionNeeded)) {