From 39f510fb536d26247997141bdfdcc7d8af890514 Mon Sep 17 00:00:00 2001 From: Nergi Rahardi Date: Thu, 23 May 2024 15:16:54 +0900 Subject: Pass dequeue timestamp along with buffer data Avoids performance penalties associated with metadata. Bug: 342174822 Test: atest libsurfaceflinger_unittest Test: Manually verified dequeueTime sent without triggering metadata change Change-Id: Ifed747818ea252b2551780ffcefd3309eb41edbe --- services/surfaceflinger/SurfaceFlinger.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 5f81cd45cc..c0faf16927 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5618,10 +5618,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime layer->setInputInfo(*s.windowInfoHandle->getInfo()); flags |= eTraversalNeeded; } - std::optional dequeueBufferTimestamp; if (what & layer_state_t::eMetadataChanged) { - dequeueBufferTimestamp = s.metadata.getInt64(gui::METADATA_DEQUEUE_TIME); - if (const int32_t gameMode = s.metadata.getInt32(gui::METADATA_GAME_MODE, -1); gameMode != -1) { // The transaction will be received on the Task layer and needs to be applied to all @@ -5763,8 +5760,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime if (what & layer_state_t::eBufferChanged) { if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime, - desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, - frameTimelineInfo)) { + desiredPresentTime, isAutoTimestamp, frameTimelineInfo)) { flags |= eTraversalNeeded; } } else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) { @@ -5850,10 +5846,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f if (what & layer_state_t::eProducerDisconnect) { layer->onDisconnect(); } - std::optional dequeueBufferTimestamp; - if (what & layer_state_t::eMetadataChanged) { - dequeueBufferTimestamp = s.metadata.getInt64(gui::METADATA_DEQUEUE_TIME); - } std::vector> callbackHandles; if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) { @@ -5900,8 +5892,7 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } layer->setTransformHint(transformHint); if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime, - desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, - frameTimelineInfo)) { + desiredPresentTime, isAutoTimestamp, frameTimelineInfo)) { flags |= eTraversalNeeded; } mLayersWithQueuedFrames.emplace(layer); -- cgit v1.2.3-59-g8ed1b From 5c989f5c72c52cba1fc2264525fac2e58bdf9f94 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Thu, 11 Apr 2024 13:57:14 -0400 Subject: SF: Isolate modesetting in DisplayModeController Move the per-display state machine for modesetting from DisplayDevice to DMC. In lieu of mStateLock, protect display lookup from multiple threads using a mutex internal to DMC, which fixes the following deadlock: OneShotTimer::loop SF::requestDisplayModes mStateLock SF::commit mStateLock SF::processDisplayChangesLocked (hotplug or resolution change) Scheduler::demotePacesetterDisplay OneShotTimer::stop A notable change is that {initiate,finalize}DisplayModeChange(s) are no longer called under mStateLock, thanks to DMC's granular, internal lock. finalizeDisplayModeChange still locks mStateLock for resolution changes. Add an ActiveModeListener to DMC and register a callback in SF to update the refresh rate overlay, which still lives in DisplayDevice for now. Fixes: 329450361 Bug: 241285876 Test: DisplayModeControllerTest Test: libsurfaceflinger_unittest Change-Id: I30ec756f134d2d67a70ac8797008dc792eac035e --- .../Display/DisplayModeController.cpp | 199 +++++++++++++++++- .../surfaceflinger/Display/DisplayModeController.h | 99 +++++++-- services/surfaceflinger/DisplayDevice.cpp | 108 +--------- services/surfaceflinger/DisplayDevice.h | 58 +---- services/surfaceflinger/SurfaceFlinger.cpp | 198 +++++++++-------- services/surfaceflinger/SurfaceFlinger.h | 17 +- services/surfaceflinger/tests/unittests/Android.bp | 2 +- .../unittests/DisplayDevice_InitiateModeChange.cpp | 152 ------------- .../tests/unittests/DisplayModeControllerTest.cpp | 234 +++++++++++++++++++++ .../SurfaceFlinger_DisplayModeSwitching.cpp | 149 ++++++------- ...ceFlinger_SetupNewDisplayDeviceInternalTest.cpp | 13 +- .../tests/unittests/TestableSurfaceFlinger.h | 14 +- 12 files changed, 733 insertions(+), 510 deletions(-) delete mode 100644 services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp create mode 100644 services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/Display/DisplayModeController.cpp b/services/surfaceflinger/Display/DisplayModeController.cpp index f093384921..a6a9bec3c3 100644 --- a/services/surfaceflinger/Display/DisplayModeController.cpp +++ b/services/surfaceflinger/Display/DisplayModeController.cpp @@ -20,30 +20,221 @@ #include "Display/DisplayModeController.h" #include "Display/DisplaySnapshot.h" +#include "DisplayHardware/HWComposer.h" +#include +#include +#include #include namespace android::display { +template +inline std::string DisplayModeController::Display::concatId(const char (&str)[N]) const { + return std::string(ftl::Concat(str, ' ', snapshot.get().displayId().value).str()); +} + +DisplayModeController::Display::Display(DisplaySnapshotRef snapshot, + RefreshRateSelectorPtr selectorPtr) + : snapshot(snapshot), + selectorPtr(std::move(selectorPtr)), + pendingModeFpsTrace(concatId("PendingModeFps")), + activeModeFpsTrace(concatId("ActiveModeFps")), + renderRateFpsTrace(concatId("RenderRateFps")), + hasDesiredModeTrace(concatId("HasDesiredMode"), false) {} + +void DisplayModeController::registerDisplay(PhysicalDisplayId displayId, + DisplaySnapshotRef snapshotRef, + RefreshRateSelectorPtr selectorPtr) { + std::lock_guard lock(mDisplayLock); + mDisplays.emplace_or_replace(displayId, std::make_unique(snapshotRef, selectorPtr)); +} + void DisplayModeController::registerDisplay(DisplaySnapshotRef snapshotRef, DisplayModeId activeModeId, scheduler::RefreshRateSelector::Config config) { const auto& snapshot = snapshotRef.get(); const auto displayId = snapshot.displayId(); - mDisplays.emplace_or_replace(displayId, snapshotRef, snapshot.displayModes(), activeModeId, - config); + std::lock_guard lock(mDisplayLock); + mDisplays.emplace_or_replace(displayId, + std::make_unique(snapshotRef, snapshot.displayModes(), + activeModeId, config)); } void DisplayModeController::unregisterDisplay(PhysicalDisplayId displayId) { + std::lock_guard lock(mDisplayLock); const bool ok = mDisplays.erase(displayId); ALOGE_IF(!ok, "%s: Unknown display %s", __func__, to_string(displayId).c_str()); } -auto DisplayModeController::selectorPtrFor(PhysicalDisplayId displayId) -> RefreshRateSelectorPtr { +auto DisplayModeController::selectorPtrFor(PhysicalDisplayId displayId) const + -> RefreshRateSelectorPtr { + std::lock_guard lock(mDisplayLock); return mDisplays.get(displayId) - .transform([](const Display& display) { return display.selectorPtr; }) + .transform([](const DisplayPtr& displayPtr) { return displayPtr->selectorPtr; }) .value_or(nullptr); } +auto DisplayModeController::setDesiredMode(PhysicalDisplayId displayId, + DisplayModeRequest&& desiredMode) -> DesiredModeAction { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DesiredModeAction::None)).get(); + + { + ATRACE_NAME(displayPtr->concatId(__func__).c_str()); + ALOGD("%s %s", displayPtr->concatId(__func__).c_str(), to_string(desiredMode).c_str()); + + std::scoped_lock lock(displayPtr->desiredModeLock); + + if (auto& desiredModeOpt = displayPtr->desiredModeOpt) { + // A mode transition was already scheduled, so just override the desired mode. + const bool emitEvent = desiredModeOpt->emitEvent; + const bool force = desiredModeOpt->force; + desiredModeOpt = std::move(desiredMode); + desiredModeOpt->emitEvent |= emitEvent; + if (FlagManager::getInstance().connected_display()) { + desiredModeOpt->force |= force; + } + return DesiredModeAction::None; + } + + // If the desired mode is already active... + const auto activeMode = displayPtr->selectorPtr->getActiveMode(); + if (const auto& desiredModePtr = desiredMode.mode.modePtr; + !desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) { + if (activeMode == desiredMode.mode) { + return DesiredModeAction::None; + } + + // ...but the render rate changed: + setActiveModeLocked(displayId, desiredModePtr->getId(), desiredModePtr->getVsyncRate(), + desiredMode.mode.fps); + return DesiredModeAction::InitiateRenderRateSwitch; + } + + // Restore peak render rate to schedule the next frame as soon as possible. + setActiveModeLocked(displayId, activeMode.modePtr->getId(), + activeMode.modePtr->getVsyncRate(), activeMode.modePtr->getPeakFps()); + + // Initiate a mode change. + displayPtr->desiredModeOpt = std::move(desiredMode); + displayPtr->hasDesiredModeTrace = true; + } + + return DesiredModeAction::InitiateDisplayModeSwitch; +} + +auto DisplayModeController::getDesiredMode(PhysicalDisplayId displayId) const + -> DisplayModeRequestOpt { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DisplayModeRequestOpt())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->desiredModeOpt; + } +} + +auto DisplayModeController::getPendingMode(PhysicalDisplayId displayId) const + -> DisplayModeRequestOpt { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DisplayModeRequestOpt())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->pendingModeOpt; + } +} + +bool DisplayModeController::isModeSetPending(PhysicalDisplayId displayId) const { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->isModeSetPending; + } +} + +scheduler::FrameRateMode DisplayModeController::getActiveMode(PhysicalDisplayId displayId) const { + return selectorPtrFor(displayId)->getActiveMode(); +} + +void DisplayModeController::clearDesiredMode(PhysicalDisplayId displayId) { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + displayPtr->desiredModeOpt.reset(); + displayPtr->hasDesiredModeTrace = false; + } +} + +bool DisplayModeController::initiateModeChange(PhysicalDisplayId displayId, + DisplayModeRequest&& desiredMode, + const hal::VsyncPeriodChangeConstraints& constraints, + hal::VsyncPeriodChangeTimeline& outTimeline) { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get(); + + // TODO: b/255635711 - Flow the DisplayModeRequest through the desired/pending/active states. + // For now, `desiredMode` and `desiredModeOpt` are one and the same, but the latter is not + // cleared until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been + // consumed at this point, so clear the `force` flag to prevent an endless loop of + // `initiateModeChange`. + if (FlagManager::getInstance().connected_display()) { + std::scoped_lock lock(displayPtr->desiredModeLock); + if (displayPtr->desiredModeOpt) { + displayPtr->desiredModeOpt->force = false; + } + } + + displayPtr->pendingModeOpt = std::move(desiredMode); + displayPtr->isModeSetPending = true; + + const auto& mode = *displayPtr->pendingModeOpt->mode.modePtr; + + if (mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(), constraints, + &outTimeline) != OK) { + return false; + } + + ATRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue()); + return true; +} + +void DisplayModeController::finalizeModeChange(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + std::lock_guard lock(mDisplayLock); + setActiveModeLocked(displayId, modeId, vsyncRate, renderFps); + + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + displayPtr->isModeSetPending = false; +} + +void DisplayModeController::setActiveMode(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + std::lock_guard lock(mDisplayLock); + setActiveModeLocked(displayId, modeId, vsyncRate, renderFps); +} + +void DisplayModeController::setActiveModeLocked(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + + ATRACE_INT(displayPtr->activeModeFpsTrace.c_str(), vsyncRate.getIntValue()); + ATRACE_INT(displayPtr->renderRateFpsTrace.c_str(), renderFps.getIntValue()); + + displayPtr->selectorPtr->setActiveMode(modeId, renderFps); + + if (mActiveModeListener) { + mActiveModeListener(displayId, vsyncRate, renderFps); + } +} + } // namespace android::display diff --git a/services/surfaceflinger/Display/DisplayModeController.h b/services/surfaceflinger/Display/DisplayModeController.h index b6a6bee714..258b04b876 100644 --- a/services/surfaceflinger/Display/DisplayModeController.h +++ b/services/surfaceflinger/Display/DisplayModeController.h @@ -17,16 +17,26 @@ #pragma once #include +#include +#include #include #include +#include +#include #include #include +#include "Display/DisplayModeRequest.h" #include "Display/DisplaySnapshotRef.h" #include "DisplayHardware/DisplayMode.h" #include "Scheduler/RefreshRateSelector.h" #include "ThreadContext.h" +#include "TracedOrdinal.h" + +namespace android { +class HWComposer; +} // namespace android namespace android::display { @@ -34,40 +44,97 @@ namespace android::display { // certain heuristic signals. class DisplayModeController { public: - // The referenced DisplaySnapshot must outlive the registration. - void registerDisplay(DisplaySnapshotRef, DisplayModeId, scheduler::RefreshRateSelector::Config) - REQUIRES(kMainThreadContext); - void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext); + using ActiveModeListener = ftl::Function; + + DisplayModeController() = default; - // TODO(b/241285876): Remove once ownership is no longer shared with DisplayDevice. + void setHwComposer(HWComposer* composerPtr) { mComposerPtr = composerPtr; } + void setActiveModeListener(const ActiveModeListener& listener) { + mActiveModeListener = listener; + } + + // TODO: b/241285876 - Remove once ownership is no longer shared with DisplayDevice. using RefreshRateSelectorPtr = std::shared_ptr; + // Used by tests to inject an existing RefreshRateSelector. + // TODO: b/241285876 - Remove this. + void registerDisplay(PhysicalDisplayId, DisplaySnapshotRef, RefreshRateSelectorPtr) + EXCLUDES(mDisplayLock); + + // The referenced DisplaySnapshot must outlive the registration. + void registerDisplay(DisplaySnapshotRef, DisplayModeId, scheduler::RefreshRateSelector::Config) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + // Returns `nullptr` if the display is no longer registered (or never was). - RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) REQUIRES(kMainThreadContext); + RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) const EXCLUDES(mDisplayLock); - // Used by tests to inject an existing RefreshRateSelector. - // TODO(b/241285876): Remove this. - void registerDisplay(PhysicalDisplayId displayId, DisplaySnapshotRef snapshotRef, - RefreshRateSelectorPtr selectorPtr) { - mDisplays.emplace_or_replace(displayId, snapshotRef, selectorPtr); - } + enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch }; + + DesiredModeAction setDesiredMode(PhysicalDisplayId, DisplayModeRequest&&) + EXCLUDES(mDisplayLock); + + using DisplayModeRequestOpt = ftl::Optional; + + DisplayModeRequestOpt getDesiredMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock); + void clearDesiredMode(PhysicalDisplayId) EXCLUDES(mDisplayLock); + + DisplayModeRequestOpt getPendingMode(PhysicalDisplayId) const REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + bool isModeSetPending(PhysicalDisplayId) const REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + + scheduler::FrameRateMode getActiveMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock); + + bool initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&, + const hal::VsyncPeriodChangeConstraints&, + hal::VsyncPeriodChangeTimeline& outTimeline) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + + void finalizeModeChange(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + + void setActiveMode(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + EXCLUDES(mDisplayLock); private: struct Display { - Display(DisplaySnapshotRef snapshot, RefreshRateSelectorPtr selectorPtr) - : snapshot(snapshot), selectorPtr(std::move(selectorPtr)) {} + template + std::string concatId(const char (&)[N]) const; + Display(DisplaySnapshotRef, RefreshRateSelectorPtr); Display(DisplaySnapshotRef snapshot, DisplayModes modes, DisplayModeId activeModeId, scheduler::RefreshRateSelector::Config config) : Display(snapshot, std::make_shared(std::move(modes), activeModeId, config)) {} - const DisplaySnapshotRef snapshot; const RefreshRateSelectorPtr selectorPtr; + + const std::string pendingModeFpsTrace; + const std::string activeModeFpsTrace; + const std::string renderRateFpsTrace; + + std::mutex desiredModeLock; + DisplayModeRequestOpt desiredModeOpt GUARDED_BY(desiredModeLock); + TracedOrdinal hasDesiredModeTrace GUARDED_BY(desiredModeLock); + + DisplayModeRequestOpt pendingModeOpt GUARDED_BY(kMainThreadContext); + bool isModeSetPending GUARDED_BY(kMainThreadContext) = false; }; - ui::PhysicalDisplayMap mDisplays; + using DisplayPtr = std::unique_ptr; + + void setActiveModeLocked(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + REQUIRES(mDisplayLock); + + // Set once when initializing the DisplayModeController, which the HWComposer must outlive. + HWComposer* mComposerPtr = nullptr; + + ActiveModeListener mActiveModeListener; + + mutable std::mutex mDisplayLock; + ui::PhysicalDisplayMap mDisplays GUARDED_BY(mDisplayLock); }; } // namespace android::display diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index a57e626224..27ea4a9865 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -24,7 +24,6 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include #include #include #include @@ -36,6 +35,7 @@ #include #include #include +#include #include #include @@ -64,15 +64,11 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) mDisplayToken(args.displayToken), mSequenceId(args.sequenceId), mCompositionDisplay{args.compositionDisplay}, - mPendingModeFpsTrace(concatId("PendingModeFps")), - 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)), - mHasDesiredModeTrace(concatId("HasDesiredMode"), false) { + mRefreshRateSelector(std::move(args.refreshRateSelector)) { mCompositionDisplay->editState().isSecure = args.isSecure; mCompositionDisplay->editState().isProtected = args.isProtected; mCompositionDisplay->createRenderSurface( @@ -204,47 +200,6 @@ bool DisplayDevice::isPoweredOn() const { return mPowerMode != hal::PowerMode::OFF; } -void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) { - ATRACE_INT(mActiveModeFpsTrace.c_str(), vsyncRate.getIntValue()); - ATRACE_INT(mRenderRateFpsTrace.c_str(), renderFps.getIntValue()); - - mRefreshRateSelector->setActiveMode(modeId, renderFps); - updateRefreshRateOverlayRate(vsyncRate, renderFps); -} - -bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode, - const hal::VsyncPeriodChangeConstraints& constraints, - hal::VsyncPeriodChangeTimeline& outTimeline) { - // TODO(b/255635711): Flow the DisplayModeRequest through the desired/pending/active states. For - // now, `desiredMode` and `mDesiredModeOpt` are one and the same, but the latter is not cleared - // until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been consumed - // at this point, so clear the `force` flag to prevent an endless loop of `initiateModeChange`. - if (FlagManager::getInstance().connected_display()) { - std::scoped_lock lock(mDesiredModeLock); - if (mDesiredModeOpt) { - mDesiredModeOpt->force = false; - } - } - - mPendingModeOpt = std::move(desiredMode); - mIsModeSetPending = true; - - const auto& mode = *mPendingModeOpt->mode.modePtr; - - if (mHwComposer.setActiveModeWithConstraints(getPhysicalId(), mode.getHwcId(), constraints, - &outTimeline) != OK) { - return false; - } - - ATRACE_INT(mPendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue()); - return true; -} - -void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) { - setActiveMode(modeId, vsyncRate, renderFps); - mIsModeSetPending = false; -} - nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { const auto physicalId = getPhysicalId(); if (!mHwComposer.isConnected(physicalId)) { @@ -450,8 +405,9 @@ void DisplayDevice::updateHdrSdrRatioOverlayRatio(float currentHdrSdrRatio) { } } -void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, bool showSpinner, - bool showRenderRate, bool showInMiddle) { +void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, Fps refreshRate, + Fps renderFps, bool showSpinner, bool showRenderRate, + bool showInMiddle) { if (!enable) { mRefreshRateOverlay.reset(); return; @@ -479,8 +435,7 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, bool sh if (mRefreshRateOverlay) { mRefreshRateOverlay->setLayerStack(getLayerStack()); mRefreshRateOverlay->setViewport(getSize()); - updateRefreshRateOverlayRate(getActiveMode().modePtr->getVsyncRate(), getActiveMode().fps, - setByHwc); + updateRefreshRateOverlayRate(refreshRate, renderFps, setByHwc); } } @@ -531,57 +486,6 @@ void DisplayDevice::animateOverlay() { } } -auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode) -> DesiredModeAction { - ATRACE_NAME(concatId(__func__).c_str()); - ALOGD("%s %s", concatId(__func__).c_str(), to_string(desiredMode).c_str()); - - std::scoped_lock lock(mDesiredModeLock); - if (mDesiredModeOpt) { - // A mode transition was already scheduled, so just override the desired mode. - const bool emitEvent = mDesiredModeOpt->emitEvent; - const bool force = mDesiredModeOpt->force; - mDesiredModeOpt = std::move(desiredMode); - mDesiredModeOpt->emitEvent |= emitEvent; - if (FlagManager::getInstance().connected_display()) { - mDesiredModeOpt->force |= force; - } - return DesiredModeAction::None; - } - - // If the desired mode is already active... - const auto activeMode = refreshRateSelector().getActiveMode(); - if (const auto& desiredModePtr = desiredMode.mode.modePtr; - !desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) { - if (activeMode == desiredMode.mode) { - return DesiredModeAction::None; - } - - // ...but the render rate changed: - setActiveMode(desiredModePtr->getId(), desiredModePtr->getVsyncRate(), - desiredMode.mode.fps); - return DesiredModeAction::InitiateRenderRateSwitch; - } - - setActiveMode(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(), - activeMode.modePtr->getPeakFps()); - - // Initiate a mode change. - mDesiredModeOpt = std::move(desiredMode); - mHasDesiredModeTrace = true; - return DesiredModeAction::InitiateDisplayModeSwitch; -} - -auto DisplayDevice::getDesiredMode() const -> DisplayModeRequestOpt { - std::scoped_lock lock(mDesiredModeLock); - return mDesiredModeOpt; -} - -void DisplayDevice::clearDesiredMode() { - std::scoped_lock lock(mDesiredModeLock); - mDesiredModeOpt.reset(); - mHasDesiredModeTrace = false; -} - void DisplayDevice::adjustRefreshRate(Fps pacesetterDisplayRefreshRate) { using fps_approx_ops::operator<=; if (mRequestedRefreshRate <= 0_Hz) { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index a21559fb45..3cc8cf5d63 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -23,8 +23,6 @@ #include #include #include -#include -#include #include #include #include @@ -42,7 +40,6 @@ #include #include -#include "Display/DisplayModeRequest.h" #include "DisplayHardware/DisplayMode.h" #include "DisplayHardware/Hal.h" #include "DisplayHardware/PowerAdvisor.h" @@ -183,37 +180,6 @@ public: ui::Dataspace getCompositionDataSpace() const; - /* ------------------------------------------------------------------------ - * Display mode management. - */ - - enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch }; - - DesiredModeAction setDesiredMode(display::DisplayModeRequest&&) EXCLUDES(mDesiredModeLock); - - using DisplayModeRequestOpt = ftl::Optional; - - DisplayModeRequestOpt getDesiredMode() const EXCLUDES(mDesiredModeLock); - void clearDesiredMode() EXCLUDES(mDesiredModeLock); - - DisplayModeRequestOpt getPendingMode() const REQUIRES(kMainThreadContext) { - return mPendingModeOpt; - } - bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; } - - scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) { - return mRefreshRateSelector->getActiveMode(); - } - - void setActiveMode(DisplayModeId, Fps vsyncRate, Fps renderFps); - - bool initiateModeChange(display::DisplayModeRequest&&, const hal::VsyncPeriodChangeConstraints&, - hal::VsyncPeriodChangeTimeline& outTimeline) - REQUIRES(kMainThreadContext); - - void finalizeModeChange(DisplayModeId, Fps vsyncRate, Fps renderFps) - REQUIRES(kMainThreadContext); - scheduler::RefreshRateSelector& refreshRateSelector() const { return *mRefreshRateSelector; } // Extends the lifetime of the RefreshRateSelector, so it can outlive this DisplayDevice. @@ -221,13 +187,14 @@ public: return mRefreshRateSelector; } - void animateOverlay(); - // Enables an overlay to be displayed with the current refresh rate - void enableRefreshRateOverlay(bool enable, bool setByHwc, bool showSpinner, bool showRenderRate, - bool showInMiddle) REQUIRES(kMainThreadContext); + // TODO(b/241285876): Move overlay to DisplayModeController. + void enableRefreshRateOverlay(bool enable, bool setByHwc, Fps refreshRate, Fps renderFps, + bool showSpinner, bool showRenderRate, bool showInMiddle) + REQUIRES(kMainThreadContext); void updateRefreshRateOverlayRate(Fps refreshRate, Fps renderFps, bool setByHwc = false); bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; } + void animateOverlay(); bool onKernelTimerChanged(std::optional, bool timerExpired); // Enables an overlay to be display with the hdr/sdr ratio @@ -249,11 +216,6 @@ public: void dump(utils::Dumper&) const; private: - template - inline std::string concatId(const char (&str)[N]) const { - return std::string(ftl::Concat(str, ' ', getId().value).str()); - } - const sp mFlinger; HWComposer& mHwComposer; const wp mDisplayToken; @@ -262,9 +224,6 @@ private: const std::shared_ptr mCompositionDisplay; std::string mDisplayName; - std::string mPendingModeFpsTrace; - std::string mActiveModeFpsTrace; - std::string mRenderRateFpsTrace; const ui::Rotation mPhysicalOrientation; ui::Rotation mOrientation = ui::ROTATION_0; @@ -296,13 +255,6 @@ private: std::unique_ptr mHdrSdrRatioOverlay; // This parameter is only used for hdr/sdr ratio overlay float mHdrSdrRatio = 1.0f; - - mutable std::mutex mDesiredModeLock; - DisplayModeRequestOpt mDesiredModeOpt GUARDED_BY(mDesiredModeLock); - TracedOrdinal mHasDesiredModeTrace GUARDED_BY(mDesiredModeLock); - - DisplayModeRequestOpt mPendingModeOpt GUARDED_BY(kMainThreadContext); - bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false; }; struct DisplayDeviceState { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 59345db67c..b1d7750a92 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -891,8 +891,12 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { } mCompositionEngine->setTimeStats(mTimeStats); + mCompositionEngine->setHwComposer(getFactory().createHWComposer(mHwcServiceName)); - mCompositionEngine->getHwComposer().setCallback(*this); + auto& composer = mCompositionEngine->getHwComposer(); + composer.setCallback(*this); + mDisplayModeController.setHwComposer(&composer); + ClientCache::getInstance().setRenderEngine(&getRenderEngine()); mHasReliablePresentFences = @@ -931,6 +935,20 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { // initializing the Scheduler after configureLocked, once decoupled from DisplayDevice. initScheduler(display); + // Start listening after creating the Scheduler, since the listener calls into it. + mDisplayModeController.setActiveModeListener( + display::DisplayModeController::ActiveModeListener::make( + [this](PhysicalDisplayId displayId, Fps vsyncRate, Fps renderRate) { + // This callback cannot lock mStateLock, as some callers already lock it. + // Instead, switch context to the main thread. + static_cast(mScheduler->schedule([=, + this]() FTL_FAKE_GUARD(mStateLock) { + if (const auto display = getDisplayDeviceLocked(displayId)) { + display->updateRefreshRateOverlayRate(vsyncRate, renderRate); + } + })); + })); + mLayerTracing.setTakeLayersSnapshotProtoFunction([&](uint32_t traceFlags) { auto snapshot = perfetto::protos::LayersSnapshotProto{}; mScheduler @@ -1296,19 +1314,19 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); - const auto display = getDisplayDeviceLocked(displayId); - if (!display) { - ALOGW("%s: display is no longer valid", __func__); - return; - } - const bool emitEvent = desiredMode.emitEvent; - switch (display->setDesiredMode(std::move(desiredMode))) { - case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch: - // DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler. - mScheduler->setRenderRate(displayId, display->refreshRateSelector().getActiveMode().fps, - /*applyImmediately*/ true); + using DesiredModeAction = display::DisplayModeController::DesiredModeAction; + + switch (mDisplayModeController.setDesiredMode(displayId, std::move(desiredMode))) { + case DesiredModeAction::InitiateDisplayModeSwitch: { + const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId); + if (!selectorPtr) break; + + const Fps renderRate = selectorPtr->getActiveMode().fps; + + // DisplayModeController::setDesiredMode updated the render rate, so inform Scheduler. + mScheduler->setRenderRate(displayId, renderRate, true /* applyImmediately */); // Schedule a new frame to initiate the display mode switch. scheduleComposite(FrameHint::kNone); @@ -1328,7 +1346,8 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { mScheduler->setModeChangePending(true); break; - case DisplayDevice::DesiredModeAction::InitiateRenderRateSwitch: + } + case DesiredModeAction::InitiateRenderRateSwitch: mScheduler->setRenderRate(displayId, mode.fps, /*applyImmediately*/ false); if (displayId == mActiveDisplayId) { @@ -1339,7 +1358,7 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { dispatchDisplayModeChangeEvent(displayId, mode); } break; - case DisplayDevice::DesiredModeAction::None: + case DesiredModeAction::None: break; } } @@ -1395,11 +1414,10 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const spmode; - if (display.getActiveMode().modePtr->getResolution() != activeMode.modePtr->getResolution()) { - auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken()); + if (const auto oldResolution = + mDisplayModeController.getActiveMode(displayId).modePtr->getResolution(); + oldResolution != activeMode.modePtr->getResolution()) { + Mutex::Autolock lock(mStateLock); + + auto& state = mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId)); // We need to generate new sequenceId in order to recreate the display (and this // way the framebuffer). state.sequenceId = DisplayDeviceState{}.sequenceId; @@ -1420,8 +1442,8 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { return; } - display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(), - activeMode.fps); + mDisplayModeController.finalizeModeChange(displayId, activeMode.modePtr->getId(), + activeMode.modePtr->getVsyncRate(), activeMode.fps); if (displayId == mActiveDisplayId) { mScheduler->updatePhaseConfiguration(activeMode.fps); @@ -1432,21 +1454,20 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { } } -void SurfaceFlinger::dropModeRequest(const sp& display) { - display->clearDesiredMode(); - if (display->getPhysicalId() == mActiveDisplayId) { +void SurfaceFlinger::dropModeRequest(PhysicalDisplayId displayId) { + mDisplayModeController.clearDesiredMode(displayId); + if (displayId == mActiveDisplayId) { // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); } } -void SurfaceFlinger::applyActiveMode(const sp& display) { - const auto activeModeOpt = display->getDesiredMode(); +void SurfaceFlinger::applyActiveMode(PhysicalDisplayId displayId) { + const auto activeModeOpt = mDisplayModeController.getDesiredMode(displayId); auto activeModePtr = activeModeOpt->mode.modePtr; - const auto displayId = activeModePtr->getPhysicalDisplayId(); const auto renderFps = activeModeOpt->mode.fps; - dropModeRequest(display); + dropModeRequest(displayId); constexpr bool kAllowToEnable = true; mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, std::move(activeModePtr).take()); @@ -1462,11 +1483,8 @@ void SurfaceFlinger::initiateDisplayModeChanges() { std::optional displayToUpdateImmediately; - for (const auto& [id, physical] : mPhysicalDisplays) { - const auto display = getDisplayDeviceLocked(id); - if (!display) continue; - - auto desiredModeOpt = display->getDesiredMode(); + for (const auto& [displayId, physical] : FTL_FAKE_GUARD(mStateLock, mPhysicalDisplays)) { + auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId); if (!desiredModeOpt) { continue; } @@ -1483,19 +1501,21 @@ void SurfaceFlinger::initiateDisplayModeChanges() { ALOGV("%s changing active mode to %d(%s) for display %s", __func__, ftl::to_underlying(desiredModeId), to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(), - to_string(display->getId()).c_str()); + to_string(displayId).c_str()); if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) && - display->getActiveMode() == desiredModeOpt->mode) { - applyActiveMode(display); + mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) { + applyActiveMode(displayId); continue; } + const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId); + // Desired active mode was set, it is different than the mode currently in use, however // allowed modes might have changed by the time we process the refresh. // Make sure the desired mode is still allowed - if (!display->refreshRateSelector().isModeAllowed(desiredModeOpt->mode)) { - dropModeRequest(display); + if (!selectorPtr->isModeAllowed(desiredModeOpt->mode)) { + dropModeRequest(displayId); continue; } @@ -1505,11 +1525,12 @@ void SurfaceFlinger::initiateDisplayModeChanges() { constraints.seamlessRequired = false; hal::VsyncPeriodChangeTimeline outTimeline; - if (!display->initiateModeChange(std::move(*desiredModeOpt), constraints, outTimeline)) { + if (!mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt), + constraints, outTimeline)) { continue; } - display->refreshRateSelector().onModeChangeInitiated(); + selectorPtr->onModeChangeInitiated(); mScheduler->onNewVsyncPeriodChangeTimeline(outTimeline); if (outTimeline.refreshRequired) { @@ -1518,17 +1539,18 @@ void SurfaceFlinger::initiateDisplayModeChanges() { // 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(); + displayToUpdateImmediately = displayId; } } if (displayToUpdateImmediately) { - const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately); - finalizeDisplayModeChange(*display); + const auto displayId = *displayToUpdateImmediately; + finalizeDisplayModeChange(displayId); - const auto desiredModeOpt = display->getDesiredMode(); - if (desiredModeOpt && display->getActiveMode() == desiredModeOpt->mode) { - applyActiveMode(display); + const auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId); + if (desiredModeOpt && + mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) { + applyActiveMode(displayId); } } } @@ -2276,8 +2298,10 @@ void SurfaceFlinger::onRefreshRateChangedDebug(const RefreshRateChangedDebugData getHwComposer().getComposer()->isVrrSupported() ? data.refreshPeriodNanos : data.vsyncPeriodNanos); ATRACE_FORMAT("%s refresh rate = %d", whence, refreshRate.getIntValue()); - display->updateRefreshRateOverlayRate(refreshRate, display->getActiveMode().fps, - /* showRefreshRate */ true); + + const auto renderRate = mDisplayModeController.getActiveMode(*displayIdOpt).fps; + constexpr bool kSetByHwc = true; + display->updateRefreshRateOverlayRate(refreshRate, renderRate, kSetByHwc); } } })); @@ -2585,33 +2609,18 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, // 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; - - if (const auto display = getDisplayDeviceLocked(pair.first)) { - return display->isModeSetPending(); - } - - return false; - })) { + [this](const auto& pair) FTL_FAKE_GUARD(kMainThreadContext) { + const auto [displayId, target] = pair; + return target->isFramePending() && + mDisplayModeController.isModeSetPending(displayId); + })) { 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); - } + for (const auto [displayId, _] : frameTargets) { + if (mDisplayModeController.isModeSetPending(displayId)) { + finalizeDisplayModeChange(displayId); } } @@ -2646,8 +2655,8 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, mPowerAdvisor->setFrameDelay(frameDelay); mPowerAdvisor->setTotalFrameTargetWorkDuration(idealSfWorkDuration); - const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get(); - const Period idealVsyncPeriod = display->getActiveMode().fps.getPeriod(); + const Period idealVsyncPeriod = + mDisplayModeController.getActiveMode(pacesetterId).fps.getPeriod(); mPowerAdvisor->updateTargetWorkDuration(idealVsyncPeriod); } @@ -2710,9 +2719,10 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, ? &mLayerHierarchyBuilder.getHierarchy() : nullptr, updateAttachedChoreographer); - initiateDisplayModeChanges(); } + initiateDisplayModeChanges(); + updateCursorAsync(); if (!mustComposite) { updateInputFlinger(vsyncId, pacesetterFrameTarget.frameBeginTime()); @@ -3758,7 +3768,8 @@ sp SurfaceFlinger::setupNewDisplayDeviceInternal( if (const auto& physical = state.physical) { const auto& mode = *physical->activeMode; - display->setActiveMode(mode.getId(), mode.getVsyncRate(), mode.getVsyncRate()); + mDisplayModeController.setActiveMode(physical->id, mode.getId(), mode.getVsyncRate(), + mode.getVsyncRate()); } display->setLayerFilter(makeLayerFilterForDisplay(display->getId(), state.layerStack)); @@ -3937,7 +3948,9 @@ void SurfaceFlinger::processDisplayChanged(const wp& displayToken, // TODO(b/175678251) Call a listener instead. if (currentState.physical->hwcDisplayId == getHwComposer().getPrimaryHwcDisplayId()) { - mScheduler->resetPhaseConfiguration(display->getActiveMode().fps); + const Fps refreshRate = + mDisplayModeController.getActiveMode(display->getPhysicalId()).fps; + mScheduler->resetPhaseConfiguration(refreshRate); } } return; @@ -7684,9 +7697,10 @@ void SurfaceFlinger::kernelTimerChanged(bool expired) { if (!display->isRefreshRateOverlayEnabled()) return; const auto desiredModeIdOpt = - display->getDesiredMode().transform([](const display::DisplayModeRequest& request) { - return request.mode.modePtr->getId(); - }); + mDisplayModeController.getDesiredMode(display->getPhysicalId()) + .transform([](const display::DisplayModeRequest& request) { + return request.mode.modePtr->getId(); + }); const bool timerExpired = mKernelIdleTimerEnabled && expired; @@ -8921,20 +8935,26 @@ status_t SurfaceFlinger::setSmallAreaDetectionThreshold(int32_t appId, float thr void SurfaceFlinger::enableRefreshRateOverlay(bool enable) { bool setByHwc = getHwComposer().hasCapability(Capability::REFRESH_RATE_CHANGED_CALLBACK_DEBUG); - for (const auto& [id, display] : mPhysicalDisplays) { - if (display.snapshot().connectionType() == ui::DisplayConnectionType::Internal || + for (const auto& [displayId, physical] : mPhysicalDisplays) { + if (physical.snapshot().connectionType() == ui::DisplayConnectionType::Internal || FlagManager::getInstance().refresh_rate_overlay_on_external_display()) { - if (const auto device = getDisplayDeviceLocked(id)) { - const auto enableOverlay = [&](const bool setByHwc) FTL_FAKE_GUARD( - kMainThreadContext) { - device->enableRefreshRateOverlay(enable, setByHwc, mRefreshRateOverlaySpinner, - mRefreshRateOverlayRenderRate, - mRefreshRateOverlayShowInMiddle); + if (const auto display = getDisplayDeviceLocked(displayId)) { + const auto enableOverlay = [&](bool setByHwc) FTL_FAKE_GUARD(kMainThreadContext) { + const auto activeMode = mDisplayModeController.getActiveMode(displayId); + const Fps refreshRate = activeMode.modePtr->getVsyncRate(); + const Fps renderFps = activeMode.fps; + + display->enableRefreshRateOverlay(enable, setByHwc, refreshRate, renderFps, + mRefreshRateOverlaySpinner, + mRefreshRateOverlayRenderRate, + mRefreshRateOverlayShowInMiddle); }; + enableOverlay(setByHwc); if (setByHwc) { const auto status = - getHwComposer().setRefreshRateChangedCallbackDebugEnabled(id, enable); + getHwComposer().setRefreshRateChangedCallbackDebugEnabled(displayId, + enable); if (status != NO_ERROR) { ALOGE("Error %s refresh rate changed callback debug", enable ? "enabling" : "disabling"); @@ -9090,7 +9110,7 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD mActiveDisplayId = activeDisplay.getPhysicalId(); activeDisplay.getCompositionDisplay()->setLayerCachingTexturePoolEnabled(true); - mScheduler->resetPhaseConfiguration(activeDisplay.getActiveMode().fps); + mScheduler->resetPhaseConfiguration(mDisplayModeController.getActiveMode(mActiveDisplayId).fps); // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 12307172f7..ee541c4364 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -737,12 +737,12 @@ private: status_t setActiveModeFromBackdoor(const sp&, DisplayModeId, Fps minFps, Fps maxFps); - void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext); - void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext); + void initiateDisplayModeChanges() REQUIRES(kMainThreadContext) EXCLUDES(mStateLock); + void finalizeDisplayModeChange(PhysicalDisplayId) REQUIRES(kMainThreadContext) + EXCLUDES(mStateLock); - // TODO(b/241285191): Replace DisplayDevice with DisplayModeRequest, and move to Scheduler. - void dropModeRequest(const sp&) REQUIRES(mStateLock); - void applyActiveMode(const sp&) REQUIRES(mStateLock); + void dropModeRequest(PhysicalDisplayId) REQUIRES(kMainThreadContext); + void applyActiveMode(PhysicalDisplayId) REQUIRES(kMainThreadContext); // Called on the main thread in response to setPowerMode() void setPowerModeInternal(const sp& display, hal::PowerMode mode) @@ -1098,8 +1098,7 @@ private: const DisplayDeviceState& drawingState) REQUIRES(mStateLock, kMainThreadContext); - void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&) - REQUIRES(mStateLock); + void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&); /* * VSYNC @@ -1339,9 +1338,7 @@ private: display::PhysicalDisplays mPhysicalDisplays GUARDED_BY(mStateLock); // The inner or outer display for foldables, assuming they have mutually exclusive power states. - // Atomic because writes from onActiveDisplayChangedLocked are not always under mStateLock, but - // reads from ISchedulerCallback::requestDisplayModes may happen concurrently. - std::atomic mActiveDisplayId GUARDED_BY(mStateLock); + std::atomic mActiveDisplayId; display::DisplayModeController mDisplayModeController; diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 5145e112d6..98d5754271 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -70,9 +70,9 @@ cc_test { "DisplayIdGeneratorTest.cpp", "DisplayTransactionTest.cpp", "DisplayDevice_GetBestColorModeTest.cpp", - "DisplayDevice_InitiateModeChange.cpp", "DisplayDevice_SetDisplayBrightnessTest.cpp", "DisplayDevice_SetProjectionTest.cpp", + "DisplayModeControllerTest.cpp", "EventThreadTest.cpp", "FlagManagerTest.cpp", "FpsReporterTest.cpp", diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp deleted file mode 100644 index c463a9271b..0000000000 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2021 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. - */ - -#undef LOG_TAG -#define LOG_TAG "LibSurfaceFlingerUnittests" - -#include "DisplayTransactionTestHelpers.h" -#include "mock/MockFrameRateMode.h" - -#include -#include - -#define EXPECT_DISPLAY_MODE_REQUEST(expected, requestOpt) \ - ASSERT_TRUE(requestOpt); \ - EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, requestOpt->mode); \ - EXPECT_EQ(expected.emitEvent, requestOpt->emitEvent) - -namespace android { -namespace { - -using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector; -using DisplayModeRequest = display::DisplayModeRequest; - -class InitiateModeChangeTest : public DisplayTransactionTest { -public: - using Action = DisplayDevice::DesiredModeAction; - void SetUp() override { - injectFakeBufferQueueFactory(); - injectFakeNativeWindowSurfaceFactory(); - - PrimaryDisplayVariant::setupHwcHotplugCallExpectations(this); - PrimaryDisplayVariant::setupFramebufferConsumerBufferQueueCallExpectations(this); - PrimaryDisplayVariant::setupFramebufferProducerBufferQueueCallExpectations(this); - PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this); - PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this); - - mFlinger.onComposerHalHotplugEvent(PrimaryDisplayVariant::HWC_DISPLAY_ID, - DisplayHotplugEvent::CONNECTED); - mFlinger.configureAndCommit(); - - mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) - .setDisplayModes(makeModes(kMode60, kMode90, kMode120), kModeId60) - .inject(); - } - -protected: - sp mDisplay; - - static constexpr DisplayModeId kModeId60{0}; - static constexpr DisplayModeId kModeId90{1}; - static constexpr DisplayModeId kModeId120{2}; - - static inline const ftl::NonNull kMode60 = - ftl::as_non_null(createDisplayMode(kModeId60, 60_Hz)); - static inline const ftl::NonNull kMode90 = - ftl::as_non_null(createDisplayMode(kModeId90, 90_Hz)); - static inline const ftl::NonNull kMode120 = - ftl::as_non_null(createDisplayMode(kModeId120, 120_Hz)); - - static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false}; - static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true}; - static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false}; - static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true}; -}; - -TEST_F(InitiateModeChangeTest, setDesiredModeToActiveMode) { - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode60))); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, setDesiredMode) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, clearDesiredMode) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_TRUE(mDisplay->getDesiredMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - const hal::VsyncPeriodChangeConstraints constraints{ - .desiredTimeNanos = systemTime(), - .seamlessRequired = false, - }; - hal::VsyncPeriodChangeTimeline timeline; - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) { - EXPECT_EQ(Action::InitiateRenderRateSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode30))); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - const hal::VsyncPeriodChangeConstraints constraints{ - .desiredTimeNanos = systemTime(), - .seamlessRequired = false, - }; - hal::VsyncPeriodChangeTimeline timeline; - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120))); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode()); - - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getPendingMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -} // namespace -} // namespace android diff --git a/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp new file mode 100644 index 0000000000..d971150d42 --- /dev/null +++ b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp @@ -0,0 +1,234 @@ +/* + * Copyright 2024 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. + */ + +#undef LOG_TAG +#define LOG_TAG "LibSurfaceFlingerUnittests" + +#include "Display/DisplayModeController.h" +#include "Display/DisplaySnapshot.h" +#include "DisplayHardware/HWComposer.h" +#include "DisplayIdentificationTestHelpers.h" +#include "FpsOps.h" +#include "mock/DisplayHardware/MockComposer.h" +#include "mock/DisplayHardware/MockDisplayMode.h" +#include "mock/MockFrameRateMode.h" + +#include +#include +#include + +#define EXPECT_DISPLAY_MODE_REQUEST(expected, requestOpt) \ + ASSERT_TRUE(requestOpt); \ + EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, requestOpt->mode); \ + EXPECT_EQ(expected.emitEvent, requestOpt->emitEvent) + +namespace android::display { +namespace { + +namespace hal = android::hardware::graphics::composer::hal; + +using testing::_; +using testing::DoAll; +using testing::Return; +using testing::SetArgPointee; + +class DisplayModeControllerTest : public testing::Test { +public: + using Action = DisplayModeController::DesiredModeAction; + + void SetUp() override { + mDmc.setHwComposer(mComposer.get()); + mDmc.setActiveModeListener( + [this](PhysicalDisplayId displayId, Fps vsyncRate, Fps renderFps) { + mActiveModeListener.Call(displayId, vsyncRate, renderFps); + }); + + constexpr uint8_t kPort = 111; + EXPECT_CALL(*mComposerHal, getDisplayIdentificationData(kHwcDisplayId, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kPort), SetArgPointee<2>(getInternalEdid()), + Return(hal::Error::NONE))); + + EXPECT_CALL(*mComposerHal, setClientTargetSlotCount(kHwcDisplayId)); + EXPECT_CALL(*mComposerHal, + setVsyncEnabled(kHwcDisplayId, hal::IComposerClient::Vsync::DISABLE)); + EXPECT_CALL(*mComposerHal, onHotplugConnect(kHwcDisplayId)); + + const auto infoOpt = mComposer->onHotplug(kHwcDisplayId, hal::Connection::CONNECTED); + ASSERT_TRUE(infoOpt); + + mDisplayId = infoOpt->id; + mDisplaySnapshotOpt.emplace(mDisplayId, ui::DisplayConnectionType::Internal, + makeModes(kMode60, kMode90, kMode120), ui::ColorModes{}, + std::nullopt); + + ftl::FakeGuard guard(kMainThreadContext); + mDmc.registerDisplay(*mDisplaySnapshotOpt, kModeId60, + scheduler::RefreshRateSelector::Config{}); + } + +protected: + hal::VsyncPeriodChangeConstraints expectModeSet(const DisplayModeRequest& request, + hal::VsyncPeriodChangeTimeline& timeline, + bool subsequent = false) { + EXPECT_CALL(*mComposerHal, + isSupported(Hwc2::Composer::OptionalFeature::RefreshRateSwitching)) + .WillOnce(Return(true)); + + if (!subsequent) { + EXPECT_CALL(*mComposerHal, getDisplayConnectionType(kHwcDisplayId, _)) + .WillOnce(DoAll(SetArgPointee<1>( + hal::IComposerClient::DisplayConnectionType::INTERNAL), + Return(hal::V2_4::Error::NONE))); + } + + const hal::VsyncPeriodChangeConstraints constraints{ + .desiredTimeNanos = systemTime(), + .seamlessRequired = false, + }; + + const hal::HWConfigId hwcModeId = request.mode.modePtr->getHwcId(); + + EXPECT_CALL(*mComposerHal, + setActiveConfigWithConstraints(kHwcDisplayId, hwcModeId, constraints, _)) + .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::V2_4::Error::NONE))); + + return constraints; + } + + static constexpr hal::HWDisplayId kHwcDisplayId = 1234; + + Hwc2::mock::Composer* mComposerHal = new testing::StrictMock(); + const std::unique_ptr mComposer{ + std::make_unique(std::unique_ptr(mComposerHal))}; + + testing::MockFunction mActiveModeListener; + + DisplayModeController mDmc; + + PhysicalDisplayId mDisplayId; + std::optional mDisplaySnapshotOpt; + + static constexpr DisplayModeId kModeId60{0}; + static constexpr DisplayModeId kModeId90{1}; + static constexpr DisplayModeId kModeId120{2}; + + static inline const ftl::NonNull kMode60 = + ftl::as_non_null(mock::createDisplayMode(kModeId60, 60_Hz)); + static inline const ftl::NonNull kMode90 = + ftl::as_non_null(mock::createDisplayMode(kModeId90, 90_Hz)); + static inline const ftl::NonNull kMode120 = + ftl::as_non_null(mock::createDisplayMode(kModeId120, 120_Hz)); + + static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false}; + static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true}; + static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false}; + static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true}; +}; + +TEST_F(DisplayModeControllerTest, setDesiredModeToActiveMode) { + EXPECT_CALL(mActiveModeListener, Call(_, _, _)).Times(0); + + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode60))); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, setDesiredMode) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + + // No action since a mode switch has already been initiated. + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, clearDesiredMode) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_TRUE(mDmc.getDesiredMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateModeChange) REQUIRES(kMainThreadContext) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + auto modeRequest = kDesiredMode90; + + hal::VsyncPeriodChangeTimeline timeline; + const auto constraints = expectModeSet(modeRequest, timeline); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateRenderRateSwitch) { + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 30_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateRenderRateSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode30))); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + auto modeRequest = kDesiredMode90; + + hal::VsyncPeriodChangeTimeline timeline; + auto constraints = expectModeSet(modeRequest, timeline); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + + // No action since a mode switch has already been initiated. + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120))); + + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId)); + modeRequest = kDesiredMode120; + + constexpr bool kSubsequent = true; + constraints = expectModeSet(modeRequest, timeline, kSubsequent); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getPendingMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +} // namespace +} // namespace android::display diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 078b4fe15e..0c3e875432 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -76,6 +76,7 @@ public: mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) .setRefreshRateSelector(std::move(selectorPtr)) .inject(std::move(vsyncController), std::move(vsyncTracker)); + mDisplayId = mDisplay->getPhysicalId(); // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy // will call setActiveConfig instead of setActiveConfigWithConstraints. @@ -112,7 +113,11 @@ public: protected: void setupScheduler(std::shared_ptr); + auto& dmc() { return mFlinger.mutableDisplayModeController(); } + sp mDisplay, mOuterDisplay; + PhysicalDisplayId mDisplayId; + mock::EventThread* mAppEventThread; static constexpr DisplayModeId kModeId60{0}; @@ -167,17 +172,17 @@ void DisplayModeSwitchingTest::setupScheduler( TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90, false, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; @@ -187,8 +192,8 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequ Mock::VerifyAndClearExpectations(mComposer); - EXPECT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that the next commit will complete the mode change and send // a onModeChanged event to the framework. @@ -198,23 +203,23 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequ mFlinger.commit(); Mock::VerifyAndClearExpectations(mAppEventThread); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90, true, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. @@ -226,8 +231,8 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshR mFlinger.commit(); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { @@ -236,8 +241,8 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { // Test that if we call setDesiredDisplayModeSpecs while a previous mode change // is still being processed the later call will be respected. - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); @@ -252,46 +257,45 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId120, false, 0, 180)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120); EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId120); mFlinger.commit(); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120); mFlinger.commit(); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId120); } TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90_4K, false, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90_4K); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90_4K); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. const VsyncPeriodChangeTimeline timeline{.refreshRequired = false}; EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90_4K); - EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), true)); + EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplayId, true)); - // Misc expecations. We don't need to enforce these method calls, but since the helper methods - // already set expectations we should add new ones here, otherwise the test will fail. + // Override expectations set up by PrimaryDisplayVariant. EXPECT_CALL(*mConsumer, setDefaultBufferSize(static_cast(kResolution4K.getWidth()), static_cast(kResolution4K.getHeight()))) @@ -304,31 +308,28 @@ TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRe injectFakeNativeWindowSurfaceFactory(); PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this); - const auto displayToken = mDisplay->getDisplayToken().promote(); - mFlinger.commit(); - // The DisplayDevice will be destroyed and recreated, - // so we need to update with the new instance. - mDisplay = mFlinger.getDisplay(displayToken); - - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90_4K); } MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { - if (!arg->getDesiredMode()) { + const auto displayId = arg->getPhysicalId(); + auto& dmc = flinger->mutableDisplayModeController(); + + if (!dmc.getDesiredMode(displayId)) { *result_listener << "No desired mode"; return false; } - if (arg->getDesiredMode()->mode.modePtr->getId() != modeId) { + if (dmc.getDesiredMode(displayId)->mode.modePtr->getId() != modeId) { *result_listener << "Unexpected desired mode " << ftl::to_underlying(modeId); return false; } // VsyncModulator should react to mode switches on the pacesetter display. - if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() && + if (displayId == flinger->scheduler()->pacesetterDisplayId() && !flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { *result_listener << "VsyncModulator did not shift to early phase"; return false; @@ -337,8 +338,10 @@ MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { return true; } -MATCHER_P(ModeSettledTo, modeId, "") { - if (const auto desiredOpt = arg->getDesiredMode()) { +MATCHER_P2(ModeSettledTo, dmc, modeId, "") { + const auto displayId = arg->getPhysicalId(); + + if (const auto desiredOpt = dmc->getDesiredMode(displayId)) { *result_listener << "Unsettled desired mode " << ftl::to_underlying(desiredOpt->mode.modePtr->getId()); return false; @@ -346,7 +349,7 @@ MATCHER_P(ModeSettledTo, modeId, "") { ftl::FakeGuard guard(kMainThreadContext); - if (arg->getActiveMode().modePtr->getId() != modeId) { + if (dmc->getActiveMode(displayId).modePtr->getId() != modeId) { *result_listener << "Settled to unexpected active mode " << ftl::to_underlying(modeId); return false; } @@ -367,14 +370,14 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -400,14 +403,14 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -420,12 +423,12 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); } TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { @@ -440,14 +443,14 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -473,13 +476,13 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); } TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { EXPECT_TRUE(mDisplay->isPoweredOn()); - EXPECT_THAT(mDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId60)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), @@ -502,7 +505,7 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { mFlinger.commit(); - EXPECT_THAT(mDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId90)); } TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { @@ -518,14 +521,14 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -555,8 +558,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); @@ -570,13 +573,13 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp index 1bae5ffd30..933d03dac1 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp @@ -232,7 +232,9 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { // Invocation DisplayDeviceState state; - if constexpr (constexpr auto connectionType = Case::Display::CONNECTION_TYPE::value) { + + constexpr auto kConnectionTypeOpt = Case::Display::CONNECTION_TYPE::value; + if constexpr (kConnectionTypeOpt) { const auto displayId = PhysicalDisplayId::tryCast(Case::Display::DISPLAY_ID::get()); ASSERT_TRUE(displayId); const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value; @@ -257,7 +259,7 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { const auto it = mFlinger.mutablePhysicalDisplays() .emplace_or_replace(*displayId, displayToken, *displayId, - *connectionType, makeModes(activeMode), + *kConnectionTypeOpt, makeModes(activeMode), std::move(colorModes), std::nullopt) .first; @@ -291,9 +293,12 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { EXPECT_EQ(Case::Display::DISPLAY_FLAGS & DisplayDevice::eReceivesInput, device->receivesInput()); - if constexpr (Case::Display::CONNECTION_TYPE::value) { + if constexpr (kConnectionTypeOpt) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode().modePtr->getHwcId()); + EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, + mFlinger.mutableDisplayModeController() + .getActiveMode(device->getPhysicalId()) + .modePtr->getHwcId()); } } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 8c72a7dba5..007383b3d9 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -191,6 +191,8 @@ public: void setupComposer(std::unique_ptr composer) { mFlinger->mCompositionEngine->setHwComposer( std::make_unique(std::move(composer))); + mFlinger->mDisplayModeController.setHwComposer( + &mFlinger->mCompositionEngine->getHwComposer()); } void setupPowerAdvisor(std::unique_ptr powerAdvisor) { @@ -1055,7 +1057,6 @@ public: auto& modes = mDisplayModes; auto& activeModeId = mActiveModeId; - std::optional refreshRateOpt; DisplayDeviceState state; state.isSecure = mCreationArgs.isSecure; @@ -1093,7 +1094,9 @@ public: const auto activeModeOpt = modes.get(activeModeId); LOG_ALWAYS_FATAL_IF(!activeModeOpt); - refreshRateOpt = activeModeOpt->get()->getPeakFps(); + + // Save a copy for use after `modes` is consumed. + const Fps refreshRate = activeModeOpt->get()->getPeakFps(); state.physical = {.id = *physicalId, .hwcDisplayId = *mHwcDisplayId, @@ -1109,6 +1112,9 @@ public: .registerDisplay(*physicalId, it->second.snapshot(), mCreationArgs.refreshRateSelector); + mFlinger.mutableDisplayModeController().setActiveMode(*physicalId, activeModeId, + refreshRate, refreshRate); + if (mFlinger.scheduler() && mSchedulerRegistration) { mFlinger.scheduler()->registerDisplay(*physicalId, mCreationArgs.refreshRateSelector, @@ -1120,10 +1126,6 @@ public: sp display = sp::make(mCreationArgs); mFlinger.mutableDisplays().emplace_or_replace(mDisplayToken, display); - if (refreshRateOpt) { - display->setActiveMode(activeModeId, *refreshRateOpt, *refreshRateOpt); - } - mFlinger.mutableCurrentState().displays.add(mDisplayToken, state); mFlinger.mutableDrawingState().displays.add(mDisplayToken, state); -- cgit v1.2.3-59-g8ed1b