Merge "SF: Fix display mode transitions for multi-display"
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 46b857b..96ae77f 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -452,7 +452,8 @@
}
}
-auto DisplayDevice::setDesiredActiveMode(const ActiveModeInfo& info) -> DesiredActiveModeAction {
+auto DisplayDevice::setDesiredActiveMode(const ActiveModeInfo& info, bool force)
+ -> DesiredActiveModeAction {
ATRACE_CALL();
LOG_ALWAYS_FATAL_IF(!info.modeOpt, "desired mode not provided");
@@ -473,7 +474,7 @@
const auto& desiredMode = *info.modeOpt->modePtr;
// Check if we are already at the desired mode
- if (refreshRateSelector().getActiveMode().modePtr->getId() == desiredMode.getId()) {
+ if (!force && refreshRateSelector().getActiveMode().modePtr->getId() == desiredMode.getId()) {
if (refreshRateSelector().getActiveMode() == info.modeOpt) {
return DesiredActiveModeAction::None;
}
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 8f9b2a1..d757673 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -210,7 +210,8 @@
InitiateDisplayModeSwitch,
InitiateRenderRateSwitch
};
- DesiredActiveModeAction setDesiredActiveMode(const ActiveModeInfo&) EXCLUDES(mActiveModeLock);
+ DesiredActiveModeAction setDesiredActiveMode(const ActiveModeInfo&, bool force = false)
+ EXCLUDES(mActiveModeLock);
std::optional<ActiveModeInfo> getDesiredActiveMode() const EXCLUDES(mActiveModeLock);
void clearDesiredActiveModeState() EXCLUDES(mActiveModeLock);
ActiveModeInfo getUpcomingActiveMode() const REQUIRES(kMainThreadContext) {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 34f8df2..856fda0 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -815,18 +815,18 @@
.powerOnImminent = powerOnImminent};
}
-ftl::Optional<FrameRateMode> Scheduler::getPreferredDisplayMode() {
+FrameRateMode Scheduler::getPreferredDisplayMode() {
std::lock_guard<std::mutex> lock(mPolicyLock);
- // Make sure the stored mode is up to date.
- if (mPolicy.modeOpt) {
- const auto ranking =
- leaderSelectorPtr()
- ->getRankedFrameRates(mPolicy.contentRequirements, makeGlobalSignals())
- .ranking;
+ const auto frameRateMode =
+ leaderSelectorPtr()
+ ->getRankedFrameRates(mPolicy.contentRequirements, makeGlobalSignals())
+ .ranking.front()
+ .frameRateMode;
- mPolicy.modeOpt = ranking.front().frameRateMode;
- }
- return mPolicy.modeOpt;
+ // Make sure the stored mode is up to date.
+ mPolicy.modeOpt = frameRateMode;
+
+ return frameRateMode;
}
void Scheduler::onNewVsyncPeriodChangeTimeline(const hal::VsyncPeriodChangeTimeline& timeline) {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index cf2ffb8..f189426 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -209,8 +209,8 @@
void dump(ConnectionHandle, std::string&) const;
void dumpVsync(std::string&) const;
- // Get the appropriate refresh for current conditions.
- ftl::Optional<FrameRateMode> getPreferredDisplayMode();
+ // Returns the preferred refresh rate and frame rate for the leader display.
+ FrameRateMode getPreferredDisplayMode();
// Notifies the scheduler about a refresh rate timeline change.
void onNewVsyncPeriodChangeTimeline(const hal::VsyncPeriodChangeTimeline& timeline);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index da26e20..b930477 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1132,7 +1132,7 @@
return NO_ERROR;
}
-void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request) {
+void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request, bool force) {
ATRACE_CALL();
auto display = getDisplayDeviceLocked(request.mode.modePtr->getPhysicalDisplayId());
@@ -1144,7 +1144,8 @@
const auto mode = request.mode;
const bool emitEvent = request.emitEvent;
- switch (display->setDesiredActiveMode(DisplayDevice::ActiveModeInfo(std::move(request)))) {
+ switch (display->setDesiredActiveMode(DisplayDevice::ActiveModeInfo(std::move(request)),
+ force)) {
case DisplayDevice::DesiredActiveModeAction::InitiateDisplayModeSwitch:
scheduleComposite(FrameHint::kNone);
@@ -3457,10 +3458,21 @@
for (auto& request : modeRequests) {
const auto& modePtr = request.mode.modePtr;
- const auto display = getDisplayDeviceLocked(modePtr->getPhysicalDisplayId());
+
+ const auto displayId = modePtr->getPhysicalDisplayId();
+ const auto display = getDisplayDeviceLocked(displayId);
if (!display) continue;
+ const bool isInternalDisplay = mPhysicalDisplays.get(displayId)
+ .transform(&PhysicalDisplay::isInternal)
+ .value_or(false);
+
+ if (isInternalDisplay && displayId != mActiveDisplayId) {
+ ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str());
+ continue;
+ }
+
if (display->refreshRateSelector().isModeAllowed(request.mode)) {
setDesiredActiveMode(std::move(request));
} else {
@@ -6725,7 +6737,7 @@
ftl::Optional<scheduler::FrameRateMode> SurfaceFlinger::getPreferredDisplayMode(
PhysicalDisplayId displayId, DisplayModeId defaultModeId) const {
if (const auto schedulerMode = mScheduler->getPreferredDisplayMode();
- schedulerMode && schedulerMode->modePtr->getPhysicalDisplayId() == displayId) {
+ schedulerMode.modePtr->getPhysicalDisplayId() == displayId) {
return schedulerMode;
}
@@ -6760,12 +6772,24 @@
case SetPolicyResult::Unchanged:
return NO_ERROR;
case SetPolicyResult::Changed:
- return applyRefreshRateSelectorPolicy(displayId, selector);
+ break;
}
+
+ const bool isInternalDisplay = mPhysicalDisplays.get(displayId)
+ .transform(&PhysicalDisplay::isInternal)
+ .value_or(false);
+
+ if (isInternalDisplay && displayId != mActiveDisplayId) {
+ // The policy will be be applied when the display becomes active.
+ ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str());
+ return NO_ERROR;
+ }
+
+ return applyRefreshRateSelectorPolicy(displayId, selector);
}
status_t SurfaceFlinger::applyRefreshRateSelectorPolicy(
- PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector) {
+ PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector, bool force) {
const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy();
ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str());
@@ -6795,7 +6819,7 @@
return INVALID_OPERATION;
}
- setDesiredActiveMode({std::move(preferredMode), .emitEvent = true});
+ setDesiredActiveMode({std::move(preferredMode), .emitEvent = true}, force);
return NO_ERROR;
}
@@ -7099,7 +7123,8 @@
// case, its preferred mode has not been propagated to HWC (via setDesiredActiveMode). In either
// case, the Scheduler's cachedModeChangedParams must be initialized to the newly active mode,
// and the kernel idle timer of the newly active display must be toggled.
- applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay->refreshRateSelector());
+ constexpr bool kForce = true;
+ applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay->refreshRateSelector(), kForce);
}
status_t SurfaceFlinger::addWindowInfosListener(
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index fa3a49f..7bb0514 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -649,7 +649,8 @@
// Show render rate with refresh rate overlay
bool mRefreshRateOverlayRenderRate = false;
- void setDesiredActiveMode(display::DisplayModeRequest&&) REQUIRES(mStateLock);
+ void setDesiredActiveMode(display::DisplayModeRequest&&, bool force = false)
+ REQUIRES(mStateLock);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId);
// Sets the active mode and a new refresh rate in SF.
@@ -678,7 +679,8 @@
// TODO(b/241285191): Look up RefreshRateSelector on Scheduler to remove redundant parameter.
status_t applyRefreshRateSelectorPolicy(PhysicalDisplayId,
- const scheduler::RefreshRateSelector&)
+ const scheduler::RefreshRateSelector&,
+ bool force = false)
REQUIRES(mStateLock, kMainThreadContext);
void commitTransactions() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext);
@@ -1187,7 +1189,9 @@
display::PhysicalDisplays mPhysicalDisplays GUARDED_BY(mStateLock);
// The inner or outer display for foldables, assuming they have mutually exclusive power states.
- PhysicalDisplayId mActiveDisplayId GUARDED_BY(mStateLock);
+ // Atomic because writes from onActiveDisplayChangedLocked are not always under mStateLock, but
+ // reads from ISchedulerCallback::requestDisplayModes may happen concurrently.
+ std::atomic<PhysicalDisplayId> mActiveDisplayId GUARDED_BY(mStateLock);
struct {
DisplayIdGenerator<GpuVirtualDisplayId> gpu;
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 074bf8c..ad3bd35 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -14,12 +14,12 @@
* limitations under the License.
*/
-#include "mock/MockDisplayModeSpecs.h"
-#include "mock/MockEventThread.h"
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
#include "DisplayTransactionTestHelpers.h"
+#include "mock/DisplayHardware/MockDisplayMode.h"
+#include "mock/MockDisplayModeSpecs.h"
#include <ftl/fake_guard.h>
#include <scheduler/Fps.h>
@@ -42,8 +42,7 @@
PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);
PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this);
- DisplayModes modes = makeModes(kMode60, kMode90, kMode120, kMode90_4K);
- auto selectorPtr = std::make_shared<scheduler::RefreshRateSelector>(modes, kModeId60);
+ auto selectorPtr = std::make_shared<scheduler::RefreshRateSelector>(kModes, kModeId60);
setupScheduler(selectorPtr);
@@ -51,7 +50,7 @@
mFlinger.configureAndCommit();
mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
- .setDisplayModes(std::move(modes), kModeId60, std::move(selectorPtr))
+ .setDisplayModes(kModes, kModeId60, std::move(selectorPtr))
.inject();
// isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy
@@ -78,6 +77,8 @@
static constexpr ui::Size kResolution4K{3840, 2160};
static inline const DisplayModePtr kMode90_4K =
createDisplayMode(kModeId90_4K, 90_Hz, 3, kResolution4K);
+
+ static inline const DisplayModes kModes = makeModes(kMode60, kMode90, kMode120, kMode90_4K);
};
void DisplayModeSwitchingTest::setupScheduler(
@@ -283,5 +284,114 @@
ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K);
}
+TEST_F(DisplayModeSwitchingTest, multiDisplay) {
+ ftl::FakeGuard guard(kMainThreadContext);
+
+ constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID;
+ constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1;
+
+ constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u);
+
+ constexpr bool kIsPrimary = false;
+ TestableSurfaceFlinger::FakeHwcDisplayInjector(kOuterDisplayId, hal::DisplayType::PHYSICAL,
+ kIsPrimary)
+ .setHwcDisplayId(kOuterDisplayHwcId)
+ .inject(&mFlinger, mComposer);
+
+ const auto outerDisplay = mFakeDisplayInjector.injectInternalDisplay(
+ [&](FakeDisplayDeviceInjector& injector) {
+ injector.setDisplayModes(mock::cloneForDisplay(kOuterDisplayId, kModes),
+ kModeId120);
+ },
+ {.displayId = kOuterDisplayId,
+ .hwcDisplayId = kOuterDisplayHwcId,
+ .isPrimary = kIsPrimary});
+
+ const auto& innerDisplay = mDisplay;
+
+ EXPECT_FALSE(innerDisplay->getDesiredActiveMode());
+ EXPECT_FALSE(outerDisplay->getDesiredActiveMode());
+
+ EXPECT_EQ(innerDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_EQ(outerDisplay->getActiveMode().modePtr->getId(), kModeId120);
+
+ mFlinger.onActiveDisplayChanged(innerDisplay);
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId90.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId60.value(),
+ false, 0.f, 120.f)));
+
+ // Transition on the inner display.
+ ASSERT_TRUE(innerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(innerDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId90);
+
+ // No transition on the outer display.
+ EXPECT_FALSE(outerDisplay->getDesiredActiveMode());
+
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kInnerDisplayHwcId,
+ hal::HWConfigId(kModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ // Transition on the inner display.
+ ASSERT_TRUE(innerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(innerDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId90);
+
+ // No transition on the outer display.
+ EXPECT_FALSE(outerDisplay->getDesiredActiveMode());
+
+ mFlinger.commit();
+
+ // Transition on the inner display.
+ EXPECT_FALSE(innerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(innerDisplay->getActiveMode().modePtr->getId(), kModeId90);
+
+ // No transition on the outer display.
+ EXPECT_FALSE(outerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(outerDisplay->getActiveMode().modePtr->getId(), kModeId120);
+
+ mFlinger.onActiveDisplayChanged(outerDisplay);
+
+ // No transition on the inner display.
+ EXPECT_FALSE(innerDisplay->getDesiredActiveMode());
+
+ // Transition on the outer display.
+ ASSERT_TRUE(outerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(outerDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId60);
+
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kOuterDisplayHwcId,
+ hal::HWConfigId(kModeId60.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ // No transition on the inner display.
+ EXPECT_FALSE(innerDisplay->getDesiredActiveMode());
+
+ // Transition on the outer display.
+ ASSERT_TRUE(outerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(outerDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId60);
+
+ mFlinger.commit();
+
+ // No transition on the inner display.
+ EXPECT_FALSE(innerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(innerDisplay->getActiveMode().modePtr->getId(), kModeId90);
+
+ // Transition on the outer display.
+ EXPECT_FALSE(outerDisplay->getDesiredActiveMode());
+ EXPECT_EQ(outerDisplay->getActiveMode().modePtr->getId(), kModeId60);
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
index c78b6bd..3b36361 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
@@ -38,4 +38,24 @@
return createDisplayMode(modeId, refreshRate, {}, {}, displayId);
}
+inline DisplayModePtr cloneForDisplay(PhysicalDisplayId displayId, const DisplayModePtr& modePtr) {
+ return DisplayMode::Builder(modePtr->getHwcId())
+ .setId(modePtr->getId())
+ .setPhysicalDisplayId(displayId)
+ .setVsyncPeriod(modePtr->getVsyncPeriod())
+ .setGroup(modePtr->getGroup())
+ .setResolution(modePtr->getResolution())
+ .build();
+}
+
+inline DisplayModes cloneForDisplay(PhysicalDisplayId displayId, const DisplayModes& modes) {
+ DisplayModes clones;
+
+ for (const auto& [id, modePtr] : modes) {
+ clones.try_emplace(id, cloneForDisplay(displayId, modePtr));
+ }
+
+ return clones;
+}
+
} // namespace android::mock