SF: Remove obsolete HWComposer members
Remove mExternalHwcDisplayId, an obsolete special case for legacy multi-
display mode. Assert primary display existence instead of propagating to
callers, which is closer to how headless mode would work, i.e. injecting
a placeholder primary display. Remove isVirtual checks now that they are
enforced at compile time. Prevent primary display disconnection for now.
Bug: 182939859
Test: libsurfaceflinger_unittest
Change-Id: I507db6a3ac0bda93005a86b38cca6ebbcd5c0155
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 58c6628..224dad1 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -111,11 +111,15 @@
MOCK_CONST_METHOD1(dump, void(std::string&));
MOCK_CONST_METHOD0(getComposer, android::Hwc2::Composer*());
- MOCK_CONST_METHOD1(getHwcDisplayId, std::optional<hal::HWDisplayId>(int32_t));
- MOCK_CONST_METHOD0(getInternalHwcDisplayId, std::optional<hal::HWDisplayId>());
- MOCK_CONST_METHOD0(getExternalHwcDisplayId, std::optional<hal::HWDisplayId>());
- MOCK_CONST_METHOD1(toPhysicalDisplayId, std::optional<PhysicalDisplayId>(hal::HWDisplayId));
- MOCK_CONST_METHOD1(fromPhysicalDisplayId, std::optional<hal::HWDisplayId>(PhysicalDisplayId));
+
+ MOCK_METHOD(hal::HWDisplayId, getPrimaryHwcDisplayId, (), (const, override));
+ MOCK_METHOD(PhysicalDisplayId, getPrimaryDisplayId, (), (const, override));
+ MOCK_METHOD(bool, isHeadless, (), (const, override));
+
+ MOCK_METHOD(std::optional<PhysicalDisplayId>, toPhysicalDisplayId, (hal::HWDisplayId),
+ (const, override));
+ MOCK_METHOD(std::optional<hal::HWDisplayId>, fromPhysicalDisplayId, (PhysicalDisplayId),
+ (const, override));
};
} // namespace mock
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index afd12b5..7db87d9 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -273,7 +273,7 @@
std::atomic<nsecs_t> mLastHwVsync = 0;
- // TODO(b/74619554): Remove special cases for primary display.
+ // TODO(b/182939859): Remove special cases for primary display.
const bool mIsPrimary;
uint32_t mFlags = 0;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index a790b4c..b0443f7 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -212,8 +212,6 @@
RETURN_IF_INVALID_DISPLAY(*displayId, false);
auto& displayData = mDisplayData[*displayId];
- LOG_FATAL_IF(displayData.isVirtual, "%s: Invalid operation on virtual display with ID %s",
- __FUNCTION__, to_string(*displayId).c_str());
{
// There have been reports of HWCs that signal several vsync events
@@ -271,7 +269,6 @@
display->setConnected(true);
auto& displayData = mDisplayData[displayId];
displayData.hwcDisplay = std::move(display);
- displayData.isVirtual = true;
return true;
}
@@ -279,10 +276,8 @@
PhysicalDisplayId displayId) {
mPhysicalDisplayIdMap[hwcDisplayId] = displayId;
- if (!mInternalHwcDisplayId) {
- mInternalHwcDisplayId = hwcDisplayId;
- } else if (mInternalHwcDisplayId != hwcDisplayId && !mExternalHwcDisplayId) {
- mExternalHwcDisplayId = hwcDisplayId;
+ if (!mPrimaryHwcDisplayId) {
+ mPrimaryHwcDisplayId = hwcDisplayId;
}
auto& displayData = mDisplayData[displayId];
@@ -372,7 +367,7 @@
ui::DisplayConnectionType type;
const auto error = hwcDisplay->getConnectionType(&type);
- const auto FALLBACK_TYPE = hwcDisplay->getId() == mInternalHwcDisplayId
+ const auto FALLBACK_TYPE = hwcDisplay->getId() == mPrimaryHwcDisplayId
? ui::DisplayConnectionType::Internal
: ui::DisplayConnectionType::External;
@@ -428,9 +423,6 @@
RETURN_IF_INVALID_DISPLAY(displayId);
auto& displayData = mDisplayData[displayId];
- LOG_FATAL_IF(displayData.isVirtual, "%s: Invalid operation on virtual display with ID %s",
- __FUNCTION__, to_string(displayId).c_str());
-
// NOTE: we use our own internal lock here because we have to call
// into the HWC with the lock held, and we want to make sure
// that even if HWC blocks (which it shouldn't), it won't
@@ -597,14 +589,11 @@
status_t HWComposer::setPowerMode(PhysicalDisplayId displayId, hal::PowerMode mode) {
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
- const auto& displayData = mDisplayData[displayId];
- LOG_FATAL_IF(displayData.isVirtual, "%s: Invalid operation on virtual display with ID %s",
- __FUNCTION__, to_string(displayId).c_str());
-
if (mode == hal::PowerMode::OFF) {
setVsyncEnabled(displayId, hal::Vsync::DISABLE);
}
+ const auto& displayData = mDisplayData[displayId];
auto& hwcDisplay = displayData.hwcDisplay;
switch (mode) {
case hal::PowerMode::OFF:
@@ -677,13 +666,8 @@
RETURN_IF_INVALID_DISPLAY(displayId);
auto& displayData = mDisplayData[displayId];
const auto hwcDisplayId = displayData.hwcDisplay->getId();
+ LOG_ALWAYS_FATAL_IF(hwcDisplayId == mPrimaryHwcDisplayId);
- // TODO(b/74619554): Select internal/external display from remaining displays.
- if (hwcDisplayId == mInternalHwcDisplayId) {
- mInternalHwcDisplayId.reset();
- } else if (hwcDisplayId == mExternalHwcDisplayId) {
- mExternalHwcDisplayId.reset();
- }
mPhysicalDisplayIdMap.erase(hwcDisplayId);
mDisplayData.erase(displayId);
}
@@ -693,9 +677,6 @@
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
const auto& displayData = mDisplayData[displayId];
- LOG_FATAL_IF(!displayData.isVirtual, "%s: Invalid operation on physical display with ID %s",
- __FUNCTION__, to_string(displayId).c_str());
-
auto error = displayData.hwcDisplay->setOutputBuffer(buffer, acquireFence);
RETURN_IF_HWC_ERROR(error, displayId, UNKNOWN_ERROR);
return NO_ERROR;
@@ -853,8 +834,7 @@
std::optional<hal::HWDisplayId> HWComposer::fromPhysicalDisplayId(
PhysicalDisplayId displayId) const {
- if (const auto it = mDisplayData.find(displayId);
- it != mDisplayData.end() && !it->second.isVirtual) {
+ if (const auto it = mDisplayData.find(displayId); it != mDisplayData.end()) {
return it->second.hwcDisplay->getId();
}
return {};
@@ -868,7 +848,8 @@
return true;
}
- if (!mHasMultiDisplaySupport && mInternalHwcDisplayId && mExternalHwcDisplayId) {
+ // Legacy mode only supports IDs LEGACY_DISPLAY_TYPE_PRIMARY and LEGACY_DISPLAY_TYPE_EXTERNAL.
+ if (!mHasMultiDisplaySupport && mPhysicalDisplayIdMap.size() == 2) {
ALOGE("Ignoring connection of tertiary display %" PRIu64, hwcDisplayId);
return true;
}
@@ -909,7 +890,7 @@
}
info = [this, hwcDisplayId, &port, &data, hasDisplayIdentificationData] {
- const bool isPrimary = !mInternalHwcDisplayId;
+ const bool isPrimary = !mPrimaryHwcDisplayId;
if (mHasMultiDisplaySupport) {
if (const auto info = parseDisplayIdentificationData(port, data)) {
return *info;
@@ -922,8 +903,8 @@
}
return DisplayIdentificationInfo{.id = PhysicalDisplayId::fromPort(port),
- .name = isPrimary ? "Internal display"
- : "External display",
+ .name = isPrimary ? "Primary display"
+ : "Secondary display",
.deviceProductInfo = std::nullopt};
}();
}
@@ -936,9 +917,12 @@
std::optional<DisplayIdentificationInfo> HWComposer::onHotplugDisconnect(
hal::HWDisplayId hwcDisplayId) {
+ LOG_ALWAYS_FATAL_IF(hwcDisplayId == mPrimaryHwcDisplayId,
+ "Primary display cannot be disconnected.");
+
const auto displayId = toPhysicalDisplayId(hwcDisplayId);
if (!displayId) {
- ALOGE("Ignoring disconnection of invalid HWC display %" PRIu64, hwcDisplayId);
+ LOG_HWC_DISPLAY_ERROR(hwcDisplayId, "Invalid HWC display");
return {};
}
@@ -947,7 +931,7 @@
if (isConnected(*displayId)) {
mDisplayData[*displayId].hwcDisplay->setConnected(false);
} else {
- ALOGW("Attempted to disconnect unknown display %" PRIu64, hwcDisplayId);
+ LOG_HWC_DISPLAY_ERROR(hwcDisplayId, "Already disconnected");
}
// The cleanup of Disconnect is handled through HWComposer::disconnectDisplay
// via SurfaceFlinger's onHotplugReceived callback handling
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 49f96d9..0a090da 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef ANDROID_SF_HWCOMPOSER_H
-#define ANDROID_SF_HWCOMPOSER_H
+#pragma once
#include <cstdint>
#include <future>
@@ -228,14 +227,18 @@
virtual const std::unordered_map<std::string, bool>& getSupportedLayerGenericMetadata()
const = 0;
- // for debugging ----------------------------------------------------------
virtual void dump(std::string& out) const = 0;
virtual Hwc2::Composer* getComposer() const = 0;
- // TODO(b/74619554): Remove special cases for internal/external display.
- virtual std::optional<hal::HWDisplayId> getInternalHwcDisplayId() const = 0;
- virtual std::optional<hal::HWDisplayId> getExternalHwcDisplayId() const = 0;
+ // Returns the first display connected at boot. It cannot be disconnected, which implies an
+ // internal connection type. Its connection via HWComposer::onHotplug, which in practice is
+ // immediately after HWComposer construction, must occur before any call to this function.
+ //
+ // TODO(b/182939859): Remove special cases for primary display.
+ virtual hal::HWDisplayId getPrimaryHwcDisplayId() const = 0;
+ virtual PhysicalDisplayId getPrimaryDisplayId() const = 0;
+ virtual bool isHeadless() const = 0;
virtual std::optional<PhysicalDisplayId> toPhysicalDisplayId(hal::HWDisplayId) const = 0;
virtual std::optional<hal::HWDisplayId> fromPhysicalDisplayId(PhysicalDisplayId) const = 0;
@@ -366,14 +369,19 @@
Hwc2::Composer* getComposer() const override { return mComposer.get(); }
- // TODO(b/74619554): Remove special cases for internal/external display.
- std::optional<hal::HWDisplayId> getInternalHwcDisplayId() const override {
- return mInternalHwcDisplayId;
+ hal::HWDisplayId getPrimaryHwcDisplayId() const override {
+ LOG_ALWAYS_FATAL_IF(!mPrimaryHwcDisplayId, "Missing HWC primary display");
+ return *mPrimaryHwcDisplayId;
}
- std::optional<hal::HWDisplayId> getExternalHwcDisplayId() const override {
- return mExternalHwcDisplayId;
+
+ PhysicalDisplayId getPrimaryDisplayId() const override {
+ const auto id = toPhysicalDisplayId(getPrimaryHwcDisplayId());
+ LOG_ALWAYS_FATAL_IF(!id, "Missing primary display");
+ return *id;
}
+ virtual bool isHeadless() const override { return !mPrimaryHwcDisplayId; }
+
std::optional<PhysicalDisplayId> toPhysicalDisplayId(hal::HWDisplayId) const override;
std::optional<hal::HWDisplayId> fromPhysicalDisplayId(PhysicalDisplayId) const override;
@@ -382,12 +390,9 @@
friend TestableSurfaceFlinger;
struct DisplayData {
- bool isVirtual = false;
std::unique_ptr<HWC2::Display> hwcDisplay;
sp<Fence> lastPresentFence = Fence::NO_FENCE; // signals when the last set op retires
std::unordered_map<HWC2::Layer*, sp<Fence>> releaseFences;
- buffer_handle_t outbufHandle = nullptr;
- sp<Fence> outbufAcquireFence = Fence::NO_FENCE;
bool validateWasSkipped;
hal::Error presentError;
@@ -418,8 +423,7 @@
bool mRegisteredCallback = false;
std::unordered_map<hal::HWDisplayId, PhysicalDisplayId> mPhysicalDisplayIdMap;
- std::optional<hal::HWDisplayId> mInternalHwcDisplayId;
- std::optional<hal::HWDisplayId> mExternalHwcDisplayId;
+ std::optional<hal::HWDisplayId> mPrimaryHwcDisplayId;
bool mHasMultiDisplaySupport = false;
const size_t mMaxVirtualDisplayDimension;
@@ -428,5 +432,3 @@
} // namespace impl
} // namespace android
-
-#endif // ANDROID_SF_HWCOMPOSER_H
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 294a67e..e306152 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -633,17 +633,14 @@
}
std::vector<PhysicalDisplayId> SurfaceFlinger::getPhysicalDisplayIdsLocked() const {
- const auto internalDisplayId = getInternalDisplayIdLocked();
- if (!internalDisplayId) {
- return {};
- }
-
std::vector<PhysicalDisplayId> displayIds;
displayIds.reserve(mPhysicalDisplayTokens.size());
- displayIds.push_back(*internalDisplayId);
+
+ const auto internalDisplayId = getInternalDisplayIdLocked();
+ displayIds.push_back(internalDisplayId);
for (const auto& [id, token] : mPhysicalDisplayTokens) {
- if (id != *internalDisplayId) {
+ if (id != internalDisplayId) {
displayIds.push_back(id);
}
}
@@ -810,10 +807,10 @@
// Process any initial hotplug and resulting display changes.
processDisplayHotplugEventsLocked();
const auto display = getDefaultDisplayDeviceLocked();
- LOG_ALWAYS_FATAL_IF(!display, "Missing internal display after registering composer callback.");
+ LOG_ALWAYS_FATAL_IF(!display, "Missing primary display after registering composer callback.");
const auto displayId = display->getPhysicalId();
LOG_ALWAYS_FATAL_IF(!getHwComposer().isConnected(displayId),
- "Internal display is disconnected.");
+ "Primary display is disconnected.");
// initialize our drawing state
mDrawingState = mCurrentState;
@@ -982,6 +979,11 @@
return NAME_NOT_FOUND;
}
+ const auto displayId = PhysicalDisplayId::tryCast(display->getId());
+ if (!displayId) {
+ return INVALID_OPERATION;
+ }
+
info->activeDisplayModeId = static_cast<int32_t>(display->getActiveMode()->getId().value());
const auto& supportedModes = display->getSupportedModes();
@@ -1041,18 +1043,18 @@
}
info->activeColorMode = display->getCompositionDisplay()->getState().colorMode;
- const auto displayId = display->getPhysicalId();
- info->supportedColorModes = getDisplayColorModes(displayId);
-
+ info->supportedColorModes = getDisplayColorModes(*display);
info->hdrCapabilities = display->getHdrCapabilities();
+
info->autoLowLatencyModeSupported =
- getHwComposer().hasDisplayCapability(displayId,
+ getHwComposer().hasDisplayCapability(*displayId,
hal::DisplayCapability::AUTO_LOW_LATENCY_MODE);
std::vector<hal::ContentType> types;
- getHwComposer().getSupportedContentTypes(displayId, &types);
+ getHwComposer().getSupportedContentTypes(*displayId, &types);
info->gameContentTypeSupported = std::any_of(types.begin(), types.end(), [](auto type) {
return type == hal::ContentType::GAME;
});
+
return NO_ERROR;
}
@@ -1276,15 +1278,15 @@
}).wait();
}
-std::vector<ColorMode> SurfaceFlinger::getDisplayColorModes(PhysicalDisplayId displayId) {
- auto modes = getHwComposer().getColorModes(displayId);
- bool isInternalDisplay = displayId == getInternalDisplayIdLocked();
+std::vector<ColorMode> SurfaceFlinger::getDisplayColorModes(const DisplayDevice& display) {
+ auto modes = getHwComposer().getColorModes(display.getPhysicalId());
- // If it's built-in display and the configuration claims it's not wide color capable,
+ // If the display is internal and the configuration claims it's not wide color capable,
// filter out all wide color modes. The typical reason why this happens is that the
// hardware is not good enough to support GPU composition of wide color, and thus the
// OEMs choose to disable this capability.
- if (isInternalDisplay && !hasWideColorDisplay) {
+ if (display.getConnectionType() == ui::DisplayConnectionType::Internal &&
+ !hasWideColorDisplay) {
const auto newEnd = std::remove_if(modes.begin(), modes.end(), isWideColorMode);
modes.erase(newEnd, modes.end());
}
@@ -1308,35 +1310,40 @@
}
status_t SurfaceFlinger::setActiveColorMode(const sp<IBinder>& displayToken, ColorMode mode) {
- schedule([=]() MAIN_THREAD {
- const auto displayId = getPhysicalDisplayIdLocked(displayToken);
- if (!displayId) {
- ALOGE("Invalid display token %p", displayToken.get());
- return;
- }
- const auto modes = getDisplayColorModes(*displayId);
- bool exists = std::find(std::begin(modes), std::end(modes), mode) != std::end(modes);
- if (mode < ColorMode::NATIVE || !exists) {
- ALOGE("Attempt to set invalid active color mode %s (%d) for display token %p",
- decodeColorMode(mode).c_str(), mode, displayToken.get());
- return;
- }
+ if (!displayToken) {
+ return BAD_VALUE;
+ }
+
+ auto future = schedule([=]() MAIN_THREAD -> status_t {
const auto display = getDisplayDeviceLocked(displayToken);
if (!display) {
ALOGE("Attempt to set active color mode %s (%d) for invalid display token %p",
decodeColorMode(mode).c_str(), mode, displayToken.get());
- } else if (display->isVirtual()) {
+ return NAME_NOT_FOUND;
+ }
+
+ if (display->isVirtual()) {
ALOGW("Attempt to set active color mode %s (%d) for virtual display",
decodeColorMode(mode).c_str(), mode);
- } else {
- display->getCompositionDisplay()->setColorProfile(
- compositionengine::Output::ColorProfile{mode, Dataspace::UNKNOWN,
- RenderIntent::COLORIMETRIC,
- Dataspace::UNKNOWN});
+ return INVALID_OPERATION;
}
- }).wait();
- return NO_ERROR;
+ const auto modes = getDisplayColorModes(*display);
+ const bool exists = std::find(modes.begin(), modes.end(), mode) != modes.end();
+
+ if (mode < ColorMode::NATIVE || !exists) {
+ ALOGE("Attempt to set invalid active color mode %s (%d) for display token %p",
+ decodeColorMode(mode).c_str(), mode, displayToken.get());
+ return BAD_VALUE;
+ }
+
+ display->getCompositionDisplay()->setColorProfile(
+ {mode, Dataspace::UNKNOWN, RenderIntent::COLORIMETRIC, Dataspace::UNKNOWN});
+
+ return NO_ERROR;
+ });
+
+ return future.get();
}
void SurfaceFlinger::setAutoLowLatencyMode(const sp<IBinder>& displayToken, bool on) {
@@ -1749,8 +1756,8 @@
void SurfaceFlinger::onComposerHalHotplug(hal::HWDisplayId hwcDisplayId,
hal::Connection connection) {
- ALOGI("%s(%" PRIu64 ", %s)", __func__, hwcDisplayId,
- connection == hal::Connection::CONNECTED ? "connected" : "disconnected");
+ const bool connected = connection == hal::Connection::CONNECTED;
+ ALOGI("%s HAL display %" PRIu64, connected ? "Connecting" : "Disconnecting", hwcDisplayId);
// 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
@@ -1942,13 +1949,9 @@
}
if (mRefreshRateOverlaySpinner) {
- if (Mutex::Autolock lock(mStateLock);
- const auto display = getDefaultDisplayDeviceLocked()) {
- if (display) {
- display->onInvalidate();
- } else {
- ALOGW("%s: default display is null", __func__);
- }
+ Mutex::Autolock lock(mStateLock);
+ if (const auto display = getDefaultDisplayDeviceLocked()) {
+ display->onInvalidate();
}
}
@@ -2842,7 +2845,7 @@
setPowerModeInternal(display, hal::PowerMode::ON);
// TODO(b/175678251) Call a listener instead.
- if (currentState.physical->hwcDisplayId == getHwComposer().getInternalHwcDisplayId()) {
+ if (currentState.physical->hwcDisplayId == getHwComposer().getPrimaryHwcDisplayId()) {
updateInternalDisplayVsyncLocked(display);
}
}
@@ -5687,12 +5690,10 @@
// Inject a hotplug connected event for the primary display. This will deallocate and
// reallocate the display state including framebuffers.
case 1037: {
- std::optional<hal::HWDisplayId> hwcId;
- {
- Mutex::Autolock lock(mStateLock);
- hwcId = getHwComposer().getInternalHwcDisplayId();
- }
- onComposerHalHotplug(*hwcId, hal::Connection::CONNECTED);
+ const hal::HWDisplayId hwcId =
+ (Mutex::Autolock(mStateLock), getHwComposer().getPrimaryHwcDisplayId());
+
+ onComposerHalHotplug(hwcId, hal::Connection::CONNECTED);
return NO_ERROR;
}
// Modify the max number of display frames stored within FrameTimeline
@@ -6774,33 +6775,29 @@
}
status_t SurfaceFlinger::getMaxAcquiredBufferCount(int* buffers) const {
- const auto maxSupportedRefreshRate = [&] {
- const auto display = getDefaultDisplayDevice();
- if (display) {
- return display->refreshRateConfigs().getSupportedRefreshRateRange().max;
+ Fps maxRefreshRate(60.f);
+
+ if (!getHwComposer().isHeadless()) {
+ if (const auto display = getDefaultDisplayDevice()) {
+ maxRefreshRate = display->refreshRateConfigs().getSupportedRefreshRateRange().max;
}
- ALOGW("%s: default display is null", __func__);
- return Fps(60);
- }();
- *buffers = getMaxAcquiredBufferCountForRefreshRate(maxSupportedRefreshRate);
+ }
+
+ *buffers = getMaxAcquiredBufferCountForRefreshRate(maxRefreshRate);
return NO_ERROR;
}
int SurfaceFlinger::getMaxAcquiredBufferCountForCurrentRefreshRate(uid_t uid) const {
- const auto refreshRate = [&] {
- const auto frameRateOverride = mScheduler->getFrameRateOverride(uid);
- if (frameRateOverride.has_value()) {
- return frameRateOverride.value();
- }
+ Fps refreshRate(60.f);
- const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked());
- if (display) {
- return display->refreshRateConfigs().getCurrentRefreshRate().getFps();
+ if (const auto frameRateOverride = mScheduler->getFrameRateOverride(uid)) {
+ refreshRate = *frameRateOverride;
+ } else if (!getHwComposer().isHeadless()) {
+ if (const auto display = ON_MAIN_THREAD(getDefaultDisplayDeviceLocked())) {
+ refreshRate = display->refreshRateConfigs().getCurrentRefreshRate().getFps();
}
+ }
- ALOGW("%s: default display is null", __func__);
- return Fps(60);
- }();
return getMaxAcquiredBufferCountForRefreshRate(refreshRate);
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 241d319..7a66aca 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1113,15 +1113,19 @@
return {};
}
- // TODO(b/74619554): Remove special cases for primary display.
+ // TODO(b/182939859): SF conflates the primary (a.k.a. default) display with the first display
+ // connected at boot, which is typically internal. (Theoretically, it must be internal because
+ // SF does not support disconnecting it, though in practice HWC may circumvent this limitation.)
+ //
+ // SF inherits getInternalDisplayToken and getInternalDisplayId from ISurfaceComposer, so these
+ // locked counterparts are named consistently. Once SF supports headless mode and can designate
+ // any display as primary, the "internal" misnomer will be phased out.
sp<IBinder> getInternalDisplayTokenLocked() const REQUIRES(mStateLock) {
- const auto displayId = getInternalDisplayIdLocked();
- return displayId ? getPhysicalDisplayTokenLocked(*displayId) : nullptr;
+ return getPhysicalDisplayTokenLocked(getInternalDisplayIdLocked());
}
- std::optional<PhysicalDisplayId> getInternalDisplayIdLocked() const REQUIRES(mStateLock) {
- const auto hwcDisplayId = getHwComposer().getInternalHwcDisplayId();
- return hwcDisplayId ? getHwComposer().toPhysicalDisplayId(*hwcDisplayId) : std::nullopt;
+ PhysicalDisplayId getInternalDisplayIdLocked() const REQUIRES(mStateLock) {
+ return getHwComposer().getPrimaryDisplayId();
}
// Toggles use of HAL/GPU virtual displays.
@@ -1200,8 +1204,7 @@
/*
* Misc
*/
- std::vector<ui::ColorMode> getDisplayColorModes(PhysicalDisplayId displayId)
- REQUIRES(mStateLock);
+ std::vector<ui::ColorMode> getDisplayColorModes(const DisplayDevice&) REQUIRES(mStateLock);
static int calculateMaxAcquiredBufferCount(Fps refreshRate,
std::chrono::nanoseconds presentLatency);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
index b713334..313ab03 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
@@ -282,7 +282,8 @@
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectPrimaryDisplay) {
- processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>();
+ EXPECT_EXIT(processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectExternalDisplay) {
@@ -290,96 +291,106 @@
}
TEST_F(HandleTransactionLockedTest, processesHotplugConnectThenDisconnectPrimary) {
- using Case = SimplePrimaryDisplayCase;
+ EXPECT_EXIT(
+ [this] {
+ using Case = SimplePrimaryDisplayCase;
- // --------------------------------------------------------------------
- // Preconditions
+ // --------------------------------------------------------------------
+ // Preconditions
- setupCommonPreconditions<Case>();
+ setupCommonPreconditions<Case>();
- // A hotplug connect event is enqueued for a display
- Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
- // A hotplug disconnect event is also enqueued for the same display
- Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
+ // A hotplug connect event is enqueued for a display
+ Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
+ // A hotplug disconnect event is also enqueued for the same display
+ Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
- // --------------------------------------------------------------------
- // Call Expectations
+ // --------------------------------------------------------------------
+ // Call Expectations
- setupCommonCallExpectationsForConnectProcessing<Case>();
- setupCommonCallExpectationsForDisconnectProcessing<Case>();
+ setupCommonCallExpectationsForConnectProcessing<Case>();
+ setupCommonCallExpectationsForDisconnectProcessing<Case>();
- EXPECT_CALL(*mComposer,
- setVsyncEnabled(Case::Display::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
- .WillOnce(Return(Error::NONE));
- EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mComposer,
+ setVsyncEnabled(Case::Display::HWC_DISPLAY_ID,
+ IComposerClient::Vsync::DISABLE))
+ .WillOnce(Return(Error::NONE));
+ EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
- // --------------------------------------------------------------------
- // Invocation
+ // --------------------------------------------------------------------
+ // Invocation
- mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
+ mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
- // --------------------------------------------------------------------
- // Postconditions
+ // --------------------------------------------------------------------
+ // Postconditions
- // HWComposer should not have an entry for the display
- EXPECT_FALSE(hasPhysicalHwcDisplay(Case::Display::HWC_DISPLAY_ID));
+ // HWComposer should not have an entry for the display
+ EXPECT_FALSE(hasPhysicalHwcDisplay(Case::Display::HWC_DISPLAY_ID));
- // SF should not have a display token.
- const auto displayId = Case::Display::DISPLAY_ID::get();
- ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
- ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 0);
+ // SF should not have a display token.
+ const auto displayId = Case::Display::DISPLAY_ID::get();
+ ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
+ ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 0);
+ }(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectThenConnectPrimary) {
- using Case = SimplePrimaryDisplayCase;
+ EXPECT_EXIT(
+ [this] {
+ using Case = SimplePrimaryDisplayCase;
- // --------------------------------------------------------------------
- // Preconditions
+ // --------------------------------------------------------------------
+ // Preconditions
- setupCommonPreconditions<Case>();
+ setupCommonPreconditions<Case>();
- // The display is already completely set up.
- Case::Display::injectHwcDisplay(this);
- auto existing = Case::Display::makeFakeExistingDisplayInjector(this);
- existing.inject();
+ // The display is already completely set up.
+ Case::Display::injectHwcDisplay(this);
+ auto existing = Case::Display::makeFakeExistingDisplayInjector(this);
+ existing.inject();
- // A hotplug disconnect event is enqueued for a display
- Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
- // A hotplug connect event is also enqueued for the same display
- Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
+ // A hotplug disconnect event is enqueued for a display
+ Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
+ // A hotplug connect event is also enqueued for the same display
+ Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
- // --------------------------------------------------------------------
- // Call Expectations
+ // --------------------------------------------------------------------
+ // Call Expectations
- setupCommonCallExpectationsForConnectProcessing<Case>();
- setupCommonCallExpectationsForDisconnectProcessing<Case>();
+ setupCommonCallExpectationsForConnectProcessing<Case>();
+ setupCommonCallExpectationsForDisconnectProcessing<Case>();
- // --------------------------------------------------------------------
- // Invocation
+ // --------------------------------------------------------------------
+ // Invocation
- mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
+ mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
- // --------------------------------------------------------------------
- // Postconditions
+ // --------------------------------------------------------------------
+ // Postconditions
- // The existing token should have been removed
- verifyDisplayIsNotConnected(existing.token());
- const auto displayId = Case::Display::DISPLAY_ID::get();
- ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
- ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 1);
- EXPECT_NE(existing.token(), mFlinger.mutablePhysicalDisplayTokens()[displayId]);
+ // The existing token should have been removed
+ verifyDisplayIsNotConnected(existing.token());
+ const auto displayId = Case::Display::DISPLAY_ID::get();
+ ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
+ ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 1);
+ EXPECT_NE(existing.token(), mFlinger.mutablePhysicalDisplayTokens()[displayId]);
- // A new display should be connected in its place
+ // A new display should be connected in its place
- verifyPhysicalDisplayIsConnected<Case>();
+ verifyPhysicalDisplayIsConnected<Case>();
- // --------------------------------------------------------------------
- // Cleanup conditions
+ // --------------------------------------------------------------------
+ // Cleanup conditions
- EXPECT_CALL(*mComposer,
- setVsyncEnabled(Case::Display::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
- .WillOnce(Return(Error::NONE));
- EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mComposer,
+ setVsyncEnabled(Case::Display::HWC_DISPLAY_ID,
+ IComposerClient::Vsync::DISABLE))
+ .WillOnce(Return(Error::NONE));
+ EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ }(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesVirtualDisplayAdded) {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index e32c4bf..7ead0af 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -292,7 +292,9 @@
}
TEST_F(SetupNewDisplayDeviceInternalTest, createSimpleExternalDisplay) {
- setupNewDisplayDeviceInternalTest<SimpleExternalDisplayCase>();
+ // External displays must be secondary, as the primary display cannot be disconnected.
+ EXPECT_EXIT(setupNewDisplayDeviceInternalTest<SimpleExternalDisplayCase>(),
+ testing::KilledBySignal(SIGABRT), "Missing primary display");
}
TEST_F(SetupNewDisplayDeviceInternalTest, createNonHwcVirtualDisplay) {
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 56ae414..e036f4d 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -439,8 +439,7 @@
auto& mutableHwcDisplayData() { return getHwComposer().mDisplayData; }
auto& mutableHwcPhysicalDisplayIdMap() { return getHwComposer().mPhysicalDisplayIdMap; }
- auto& mutableInternalHwcDisplayId() { return getHwComposer().mInternalHwcDisplayId; }
- auto& mutableExternalHwcDisplayId() { return getHwComposer().mExternalHwcDisplayId; }
+ auto& mutablePrimaryHwcDisplayId() { return getHwComposer().mPrimaryHwcDisplayId; }
auto& mutableUseFrameRateApi() { return mFlinger->useFrameRateApi; }
auto fromHandle(const sp<IBinder>& handle) {
@@ -600,13 +599,12 @@
LOG_ALWAYS_FATAL_IF(!physicalId);
flinger->mutableHwcPhysicalDisplayIdMap().emplace(mHwcDisplayId, *physicalId);
if (mIsPrimary) {
- flinger->mutableInternalHwcDisplayId() = mHwcDisplayId;
+ flinger->mutablePrimaryHwcDisplayId() = mHwcDisplayId;
} else {
- // If there is an external HWC display there should always be an internal ID
+ // If there is an external HWC display, there should always be a primary ID
// as well. Set it to some arbitrary value.
- auto& internalId = flinger->mutableInternalHwcDisplayId();
- if (!internalId) internalId = mHwcDisplayId - 1;
- flinger->mutableExternalHwcDisplayId() = mHwcDisplayId;
+ auto& primaryId = flinger->mutablePrimaryHwcDisplayId();
+ if (!primaryId) primaryId = mHwcDisplayId - 1;
}
}
}