diff options
| -rw-r--r-- | services/surfaceflinger/Layer.cpp | 3 | ||||
| -rw-r--r-- | services/surfaceflinger/Scheduler/RefreshRateSelector.h | 14 | ||||
| -rw-r--r-- | services/surfaceflinger/Scheduler/Scheduler.cpp | 172 | ||||
| -rw-r--r-- | services/surfaceflinger/Scheduler/Scheduler.h | 94 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 25 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 6 | ||||
| -rw-r--r-- | services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h | 7 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/SchedulerTest.cpp | 14 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/TestableScheduler.h | 35 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 19 |
10 files changed, 214 insertions, 175 deletions
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 2a18521b5b..b4e16fc3e8 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -195,8 +195,7 @@ Layer::Layer(const LayerCreationArgs& args) mDrawingState.color.b = -1.0_hf; } - mFrameTracker.setDisplayRefreshPeriod( - args.flinger->mScheduler->getVsyncPeriodFromRefreshRateSelector()); + mFrameTracker.setDisplayRefreshPeriod(args.flinger->mScheduler->getLeaderVsyncPeriod()); mOwnerUid = args.ownerUid; mOwnerPid = args.ownerPid; diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h index 0e80817521..65ee487112 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h @@ -363,14 +363,16 @@ public: } } - void resetIdleTimer(bool kernelOnly) { - if (!mIdleTimer) { - return; + void resetKernelIdleTimer() { + if (mIdleTimer && mConfig.kernelIdleTimerController) { + mIdleTimer->reset(); } - if (kernelOnly && !mConfig.kernelIdleTimerController.has_value()) { - return; + } + + void resetIdleTimer() { + if (mIdleTimer) { + mIdleTimer->reset(); } - mIdleTimer->reset(); } void dump(utils::Dumper&) const EXCLUDES(mLock); diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 6108d92a50..f1fcc884e5 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -70,7 +70,7 @@ Scheduler::~Scheduler() { mTouchTimer.reset(); // Stop idle timer and clear callbacks, as the RefreshRateSelector may outlive the Scheduler. - setRefreshRateSelector(nullptr); + demoteLeaderDisplay(); } void Scheduler::startTimers() { @@ -95,40 +95,29 @@ void Scheduler::startTimers() { } } -void Scheduler::setRefreshRateSelector(RefreshRateSelectorPtr newSelectorPtr) { - // No need to lock for reads on kMainThreadContext. - if (const auto& selectorPtr = FTL_FAKE_GUARD(mRefreshRateSelectorLock, mRefreshRateSelector)) { - unbindIdleTimer(*selectorPtr); - } - - { - // Clear state that depends on the current RefreshRateSelector. - std::scoped_lock lock(mPolicyLock); - mPolicy = {}; - } - - std::scoped_lock lock(mRefreshRateSelectorLock); - mRefreshRateSelector = std::move(newSelectorPtr); +void Scheduler::setLeaderDisplay(std::optional<PhysicalDisplayId> leaderIdOpt) { + demoteLeaderDisplay(); - if (mRefreshRateSelector) { - bindIdleTimer(*mRefreshRateSelector); - } + std::scoped_lock lock(mDisplayLock); + promoteLeaderDisplay(leaderIdOpt); } void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr) { - if (!mLeaderDisplayId) { - mLeaderDisplayId = displayId; - } + demoteLeaderDisplay(); + std::scoped_lock lock(mDisplayLock); mRefreshRateSelectors.emplace_or_replace(displayId, std::move(selectorPtr)); + + promoteLeaderDisplay(); } void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) { - if (mLeaderDisplayId == displayId) { - mLeaderDisplayId.reset(); - } + demoteLeaderDisplay(); + std::scoped_lock lock(mDisplayLock); mRefreshRateSelectors.erase(displayId); + + promoteLeaderDisplay(); } void Scheduler::run() { @@ -163,7 +152,7 @@ std::unique_ptr<VSyncSource> Scheduler::makePrimaryDispSyncSource( std::optional<Fps> Scheduler::getFrameRateOverride(uid_t uid) const { const bool supportsFrameRateOverrideByContent = - holdRefreshRateSelector()->supportsFrameRateOverrideByContent(); + leaderSelectorPtr()->supportsFrameRateOverrideByContent(); return mFrameRateOverrideMappings .getFrameRateOverrideForUid(uid, supportsFrameRateOverrideByContent); } @@ -178,8 +167,6 @@ bool Scheduler::isVsyncValid(TimePoint expectedVsyncTimestamp, uid_t uid) const } impl::EventThread::ThrottleVsyncCallback Scheduler::makeThrottleVsyncCallback() const { - std::scoped_lock lock(mRefreshRateSelectorLock); - return [this](nsecs_t expectedVsyncTimestamp, uid_t uid) { return !isVsyncValid(TimePoint::fromNs(expectedVsyncTimestamp), uid); }; @@ -187,7 +174,7 @@ impl::EventThread::ThrottleVsyncCallback Scheduler::makeThrottleVsyncCallback() impl::EventThread::GetVsyncPeriodFunction Scheduler::makeGetVsyncPeriodFunction() const { return [this](uid_t uid) { - const Fps refreshRate = holdRefreshRateSelector()->getActiveModePtr()->getFps(); + const Fps refreshRate = leaderSelectorPtr()->getActiveModePtr()->getFps(); const nsecs_t currentPeriod = mVsyncSchedule->period().ns() ?: refreshRate.getPeriodNsecs(); const auto frameRate = getFrameRateOverride(uid); @@ -281,7 +268,7 @@ void Scheduler::onScreenReleased(ConnectionHandle handle) { void Scheduler::onFrameRateOverridesChanged(ConnectionHandle handle, PhysicalDisplayId displayId) { const bool supportsFrameRateOverrideByContent = - holdRefreshRateSelector()->supportsFrameRateOverrideByContent(); + leaderSelectorPtr()->supportsFrameRateOverrideByContent(); std::vector<FrameRateOverride> overrides = mFrameRateOverrideMappings.getAllFrameRateOverrides(supportsFrameRateOverrideByContent); @@ -322,8 +309,7 @@ void Scheduler::dispatchCachedReportedMode() { // If the mode is not the current mode, this means that a // mode change is in progress. In that case we shouldn't dispatch an event // as it will be dispatched when the current mode changes. - if (std::scoped_lock lock(mRefreshRateSelectorLock); - mRefreshRateSelector->getActiveModePtr() != mPolicy.mode) { + if (leaderSelectorPtr()->getActiveModePtr() != mPolicy.mode) { return; } @@ -416,10 +402,7 @@ void Scheduler::resync() { const nsecs_t last = mLastResyncTime.exchange(now); if (now - last > kIgnoreDelay) { - const auto refreshRate = [&] { - std::scoped_lock lock(mRefreshRateSelectorLock); - return mRefreshRateSelector->getActiveModePtr()->getFps(); - }(); + const auto refreshRate = leaderSelectorPtr()->getActiveModePtr()->getFps(); resyncToHardwareVsync(false, refreshRate); } } @@ -478,12 +461,9 @@ void Scheduler::deregisterLayer(Layer* layer) { void Scheduler::recordLayerHistory(Layer* layer, nsecs_t presentTime, LayerHistory::LayerUpdateType updateType) { - { - std::scoped_lock lock(mRefreshRateSelectorLock); - if (!mRefreshRateSelector->canSwitch()) return; + if (leaderSelectorPtr()->canSwitch()) { + mLayerHistory.record(layer, presentTime, systemTime(), updateType); } - - mLayerHistory.record(layer, presentTime, systemTime(), updateType); } void Scheduler::setModeChangePending(bool pending) { @@ -496,7 +476,7 @@ void Scheduler::setDefaultFrameRateCompatibility(Layer* layer) { } void Scheduler::chooseRefreshRateForContent() { - const auto selectorPtr = holdRefreshRateSelector(); + const auto selectorPtr = leaderSelectorPtr(); if (!selectorPtr->canSwitch()) return; ATRACE_CALL(); @@ -506,16 +486,13 @@ void Scheduler::chooseRefreshRateForContent() { } void Scheduler::resetIdleTimer() { - std::scoped_lock lock(mRefreshRateSelectorLock); - mRefreshRateSelector->resetIdleTimer(/*kernelOnly*/ false); + leaderSelectorPtr()->resetIdleTimer(); } void Scheduler::onTouchHint() { if (mTouchTimer) { mTouchTimer->reset(); - - std::scoped_lock lock(mRefreshRateSelectorLock); - mRefreshRateSelector->resetIdleTimer(/*kernelOnly*/ true); + leaderSelectorPtr()->resetKernelIdleTimer(); } } @@ -535,30 +512,12 @@ void Scheduler::setDisplayPowerMode(hal::PowerMode powerMode) { mLayerHistory.clear(); } -void Scheduler::bindIdleTimer(RefreshRateSelector& selector) { - selector.setIdleTimerCallbacks( - {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); }, - .onExpired = [this] { idleTimerCallback(TimerState::Expired); }}, - .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); }, - .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}}); - - selector.startIdleTimer(); -} - -void Scheduler::unbindIdleTimer(RefreshRateSelector& selector) { - selector.stopIdleTimer(); - selector.clearIdleTimerCallbacks(); -} - void Scheduler::kernelIdleTimerCallback(TimerState state) { ATRACE_INT("ExpiredKernelIdleTimer", static_cast<int>(state)); // TODO(145561154): cleanup the kernel idle timer implementation and the refresh rate // magic number - const Fps refreshRate = [&] { - std::scoped_lock lock(mRefreshRateSelectorLock); - return mRefreshRateSelector->getActiveModePtr()->getFps(); - }(); + const Fps refreshRate = leaderSelectorPtr()->getActiveModePtr()->getFps(); constexpr Fps FPS_THRESHOLD_FOR_KERNEL_TIMER = 65_Hz; using namespace fps_approx_ops; @@ -614,7 +573,11 @@ void Scheduler::dump(utils::Dumper& dumper) const { } { utils::Dumper::Section section(dumper, "Policy"sv); - + { + std::scoped_lock lock(mDisplayLock); + ftl::FakeGuard guard(kMainThreadContext); + dumper.dump("leaderDisplayId"sv, mLeaderDisplayId); + } dumper.dump("layerHistory"sv, mLayerHistory.dump()); dumper.dump("touchTimer"sv, mTouchTimer.transform(&OneShotTimer::interval)); dumper.dump("displayPowerTimer"sv, mDisplayPowerTimer.transform(&OneShotTimer::interval)); @@ -638,17 +601,44 @@ void Scheduler::dumpVsync(std::string& out) const { } bool Scheduler::updateFrameRateOverrides(GlobalSignals consideredSignals, Fps displayRefreshRate) { - // we always update mFrameRateOverridesByContent here - // supportsFrameRateOverridesByContent will be checked - // when getting FrameRateOverrides from mFrameRateOverrideMappings - if (!consideredSignals.idle) { - const auto frameRateOverrides = - holdRefreshRateSelector()->getFrameRateOverrides(mPolicy.contentRequirements, - displayRefreshRate, - consideredSignals); - return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides); + if (consideredSignals.idle) return false; + + const auto frameRateOverrides = + leaderSelectorPtr()->getFrameRateOverrides(mPolicy.contentRequirements, + displayRefreshRate, consideredSignals); + + // Note that RefreshRateSelector::supportsFrameRateOverrideByContent is checked when querying + // the FrameRateOverrideMappings rather than here. + return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides); +} + +void Scheduler::promoteLeaderDisplay(std::optional<PhysicalDisplayId> leaderIdOpt) { + // TODO(b/241286431): Choose the leader display. + mLeaderDisplayId = leaderIdOpt.value_or(mRefreshRateSelectors.begin()->first); + ALOGI("Display %s is the leader", to_string(*mLeaderDisplayId).c_str()); + + if (const auto leaderPtr = leaderSelectorPtrLocked()) { + leaderPtr->setIdleTimerCallbacks( + {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); }, + .onExpired = [this] { idleTimerCallback(TimerState::Expired); }}, + .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); }, + .onExpired = + [this] { kernelIdleTimerCallback(TimerState::Expired); }}}); + + leaderPtr->startIdleTimer(); } - return false; +} + +void Scheduler::demoteLeaderDisplay() { + // No need to lock for reads on kMainThreadContext. + if (const auto leaderPtr = FTL_FAKE_GUARD(mDisplayLock, leaderSelectorPtrLocked())) { + leaderPtr->stopIdleTimer(); + leaderPtr->clearIdleTimerCallbacks(); + } + + // Clear state that depends on the leader's RefreshRateSelector. + std::scoped_lock lock(mPolicyLock); + mPolicy = {}; } template <typename S, typename T> @@ -660,23 +650,29 @@ auto Scheduler::applyPolicy(S Policy::*statePtr, T&& newState) -> GlobalSignals bool frameRateOverridesChanged; { - std::lock_guard<std::mutex> lock(mPolicyLock); + std::scoped_lock lock(mPolicyLock); auto& currentState = mPolicy.*statePtr; if (currentState == newState) return {}; currentState = std::forward<T>(newState); - auto modeChoices = chooseDisplayModes(); - - // TODO(b/240743786): The leader display's mode must change for any DisplayModeRequest to go - // through. Fix this by tracking per-display Scheduler::Policy and timers. + DisplayModeChoiceMap modeChoices; DisplayModePtr modePtr; - std::tie(modePtr, consideredSignals) = - modeChoices.get(*mLeaderDisplayId) - .transform([](const DisplayModeChoice& choice) { - return std::make_pair(choice.modePtr, choice.consideredSignals); - }) - .value(); + { + std::scoped_lock lock(mDisplayLock); + ftl::FakeGuard guard(kMainThreadContext); + + modeChoices = chooseDisplayModes(); + + // TODO(b/240743786): The leader display's mode must change for any DisplayModeRequest + // to go through. Fix this by tracking per-display Scheduler::Policy and timers. + std::tie(modePtr, consideredSignals) = + modeChoices.get(*mLeaderDisplayId) + .transform([](const DisplayModeChoice& choice) { + return std::make_pair(choice.modePtr, choice.consideredSignals); + }) + .value(); + } modeRequests.reserve(modeChoices.size()); for (auto& [id, choice] : modeChoices) { @@ -807,7 +803,7 @@ DisplayModePtr Scheduler::getPreferredDisplayMode() { // Make sure the stored mode is up to date. if (mPolicy.mode) { const auto ranking = - holdRefreshRateSelector() + leaderSelectorPtr() ->getRankedRefreshRates(mPolicy.contentRequirements, makeGlobalSignals()) .ranking; diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 04f3b69b98..fb2307100f 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -22,7 +22,6 @@ #include <future> #include <memory> #include <mutex> -#include <optional> #include <unordered_map> #include <utility> @@ -33,6 +32,8 @@ #include <ui/GraphicTypes.h> #pragma clang diagnostic pop // ignored "-Wconversion -Wextra" +#include <ftl/fake_guard.h> +#include <ftl/optional.h> #include <scheduler/Features.h> #include <scheduler/Time.h> #include <ui/DisplayId.h> @@ -108,12 +109,15 @@ public: void startTimers(); + // TODO(b/241285191): Remove this API by promoting leader in onScreen{Acquired,Released}. + void setLeaderDisplay(std::optional<PhysicalDisplayId>) REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + using RefreshRateSelectorPtr = std::shared_ptr<RefreshRateSelector>; - void setRefreshRateSelector(RefreshRateSelectorPtr) REQUIRES(kMainThreadContext) - EXCLUDES(mRefreshRateSelectorLock); - void registerDisplay(PhysicalDisplayId, RefreshRateSelectorPtr); - void unregisterDisplay(PhysicalDisplayId); + void registerDisplay(PhysicalDisplayId, RefreshRateSelectorPtr) REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); void run(); @@ -165,7 +169,7 @@ public: // Otherwise, if hardware vsync is not already enabled then this method will // no-op. void resyncToHardwareVsync(bool makeAvailable, Fps refreshRate); - void resync() EXCLUDES(mRefreshRateSelectorLock); + void resync() EXCLUDES(mDisplayLock); void forceNextResync() { mLastResyncTime = 0; } // Passes a vsync sample to VsyncController. periodFlushed will be true if @@ -176,14 +180,14 @@ public: // Layers are registered on creation, and unregistered when the weak reference expires. void registerLayer(Layer*); - void recordLayerHistory(Layer*, nsecs_t presentTime, LayerHistory::LayerUpdateType updateType) - EXCLUDES(mRefreshRateSelectorLock); + void recordLayerHistory(Layer*, nsecs_t presentTime, LayerHistory::LayerUpdateType) + EXCLUDES(mDisplayLock); void setModeChangePending(bool pending); void setDefaultFrameRateCompatibility(Layer*); void deregisterLayer(Layer*); // Detects content using layer history, and selects a matching refresh rate. - void chooseRefreshRateForContent() EXCLUDES(mRefreshRateSelectorLock); + void chooseRefreshRateForContent() EXCLUDES(mDisplayLock); void resetIdleTimer(); @@ -228,11 +232,10 @@ public: void setGameModeRefreshRateForUid(FrameRateOverride); // Retrieves the overridden refresh rate for a given uid. - std::optional<Fps> getFrameRateOverride(uid_t uid) const EXCLUDES(mRefreshRateSelectorLock); + std::optional<Fps> getFrameRateOverride(uid_t) const EXCLUDES(mDisplayLock); - nsecs_t getVsyncPeriodFromRefreshRateSelector() const EXCLUDES(mRefreshRateSelectorLock) { - std::scoped_lock lock(mRefreshRateSelectorLock); - return mRefreshRateSelector->getActiveModePtr()->getFps().getPeriodNsecs(); + nsecs_t getLeaderVsyncPeriod() const EXCLUDES(mDisplayLock) { + return leaderSelectorPtr()->getActiveModePtr()->getFps().getPeriodNsecs(); } // Returns the framerate of the layer with the given sequence ID @@ -255,21 +258,23 @@ private: sp<EventThreadConnection> createConnectionInternal( EventThread*, EventRegistrationFlags eventRegistration = {}); - void bindIdleTimer(RefreshRateSelector&) REQUIRES(kMainThreadContext, mRefreshRateSelectorLock); - - // Blocks until the timer thread exits. `mRefreshRateSelectorLock` must not be locked by the - // caller on the main thread to avoid deadlock, since the timer thread locks it before exit. - static void unbindIdleTimer(RefreshRateSelector&) REQUIRES(kMainThreadContext) - EXCLUDES(mRefreshRateSelectorLock); - // Update feature state machine to given state when corresponding timer resets or expires. - void kernelIdleTimerCallback(TimerState) EXCLUDES(mRefreshRateSelectorLock); + void kernelIdleTimerCallback(TimerState) EXCLUDES(mDisplayLock); void idleTimerCallback(TimerState); void touchTimerCallback(TimerState); void displayPowerTimerCallback(TimerState); void setVsyncPeriod(nsecs_t period); + // Chooses a leader among the registered displays, unless `leaderIdOpt` is specified. The new + // `mLeaderDisplayId` is never `std::nullopt`. + void promoteLeaderDisplay(std::optional<PhysicalDisplayId> leaderIdOpt = std::nullopt) + REQUIRES(kMainThreadContext, mDisplayLock); + + // Blocks until the leader's idle timer thread exits. `mDisplayLock` must not be locked by the + // caller on the main thread to avoid deadlock, since the timer thread locks it before exit. + void demoteLeaderDisplay() REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock, mPolicyLock); + struct Policy; // Sets the S state of the policy to the T value under mPolicyLock, and chooses a display mode @@ -296,23 +301,20 @@ private: }; using DisplayModeChoiceMap = display::PhysicalDisplayMap<PhysicalDisplayId, DisplayModeChoice>; - DisplayModeChoiceMap chooseDisplayModes() const REQUIRES(mPolicyLock); + + // See mDisplayLock for thread safety. + DisplayModeChoiceMap chooseDisplayModes() const + REQUIRES(mPolicyLock, mDisplayLock, kMainThreadContext); GlobalSignals makeGlobalSignals() const REQUIRES(mPolicyLock); bool updateFrameRateOverrides(GlobalSignals, Fps displayRefreshRate) REQUIRES(mPolicyLock); - void dispatchCachedReportedMode() REQUIRES(mPolicyLock) EXCLUDES(mRefreshRateSelectorLock); + void dispatchCachedReportedMode() REQUIRES(mPolicyLock) EXCLUDES(mDisplayLock); - android::impl::EventThread::ThrottleVsyncCallback makeThrottleVsyncCallback() const - EXCLUDES(mRefreshRateSelectorLock); + android::impl::EventThread::ThrottleVsyncCallback makeThrottleVsyncCallback() const; android::impl::EventThread::GetVsyncPeriodFunction makeGetVsyncPeriodFunction() const; - RefreshRateSelectorPtr holdRefreshRateSelector() const EXCLUDES(mRefreshRateSelectorLock) { - std::scoped_lock lock(mRefreshRateSelectorLock); - return mRefreshRateSelector; - } - // Stores EventThread associated with a given VSyncSource, and an initial EventThreadConnection. struct Connection { sp<EventThreadConnection> connection; @@ -342,10 +344,34 @@ private: ISchedulerCallback& mSchedulerCallback; + // mDisplayLock may be locked while under mPolicyLock. mutable std::mutex mPolicyLock; - display::PhysicalDisplayMap<PhysicalDisplayId, RefreshRateSelectorPtr> mRefreshRateSelectors; - std::optional<PhysicalDisplayId> mLeaderDisplayId; + // Only required for reads outside kMainThreadContext. kMainThreadContext is the only writer, so + // must lock for writes but not reads. See also mPolicyLock for locking order. + mutable std::mutex mDisplayLock; + + display::PhysicalDisplayMap<PhysicalDisplayId, RefreshRateSelectorPtr> mRefreshRateSelectors + GUARDED_BY(mDisplayLock) GUARDED_BY(kMainThreadContext); + + ftl::Optional<PhysicalDisplayId> mLeaderDisplayId GUARDED_BY(mDisplayLock) + GUARDED_BY(kMainThreadContext); + + RefreshRateSelectorPtr leaderSelectorPtr() const EXCLUDES(mDisplayLock) { + std::scoped_lock lock(mDisplayLock); + return leaderSelectorPtrLocked(); + } + + RefreshRateSelectorPtr leaderSelectorPtrLocked() const REQUIRES(mDisplayLock) { + ftl::FakeGuard guard(kMainThreadContext); + const RefreshRateSelectorPtr noLeader; + return mLeaderDisplayId + .and_then([this](PhysicalDisplayId leaderId) + REQUIRES(mDisplayLock, kMainThreadContext) { + return mRefreshRateSelectors.get(leaderId); + }) + .value_or(std::cref(noLeader)); + } struct Policy { // Policy for choosing the display mode. @@ -367,10 +393,6 @@ private: std::optional<ModeChangedParams> cachedModeChangedParams; } mPolicy GUARDED_BY(mPolicyLock); - // TODO(b/255635821): Remove this by instead looking up the `mLeaderDisplayId` selector. - mutable std::mutex mRefreshRateSelectorLock; - RefreshRateSelectorPtr mRefreshRateSelector GUARDED_BY(mRefreshRateSelectorLock); - std::mutex mVsyncTimelineLock; std::optional<hal::VsyncPeriodChangeTimeline> mLastVsyncPeriodChangeTimeline GUARDED_BY(mVsyncTimelineLock); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f0d6aebede..3e12a77eeb 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2954,18 +2954,16 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, displaySurface, producer); if (mScheduler && !display->isVirtual()) { - auto selectorPtr = display->holdRefreshRateSelector(); - - // Display modes are reloaded on hotplug reconnect. - if (display->isPrimary()) { + const auto displayId = display->getPhysicalId(); + { // TODO(b/241285876): Annotate `processDisplayAdded` instead. ftl::FakeGuard guard(kMainThreadContext); - mScheduler->setRefreshRateSelector(selectorPtr); + + // For hotplug reconnect, renew the registration since display modes have been reloaded. + mScheduler->registerDisplay(displayId, display->holdRefreshRateSelector()); } - const auto displayId = display->getPhysicalId(); - mScheduler->registerDisplay(displayId, std::move(selectorPtr)); - dispatchDisplayHotplugEvent(display->getPhysicalId(), true); + dispatchDisplayHotplugEvent(displayId, true); } mDisplays.try_emplace(displayToken, std::move(display)); @@ -3424,9 +3422,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { !getHwComposer().hasCapability(Capability::PRESENT_FENCE_IS_NOT_RELIABLE)) { features |= Feature::kPresentFences; } - - auto selectorPtr = display->holdRefreshRateSelector(); - if (selectorPtr->kernelIdleTimerController()) { + if (display->refreshRateSelector().kernelIdleTimerController()) { features |= Feature::kKernelIdleTimer; } @@ -3434,8 +3430,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { static_cast<ISchedulerCallback&>(*this), features); mScheduler->createVsyncSchedule(features); - mScheduler->setRefreshRateSelector(selectorPtr); - mScheduler->registerDisplay(display->getPhysicalId(), std::move(selectorPtr)); + mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector()); setVsyncEnabled(false); mScheduler->startTimers(); @@ -6980,9 +6975,11 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const sp<DisplayDevice>& activ } mActiveDisplayId = activeDisplay->getPhysicalId(); activeDisplay->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(true); + updateInternalDisplayVsyncLocked(activeDisplay); mScheduler->setModeChangePending(false); - mScheduler->setRefreshRateSelector(activeDisplay->holdRefreshRateSelector()); + mScheduler->setLeaderDisplay(mActiveDisplayId); + onActiveDisplaySizeChanged(activeDisplay); mActiveDisplayTransformHint = activeDisplay->getTransformHint(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 1918913419..b51210d882 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -464,8 +464,7 @@ private: typename Handler = VsyncModulator::VsyncConfigOpt (VsyncModulator::*)(Args...)> void modulateVsync(Handler handler, Args... args) { if (const auto config = (*mVsyncModulator.*handler)(args...)) { - const auto vsyncPeriod = mScheduler->getVsyncPeriodFromRefreshRateSelector(); - setVsyncConfig(*config, vsyncPeriod); + setVsyncConfig(*config, mScheduler->getLeaderVsyncPeriod()); } } @@ -928,7 +927,8 @@ private: const sp<compositionengine::DisplaySurface>& displaySurface, const sp<IGraphicBufferProducer>& producer) REQUIRES(mStateLock); void processDisplayChangesLocked() REQUIRES(mStateLock, kMainThreadContext); - void processDisplayRemoved(const wp<IBinder>& displayToken) REQUIRES(mStateLock); + void processDisplayRemoved(const wp<IBinder>& displayToken) + REQUIRES(mStateLock, kMainThreadContext); void processDisplayChanged(const wp<IBinder>& displayToken, const DisplayDeviceState& currentState, const DisplayDeviceState& drawingState) diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h index 9ba9b90b82..ef7903949f 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h @@ -230,7 +230,10 @@ public: ISchedulerCallback& callback) : Scheduler(*this, callback, Feature::kContentDetection) { mVsyncSchedule.emplace(VsyncSchedule(std::move(tracker), nullptr, std::move(controller))); - setRefreshRateSelector(std::move(selectorPtr)); + + const auto displayId = FTL_FAKE_GUARD(kMainThreadContext, + selectorPtr->getActiveMode().getPhysicalDisplayId()); + registerDisplay(displayId, std::move(selectorPtr)); } ConnectionHandle createConnection(std::unique_ptr<EventThread> eventThread) { @@ -242,7 +245,7 @@ public: auto &mutableLayerHistory() { return mLayerHistory; } - auto refreshRateSelector() { return holdRefreshRateSelector(); } + auto refreshRateSelector() { return leaderSelectorPtr(); } void replaceTouchTimer(int64_t millis) { if (mTouchTimer) { diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index ea4666ed4b..8e333a3482 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -190,8 +190,10 @@ TEST_F(SchedulerTest, updateDisplayModes) { sp<MockLayer> layer = sp<MockLayer>::make(mFlinger.flinger()); ASSERT_EQ(1u, mScheduler->layerHistorySize()); - mScheduler->setRefreshRateSelector( - std::make_shared<RefreshRateSelector>(kDisplay1Modes, kDisplay1Mode60->getId())); + // Replace `mSelector` with a new `RefreshRateSelector` that has different display modes. + mScheduler->registerDisplay(kDisplayId1, + std::make_shared<RefreshRateSelector>(kDisplay1Modes, + kDisplay1Mode60->getId())); ASSERT_EQ(0u, mScheduler->getNumActiveLayers()); mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer); @@ -234,11 +236,9 @@ MATCHER(Is120Hz, "") { } TEST_F(SchedulerTest, chooseRefreshRateForContentSelectsMaxRefreshRate) { - const auto selectorPtr = - std::make_shared<RefreshRateSelector>(kDisplay1Modes, kDisplay1Mode60->getId()); - - mScheduler->registerDisplay(kDisplayId1, selectorPtr); - mScheduler->setRefreshRateSelector(selectorPtr); + mScheduler->registerDisplay(kDisplayId1, + std::make_shared<RefreshRateSelector>(kDisplay1Modes, + kDisplay1Mode60->getId())); const sp<MockLayer> layer = sp<MockLayer>::make(mFlinger.flinger()); EXPECT_CALL(*layer, isVisible()).WillOnce(Return(true)); diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index ba214d534f..3f8fe0d036 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -43,7 +43,10 @@ public: ISchedulerCallback& callback) : Scheduler(*this, callback, Feature::kContentDetection) { mVsyncSchedule.emplace(VsyncSchedule(std::move(tracker), nullptr, std::move(controller))); - setRefreshRateSelector(std::move(selectorPtr)); + + const auto displayId = FTL_FAKE_GUARD(kMainThreadContext, + selectorPtr->getActiveMode().getPhysicalDisplayId()); + registerDisplay(displayId, std::move(selectorPtr)); ON_CALL(*this, postMessage).WillByDefault([](sp<MessageHandler>&& handler) { // Execute task to prevent broken promise exception on destruction. @@ -67,12 +70,27 @@ public: auto& mutablePrimaryHWVsyncEnabled() { return mPrimaryHWVsyncEnabled; } auto& mutableHWVsyncAvailable() { return mHWVsyncAvailable; } - auto refreshRateSelector() { return holdRefreshRateSelector(); } - bool hasRefreshRateSelectors() const { return !mRefreshRateSelectors.empty(); } + auto refreshRateSelector() { return leaderSelectorPtr(); } + + const auto& refreshRateSelectors() const NO_THREAD_SAFETY_ANALYSIS { + return mRefreshRateSelectors; + } + + bool hasRefreshRateSelectors() const { return !refreshRateSelectors().empty(); } - void setRefreshRateSelector(RefreshRateSelectorPtr selectorPtr) { + void registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr) { ftl::FakeGuard guard(kMainThreadContext); - return Scheduler::setRefreshRateSelector(std::move(selectorPtr)); + Scheduler::registerDisplay(displayId, std::move(selectorPtr)); + } + + void unregisterDisplay(PhysicalDisplayId displayId) { + ftl::FakeGuard guard(kMainThreadContext); + Scheduler::unregisterDisplay(displayId); + } + + void setLeaderDisplay(PhysicalDisplayId displayId) { + ftl::FakeGuard guard(kMainThreadContext); + Scheduler::setLeaderDisplay(displayId); } auto& mutableLayerHistory() { return mLayerHistory; } @@ -115,14 +133,13 @@ public: using Scheduler::DisplayModeChoice; using Scheduler::DisplayModeChoiceMap; - DisplayModeChoiceMap chooseDisplayModes() { - std::lock_guard<std::mutex> lock(mPolicyLock); + DisplayModeChoiceMap chooseDisplayModes() NO_THREAD_SAFETY_ANALYSIS { return Scheduler::chooseDisplayModes(); } void dispatchCachedReportedMode() { std::lock_guard<std::mutex> lock(mPolicyLock); - return Scheduler::dispatchCachedReportedMode(); + Scheduler::dispatchCachedReportedMode(); } void clearCachedReportedMode() { @@ -131,7 +148,7 @@ public: } void onNonPrimaryDisplayModeChanged(ConnectionHandle handle, DisplayModePtr mode) { - return Scheduler::onNonPrimaryDisplayModeChanged(handle, mode); + Scheduler::onNonPrimaryDisplayModeChanged(handle, mode); } private: diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index df53f1560a..2d5eb92080 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -837,33 +837,36 @@ public: sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs); mFlinger.mutableDisplays().emplace_or_replace(mDisplayToken, display); - if (mFlinger.scheduler()) { - mFlinger.scheduler()->registerDisplay(display->getPhysicalId(), - display->holdRefreshRateSelector()); - } DisplayDeviceState state; state.isSecure = mCreationArgs.isSecure; if (mConnectionType) { LOG_ALWAYS_FATAL_IF(!displayId); - const auto physicalId = PhysicalDisplayId::tryCast(*displayId); - LOG_ALWAYS_FATAL_IF(!physicalId); + const auto physicalIdOpt = PhysicalDisplayId::tryCast(*displayId); + LOG_ALWAYS_FATAL_IF(!physicalIdOpt); + const auto physicalId = *physicalIdOpt; + LOG_ALWAYS_FATAL_IF(!mHwcDisplayId); const auto activeMode = modes.get(activeModeId); LOG_ALWAYS_FATAL_IF(!activeMode); - state.physical = {.id = *physicalId, + state.physical = {.id = physicalId, .hwcDisplayId = *mHwcDisplayId, .activeMode = activeMode->get()}; const auto it = mFlinger.mutablePhysicalDisplays() - .emplace_or_replace(*physicalId, mDisplayToken, *physicalId, + .emplace_or_replace(physicalId, mDisplayToken, physicalId, *mConnectionType, std::move(modes), ui::ColorModes(), std::nullopt) .first; + if (mFlinger.scheduler()) { + mFlinger.scheduler()->registerDisplay(physicalId, + display->holdRefreshRateSelector()); + } + display->setActiveMode(activeModeId, it->second.snapshot()); } |