From fc94b41a23e05693c98fc26182bb6724e96009c2 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Sat, 3 Aug 2024 15:02:23 -0400 Subject: SF: Clean up emitting of mode change event Clarify the special case of emitting despite the unchanged mode. Rename functions and state for consistency with DisplayModeRequest::emitEvent. Remove Cycle parameters for which the argument is always Cycle::Render. This does not change behavior. Bug: 255635821 Flag: EXEMPT refactor Test: SchedulerTest.emitModeChangeEvent Change-Id: I99debb33ba84dff707d6da0a92dacc4e275fc5ce --- services/surfaceflinger/SurfaceFlinger.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e7d802a524..c616a7076e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3596,7 +3596,7 @@ void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId, ? &scheduler::Scheduler::onPrimaryDisplayModeChanged : &scheduler::Scheduler::onNonPrimaryDisplayModeChanged; - ((*mScheduler).*onDisplayModeChanged)(scheduler::Cycle::Render, mode); + ((*mScheduler).*onDisplayModeChanged)(mode); } sp SurfaceFlinger::setupNewDisplayDeviceInternal( @@ -7962,10 +7962,10 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might // be depending in this callback. if (const auto activeMode = selector.getActiveMode(); displayId == mActiveDisplayId) { - mScheduler->onPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode); + mScheduler->onPrimaryDisplayModeChanged(activeMode); toggleKernelIdleTimer(); } else { - mScheduler->onNonPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode); + mScheduler->onNonPrimaryDisplayModeChanged(activeMode); } auto preferredModeOpt = getPreferredDisplayMode(displayId, currentPolicy.defaultMode); -- cgit v1.2.3-59-g8ed1b From bda5236d4a6bfc7533b62f92b12b7ba8a0d5cc6f Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Sun, 4 Aug 2024 00:41:46 -0400 Subject: SF: Merge on{,Non}PrimaryDisplayModeChanged Branch on the pacesetter rather than active display, though they are the same display in practice for now. Bug: 255635821 Flag: EXEMPT refactor Test: presubmit Change-Id: I40d73e79075893826492b85c27e35eed59b4b289 --- services/surfaceflinger/Scheduler/Scheduler.cpp | 18 ++++++++++++------ services/surfaceflinger/Scheduler/Scheduler.h | 4 ++-- services/surfaceflinger/SurfaceFlinger.cpp | 22 ++++------------------ services/surfaceflinger/SurfaceFlinger.h | 2 -- .../tests/unittests/SchedulerTest.cpp | 4 ++-- .../tests/unittests/TestableScheduler.h | 2 +- 6 files changed, 21 insertions(+), 31 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index b7110272bb..6b3113512e 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -424,8 +424,12 @@ void Scheduler::onHdcpLevelsChanged(Cycle cycle, PhysicalDisplayId displayId, eventThreadFor(cycle).onHdcpLevelsChanged(displayId, connectedLevel, maxLevel); } -void Scheduler::onPrimaryDisplayModeChanged(const FrameRateMode& mode) { - { +bool Scheduler::onDisplayModeChanged(PhysicalDisplayId displayId, const FrameRateMode& mode) { + const bool isPacesetter = + FTL_FAKE_GUARD(kMainThreadContext, + (std::scoped_lock(mDisplayLock), displayId == mPacesetterDisplayId)); + + if (isPacesetter) { std::lock_guard lock(mPolicyLock); mPolicy.emittedModeOpt = mode; @@ -433,7 +437,12 @@ void Scheduler::onPrimaryDisplayModeChanged(const FrameRateMode& mode) { // again for the new refresh rate. mPolicy.contentRequirements.clear(); } - onNonPrimaryDisplayModeChanged(mode); + + if (hasEventThreads()) { + eventThreadFor(Cycle::Render).onModeChanged(mode); + } + + return isPacesetter; } void Scheduler::emitModeChangeIfNeeded() { @@ -455,10 +464,7 @@ void Scheduler::emitModeChangeIfNeeded() { } mPolicy.emittedModeOpt = mode; - onNonPrimaryDisplayModeChanged(mode); -} -void Scheduler::onNonPrimaryDisplayModeChanged(const FrameRateMode& mode) { if (hasEventThreads()) { eventThreadFor(Cycle::Render).onModeChanged(mode); } diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index fc22cc33e4..4003d9f840 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -154,8 +154,8 @@ public: void dispatchHotplugError(int32_t errorCode); - void onPrimaryDisplayModeChanged(const FrameRateMode&) EXCLUDES(mPolicyLock); - void onNonPrimaryDisplayModeChanged(const FrameRateMode&); + // Returns true if the PhysicalDisplayId is the pacesetter. + bool onDisplayModeChanged(PhysicalDisplayId, const FrameRateMode&) EXCLUDES(mPolicyLock); void enableSyntheticVsync(bool = true) REQUIRES(kMainThreadContext); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c616a7076e..9633cc5c9d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1358,7 +1358,7 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { } if (emitEvent) { - dispatchDisplayModeChangeEvent(displayId, mode); + mScheduler->onDisplayModeChanged(displayId, mode); } break; case DesiredModeAction::None: @@ -1455,7 +1455,7 @@ void SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) { } if (pendingModeOpt->emitEvent) { - dispatchDisplayModeChangeEvent(displayId, activeMode); + mScheduler->onDisplayModeChanged(displayId, activeMode); } } @@ -3589,16 +3589,6 @@ void SurfaceFlinger::processHotplugDisconnect(PhysicalDisplayId displayId, mPhysicalDisplays.erase(displayId); } -void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId, - const scheduler::FrameRateMode& mode) { - // TODO(b/255635821): Merge code paths and move to Scheduler. - const auto onDisplayModeChanged = displayId == mActiveDisplayId - ? &scheduler::Scheduler::onPrimaryDisplayModeChanged - : &scheduler::Scheduler::onNonPrimaryDisplayModeChanged; - - ((*mScheduler).*onDisplayModeChanged)(mode); -} - sp SurfaceFlinger::setupNewDisplayDeviceInternal( const wp& displayToken, std::shared_ptr compositionDisplay, @@ -7959,13 +7949,9 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy(); ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str()); - // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might - // be depending in this callback. - if (const auto activeMode = selector.getActiveMode(); displayId == mActiveDisplayId) { - mScheduler->onPrimaryDisplayModeChanged(activeMode); + if (const bool isPacesetter = + mScheduler->onDisplayModeChanged(displayId, selector.getActiveMode())) { toggleKernelIdleTimer(); - } else { - mScheduler->onNonPrimaryDisplayModeChanged(activeMode); } auto preferredModeOpt = getPreferredDisplayMode(displayId, currentPolicy.defaultMode); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 9b2dea2dae..8b71f3be55 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1067,8 +1067,6 @@ private: const DisplayDeviceState& drawingState) REQUIRES(mStateLock, kMainThreadContext); - void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&); - /* * VSYNC */ diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index 5b5d6aef5a..dddda05544 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -203,7 +203,7 @@ TEST_F(SchedulerTest, emitModeChangeEvent) { const auto selectorPtr = std::make_shared(kDisplay1Modes, kDisplay1Mode120->getId()); mScheduler->registerDisplay(kDisplayId1, selectorPtr); - mScheduler->onPrimaryDisplayModeChanged(kDisplay1Mode120_120); + mScheduler->onDisplayModeChanged(kDisplayId1, kDisplay1Mode120_120); mScheduler->setContentRequirements({kLayer}); @@ -231,7 +231,7 @@ TEST_F(SchedulerTest, emitModeChangeEvent) { EXPECT_CALL(*mEventThread, onModeChanged(kDisplay1Mode120_120)).Times(1); mScheduler->touchTimerCallback(TimerState::Reset); - mScheduler->onPrimaryDisplayModeChanged(kDisplay1Mode120_120); + mScheduler->onDisplayModeChanged(kDisplayId1, kDisplay1Mode120_120); } TEST_F(SchedulerTest, calculateMaxAcquiredBufferCount) { diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index b272cf179a..df16b2e058 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -193,7 +193,7 @@ public: return Scheduler::chooseDisplayModes(); } - using Scheduler::onPrimaryDisplayModeChanged; + using Scheduler::onDisplayModeChanged; void setInitialHwVsyncEnabled(PhysicalDisplayId id, bool enabled) { auto schedule = getVsyncSchedule(id); -- cgit v1.2.3-59-g8ed1b