Fix frozen screen after exiting vr

When a hotplug connected event occurs, don't recreate any
already-existing entries in mBuiltinDisplays. This fixes a problem where
the screen would freeze after exiting vr.

In the future, we may want to use hotplug connected events to notify
surface flinger to reinitialize display state. This CL includes a change
to the code in HWC2.cpp to always reinitialize the display state when we
get a hotplug connected event, even if the display is already connected.

Bug: 74985350

Test: - Confirmed exiting vr no longer freezes the screen.

- Added code to simulate a hotplug connected event while the device is
  running, to test the changes in HWC2.cpp. Confirmed the device
  continued to function normally.

Change-Id: I6afda67cae84842b2568c773e6b5aa4f38df6a96
Merged-In: I6afda67cae84842b2568c773e6b5aa4f38df6a96

2 files changed, 33 insertions, 33 deletions

diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index b7bf964f29..47f4e46361 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -144,31 +144,31 @@ void Device::destroyDisplay(hwc2_display_t displayId)
 
 void Device::onHotplug(hwc2_display_t displayId, Connection connection) {
     if (connection == Connection::Connected) {
-        auto display = getDisplayById(displayId);
-        if (display) {
-            if (display->isConnected()) {
-                ALOGW("Attempt to hotplug connect display %" PRIu64
-                        " , which is already connected.", displayId);
-            } else {
-                display->setConnected(true);
-            }
-        } else {
-            DisplayType displayType;
-            auto intError = mComposer->getDisplayType(displayId,
-                    reinterpret_cast<Hwc2::IComposerClient::DisplayType *>(
-                            &displayType));
-            auto error = static_cast<Error>(intError);
-            if (error != Error::None) {
-                ALOGE("getDisplayType(%" PRIu64 ") failed: %s (%d). "
-                        "Aborting hotplug attempt.",
-                        displayId, to_string(error).c_str(), intError);
-                return;
-            }
+        // If we get a hotplug connected event for a display we already have,
+        // destroy the display and recreate it. This will force us to requery
+        // the display params and recreate all layers on that display.
+        auto oldDisplay = getDisplayById(displayId);
+        if (oldDisplay != nullptr && oldDisplay->isConnected()) {
+            ALOGI("Hotplug connecting an already connected display."
+                    " Clearing old display state.");
+        }
+        mDisplays.erase(displayId);
 
-            auto newDisplay = std::make_unique<Display>(
-                    *mComposer.get(), mCapabilities, displayId, displayType);
-            mDisplays.emplace(displayId, std::move(newDisplay));
+        DisplayType displayType;
+        auto intError = mComposer->getDisplayType(displayId,
+                reinterpret_cast<Hwc2::IComposerClient::DisplayType *>(
+                        &displayType));
+        auto error = static_cast<Error>(intError);
+        if (error != Error::None) {
+            ALOGE("getDisplayType(%" PRIu64 ") failed: %s (%d). "
+                    "Aborting hotplug attempt.",
+                    displayId, to_string(error).c_str(), intError);
+            return;
         }
+
+        auto newDisplay = std::make_unique<Display>(
+                *mComposer.get(), mCapabilities, displayId, displayType);
+        mDisplays.emplace(displayId, std::move(newDisplay));
     } else if (connection == Connection::Disconnected) {
         // The display will later be destroyed by a call to
         // destroyDisplay(). For now we just mark it disconnected.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 11658e8b49..7e02922405 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2120,16 +2120,16 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() {
         getBE().mHwc->onHotplug(event.display, displayType, event.connection);
 
         if (event.connection == HWC2::Connection::Connected) {
-            ALOGV("Creating built in display %d", displayType);
-            ALOGW_IF(mBuiltinDisplays[displayType],
-                    "Overwriting display token for display type %d", displayType);
-            mBuiltinDisplays[displayType] = new BBinder();
-            // All non-virtual displays are currently considered secure.
-            DisplayDeviceState info(displayType, true);
-            info.displayName = displayType == DisplayDevice::DISPLAY_PRIMARY ?
-                    "Built-in Screen" : "External Screen";
-            mCurrentState.displays.add(mBuiltinDisplays[displayType], info);
-            mInterceptor.saveDisplayCreation(info);
+            if (!mBuiltinDisplays[displayType].get()) {
+                ALOGV("Creating built in display %d", displayType);
+                mBuiltinDisplays[displayType] = new BBinder();
+                // All non-virtual displays are currently considered secure.
+                DisplayDeviceState info(displayType, true);
+                info.displayName = displayType == DisplayDevice::DISPLAY_PRIMARY ?
+                        "Built-in Screen" : "External Screen";
+                mCurrentState.displays.add(mBuiltinDisplays[displayType], info);
+                mInterceptor.saveDisplayCreation(info);
+            }
         } else {
             ALOGV("Removing built in display %d", displayType);