diff options
author | 2022-09-29 14:26:53 -0400 | |
---|---|---|
committer | 2024-01-24 13:48:33 -0500 | |
commit | a42d539940191c7c3ada290c52b3a4462cb0cac1 (patch) | |
tree | f24c9f3c418c508c01979102c5fe90d84f8a778e | |
parent | a9c519873255af50508ba47d483b951b272c88e9 (diff) |
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
14 files changed, 186 insertions, 180 deletions
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index e1dc7911a1..6b8e8248b8 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -416,26 +416,36 @@ public: }; 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 @@ struct DisplayState { // 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 c71c517d94..39748b8417 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h @@ -60,7 +60,7 @@ public: 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 c53b46140b..2dc9a1a49a 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h @@ -73,7 +73,7 @@ public: 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 d907bf538d..6428d089a7 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -204,13 +204,12 @@ void Display::setReleasedLayers(const compositionengine::CompositionRefreshArgs& 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 b96264b65e..ef9d1184da 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -68,6 +68,7 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) 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 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) 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 @@ auto DisplayDevice::getFrontEndInfo() const -> frontend::DisplayInfo { } 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 @@ void DisplayDevice::setPowerMode(hal::PowerMode mode) { 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 46534de25e..edd57cce91 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -173,8 +173,8 @@ public: /* ------------------------------------------------------------------------ * 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 @@ private: 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 @@ struct DisplayDeviceCreationArgs { 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 fe5b159051..8eff1b6f17 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1952,6 +1952,7 @@ status_t SurfaceFlinger::setDisplayBrightness(const sp<IBinder>& displayToken, 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 @@ status_t SurfaceFlinger::setDisplayBrightness(const sp<IBinder>& displayToken, 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 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( 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::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32 } 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 @@ void SurfaceFlinger::initializeDisplays() { 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 @@ void SurfaceFlinger::initializeDisplays() { { 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 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 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 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 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 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 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 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: } 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 6ae05d604a..c4cf425eb7 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -903,7 +903,8 @@ private: * 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 f1bb231f26..b17b529793 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 @@ struct ActiveDisplayRotationFlagsTest : DisplayTransactionTest { 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 99fef7ea24..f26336a655 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h @@ -347,7 +347,6 @@ struct HwcDisplayVariant { // 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 @@ struct HwcDisplayVariant { } // 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 @@ struct HwcDisplayVariant { .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) { + 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, kPowerMode)) + .WillOnce(Return(Error::NONE)); + } else { EXPECT_CALL(*test->mComposer, getDisplayCapabilities(HWC_DISPLAY_ID, _)) - .WillOnce(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})), - Return(Error::NONE))); + .WillRepeatedly(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})), + Return(Error::NONE))); - EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, INIT_POWER_MODE)) - .WillOnce(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 0000000000..90e716ff1f --- /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 844b96c5cc..93c282963f 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 1583f64c0a..eaf468432c 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); + + ASSERT_TRUE(hasCurrentDisplayState(token)); + const auto& state = getCurrentDisplayState(token); - // The primary display should have a current state - ASSERT_TRUE(hasCurrentDisplayState(primaryDisplay.token())); - const auto& primaryDisplayState = getCurrentDisplayState(primaryDisplay.token()); + const ui::LayerStack expectedLayerStack = display == mInnerDisplay + ? ui::DEFAULT_LAYER_STACK + : ui::LayerStack::fromValue(ui::DEFAULT_LAYER_STACK.id + 1); - // 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); + 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 width and height should both be zero - EXPECT_EQ(0u, primaryDisplayState.width); - EXPECT_EQ(0u, primaryDisplayState.height); + EXPECT_EQ(0u, state.width); + EXPECT_EQ(0u, state.height); - // The display should be set to PowerMode::ON - ASSERT_TRUE(hasDisplayDevice(primaryDisplay.token())); - auto displayDevice = primaryDisplay.mutableDisplayDevice(); - EXPECT_EQ(PowerMode::ON, displayDevice->getPowerMode()); + ASSERT_TRUE(hasDisplayDevice(token)); + EXPECT_EQ(PowerMode::ON, getDisplayDevice(token).getPowerMode()); + } - // The display transaction needed flag should be set. EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded)); } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index f00eacce8b..4e73733d1d 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -750,7 +750,6 @@ public: 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 @@ public: return *this; } - auto& setPowerMode(std::optional<hal::PowerMode> mode) { + auto& setPowerMode(hal::PowerMode mode) { mPowerMode = mode; return *this; } @@ -817,9 +816,7 @@ public: mHwcDisplayType); display->mutableIsConnected() = true; - if (mPowerMode) { - display->setPowerMode(*mPowerMode); - } + display->setPowerMode(mPowerMode); flinger->mutableHwcDisplayData()[mDisplayId].hwcDisplay = std::move(display); @@ -885,7 +882,7 @@ public: 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 @@ public: return *this; } - auto& setPowerMode(std::optional<hal::PowerMode> mode) { + auto& setPowerMode(hal::PowerMode mode) { mCreationArgs.initialPowerMode = mode; return *this; } |