SF: Fix condition to activate display on power-on
Since [1], getDefaultDisplayDeviceLocked's fallback when the active
display is outdated is to return the primary display, not `nullptr`.
During the initial call to setPowerModeInternal, `activeDisplay` is
no longer `nullptr`, but the primary display.
After [2], onActiveDisplayChangedLocked is expected to be called for
the primary display during boot. This was not the case though, since
the DisplayDevice::setPowerMode() update precedes the DisplayDevice::
isPoweredOn() query.
Fix this by querying before updating. Also, clean up `nullptr` checks
around onActiveDisplayChangedLocked.
[1] If83c908446e5e5267dfcb15189a26b779d75b216
[2] I15e0f5a280e62baf6d4e6ea2748d95342e79ac44
Fixes: 260663220
Test: The leader display ID is logged during single-display boot.
Test: SetPowerModeInternalTest.designatesLeaderDisplay
Change-Id: I0d20ce7f7ab3f9e90ef1c72263b8166865486d62
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 19c7d5c..d58e644 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -359,6 +359,7 @@
}
// Called by tests to inject a HWC display setup
+ template <bool kInitPowerMode = true>
static void injectHwcDisplayWithNoDefaultCapabilities(DisplayTransactionTest* test) {
const auto displayId = DisplayVariant::DISPLAY_ID::get();
ASSERT_FALSE(GpuVirtualDisplayId::tryCast(displayId));
@@ -367,18 +368,21 @@
.setHwcDisplayId(HWC_DISPLAY_ID)
.setResolution(DisplayVariant::RESOLUTION)
.setActiveConfig(HWC_ACTIVE_CONFIG_ID)
- .setPowerMode(INIT_POWER_MODE)
+ .setPowerMode(kInitPowerMode ? std::make_optional(INIT_POWER_MODE) : std::nullopt)
.inject(&test->mFlinger, test->mComposer);
}
// Called by tests to inject a HWC display setup
+ template <bool kInitPowerMode = true>
static void injectHwcDisplay(DisplayTransactionTest* test) {
EXPECT_CALL(*test->mComposer, getDisplayCapabilities(HWC_DISPLAY_ID, _))
.WillOnce(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})),
Return(Error::NONE)));
- EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, INIT_POWER_MODE))
- .WillOnce(Return(Error::NONE));
- injectHwcDisplayWithNoDefaultCapabilities(test);
+ if constexpr (kInitPowerMode) {
+ EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, INIT_POWER_MODE))
+ .WillOnce(Return(Error::NONE));
+ }
+ injectHwcDisplayWithNoDefaultCapabilities<kInitPowerMode>(test);
}
static std::shared_ptr<compositionengine::Display> injectCompositionDisplay(
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
index bc66961..622717f 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
@@ -97,7 +97,6 @@
.setNativeWindow(mNativeWindow)
.setPowerMode(hal::PowerMode::ON)
.inject();
- mFlinger.mutableActiveDisplayId() = mDisplay->getPhysicalId();
}
void SurfaceFlingerPowerHintTest::setupScheduler() {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index 25857ec..0fb8e2b 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -259,12 +259,6 @@
auto injector = Display::makeFakeExistingDisplayInjector(test);
const auto display = injector.inject();
display->setPowerMode(mode);
- if (injector.physicalDisplay()
- .transform(&display::PhysicalDisplay::isInternal)
- .value_or(false)) {
- test->mFlinger.mutableActiveDisplayId() = display->getPhysicalId();
- }
-
return display;
}
@@ -490,5 +484,37 @@
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOnToUnknownVariant>>();
}
+TEST_F(SetPowerModeInternalTest, designatesLeaderDisplay) {
+ using Case = SimplePrimaryDisplayCase;
+
+ // --------------------------------------------------------------------
+ // Preconditions
+
+ // Inject a primary display with uninitialized power mode.
+ constexpr bool kInitPowerMode = false;
+ Case::Display::injectHwcDisplay<kInitPowerMode>(this);
+ auto injector = Case::Display::makeFakeExistingDisplayInjector(this);
+ injector.setPowerMode(std::nullopt);
+ const auto display = injector.inject();
+
+ // --------------------------------------------------------------------
+ // Invocation
+
+ // FakeDisplayDeviceInjector registers the display with Scheduler, so it has already been
+ // designated as the leader. Set an arbitrary leader to verify that `setPowerModeInternal`
+ // designates a leader regardless of any preceding `Scheduler::registerDisplay` call(s).
+ constexpr PhysicalDisplayId kPlaceholderId = PhysicalDisplayId::fromPort(42);
+ ASSERT_NE(display->getPhysicalId(), kPlaceholderId);
+ mFlinger.scheduler()->setLeaderDisplay(kPlaceholderId);
+
+ mFlinger.setPowerModeInternal(display, PowerMode::ON);
+
+ // --------------------------------------------------------------------
+ // Postconditions
+
+ // The primary display should be designated as the leader.
+ EXPECT_EQ(mFlinger.scheduler()->leaderDisplayId(), display->getPhysicalId());
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 54c10c5..b8a6063 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -87,6 +87,10 @@
Scheduler::unregisterDisplay(displayId);
}
+ std::optional<PhysicalDisplayId> leaderDisplayId() const NO_THREAD_SAFETY_ANALYSIS {
+ return mLeaderDisplayId;
+ }
+
void setLeaderDisplay(PhysicalDisplayId displayId) {
ftl::FakeGuard guard(kMainThreadContext);
Scheduler::setLeaderDisplay(displayId);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index e29dd67..fb473f5 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -470,7 +470,7 @@
void onActiveDisplayChanged(const sp<DisplayDevice>& activeDisplay) {
Mutex::Autolock lock(mFlinger->mStateLock);
ftl::FakeGuard guard(kMainThreadContext);
- mFlinger->onActiveDisplayChangedLocked(activeDisplay);
+ mFlinger->onActiveDisplayChangedLocked(nullptr, activeDisplay);
}
auto createLayer(LayerCreationArgs& args, const sp<IBinder>& parentHandle,
@@ -621,7 +621,7 @@
return *this;
}
- auto& setPowerMode(hal::PowerMode mode) {
+ auto& setPowerMode(std::optional<hal::PowerMode> mode) {
mPowerMode = mode;
return *this;
}
@@ -645,7 +645,11 @@
mHwcDisplayType);
display->mutableIsConnected() = true;
- display->setPowerMode(mPowerMode);
+
+ if (mPowerMode) {
+ display->setPowerMode(*mPowerMode);
+ }
+
flinger->mutableHwcDisplayData()[mDisplayId].hwcDisplay = std::move(display);
EXPECT_CALL(*composer, getDisplayConfigs(mHwcDisplayId, _))
@@ -711,7 +715,7 @@
int32_t mDpiY = DEFAULT_DPI;
int32_t mConfigGroup = DEFAULT_CONFIG_GROUP;
hal::HWConfigId mActiveConfig = DEFAULT_ACTIVE_CONFIG;
- hal::PowerMode mPowerMode = DEFAULT_POWER_MODE;
+ std::optional<hal::PowerMode> mPowerMode = DEFAULT_POWER_MODE;
const std::unordered_set<aidl::android::hardware::graphics::composer3::Capability>*
mCapabilities = nullptr;
};
@@ -788,7 +792,7 @@
return *this;
}
- auto& setPowerMode(hal::PowerMode mode) {
+ auto& setPowerMode(std::optional<hal::PowerMode> mode) {
mCreationArgs.initialPowerMode = mode;
return *this;
}
@@ -853,6 +857,10 @@
LOG_ALWAYS_FATAL_IF(!physicalIdOpt);
const auto physicalId = *physicalIdOpt;
+ if (mCreationArgs.isPrimary) {
+ mFlinger.mutableActiveDisplayId() = physicalId;
+ }
+
LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
const auto activeMode = modes.get(activeModeId);