SF: Update display properties on hotplug connect
When a hotplug connect event for already connected display
is received, destroy and recreate the display device in SF,
then send a "hotplug connect" event to Display manager. This
way display properties will be updated.
Bug: 143451809
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFligner_test
Test: atest CompositionTest
Test: Manually on device:
2. adb shell dumpsys display
3. unplug primary display
4. plug another display
5. adb shell dumpsys display
Test: Manually on device:
1. disconnect and reconnect display
2. power off and on the screen
3. make sure the device didn't crash
Change-Id: I89996d9340f6570fa5ae0cc0977eaba7a2d3693c
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index fb6c817..397fedc 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -183,9 +183,10 @@
struct Physical {
DisplayId id;
DisplayConnectionType type;
+ hwc2_display_t hwcDisplayId;
bool operator==(const Physical& other) const {
- return id == other.id && type == other.type;
+ return id == other.id && type == other.type && hwcDisplayId == other.hwcDisplayId;
}
};
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8489adb..c69e106 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2353,7 +2353,8 @@
}
DisplayDeviceState state;
- state.physical = {displayId, getHwComposer().getDisplayConnectionType(displayId)};
+ state.physical = {displayId, getHwComposer().getDisplayConnectionType(displayId),
+ event.hwcDisplayId};
state.isSecure = true; // All physical displays are currently considered secure.
state.displayName = info->name;
@@ -2362,6 +2363,12 @@
mPhysicalDisplayTokens.emplace(displayId, std::move(token));
mInterceptor->saveDisplayCreation(state);
+ } else {
+ ALOGV("Recreating display %s", to_string(displayId).c_str());
+
+ const auto token = it->second;
+ auto& state = mCurrentState.displays.editValueFor(token);
+ state.sequenceId = DisplayDeviceState{}.sequenceId;
}
} else {
ALOGV("Removing display %s", to_string(displayId).c_str());
@@ -2539,20 +2546,17 @@
producer = bqProducer;
}
- if (displaySurface != nullptr) {
- mDisplays.emplace(displayToken,
- setupNewDisplayDeviceInternal(displayToken, compositionDisplay, state,
- displaySurface, producer));
- if (!state.isVirtual()) {
- LOG_ALWAYS_FATAL_IF(!displayId);
- dispatchDisplayHotplugEvent(displayId->value, true);
- }
+ LOG_FATAL_IF(!displaySurface);
+ const auto display = setupNewDisplayDeviceInternal(displayToken, compositionDisplay, state,
+ displaySurface, producer);
+ mDisplays.emplace(displayToken, display);
+ if (!state.isVirtual()) {
+ LOG_FATAL_IF(!displayId);
+ dispatchDisplayHotplugEvent(displayId->value, true);
+ }
- const auto displayDevice = mDisplays[displayToken];
- if (displayDevice->isPrimary()) {
- mScheduler->onPrimaryDisplayAreaChanged(displayDevice->getWidth() *
- displayDevice->getHeight());
- }
+ if (display->isPrimary()) {
+ mScheduler->onPrimaryDisplayAreaChanged(display->getWidth() * display->getHeight());
}
}
@@ -2563,7 +2567,7 @@
display->disconnect();
if (!display->isVirtual()) {
- LOG_ALWAYS_FATAL_IF(!displayId);
+ LOG_FATAL_IF(!displayId);
dispatchDisplayHotplugEvent(displayId->value, false);
}
}
@@ -2576,13 +2580,19 @@
const DisplayDeviceState& drawingState) {
const sp<IBinder> currentBinder = IInterface::asBinder(currentState.surface);
const sp<IBinder> drawingBinder = IInterface::asBinder(drawingState.surface);
- if (currentBinder != drawingBinder) {
+ if (currentBinder != drawingBinder || currentState.sequenceId != drawingState.sequenceId) {
// changing the surface is like destroying and recreating the DisplayDevice
if (const auto display = getDisplayDeviceLocked(displayToken)) {
display->disconnect();
}
mDisplays.erase(displayToken);
+ if (const auto& physical = currentState.physical) {
+ getHwComposer().allocatePhysicalDisplay(physical->hwcDisplayId, physical->id);
+ }
processDisplayAdded(displayToken, currentState);
+ if (currentState.physical) {
+ initializeDisplays();
+ }
return;
}
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 21c33ae..ad159b2 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -305,14 +305,14 @@
compositionengine::impl::createDisplay(test->mFlinger.getCompositionEngine(),
ceDisplayArgs);
- test->mDisplay =
- FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,
- DisplayConnectionType::Internal, true /* isPrimary */)
- .setDisplaySurface(test->mDisplaySurface)
- .setNativeWindow(test->mNativeWindow)
- .setSecure(Derived::IS_SECURE)
- .setPowerMode(Derived::INIT_POWER_MODE)
- .inject();
+ test->mDisplay = FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,
+ DisplayConnectionType::Internal, HWC_DISPLAY,
+ true /* isPrimary */)
+ .setDisplaySurface(test->mDisplaySurface)
+ .setNativeWindow(test->mNativeWindow)
+ .setSecure(Derived::IS_SECURE)
+ .setPowerMode(Derived::INIT_POWER_MODE)
+ .inject();
Mock::VerifyAndClear(test->mNativeWindow);
test->mDisplay->setLayerStack(DEFAULT_LAYER_STACK);
}
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 6d00ccc..24eeac7 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -246,6 +246,7 @@
constexpr DisplayId DEFAULT_DISPLAY_ID = DisplayId{777};
constexpr int DEFAULT_DISPLAY_WIDTH = 1080;
constexpr int DEFAULT_DISPLAY_HEIGHT = 1920;
+ constexpr hwc2_display_t DEFAULT_DISPLAY_HWC_DISPLAY_ID = 0;
// The DisplayDevice is required to have a framebuffer (behind the
// ANativeWindow interface) which uses the actual hardware display
@@ -270,7 +271,7 @@
auto injector =
FakeDisplayDeviceInjector(mFlinger, compositionDisplay, DisplayConnectionType::Internal,
- true /* isPrimary */);
+ DEFAULT_DISPLAY_HWC_DISPLAY_ID, true /* isPrimary */);
injector.setNativeWindow(mNativeWindow);
if (injectExtra) {
@@ -373,6 +374,23 @@
static constexpr std::optional<DisplayConnectionType> value = PhysicalDisplay::CONNECTION_TYPE;
};
+template <typename>
+struct HwcDisplayIdGetter {
+ static constexpr std::optional<hwc2_display_t> value;
+};
+
+constexpr hwc2_display_t HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID = 1010;
+
+template <DisplayId::Type displayId>
+struct HwcDisplayIdGetter<VirtualDisplayId<displayId>> {
+ static constexpr std::optional<hwc2_display_t> value = HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID;
+};
+
+template <typename PhysicalDisplay>
+struct HwcDisplayIdGetter<PhysicalDisplayId<PhysicalDisplay>> {
+ static constexpr std::optional<hwc2_display_t> value = PhysicalDisplay::HWC_DISPLAY_ID;
+};
+
// DisplayIdType can be:
// 1) PhysicalDisplayId<...> for generated ID of physical display backed by HWC.
// 2) VirtualDisplayId<...> for hard-coded ID of virtual display backed by HWC.
@@ -382,6 +400,7 @@
struct DisplayVariant {
using DISPLAY_ID = DisplayIdGetter<DisplayIdType>;
using CONNECTION_TYPE = DisplayConnectionTypeGetter<DisplayIdType>;
+ using HWC_DISPLAY_ID_OPT = HwcDisplayIdGetter<DisplayIdType>;
// The display width and height
static constexpr int WIDTH = width;
@@ -418,9 +437,9 @@
compositionengine::impl::createDisplay(test->mFlinger.getCompositionEngine(),
ceDisplayArgs.build());
- auto injector =
- FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,
- CONNECTION_TYPE::value, static_cast<bool>(PRIMARY));
+ auto injector = FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay,
+ CONNECTION_TYPE::value, HWC_DISPLAY_ID_OPT::value,
+ static_cast<bool>(PRIMARY));
injector.setSecure(static_cast<bool>(SECURE));
injector.setNativeWindow(test->mNativeWindow);
@@ -603,12 +622,11 @@
constexpr uint32_t GRALLOC_USAGE_PHYSICAL_DISPLAY =
GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_FB;
-template <hwc2_display_t hwcDisplayId, typename PhysicalDisplay, int width, int height,
- Critical critical>
+template <typename PhysicalDisplay, int width, int height, Critical critical>
struct PhysicalDisplayVariant
: DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height, critical, Async::FALSE,
Secure::TRUE, PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,
- HwcDisplayVariant<hwcDisplayId, HWC2::DisplayType::Physical,
+ HwcDisplayVariant<PhysicalDisplay::HWC_DISPLAY_ID, HWC2::DisplayType::Physical,
DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height,
critical, Async::FALSE, Secure::TRUE,
PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,
@@ -619,6 +637,7 @@
static constexpr auto CONNECTION_TYPE = DisplayConnectionType::Internal;
static constexpr Primary PRIMARY = Primary::TRUE;
static constexpr uint8_t PORT = 255;
+ static constexpr hwc2_display_t HWC_DISPLAY_ID = 1001;
static constexpr bool HAS_IDENTIFICATION_DATA = hasIdentificationData;
static constexpr auto GET_IDENTIFICATION_DATA = getInternalEdid;
};
@@ -628,6 +647,7 @@
static constexpr auto CONNECTION_TYPE = DisplayConnectionType::External;
static constexpr Primary PRIMARY = Primary::FALSE;
static constexpr uint8_t PORT = 254;
+ static constexpr hwc2_display_t HWC_DISPLAY_ID = 1002;
static constexpr bool HAS_IDENTIFICATION_DATA = hasIdentificationData;
static constexpr auto GET_IDENTIFICATION_DATA = getExternalEdid;
};
@@ -635,19 +655,19 @@
struct TertiaryDisplay {
static constexpr Primary PRIMARY = Primary::FALSE;
static constexpr uint8_t PORT = 253;
+ static constexpr hwc2_display_t HWC_DISPLAY_ID = 1003;
static constexpr auto GET_IDENTIFICATION_DATA = getExternalEdid;
};
// A primary display is a physical display that is critical
using PrimaryDisplayVariant =
- PhysicalDisplayVariant<1001, PrimaryDisplay<false>, 3840, 2160, Critical::TRUE>;
+ PhysicalDisplayVariant<PrimaryDisplay<false>, 3840, 2160, Critical::TRUE>;
// An external display is physical display that is not critical.
using ExternalDisplayVariant =
- PhysicalDisplayVariant<1002, ExternalDisplay<false>, 1920, 1280, Critical::FALSE>;
+ PhysicalDisplayVariant<ExternalDisplay<false>, 1920, 1280, Critical::FALSE>;
-using TertiaryDisplayVariant =
- PhysicalDisplayVariant<1003, TertiaryDisplay, 1600, 1200, Critical::FALSE>;
+using TertiaryDisplayVariant = PhysicalDisplayVariant<TertiaryDisplay, 1600, 1200, Critical::FALSE>;
// A virtual display not supported by the HWC.
constexpr uint32_t GRALLOC_USAGE_NONHWC_VIRTUAL_DISPLAY = 0;
@@ -696,7 +716,7 @@
: DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE, secure,
Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>,
HwcDisplayVariant<
- 1010, HWC2::DisplayType::Virtual,
+ HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID, HWC2::DisplayType::Virtual,
DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,
secure, Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>> {
using Base = DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,
@@ -1767,7 +1787,9 @@
if (const auto connectionType = Case::Display::CONNECTION_TYPE::value) {
const auto displayId = Case::Display::DISPLAY_ID::get();
ASSERT_TRUE(displayId);
- state.physical = {*displayId, *connectionType};
+ const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
+ ASSERT_TRUE(hwcDisplayId);
+ state.physical = {*displayId, *connectionType, *hwcDisplayId};
}
state.isSecure = static_cast<bool>(Case::Display::SECURE);
@@ -1941,7 +1963,9 @@
if (const auto connectionType = Case::Display::CONNECTION_TYPE::value) {
const auto displayId = Case::Display::DISPLAY_ID::get();
ASSERT_TRUE(displayId);
- expectedPhysical = {*displayId, *connectionType};
+ const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
+ ASSERT_TRUE(hwcDisplayId);
+ expectedPhysical = {*displayId, *connectionType, *hwcDisplayId};
}
// The display should have been set up in the current display state
@@ -2124,11 +2148,11 @@
ignoresHotplugConnectCommon<SimpleExternalDisplayCase>();
}
-TEST_F(HandleTransactionLockedTest, processHotplugDisconnectPrimaryDisplay) {
+TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectPrimaryDisplay) {
processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>();
}
-TEST_F(HandleTransactionLockedTest, processHotplugDisconnectExternalDisplay) {
+TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectExternalDisplay) {
processesHotplugDisconnectCommon<SimpleExternalDisplayCase>();
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 53525f3..fdfec4e 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -549,9 +549,10 @@
FakeDisplayDeviceInjector(TestableSurfaceFlinger& flinger,
std::shared_ptr<compositionengine::Display> compositionDisplay,
std::optional<DisplayConnectionType> connectionType,
- bool isPrimary)
+ std::optional<hwc2_display_t> hwcDisplayId, bool isPrimary)
: mFlinger(flinger),
- mCreationArgs(flinger.mFlinger.get(), mDisplayToken, compositionDisplay) {
+ mCreationArgs(flinger.mFlinger.get(), mDisplayToken, compositionDisplay),
+ mHwcDisplayId(hwcDisplayId) {
mCreationArgs.connectionType = connectionType;
mCreationArgs.isPrimary = isPrimary;
}
@@ -619,7 +620,8 @@
DisplayDeviceState state;
if (const auto type = mCreationArgs.connectionType) {
LOG_ALWAYS_FATAL_IF(!displayId);
- state.physical = {*displayId, *type};
+ LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
+ state.physical = {*displayId, *type, *mHwcDisplayId};
}
state.isSecure = mCreationArgs.isSecure;
@@ -640,6 +642,7 @@
TestableSurfaceFlinger& mFlinger;
sp<BBinder> mDisplayToken = new BBinder();
DisplayDeviceCreationArgs mCreationArgs;
+ const std::optional<hwc2_display_t> mHwcDisplayId;
};
surfaceflinger::test::Factory mFactory;