SF: Initialize all displays on boot/restart
Generalize SF::initializeDisplays (called on boot and restart) to:
- Apply the transaction that clears DisplayState to all displays.
- Power on all displays.
The first change removes a special case for the primary display, setting
the stage for multi-display boot animation. Each display is assigned its
own LayerStack, and set up with a projection to its active resolution.
The second change fixes a bug where DisplayCapability::BRIGHTNESS was
not detected for secondary displays present during boot. SF queries
capabilities when a display is first powered on, but DM asks SF about
brightness when the display is hotplugged, regardless of power mode.
The general fix (covering external displays) is for DM to defer its
query, but this stopgap covers internal displays.
Revert I3a2eae4efc4a5c6113700a9ca9e9b261e364a878, which let the initial
power mode be std::nullopt. This effectively forced DM's first request
to setPowerMode(<rear display>, OFF), which would otherwise be ignored
because OFF had been the default power mode on DisplayDevice creation.
However, that special case confusingly took the same branch as the OFF
to ON transition, and is no longer needed now that all displays are ON
(from SF's perspective, not just HWC's) until the boot animation ends.
Fixes: 267633741
Fixes: 150889228
Bug: 269510347
Test: Boot unfolded and folded.
Test: Induce system_server crash.
Test: InitializeDisplaysTest.initializesDisplays
Change-Id: I5277a629f39b3b285452aa84d49ff84e3dc957ca
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index e1dc791..6b8e824 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -416,26 +416,36 @@
};
struct DisplayState {
- enum {
+ enum : uint32_t {
eSurfaceChanged = 0x01,
eLayerStackChanged = 0x02,
eDisplayProjectionChanged = 0x04,
eDisplaySizeChanged = 0x08,
- eFlagsChanged = 0x10
+ eFlagsChanged = 0x10,
+
+ eAllChanged = ~0u
};
+ // Not for direct use. Prefer constructor below for new displays.
DisplayState();
+
+ DisplayState(sp<IBinder> token, ui::LayerStack layerStack)
+ : what(eAllChanged),
+ token(std::move(token)),
+ layerStack(layerStack),
+ layerStackSpaceRect(Rect::INVALID_RECT),
+ orientedDisplaySpaceRect(Rect::INVALID_RECT) {}
+
void merge(const DisplayState& other);
void sanitize(int32_t permissions);
uint32_t what = 0;
uint32_t flags = 0;
sp<IBinder> token;
- sp<IGraphicBufferProducer> surface;
ui::LayerStack layerStack = ui::DEFAULT_LAYER_STACK;
- // These states define how layers are projected onto the physical display.
+ // These states define how layers are projected onto the physical or virtual display.
//
// Layers are first clipped to `layerStackSpaceRect'. They are then translated and
// scaled from `layerStackSpaceRect' to `orientedDisplaySpaceRect'. Finally, they are rotated
@@ -446,10 +456,17 @@
// will be scaled by a factor of 2 and translated by (20, 10). When orientation is 1, layers
// will be additionally rotated by 90 degrees around the origin clockwise and translated by (W,
// 0).
+ //
+ // Rect::INVALID_RECT sizes the space to the active resolution of the physical display, or the
+ // default dimensions of the virtual display surface.
+ //
ui::Rotation orientation = ui::ROTATION_0;
Rect layerStackSpaceRect = Rect::EMPTY_RECT;
Rect orientedDisplaySpaceRect = Rect::EMPTY_RECT;
+ // Exclusive to virtual displays: The sink surface into which the virtual display is rendered,
+ // and an optional resolution that overrides its default dimensions.
+ sp<IGraphicBufferProducer> surface;
uint32_t width = 0;
uint32_t height = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h
index c71c517..39748b8 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h
@@ -60,7 +60,7 @@
virtual void createClientCompositionCache(uint32_t cacheSize) = 0;
// Sends the brightness setting to HWC
- virtual void applyDisplayBrightness(const bool applyImmediately) = 0;
+ virtual void applyDisplayBrightness(bool applyImmediately) = 0;
protected:
~Display() = default;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
index c53b461..2dc9a1a 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
@@ -73,7 +73,7 @@
const compositionengine::DisplayColorProfileCreationArgs&) override;
void createRenderSurface(const compositionengine::RenderSurfaceCreationArgs&) override;
void createClientCompositionCache(uint32_t cacheSize) override;
- void applyDisplayBrightness(const bool applyImmediately) override;
+ void applyDisplayBrightness(bool applyImmediately) override;
void setSecure(bool secure) override;
// Internal helpers used by chooseCompositionStrategy()
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index d907bf5..6428d08 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -204,13 +204,12 @@
setReleasedLayers(std::move(releasedLayers));
}
-void Display::applyDisplayBrightness(const bool applyImmediately) {
- auto& hwc = getCompositionEngine().getHwComposer();
- const auto halDisplayId = HalDisplayId::tryCast(*getDisplayId());
- if (const auto physicalDisplayId = PhysicalDisplayId::tryCast(*halDisplayId);
- physicalDisplayId && getState().displayBrightness) {
+void Display::applyDisplayBrightness(bool applyImmediately) {
+ if (const auto displayId = ftl::Optional(getDisplayId()).and_then(PhysicalDisplayId::tryCast);
+ displayId && getState().displayBrightness) {
+ auto& hwc = getCompositionEngine().getHwComposer();
const status_t result =
- hwc.setDisplayBrightness(*physicalDisplayId, *getState().displayBrightness,
+ hwc.setDisplayBrightness(*displayId, *getState().displayBrightness,
getState().displayBrightnessNits,
Hwc2::Composer::DisplayBrightnessOptions{
.applyImmediately = applyImmediately})
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index b96264b..ef9d118 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -68,6 +68,7 @@
mActiveModeFpsTrace(concatId("ActiveModeFps")),
mRenderRateFpsTrace(concatId("RenderRateFps")),
mPhysicalOrientation(args.physicalOrientation),
+ mPowerMode(ftl::Concat("PowerMode ", getId().value).c_str(), args.initialPowerMode),
mIsPrimary(args.isPrimary),
mRequestedRefreshRate(args.requestedRefreshRate),
mRefreshRateSelector(std::move(args.refreshRateSelector)),
@@ -106,9 +107,7 @@
mCompositionDisplay->getRenderSurface()->initialize();
- if (const auto powerModeOpt = args.initialPowerMode) {
- setPowerMode(*powerModeOpt);
- }
+ setPowerMode(args.initialPowerMode);
// initialize the display orientation transform.
setProjection(ui::ROTATION_0, Rect::INVALID_RECT, Rect::INVALID_RECT);
@@ -173,6 +172,7 @@
}
void DisplayDevice::setPowerMode(hal::PowerMode mode) {
+ // TODO(b/241285876): Skip this for virtual displays.
if (mode == hal::PowerMode::OFF || mode == hal::PowerMode::ON) {
if (mStagedBrightness && mBrightness != mStagedBrightness) {
getCompositionDisplay()->setNextBrightness(*mStagedBrightness);
@@ -182,33 +182,26 @@
getCompositionDisplay()->applyDisplayBrightness(true);
}
- if (mPowerMode) {
- *mPowerMode = mode;
- } else {
- mPowerMode.emplace("PowerMode -" + to_string(getId()), mode);
- }
+ mPowerMode = mode;
getCompositionDisplay()->setCompositionEnabled(isPoweredOn());
}
void DisplayDevice::tracePowerMode() {
- // assign the same value for tracing
- if (mPowerMode) {
- const hal::PowerMode powerMode = *mPowerMode;
- *mPowerMode = powerMode;
- }
+ // Assign the same value for tracing.
+ mPowerMode = mPowerMode.get();
}
void DisplayDevice::enableLayerCaching(bool enable) {
getCompositionDisplay()->setLayerCachingEnabled(enable);
}
-std::optional<hal::PowerMode> DisplayDevice::getPowerMode() const {
+hal::PowerMode DisplayDevice::getPowerMode() const {
return mPowerMode;
}
bool DisplayDevice::isPoweredOn() const {
- return mPowerMode && *mPowerMode != hal::PowerMode::OFF;
+ return mPowerMode != hal::PowerMode::OFF;
}
void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 46534de..edd57cc 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -173,8 +173,8 @@
/* ------------------------------------------------------------------------
* Display power mode management.
*/
- std::optional<hardware::graphics::composer::hal::PowerMode> getPowerMode() const;
- void setPowerMode(hardware::graphics::composer::hal::PowerMode mode);
+ hardware::graphics::composer::hal::PowerMode getPowerMode() const;
+ void setPowerMode(hardware::graphics::composer::hal::PowerMode);
bool isPoweredOn() const;
void tracePowerMode();
@@ -270,9 +270,7 @@
ui::Rotation mOrientation = ui::ROTATION_0;
bool mIsOrientationChanged = false;
- // Allow nullopt as initial power mode.
- using TracedPowerMode = TracedOrdinal<hardware::graphics::composer::hal::PowerMode>;
- std::optional<TracedPowerMode> mPowerMode;
+ TracedOrdinal<hardware::graphics::composer::hal::PowerMode> mPowerMode;
std::optional<float> mStagedBrightness;
std::optional<float> mBrightness;
@@ -362,7 +360,8 @@
HdrCapabilities hdrCapabilities;
int32_t supportedPerFrameMetadata{0};
std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>> hwcColorModes;
- std::optional<hardware::graphics::composer::hal::PowerMode> initialPowerMode;
+ hardware::graphics::composer::hal::PowerMode initialPowerMode{
+ hardware::graphics::composer::hal::PowerMode::OFF};
bool isPrimary{false};
DisplayModeId activeModeId;
// Refer to DisplayDevice::mRequestedRefreshRate, for virtual display only
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fe5b159..8eff1b6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1952,6 +1952,7 @@
const char* const whence = __func__;
return ftl::Future(mScheduler->schedule([=, this]() FTL_FAKE_GUARD(mStateLock) {
+ // TODO(b/241285876): Validate that the display is physical instead of failing later.
if (const auto display = getDisplayDeviceLocked(displayToken)) {
const bool supportsDisplayBrightnessCommand =
getHwComposer().getComposer()->isSupported(
@@ -2001,7 +2002,6 @@
Hwc2::Composer::DisplayBrightnessOptions{
.applyImmediately = true});
}
-
} else {
ALOGE("%s: Invalid display token %p", whence, displayToken.get());
return ftl::yield<status_t>(NAME_NOT_FOUND);
@@ -3624,9 +3624,7 @@
getPhysicalDisplayOrientation(compositionDisplay->getId(), creationArgs.isPrimary);
ALOGV("Display Orientation: %s", toCString(creationArgs.physicalOrientation));
- // virtual displays are always considered enabled
- creationArgs.initialPowerMode =
- state.isVirtual() ? std::make_optional(hal::PowerMode::ON) : std::nullopt;
+ creationArgs.initialPowerMode = state.isVirtual() ? hal::PowerMode::ON : hal::PowerMode::OFF;
creationArgs.requestedRefreshRate = state.requestedRefreshRate;
@@ -5912,12 +5910,6 @@
}
void SurfaceFlinger::initializeDisplays() {
- const auto display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked());
- if (!display) return;
-
- const sp<IBinder> token = display->getDisplayToken().promote();
- LOG_ALWAYS_FATAL_IF(token == nullptr);
-
TransactionState state;
state.inputWindowCommands = mInputWindowCommands;
const nsecs_t now = systemTime();
@@ -5928,18 +5920,10 @@
const uint64_t transactionId = (static_cast<uint64_t>(mPid) << 32) | mUniqueTransactionId++;
state.id = transactionId;
- // reset screen orientation and use primary layer stack
- DisplayState d;
- d.what = DisplayState::eDisplayProjectionChanged |
- DisplayState::eLayerStackChanged;
- d.token = token;
- d.layerStack = ui::DEFAULT_LAYER_STACK;
- d.orientation = ui::ROTATION_0;
- d.orientedDisplaySpaceRect.makeInvalid();
- d.layerStackSpaceRect.makeInvalid();
- d.width = 0;
- d.height = 0;
- state.displays.add(d);
+ auto layerStack = ui::DEFAULT_LAYER_STACK.id;
+ for (const auto& [id, display] : FTL_FAKE_GUARD(mStateLock, mPhysicalDisplays)) {
+ state.displays.push(DisplayState(display.token(), ui::LayerStack::fromValue(layerStack++)));
+ }
std::vector<TransactionState> transactions;
transactions.emplace_back(state);
@@ -5952,12 +5936,25 @@
{
ftl::FakeGuard guard(mStateLock);
- setPowerModeInternal(display, hal::PowerMode::ON);
+
+ // In case of a restart, ensure all displays are off.
+ for (const auto& [id, display] : mPhysicalDisplays) {
+ setPowerModeInternal(getDisplayDeviceLocked(id), hal::PowerMode::OFF);
+ }
+
+ // Power on all displays. The primary display is first, so becomes the active display. Also,
+ // the DisplayCapability set of a display is populated on its first powering on. Do this now
+ // before responding to any Binder query from DisplayManager about display capabilities.
+ for (const auto& [id, display] : mPhysicalDisplays) {
+ setPowerModeInternal(getDisplayDeviceLocked(id), hal::PowerMode::ON);
+ }
}
}
void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) {
if (display->isVirtual()) {
+ // TODO(b/241285876): This code path should not be reachable, so enforce this at compile
+ // time.
ALOGE("%s: Invalid operation on virtual display", __func__);
return;
}
@@ -5965,8 +5962,8 @@
const auto displayId = display->getPhysicalId();
ALOGD("Setting power mode %d on display %s", mode, to_string(displayId).c_str());
- const auto currentModeOpt = display->getPowerMode();
- if (currentModeOpt == mode) {
+ const auto currentMode = display->getPowerMode();
+ if (currentMode == mode) {
return;
}
@@ -5983,7 +5980,7 @@
display->setPowerMode(mode);
const auto activeMode = display->refreshRateSelector().getActiveMode().modePtr;
- if (!currentModeOpt || *currentModeOpt == hal::PowerMode::OFF) {
+ if (currentMode == hal::PowerMode::OFF) {
// Turn on the display
// Activate the display (which involves a modeset to the active mode) when the inner or
@@ -6028,7 +6025,7 @@
mVisibleRegionsDirty = true;
scheduleComposite(FrameHint::kActive);
} else if (mode == hal::PowerMode::OFF) {
- const bool currentModeNotDozeSuspend = (*currentModeOpt != hal::PowerMode::DOZE_SUSPEND);
+ const bool currentModeNotDozeSuspend = (currentMode != hal::PowerMode::DOZE_SUSPEND);
// Turn off the display
if (displayId == mActiveDisplayId) {
if (const auto display = getActivatableDisplay()) {
@@ -6069,7 +6066,7 @@
} else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) {
// Update display while dozing
getHwComposer().setPowerMode(displayId, mode);
- if (*currentModeOpt == hal::PowerMode::DOZE_SUSPEND &&
+ if (currentMode == hal::PowerMode::DOZE_SUSPEND &&
(displayId == mActiveDisplayId || FlagManager::getInstance().multithreaded_present())) {
if (displayId == mActiveDisplayId) {
ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6ae05d6..c4cf425 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -903,7 +903,8 @@
* Display and layer stack management
*/
- // Called during boot, and restart after system_server death.
+ // Called during boot and restart after system_server death, setting the stage for bootanimation
+ // before DisplayManager takes over.
void initializeDisplays() REQUIRES(kMainThreadContext);
sp<const DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) const
diff --git a/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp b/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp
index f1bb231..b17b529 100644
--- a/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp
@@ -17,7 +17,7 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
-#include "DisplayTransactionTestHelpers.h"
+#include "DualDisplayTransactionTest.h"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -25,31 +25,10 @@
namespace android {
namespace {
-struct ActiveDisplayRotationFlagsTest : DisplayTransactionTest {
- static constexpr bool kWithMockScheduler = false;
- ActiveDisplayRotationFlagsTest() : DisplayTransactionTest(kWithMockScheduler) {}
-
+struct ActiveDisplayRotationFlagsTest
+ : DualDisplayTransactionTest<hal::PowerMode::ON, hal::PowerMode::OFF> {
void SetUp() override {
- injectMockScheduler(kInnerDisplayId);
-
- // Inject inner and outer displays with uninitialized power modes.
- constexpr bool kInitPowerMode = false;
- {
- InnerDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
- auto injector = InnerDisplayVariant::makeFakeExistingDisplayInjector(this);
- injector.setPowerMode(std::nullopt);
- injector.setRefreshRateSelector(mFlinger.scheduler()->refreshRateSelector());
- mInnerDisplay = injector.inject();
- }
- {
- OuterDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
- auto injector = OuterDisplayVariant::makeFakeExistingDisplayInjector(this);
- injector.setPowerMode(std::nullopt);
- mOuterDisplay = injector.inject();
- }
-
- mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);
- mFlinger.setPowerModeInternal(mOuterDisplay, PowerMode::ON);
+ DualDisplayTransactionTest::SetUp();
// The flags are a static variable, so by modifying them in the test, we
// are modifying the real ones used by SurfaceFlinger. Save the original
@@ -64,10 +43,6 @@
void TearDown() override { mFlinger.mutableActiveDisplayRotationFlags() = mOldRotationFlags; }
- static inline PhysicalDisplayId kInnerDisplayId = InnerDisplayVariant::DISPLAY_ID::get();
- static inline PhysicalDisplayId kOuterDisplayId = OuterDisplayVariant::DISPLAY_ID::get();
-
- sp<DisplayDevice> mInnerDisplay, mOuterDisplay;
ui::Transform::RotationFlags mOldRotationFlags;
};
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 99fef7e..f26336a 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -347,7 +347,6 @@
// The HWC active configuration id
static constexpr hal::HWConfigId HWC_ACTIVE_CONFIG_ID = 2001;
- static constexpr PowerMode INIT_POWER_MODE = hal::PowerMode::ON;
static void injectPendingHotplugEvent(DisplayTransactionTest* test, Connection connection) {
test->mFlinger.mutablePendingHotplugEvents().emplace_back(
@@ -355,7 +354,7 @@
}
// Called by tests to inject a HWC display setup
- template <bool kInitPowerMode = true>
+ template <hal::PowerMode kPowerMode = hal::PowerMode::ON>
static void injectHwcDisplayWithNoDefaultCapabilities(DisplayTransactionTest* test) {
const auto displayId = DisplayVariant::DISPLAY_ID::get();
ASSERT_FALSE(GpuVirtualDisplayId::tryCast(displayId));
@@ -364,22 +363,37 @@
.setHwcDisplayId(HWC_DISPLAY_ID)
.setResolution(DisplayVariant::RESOLUTION)
.setActiveConfig(HWC_ACTIVE_CONFIG_ID)
- .setPowerMode(kInitPowerMode ? std::make_optional(INIT_POWER_MODE) : std::nullopt)
+ .setPowerMode(kPowerMode)
.inject(&test->mFlinger, test->mComposer);
}
// Called by tests to inject a HWC display setup
- template <bool kInitPowerMode = true>
+ //
+ // TODO(b/241285876): The `kExpectSetPowerModeOnce` argument is set to `false` by tests that
+ // power on/off displays several times. Replace those catch-all expectations with `InSequence`
+ // and `RetiresOnSaturation`.
+ //
+ template <hal::PowerMode kPowerMode = hal::PowerMode::ON, bool kExpectSetPowerModeOnce = true>
static void injectHwcDisplay(DisplayTransactionTest* test) {
- if constexpr (kInitPowerMode) {
- EXPECT_CALL(*test->mComposer, getDisplayCapabilities(HWC_DISPLAY_ID, _))
- .WillOnce(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})),
- Return(Error::NONE)));
+ if constexpr (kExpectSetPowerModeOnce) {
+ if constexpr (kPowerMode == hal::PowerMode::ON) {
+ 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))
+ EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, kPowerMode))
.WillOnce(Return(Error::NONE));
+ } else {
+ EXPECT_CALL(*test->mComposer, getDisplayCapabilities(HWC_DISPLAY_ID, _))
+ .WillRepeatedly(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})),
+ Return(Error::NONE)));
+
+ EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, _))
+ .WillRepeatedly(Return(Error::NONE));
}
- injectHwcDisplayWithNoDefaultCapabilities<kInitPowerMode>(test);
+
+ injectHwcDisplayWithNoDefaultCapabilities<kPowerMode>(test);
}
static std::shared_ptr<compositionengine::Display> injectCompositionDisplay(
diff --git a/services/surfaceflinger/tests/unittests/DualDisplayTransactionTest.h b/services/surfaceflinger/tests/unittests/DualDisplayTransactionTest.h
new file mode 100644
index 0000000..90e716f
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/DualDisplayTransactionTest.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include "DisplayTransactionTestHelpers.h"
+
+namespace android {
+
+template <hal::PowerMode kInnerDisplayPowerMode, hal::PowerMode kOuterDisplayPowerMode,
+ bool kExpectSetPowerModeOnce = true>
+struct DualDisplayTransactionTest : DisplayTransactionTest {
+ static constexpr bool kWithMockScheduler = false;
+ DualDisplayTransactionTest() : DisplayTransactionTest(kWithMockScheduler) {}
+
+ void SetUp() override {
+ injectMockScheduler(kInnerDisplayId);
+
+ {
+ InnerDisplayVariant::injectHwcDisplay<kInnerDisplayPowerMode, kExpectSetPowerModeOnce>(
+ this);
+
+ auto injector = InnerDisplayVariant::makeFakeExistingDisplayInjector(this);
+ injector.setRefreshRateSelector(mFlinger.scheduler()->refreshRateSelector());
+ injector.setPowerMode(kInnerDisplayPowerMode);
+ mInnerDisplay = injector.inject();
+ }
+ {
+ OuterDisplayVariant::injectHwcDisplay<kOuterDisplayPowerMode, kExpectSetPowerModeOnce>(
+ this);
+
+ auto injector = OuterDisplayVariant::makeFakeExistingDisplayInjector(this);
+ injector.setPowerMode(kOuterDisplayPowerMode);
+ mOuterDisplay = injector.inject();
+ }
+ }
+
+ static inline PhysicalDisplayId kInnerDisplayId = InnerDisplayVariant::DISPLAY_ID::get();
+ static inline PhysicalDisplayId kOuterDisplayId = OuterDisplayVariant::DISPLAY_ID::get();
+
+ sp<DisplayDevice> mInnerDisplay, mOuterDisplay;
+};
+
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
index 844b96c..93c2829 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp
@@ -17,7 +17,7 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
-#include "DisplayTransactionTestHelpers.h"
+#include "DualDisplayTransactionTest.h"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -25,35 +25,9 @@
namespace android {
namespace {
-struct FoldableTest : DisplayTransactionTest {
- static constexpr bool kWithMockScheduler = false;
- FoldableTest() : DisplayTransactionTest(kWithMockScheduler) {}
-
- void SetUp() override {
- injectMockScheduler(kInnerDisplayId);
-
- // Inject inner and outer displays with uninitialized power modes.
- constexpr bool kInitPowerMode = false;
- {
- InnerDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
- auto injector = InnerDisplayVariant::makeFakeExistingDisplayInjector(this);
- injector.setPowerMode(std::nullopt);
- injector.setRefreshRateSelector(mFlinger.scheduler()->refreshRateSelector());
- mInnerDisplay = injector.inject();
- }
- {
- OuterDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
- auto injector = OuterDisplayVariant::makeFakeExistingDisplayInjector(this);
- injector.setPowerMode(std::nullopt);
- mOuterDisplay = injector.inject();
- }
- }
-
- static inline PhysicalDisplayId kInnerDisplayId = InnerDisplayVariant::DISPLAY_ID::get();
- static inline PhysicalDisplayId kOuterDisplayId = OuterDisplayVariant::DISPLAY_ID::get();
-
- sp<DisplayDevice> mInnerDisplay, mOuterDisplay;
-};
+constexpr bool kExpectSetPowerModeOnce = false;
+struct FoldableTest : DualDisplayTransactionTest<hal::PowerMode::OFF, hal::PowerMode::OFF,
+ kExpectSetPowerModeOnce> {};
TEST_F(FoldableTest, promotesPacesetterOnBoot) {
// When the device boots, the inner display should be the pacesetter.
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
index 1583f64..eaf4684 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp
@@ -17,66 +17,49 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
-#include "DisplayTransactionTestHelpers.h"
+#include "DualDisplayTransactionTest.h"
namespace android {
namespace {
-class InitializeDisplaysTest : public DisplayTransactionTest {};
+constexpr bool kExpectSetPowerModeOnce = false;
+struct InitializeDisplaysTest : DualDisplayTransactionTest<hal::PowerMode::OFF, hal::PowerMode::OFF,
+ kExpectSetPowerModeOnce> {};
-TEST_F(InitializeDisplaysTest, commitsPrimaryDisplay) {
- using Case = SimplePrimaryDisplayCase;
-
- // --------------------------------------------------------------------
- // Preconditions
-
- // A primary display is set up
- Case::Display::injectHwcDisplay(this);
- auto primaryDisplay = Case::Display::makeFakeExistingDisplayInjector(this);
- primaryDisplay.inject();
-
- // --------------------------------------------------------------------
- // Call Expectations
-
- // We expect a call to get the active display config.
- Case::Display::setupHwcGetActiveConfigCallExpectations(this);
-
- // We expect a scheduled commit for the display transaction.
- EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1);
+TEST_F(InitializeDisplaysTest, initializesDisplays) {
+ // Scheduled by the display transaction, and by powering on each display.
+ EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(3);
EXPECT_CALL(static_cast<mock::VSyncTracker&>(
mFlinger.scheduler()->getVsyncSchedule()->getTracker()),
nextAnticipatedVSyncTimeFrom(_, _))
.WillRepeatedly(Return(0));
- // --------------------------------------------------------------------
- // Invocation
-
FTL_FAKE_GUARD(kMainThreadContext, mFlinger.initializeDisplays());
- // --------------------------------------------------------------------
- // Postconditions
+ for (const auto& display : {mInnerDisplay, mOuterDisplay}) {
+ const auto token = display->getDisplayToken().promote();
+ ASSERT_TRUE(token);
- // The primary display should have a current state
- ASSERT_TRUE(hasCurrentDisplayState(primaryDisplay.token()));
- const auto& primaryDisplayState = getCurrentDisplayState(primaryDisplay.token());
+ ASSERT_TRUE(hasCurrentDisplayState(token));
+ const auto& state = getCurrentDisplayState(token);
- // The primary display state should be reset
- EXPECT_EQ(ui::DEFAULT_LAYER_STACK, primaryDisplayState.layerStack);
- EXPECT_EQ(ui::ROTATION_0, primaryDisplayState.orientation);
- EXPECT_EQ(Rect::INVALID_RECT, primaryDisplayState.orientedDisplaySpaceRect);
- EXPECT_EQ(Rect::INVALID_RECT, primaryDisplayState.layerStackSpaceRect);
+ const ui::LayerStack expectedLayerStack = display == mInnerDisplay
+ ? ui::DEFAULT_LAYER_STACK
+ : ui::LayerStack::fromValue(ui::DEFAULT_LAYER_STACK.id + 1);
- // The width and height should both be zero
- EXPECT_EQ(0u, primaryDisplayState.width);
- EXPECT_EQ(0u, primaryDisplayState.height);
+ EXPECT_EQ(expectedLayerStack, state.layerStack);
+ EXPECT_EQ(ui::ROTATION_0, state.orientation);
+ EXPECT_EQ(Rect::INVALID_RECT, state.orientedDisplaySpaceRect);
+ EXPECT_EQ(Rect::INVALID_RECT, state.layerStackSpaceRect);
- // The display should be set to PowerMode::ON
- ASSERT_TRUE(hasDisplayDevice(primaryDisplay.token()));
- auto displayDevice = primaryDisplay.mutableDisplayDevice();
- EXPECT_EQ(PowerMode::ON, displayDevice->getPowerMode());
+ EXPECT_EQ(0u, state.width);
+ EXPECT_EQ(0u, state.height);
- // The display transaction needed flag should be set.
+ ASSERT_TRUE(hasDisplayDevice(token));
+ EXPECT_EQ(PowerMode::ON, getDisplayDevice(token).getPowerMode());
+ }
+
EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded));
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index f00eacc..4e73733 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -750,7 +750,6 @@
static constexpr int32_t DEFAULT_CONFIG_GROUP = 7;
static constexpr int32_t DEFAULT_DPI = 320;
static constexpr hal::HWConfigId DEFAULT_ACTIVE_CONFIG = 0;
- static constexpr hal::PowerMode DEFAULT_POWER_MODE = hal::PowerMode::ON;
FakeHwcDisplayInjector(HalDisplayId displayId, hal::DisplayType hwcDisplayType,
bool isPrimary)
@@ -793,7 +792,7 @@
return *this;
}
- auto& setPowerMode(std::optional<hal::PowerMode> mode) {
+ auto& setPowerMode(hal::PowerMode mode) {
mPowerMode = mode;
return *this;
}
@@ -817,9 +816,7 @@
mHwcDisplayType);
display->mutableIsConnected() = true;
- if (mPowerMode) {
- display->setPowerMode(*mPowerMode);
- }
+ display->setPowerMode(mPowerMode);
flinger->mutableHwcDisplayData()[mDisplayId].hwcDisplay = std::move(display);
@@ -885,7 +882,7 @@
int32_t mDpiY = DEFAULT_DPI;
int32_t mConfigGroup = DEFAULT_CONFIG_GROUP;
hal::HWConfigId mActiveConfig = DEFAULT_ACTIVE_CONFIG;
- std::optional<hal::PowerMode> mPowerMode = DEFAULT_POWER_MODE;
+ hal::PowerMode mPowerMode = hal::PowerMode::ON;
const std::unordered_set<aidl::android::hardware::graphics::composer3::Capability>*
mCapabilities = nullptr;
};
@@ -962,7 +959,7 @@
return *this;
}
- auto& setPowerMode(std::optional<hal::PowerMode> mode) {
+ auto& setPowerMode(hal::PowerMode mode) {
mCreationArgs.initialPowerMode = mode;
return *this;
}