diff options
11 files changed, 292 insertions, 128 deletions
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 32bd890aee..3c7cbe693f 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -38,7 +38,6 @@ #include <log/log.h> #include <system/window.h> -#include "Display/DisplaySnapshot.h" #include "DisplayDevice.h" #include "FrontEnd/DisplayInfo.h" #include "HdrSdrRatioOverlay.h" @@ -231,10 +230,18 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info, return BAD_VALUE; } mUpcomingActiveMode = info; - ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), info.modeOpt->modePtr->getFps().getIntValue()); - return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), - info.modeOpt->modePtr->getHwcId(), constraints, - outTimeline); + mIsModeSetPending = true; + + const auto& pendingMode = *info.modeOpt->modePtr; + ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), pendingMode.getFps().getIntValue()); + + return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), pendingMode.getHwcId(), + constraints, outTimeline); +} + +void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps displayFps, Fps renderFps) { + setActiveMode(modeId, displayFps, renderFps); + mIsModeSetPending = false; } nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index e92125a45d..a3fa701be2 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -218,6 +218,8 @@ public: return mUpcomingActiveMode; } + bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; } + scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) { return mRefreshRateSelector->getActiveMode(); } @@ -229,6 +231,9 @@ public: hal::VsyncPeriodChangeTimeline* outTimeline) REQUIRES(kMainThreadContext); + void finalizeModeChange(DisplayModeId, Fps displayFps, Fps renderFps) + REQUIRES(kMainThreadContext); + scheduler::RefreshRateSelector& refreshRateSelector() const { return *mRefreshRateSelector; } // Extends the lifetime of the RefreshRateSelector, so it can outlive this DisplayDevice. @@ -313,7 +318,9 @@ private: ActiveModeInfo mDesiredActiveMode GUARDED_BY(mActiveModeLock); TracedOrdinal<bool> mDesiredActiveModeChanged GUARDED_BY(mActiveModeLock) = {ftl::Concat("DesiredActiveModeChanged-", getId().value).c_str(), false}; + ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext); + bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false; }; struct DisplayDeviceState { diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index d6d7725f6c..5a19ec5095 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -186,7 +186,17 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, FrameTargeter& pacesetterTargeter = *pacesetterOpt->get().targeterPtr; pacesetterTargeter.beginFrame(beginFrameArgs, *pacesetterOpt->get().schedulePtr); - if (!compositor.commit(pacesetterTargeter.target())) return; + FrameTargets targets; + targets.try_emplace(pacesetterId, &pacesetterTargeter.target()); + + for (const auto& [id, display] : mDisplays) { + if (id == pacesetterId) continue; + + const FrameTargeter& targeter = *display.targeterPtr; + targets.try_emplace(id, &targeter.target()); + } + + if (!compositor.commit(pacesetterId, targets)) return; // TODO(b/256196556): Choose the frontrunner display. FrameTargeters targeters; diff --git a/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h b/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h index 6fe813a181..12ee36e084 100644 --- a/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h +++ b/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h @@ -29,6 +29,7 @@ namespace scheduler { class FrameTarget; class FrameTargeter; +using FrameTargets = ui::PhysicalDisplayMap<PhysicalDisplayId, const scheduler::FrameTarget*>; using FrameTargeters = ui::PhysicalDisplayMap<PhysicalDisplayId, scheduler::FrameTargeter*>; } // namespace scheduler @@ -39,7 +40,7 @@ struct ICompositor { // Commits transactions for layers and displays. Returns whether any state has been invalidated, // i.e. whether a frame should be composited for each display. - virtual bool commit(const scheduler::FrameTarget&) = 0; + virtual bool commit(PhysicalDisplayId pacesetterId, const scheduler::FrameTargets&) = 0; // Composites a frame for each display. CompositionEngine performs GPU and/or HAL composition // via RenderEngine and the Composer HAL, respectively. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 9f24dd6341..6caf5df0e2 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1195,9 +1195,9 @@ status_t SurfaceFlinger::getDisplayStats(const sp<IBinder>& displayToken, } void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request, bool force) { - ATRACE_CALL(); - const auto displayId = request.mode.modePtr->getPhysicalDisplayId(); + ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); + const auto display = getDisplayDeviceLocked(displayId); if (!display) { ALOGW("%s: display is no longer valid", __func__); @@ -1225,17 +1225,24 @@ void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request, // As we called to set period, we will call to onRefreshRateChangeCompleted once // VsyncController model is locked. mScheduler->modulateVsync(displayId, &VsyncModulator::onRefreshRateChangeInitiated); - updatePhaseConfiguration(mode.fps); + + if (displayId == mActiveDisplayId) { + updatePhaseConfiguration(mode.fps); + } + mScheduler->setModeChangePending(true); break; case DisplayDevice::DesiredActiveModeAction::InitiateRenderRateSwitch: mScheduler->setRenderRate(displayId, mode.fps); - updatePhaseConfiguration(mode.fps); - mRefreshRateStats->setRefreshRate(mode.fps); - if (display->getPhysicalId() == mActiveDisplayId && emitEvent) { - mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, mode); + + if (displayId == mActiveDisplayId) { + updatePhaseConfiguration(mode.fps); + mRefreshRateStats->setRefreshRate(mode.fps); } + if (emitEvent) { + dispatchDisplayModeChangeEvent(displayId, mode); + } break; case DisplayDevice::DesiredActiveModeAction::None: break; @@ -1291,24 +1298,20 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const sp<display::DisplayToke return future.get(); } -void SurfaceFlinger::updateInternalStateWithChangedMode() { - ATRACE_CALL(); - - const auto display = getDefaultDisplayDeviceLocked(); - if (!display) { - return; - } +void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { + const auto displayId = display.getPhysicalId(); + ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); - const auto upcomingModeInfo = display->getUpcomingActiveMode(); + const auto upcomingModeInfo = display.getUpcomingActiveMode(); if (!upcomingModeInfo.modeOpt) { // There is no pending mode change. This can happen if the active // display changed and the mode change happened on a different display. return; } - if (display->getActiveMode().modePtr->getResolution() != + if (display.getActiveMode().modePtr->getResolution() != upcomingModeInfo.modeOpt->modePtr->getResolution()) { - auto& state = mCurrentState.displays.editValueFor(display->getDisplayToken()); + auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken()); // We need to generate new sequenceId in order to recreate the display (and this // way the framebuffer). state.sequenceId = DisplayDeviceState{}.sequenceId; @@ -1319,27 +1322,24 @@ void SurfaceFlinger::updateInternalStateWithChangedMode() { return; } - mPhysicalDisplays.get(display->getPhysicalId()) - .transform(&PhysicalDisplay::snapshotRef) - .transform(ftl::unit_fn([&](const display::DisplaySnapshot& snapshot) { - FTL_FAKE_GUARD(kMainThreadContext, - display->setActiveMode(upcomingModeInfo.modeOpt->modePtr->getId(), - upcomingModeInfo.modeOpt->modePtr->getFps(), - upcomingModeInfo.modeOpt->fps)); - })); - - const Fps refreshRate = upcomingModeInfo.modeOpt->fps; - mRefreshRateStats->setRefreshRate(refreshRate); - updatePhaseConfiguration(refreshRate); + const auto& activeMode = *upcomingModeInfo.modeOpt; + display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getFps(), + activeMode.fps); + + if (displayId == mActiveDisplayId) { + mRefreshRateStats->setRefreshRate(activeMode.fps); + updatePhaseConfiguration(activeMode.fps); + } if (upcomingModeInfo.event != scheduler::DisplayModeEvent::None) { - mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, *upcomingModeInfo.modeOpt); + dispatchDisplayModeChangeEvent(displayId, activeMode); } } void SurfaceFlinger::clearDesiredActiveModeState(const sp<DisplayDevice>& display) { display->clearDesiredActiveModeState(); if (display->getPhysicalId() == mActiveDisplayId) { + // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); } } @@ -1353,21 +1353,18 @@ void SurfaceFlinger::desiredActiveModeChangeDone(const sp<DisplayDevice>& displa clearDesiredActiveModeState(display); mScheduler->resyncToHardwareVsync(displayId, true /* allowToEnable */, displayFps); mScheduler->setRenderRate(displayId, renderFps); - updatePhaseConfiguration(renderFps); + + if (displayId == mActiveDisplayId) { + updatePhaseConfiguration(renderFps); + } } -void SurfaceFlinger::setActiveModeInHwcIfNeeded() { +void SurfaceFlinger::initiateDisplayModeChanges() { ATRACE_CALL(); std::optional<PhysicalDisplayId> displayToUpdateImmediately; for (const auto& [id, physical] : mPhysicalDisplays) { - const auto& snapshot = physical.snapshot(); - - if (snapshot.connectionType() != ui::DisplayConnectionType::Internal) { - continue; - } - const auto display = getDisplayDeviceLocked(id); if (!display) continue; @@ -1378,14 +1375,14 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { continue; } - if (id != mActiveDisplayId) { - // Display is no longer the active display, so abort the mode change. + if (!display->isPoweredOn()) { + // Display is no longer powered on, so abort the mode change. clearDesiredActiveModeState(display); continue; } const auto desiredModeId = desiredActiveMode->modeOpt->modePtr->getId(); - const auto displayModePtrOpt = snapshot.displayModes().get(desiredModeId); + const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId); if (!displayModePtrOpt) { ALOGW("Desired display mode is no longer supported. Mode ID = %d", @@ -1435,19 +1432,18 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { if (outTimeline.refreshRequired) { scheduleComposite(FrameHint::kNone); - mSetActiveModePending = true; } else { - // Updating the internal state should be done outside the loop, - // because it can recreate a DisplayDevice and modify mDisplays - // which will invalidate the iterator. + // TODO(b/255635711): Remove `displayToUpdateImmediately` to `finalizeDisplayModeChange` + // for all displays. This was only needed when the loop iterated over `mDisplays` rather + // than `mPhysicalDisplays`. displayToUpdateImmediately = display->getPhysicalId(); } } if (displayToUpdateImmediately) { - updateInternalStateWithChangedMode(); - const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately); + finalizeDisplayModeChange(*display); + const auto desiredActiveMode = display->getDesiredActiveMode(); if (desiredActiveMode && display->getActiveMode() == desiredActiveMode->modeOpt) { desiredActiveModeChangeDone(display); @@ -2398,7 +2394,10 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, return mustComposite; } -bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget) { +bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, + const scheduler::FrameTargets& frameTargets) { + const scheduler::FrameTarget& pacesetterFrameTarget = *frameTargets.get(pacesetterId)->get(); + const VsyncId vsyncId = pacesetterFrameTarget.vsyncId(); ATRACE_NAME(ftl::Concat(__func__, ' ', ftl::to_underlying(vsyncId)).c_str()); @@ -2411,20 +2410,35 @@ bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget) mTracingEnabledChanged = false; } - // If we are in the middle of a mode change and the fence hasn't - // fired yet just wait for the next commit. - if (mSetActiveModePending) { - if (pacesetterFrameTarget.isFramePending()) { - mScheduler->scheduleFrame(); - return false; - } + // If a mode set is pending and the fence hasn't fired yet, wait for the next commit. + if (std::any_of(frameTargets.begin(), frameTargets.end(), + [this](const auto& pair) FTL_FAKE_GUARD(mStateLock) + FTL_FAKE_GUARD(kMainThreadContext) { + if (!pair.second->isFramePending()) return false; - // We received the present fence from the HWC, so we assume it successfully updated - // the mode, hence we update SF. - mSetActiveModePending = false; - { - Mutex::Autolock lock(mStateLock); - updateInternalStateWithChangedMode(); + if (const auto display = getDisplayDeviceLocked(pair.first)) { + return display->isModeSetPending(); + } + + return false; + })) { + mScheduler->scheduleFrame(); + return false; + } + + { + Mutex::Autolock lock(mStateLock); + + for (const auto [id, target] : frameTargets) { + // TODO(b/241285876): This is `nullptr` when the DisplayDevice is about to be removed in + // this commit, since the PhysicalDisplay has already been removed. Rather than checking + // for `nullptr` below, change Scheduler::onFrameSignal to filter out the FrameTarget of + // the removed display. + const auto display = getDisplayDeviceLocked(id); + + if (display && display->isModeSetPending()) { + finalizeDisplayModeChange(*display); + } } } @@ -2515,7 +2529,7 @@ bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget) ? &mLayerHierarchyBuilder.getHierarchy() : nullptr, updateAttachedChoreographer); - setActiveModeInHwcIfNeeded(); + initiateDisplayModeChanges(); } updateCursorAsync(); @@ -3322,6 +3336,16 @@ void SurfaceFlinger::dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bo mScheduler->onHotplugReceived(mSfConnectionHandle, displayId, connected); } +void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId, + const scheduler::FrameRateMode& mode) { + // TODO(b/255635821): Merge code paths and move to Scheduler. + const auto onDisplayModeChanged = displayId == mActiveDisplayId + ? &scheduler::Scheduler::onPrimaryDisplayModeChanged + : &scheduler::Scheduler::onNonPrimaryDisplayModeChanged; + + ((*mScheduler).*onDisplayModeChanged)(mAppConnectionHandle, mode); +} + sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( const wp<IBinder>& displayToken, std::shared_ptr<compositionengine::Display> compositionDisplay, @@ -3420,14 +3444,8 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( RenderIntent::COLORIMETRIC}); if (const auto& physical = state.physical) { - mPhysicalDisplays.get(physical->id) - .transform(&PhysicalDisplay::snapshotRef) - .transform(ftl::unit_fn([&](const display::DisplaySnapshot& snapshot) { - FTL_FAKE_GUARD(kMainThreadContext, - display->setActiveMode(physical->activeMode->getId(), - physical->activeMode->getFps(), - physical->activeMode->getFps())); - })); + const auto& mode = *physical->activeMode; + display->setActiveMode(mode.getId(), mode.getFps(), mode.getFps()); } display->setLayerFilter(makeLayerFilterForDisplay(display->getId(), state.layerStack)); @@ -3946,12 +3964,8 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest if (!display) continue; - const bool isInternalDisplay = mPhysicalDisplays.get(displayId) - .transform(&PhysicalDisplay::isInternal) - .value_or(false); - - if (isInternalDisplay && displayId != mActiveDisplayId) { - ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str()); + if (!display->isPoweredOn()) { + ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str()); continue; } @@ -3959,7 +3973,7 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest setDesiredActiveMode(std::move(request)); } else { ALOGV("%s: Mode %d is disallowed for display %s", __func__, modePtr->getId().value(), - to_string(display->getId()).c_str()); + to_string(displayId).c_str()); } } } @@ -7922,6 +7936,7 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( const sp<DisplayDevice>& display, const scheduler::RefreshRateSelector::PolicyVariant& policy) { const auto displayId = display->getPhysicalId(); + ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); Mutex::Autolock lock(mStateLock); @@ -7942,13 +7957,11 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( break; } - const bool isInternalDisplay = mPhysicalDisplays.get(displayId) - .transform(&PhysicalDisplay::isInternal) - .value_or(false); - - if (isInternalDisplay && displayId != mActiveDisplayId) { - // The policy will be be applied when the display becomes active. - ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str()); + // TODO(b/255635711): Apply the policy once the display is powered on, which is currently only + // done for the internal display that becomes active on fold/unfold. For now, assume that DM + // always powers on the secondary (internal or external) display before setting its policy. + if (!display->isPoweredOn()) { + ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str()); return NO_ERROR; } @@ -8311,7 +8324,9 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD resetPhaseConfiguration(activeDisplay.getActiveMode().fps); + // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); + mScheduler->setPacesetterDisplay(mActiveDisplayId); onActiveDisplaySizeChanged(activeDisplay); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b07910d360..f61f9a2345 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -638,7 +638,8 @@ private: // ICompositor overrides: void configure() override REQUIRES(kMainThreadContext); - bool commit(const scheduler::FrameTarget&) override REQUIRES(kMainThreadContext); + bool commit(PhysicalDisplayId pacesetterId, const scheduler::FrameTargets&) override + REQUIRES(kMainThreadContext); CompositeResultsPerDisplay composite(PhysicalDisplayId pacesetterId, const scheduler::FrameTargeters&) override REQUIRES(kMainThreadContext); @@ -684,11 +685,10 @@ private: REQUIRES(mStateLock); status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId); - // Sets the active mode and a new refresh rate in SF. - void updateInternalStateWithChangedMode() REQUIRES(mStateLock, kMainThreadContext); - // Calls to setActiveMode on the main thread if there is a pending mode change - // that needs to be applied. - void setActiveModeInHwcIfNeeded() REQUIRES(mStateLock, kMainThreadContext); + + void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext); + void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext); + void clearDesiredActiveModeState(const sp<DisplayDevice>&) REQUIRES(mStateLock); // Called when active mode is no longer is progress void desiredActiveModeChangeDone(const sp<DisplayDevice>&) REQUIRES(mStateLock); @@ -1011,7 +1011,9 @@ private: const DisplayDeviceState& drawingState) REQUIRES(mStateLock, kMainThreadContext); - void dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected); + void dispatchDisplayHotplugEvent(PhysicalDisplayId, bool connected); + void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&) + REQUIRES(mStateLock); /* * VSYNC @@ -1330,9 +1332,6 @@ private: std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats; scheduler::PresentLatencyTracker mPresentLatencyTracker GUARDED_BY(kMainThreadContext); - // below flags are set by main thread only - bool mSetActiveModePending = false; - bool mLumaSampling = true; sp<RegionSamplingThread> mRegionSamplingThread; sp<FpsReporter> mFpsReporter; diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h index 28ac664ba3..ca1af6ed84 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h @@ -286,7 +286,7 @@ public: private: // ICompositor overrides: void configure() override {} - bool commit(const scheduler::FrameTarget&) override { return false; } + bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; } CompositeResultsPerDisplay composite(PhysicalDisplayId, const scheduler::FrameTargeters&) override { return {}; diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp index 1dcf222834..9aa089f900 100644 --- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp @@ -35,7 +35,7 @@ using CallbackToken = scheduler::VSyncDispatch::CallbackToken; struct NoOpCompositor final : ICompositor { void configure() override {} - bool commit(const scheduler::FrameTarget&) override { return false; } + bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; } CompositeResultsPerDisplay composite(PhysicalDisplayId, const scheduler::FrameTargeters&) override { return {}; diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 703bdda694..24eb31821a 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -67,10 +67,36 @@ public: .WillByDefault(Return(true)); } + static constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID; + static constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1; + + auto injectOuterDisplay() { + constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u); + + constexpr bool kIsPrimary = false; + TestableSurfaceFlinger::FakeHwcDisplayInjector(kOuterDisplayId, hal::DisplayType::PHYSICAL, + kIsPrimary) + .setHwcDisplayId(kOuterDisplayHwcId) + .setPowerMode(hal::PowerMode::OFF) + .inject(&mFlinger, mComposer); + + mOuterDisplay = mFakeDisplayInjector.injectInternalDisplay( + [&](FakeDisplayDeviceInjector& injector) { + injector.setPowerMode(hal::PowerMode::OFF); + injector.setDisplayModes(mock::cloneForDisplay(kOuterDisplayId, kModes), + kModeId120); + }, + {.displayId = kOuterDisplayId, + .hwcDisplayId = kOuterDisplayHwcId, + .isPrimary = kIsPrimary}); + + return std::forward_as_tuple(mDisplay, mOuterDisplay); + } + protected: void setupScheduler(std::shared_ptr<scheduler::RefreshRateSelector>); - sp<DisplayDevice> mDisplay; + sp<DisplayDevice> mDisplay, mOuterDisplay; mock::EventThread* mAppEventThread; static constexpr DisplayModeId kModeId60{0}; @@ -328,32 +354,16 @@ MATCHER_P(ModeSettledTo, modeId, "") { return true; } -TEST_F(DisplayModeSwitchingTest, multiDisplay) { - constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID; - constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1; - - constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u); +TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { + const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); - constexpr bool kIsPrimary = false; - TestableSurfaceFlinger::FakeHwcDisplayInjector(kOuterDisplayId, hal::DisplayType::PHYSICAL, - kIsPrimary) - .setHwcDisplayId(kOuterDisplayHwcId) - .inject(&mFlinger, mComposer); - - const auto outerDisplay = mFakeDisplayInjector.injectInternalDisplay( - [&](FakeDisplayDeviceInjector& injector) { - injector.setDisplayModes(mock::cloneForDisplay(kOuterDisplayId, kModes), - kModeId120); - }, - {.displayId = kOuterDisplayId, - .hwcDisplayId = kOuterDisplayHwcId, - .isPrimary = kIsPrimary}); - - const auto& innerDisplay = mDisplay; + EXPECT_TRUE(innerDisplay->isPoweredOn()); + EXPECT_FALSE(outerDisplay->isPoweredOn()); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + // Only the inner display is powered on. mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); @@ -388,6 +398,10 @@ TEST_F(DisplayModeSwitchingTest, multiDisplay) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + innerDisplay->setPowerMode(hal::PowerMode::OFF); + outerDisplay->setPowerMode(hal::PowerMode::ON); + + // Only the outer display is powered on. mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); @@ -409,5 +423,107 @@ TEST_F(DisplayModeSwitchingTest, multiDisplay) { EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); } +TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { + const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); + + EXPECT_TRUE(innerDisplay->isPoweredOn()); + EXPECT_FALSE(outerDisplay->isPoweredOn()); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + + outerDisplay->setPowerMode(hal::PowerMode::ON); + + // Both displays are powered on. + mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId90.value(), + false, 0.f, 120.f))); + + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId60.value(), + false, 0.f, 120.f))); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + + const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; + EXPECT_CALL(*mComposer, + setActiveConfigWithConstraints(kInnerDisplayHwcId, + hal::HWConfigId(kModeId90.value()), _, _)) + .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE))); + + EXPECT_CALL(*mComposer, + setActiveConfigWithConstraints(kOuterDisplayHwcId, + hal::HWConfigId(kModeId60.value()), _, _)) + .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE))); + + mFlinger.commit(); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + + mFlinger.commit(); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); +} + +TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { + const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); + + EXPECT_TRUE(innerDisplay->isPoweredOn()); + EXPECT_FALSE(outerDisplay->isPoweredOn()); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + + outerDisplay->setPowerMode(hal::PowerMode::ON); + + // Both displays are powered on. + mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId90.value(), + false, 0.f, 120.f))); + + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId60.value(), + false, 0.f, 120.f))); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + + // Power off the outer display before the mode has been set. + outerDisplay->setPowerMode(hal::PowerMode::OFF); + + const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; + EXPECT_CALL(*mComposer, + setActiveConfigWithConstraints(kInnerDisplayHwcId, + hal::HWConfigId(kModeId90.value()), _, _)) + .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE))); + + mFlinger.commit(); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + + mFlinger.commit(); + + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); +} + } // namespace } // namespace android diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index f3c9d0dd44..151b178c01 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -180,7 +180,7 @@ public: private: // ICompositor overrides: void configure() override {} - bool commit(const scheduler::FrameTarget&) override { return false; } + bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; } CompositeResultsPerDisplay composite(PhysicalDisplayId, const scheduler::FrameTargeters&) override { return {}; diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 9b3a893409..e59d44d745 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -386,10 +386,19 @@ public: .sfWorkDuration = 10ms}, *mScheduler->getVsyncSchedule()); - mFlinger->commit(frameTargeter.target()); + scheduler::FrameTargets targets; + scheduler::FrameTargeters targeters; + + for (const auto& [id, display] : + FTL_FAKE_GUARD(mFlinger->mStateLock, mFlinger->mPhysicalDisplays)) { + targets.try_emplace(id, &frameTargeter.target()); + targeters.try_emplace(id, &frameTargeter); + } + + mFlinger->commit(displayId, targets); if (composite) { - mFlinger->composite(displayId, ftl::init::map(displayId, &frameTargeter)); + mFlinger->composite(displayId, targeters); } } |