diff options
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 799d62c19d..4f81482814 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -67,6 +67,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)), @@ -105,9 +106,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); @@ -172,6 +171,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); @@ -181,33 +181,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 97b56a2f19..4ab6321064 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(); @@ -271,9 +271,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; @@ -363,7 +361,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 ddd78ae163..9426f975be 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1950,6 +1950,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( @@ -1999,7 +2000,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); @@ -3544,9 +3544,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; @@ -5810,12 +5808,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(); @@ -5826,18 +5818,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); @@ -5850,12 +5834,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; } @@ -5863,8 +5860,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; } @@ -5881,7 +5878,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 @@ -5926,7 +5923,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()) { @@ -5967,7 +5964,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 6e12172538..7cfe46cbea 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 6671414ba6..387d2f26e6 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 4e0b5afa99..9ef0749dc6 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; } |