SF: Sequential display mode IDs
If the supported display modes differ after a hotplug
assing the new modes new IDs. This way the old IDs will
self invalidate.
This breaks the assumption that the modes IDs are
equivalent to indices in the array of supported modes.
Bug: 159590486
Test: manually 0. device boots in 60hz mode
1. switch to 50hz from app
2. disconnect display
3. reconnect display
dumpsys SurfaceFlinger after each step to
verify mode IDs change as expected
Change-Id: I44c90e8151f31e998b43649dad73c51d6948195d
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 36c4c4d..f4a2a3f 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -145,9 +145,9 @@
}
void DisplayDevice::setActiveMode(DisplayModeId id) {
- LOG_FATAL_IF(id.value() >= mSupportedModes.size(),
- "Cannot set active mode which is not supported.");
- mActiveModeId = id;
+ const auto mode = getMode(id);
+ LOG_FATAL_IF(!mode, "Cannot set active mode which is not supported.");
+ mActiveMode = mode;
}
status_t DisplayDevice::initiateModeChange(DisplayModeId modeId,
@@ -164,7 +164,7 @@
}
const DisplayModePtr& DisplayDevice::getActiveMode() const {
- return mSupportedModes[mActiveModeId.value()];
+ return mActiveMode;
}
const DisplayModes& DisplayDevice::getSupportedModes() const {
@@ -172,9 +172,10 @@
}
DisplayModePtr DisplayDevice::getMode(DisplayModeId modeId) const {
- const auto id = modeId.value();
- if (static_cast<size_t>(id) < mSupportedModes.size()) {
- return mSupportedModes[id];
+ const auto it = std::find_if(mSupportedModes.begin(), mSupportedModes.end(),
+ [&](DisplayModePtr mode) { return mode->getId() == modeId; });
+ if (it != mSupportedModes.end()) {
+ return *it;
}
return nullptr;
}
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index a94bfa2..7156613 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -208,7 +208,7 @@
hardware::graphics::composer::hal::PowerMode mPowerMode =
hardware::graphics::composer::hal::PowerMode::OFF;
- DisplayModeId mActiveModeId;
+ DisplayModePtr mActiveMode;
const DisplayModes mSupportedModes;
std::atomic<nsecs_t> mLastHwVsync = 0;
diff --git a/services/surfaceflinger/DisplayHardware/DisplayMode.h b/services/surfaceflinger/DisplayHardware/DisplayMode.h
index 853c05b..85cc993 100644
--- a/services/surfaceflinger/DisplayHardware/DisplayMode.h
+++ b/services/surfaceflinger/DisplayHardware/DisplayMode.h
@@ -125,6 +125,12 @@
// without visual interruptions such as a black screen.
int32_t getGroup() const { return mGroup; }
+ bool equalsExceptDisplayModeId(const DisplayModePtr& other) const {
+ return mHwcId == other->mHwcId && mWidth == other->mWidth && mHeight == other->mHeight &&
+ getVsyncPeriod() == other->getVsyncPeriod() && mDpiX == other->mDpiX &&
+ mDpiY == other->mDpiY && mGroup == other->mGroup;
+ }
+
private:
explicit DisplayMode(hal::HWConfigId id) : mHwcId(id) {}
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index de11c16..91e8043 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -610,15 +610,16 @@
void RefreshRateConfigs::updateDisplayModes(const DisplayModes& modes,
DisplayModeId currentModeId) {
std::lock_guard lock(mLock);
- LOG_ALWAYS_FATAL_IF(modes.empty());
- LOG_ALWAYS_FATAL_IF(currentModeId.value() >= modes.size());
+ // The current mode should be supported
+ LOG_ALWAYS_FATAL_IF(std::none_of(modes.begin(), modes.end(), [&](DisplayModePtr mode) {
+ return mode->getId() == currentModeId;
+ }));
mRefreshRates.clear();
for (const auto& mode : modes) {
const auto modeId = mode->getId();
- const auto fps = Fps::fromPeriodNsecs(mode->getVsyncPeriod());
mRefreshRates.emplace(modeId,
- std::make_unique<RefreshRate>(modeId, mode, fps,
+ std::make_unique<RefreshRate>(modeId, mode, mode->getFps(),
RefreshRate::ConstructorTag(0)));
if (modeId == currentModeId) {
mCurrentRefreshRate = mRefreshRates.at(modeId).get();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 350a3c8..056dd9f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2308,20 +2308,47 @@
DisplayModes SurfaceFlinger::loadSupportedDisplayModes(PhysicalDisplayId displayId) const {
const auto hwcModes = getHwComposer().getModes(displayId);
- DisplayModes modes;
- int32_t nextModeId = 0;
- for (const auto& hwcMode : hwcModes) {
- modes.push_back(DisplayMode::Builder(hwcMode.hwcId)
- .setId(DisplayModeId{nextModeId++})
- .setWidth(hwcMode.width)
- .setHeight(hwcMode.height)
- .setVsyncPeriod(hwcMode.vsyncPeriod)
- .setDpiX(hwcMode.dpiX)
- .setDpiY(hwcMode.dpiY)
- .setGroup(hwcMode.configGroup)
- .build());
+
+ DisplayModes oldModes;
+
+ if (const auto token = getPhysicalDisplayTokenLocked(displayId)) {
+ oldModes = getDisplayDeviceLocked(token)->getSupportedModes();
}
- return modes;
+
+ int largestUsedModeId = -1; // Use int instead of DisplayModeId for signedness
+ for (const auto& mode : oldModes) {
+ const auto id = static_cast<int>(mode->getId().value());
+ if (id > largestUsedModeId) {
+ largestUsedModeId = id;
+ }
+ }
+
+ DisplayModes newModes;
+ int32_t nextModeId = largestUsedModeId + 1;
+ for (const auto& hwcMode : hwcModes) {
+ newModes.push_back(DisplayMode::Builder(hwcMode.hwcId)
+ .setId(DisplayModeId{nextModeId++})
+ .setWidth(hwcMode.width)
+ .setHeight(hwcMode.height)
+ .setVsyncPeriod(hwcMode.vsyncPeriod)
+ .setDpiX(hwcMode.dpiX)
+ .setDpiY(hwcMode.dpiY)
+ .setGroup(hwcMode.configGroup)
+ .build());
+ }
+
+ const bool modesAreSame =
+ std::equal(newModes.begin(), newModes.end(), oldModes.begin(), oldModes.end(),
+ [](DisplayModePtr left, DisplayModePtr right) {
+ return left->equalsExceptDisplayModeId(right);
+ });
+
+ if (modesAreSame) {
+ // The supported modes have not changed, keep the old IDs.
+ return oldModes;
+ }
+
+ return newModes;
}
void SurfaceFlinger::processDisplayHotplugEventsLocked() {
@@ -2414,7 +2441,6 @@
const sp<IGraphicBufferProducer>& producer) {
DisplayDeviceCreationArgs creationArgs(this, getHwComposer(), displayToken, compositionDisplay);
creationArgs.sequenceId = state.sequenceId;
- creationArgs.hwComposer = getHwComposer();
creationArgs.isSecure = state.isSecure;
creationArgs.displaySurface = displaySurface;
creationArgs.hasWideColorGamut = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1615ab6..d1eae1c 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -906,7 +906,7 @@
/*
* Display management
*/
- DisplayModes loadSupportedDisplayModes(PhysicalDisplayId) const;
+ DisplayModes loadSupportedDisplayModes(PhysicalDisplayId) const REQUIRES(mStateLock);
sp<DisplayDevice> setupNewDisplayDeviceInternal(
const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay,