diff options
16 files changed, 228 insertions, 187 deletions
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp index 71d563130e..a94952f415 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp @@ -714,20 +714,19 @@ RefreshRateConfigs::RefreshRateConfigs(const DisplayModes& modes, DisplayModeId void RefreshRateConfigs::initializeIdleTimer() { if (mConfig.idleTimerTimeoutMs > 0) { - const auto getCallback = [this]() -> std::optional<IdleTimerCallbacks::Callbacks> { - std::scoped_lock lock(mIdleTimerCallbacksMutex); - if (!mIdleTimerCallbacks.has_value()) return {}; - return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel - : mIdleTimerCallbacks->platform; - }; - mIdleTimer.emplace( "IdleTimer", std::chrono::milliseconds(mConfig.idleTimerTimeoutMs), - [getCallback] { - if (const auto callback = getCallback()) callback->onReset(); + [this] { + std::scoped_lock lock(mIdleTimerCallbacksMutex); + if (const auto callbacks = getIdleTimerCallbacks()) { + callbacks->onReset(); + } }, - [getCallback] { - if (const auto callback = getCallback()) callback->onExpired(); + [this] { + std::scoped_lock lock(mIdleTimerCallbacksMutex); + if (const auto callbacks = getIdleTimerCallbacks()) { + callbacks->onExpired(); + } }); } } diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h index 4bbdab6bbe..fc45d2b909 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h @@ -348,16 +348,24 @@ public: bool supportsKernelIdleTimer() const { return mConfig.supportKernelIdleTimer; } - void setIdleTimerCallbacks(std::function<void()> platformTimerReset, - std::function<void()> platformTimerExpired, - std::function<void()> kernelTimerReset, - std::function<void()> kernelTimerExpired) { + struct IdleTimerCallbacks { + struct Callbacks { + std::function<void()> onReset; + std::function<void()> onExpired; + }; + + Callbacks platform; + Callbacks kernel; + }; + + void setIdleTimerCallbacks(IdleTimerCallbacks callbacks) EXCLUDES(mIdleTimerCallbacksMutex) { std::scoped_lock lock(mIdleTimerCallbacksMutex); - mIdleTimerCallbacks.emplace(); - mIdleTimerCallbacks->platform.onReset = std::move(platformTimerReset); - mIdleTimerCallbacks->platform.onExpired = std::move(platformTimerExpired); - mIdleTimerCallbacks->kernel.onReset = std::move(kernelTimerReset); - mIdleTimerCallbacks->kernel.onExpired = std::move(kernelTimerExpired); + mIdleTimerCallbacks = std::move(callbacks); + } + + void clearIdleTimerCallbacks() EXCLUDES(mIdleTimerCallbacksMutex) { + std::scoped_lock lock(mIdleTimerCallbacksMutex); + mIdleTimerCallbacks.reset(); } void startIdleTimer() { @@ -380,7 +388,7 @@ public: return; } mIdleTimer->reset(); - }; + } void dump(std::string& result) const EXCLUDES(mLock); @@ -448,6 +456,13 @@ private: void initializeIdleTimer(); + std::optional<IdleTimerCallbacks::Callbacks> getIdleTimerCallbacks() const + REQUIRES(mIdleTimerCallbacksMutex) { + if (!mIdleTimerCallbacks) return {}; + return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel + : mIdleTimerCallbacks->platform; + } + // The list of refresh rates, indexed by display modes ID. This may change after this // object is initialized. AllRefreshRatesMapType mRefreshRates GUARDED_BY(mLock); @@ -492,21 +507,11 @@ private: mutable std::optional<GetBestRefreshRateInvocation> lastBestRefreshRateInvocation GUARDED_BY(mLock); - // Timer that records time between requests for next vsync. - std::optional<scheduler::OneShotTimer> mIdleTimer; - - struct IdleTimerCallbacks { - struct Callbacks { - std::function<void()> onReset; - std::function<void()> onExpired; - }; - - Callbacks platform; - Callbacks kernel; - }; - + // Declare mIdleTimer last to ensure its thread joins before the mutex/callbacks are destroyed. std::mutex mIdleTimerCallbacksMutex; std::optional<IdleTimerCallbacks> mIdleTimerCallbacks GUARDED_BY(mIdleTimerCallbacksMutex); + // Used to detect (lack of) frame activity. + std::optional<scheduler::OneShotTimer> mIdleTimer; }; } // namespace android::scheduler diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index cbe4552b0b..4173088d99 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -62,6 +62,15 @@ namespace android::scheduler { Scheduler::Scheduler(ICompositor& compositor, ISchedulerCallback& callback, FeatureFlags features) : impl::MessageQueue(compositor), mFeatures(features), mSchedulerCallback(callback) {} +Scheduler::~Scheduler() { + // Stop timers and wait for their threads to exit. + mDisplayPowerTimer.reset(); + mTouchTimer.reset(); + + // Stop idle timer and clear callbacks, as the RefreshRateConfigs may outlive the Scheduler. + setRefreshRateConfigs(nullptr); +} + void Scheduler::startTimers() { using namespace sysprop; using namespace std::string_literals; @@ -84,11 +93,32 @@ void Scheduler::startTimers() { } } -Scheduler::~Scheduler() { - // Ensure the OneShotTimer threads are joined before we start destroying state. - mDisplayPowerTimer.reset(); - mTouchTimer.reset(); - mRefreshRateConfigs.reset(); +void Scheduler::setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> configs) { + { + // The current RefreshRateConfigs instance may outlive this call, so unbind its idle timer. + std::scoped_lock lock(mRefreshRateConfigsLock); + if (mRefreshRateConfigs) { + mRefreshRateConfigs->stopIdleTimer(); + mRefreshRateConfigs->clearIdleTimerCallbacks(); + } + } + { + // Clear state that depends on the current instance. + std::scoped_lock lock(mPolicyLock); + mPolicy = {}; + } + + std::scoped_lock lock(mRefreshRateConfigsLock); + mRefreshRateConfigs = std::move(configs); + if (!mRefreshRateConfigs) return; + + mRefreshRateConfigs->setIdleTimerCallbacks( + {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); }, + .onExpired = [this] { idleTimerCallback(TimerState::Expired); }}, + .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); }, + .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}}); + + mRefreshRateConfigs->startIdleTimer(); } void Scheduler::run() { diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 818f1edb41..548c34b5d2 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -74,12 +74,16 @@ class Scheduler : impl::MessageQueue { public: Scheduler(ICompositor&, ISchedulerCallback&, FeatureFlags); - ~Scheduler(); + virtual ~Scheduler(); - void createVsyncSchedule(FeatureFlags); void startTimers(); + void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs>) + EXCLUDES(mRefreshRateConfigsLock); + void run(); + void createVsyncSchedule(FeatureFlags); + using Impl::initVsync; using Impl::setInjector; @@ -198,36 +202,6 @@ public: std::optional<Fps> getFrameRateOverride(uid_t uid) const EXCLUDES(mRefreshRateConfigsLock, mFrameRateOverridesLock); - void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> refreshRateConfigs) - EXCLUDES(mRefreshRateConfigsLock) { - // We need to stop the idle timer on the previous RefreshRateConfigs instance - // and cleanup the scheduler's state before we switch to the other RefreshRateConfigs. - { - std::scoped_lock lock(mRefreshRateConfigsLock); - if (mRefreshRateConfigs) mRefreshRateConfigs->stopIdleTimer(); - } - { - std::scoped_lock lock(mPolicyLock); - mPolicy = {}; - } - { - std::scoped_lock lock(mRefreshRateConfigsLock); - mRefreshRateConfigs = std::move(refreshRateConfigs); - mRefreshRateConfigs->setIdleTimerCallbacks( - [this] { std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Reset); }, - [this] { - std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Expired); - }, - [this] { - std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Reset); - }, - [this] { - std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Expired); - }); - mRefreshRateConfigs->startIdleTimer(); - } - } - nsecs_t getVsyncPeriodFromRefreshRateConfigs() const EXCLUDES(mRefreshRateConfigsLock) { std::scoped_lock lock(mRefreshRateConfigsLock); return mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod(); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 64a18c6d5c..f8c7aec216 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -384,7 +384,7 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory, SkipInitializationTag) mInternalDisplayDensity(getDensityFromProperty("ro.sf.lcd_density", true)), mEmulatedDisplayDensity(getDensityFromProperty("qemu.sf.lcd_density", false)), mPowerAdvisor(*this), - mWindowInfosListenerInvoker(new WindowInfosListenerInvoker(this)) { + mWindowInfosListenerInvoker(sp<WindowInfosListenerInvoker>::make(*this)) { ALOGI("Using HWComposer service: %s", mHwcServiceName.c_str()); } diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.cpp b/services/surfaceflinger/WindowInfosListenerInvoker.cpp index b93d127ab8..72434e943a 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.cpp +++ b/services/surfaceflinger/WindowInfosListenerInvoker.cpp @@ -25,24 +25,21 @@ using gui::DisplayInfo; using gui::IWindowInfosListener; using gui::WindowInfo; -struct WindowInfosReportedListener : gui::BnWindowInfosReportedListener { - explicit WindowInfosReportedListener(std::function<void()> listenerCb) - : mListenerCb(listenerCb) {} +struct WindowInfosListenerInvoker::WindowInfosReportedListener + : gui::BnWindowInfosReportedListener { + explicit WindowInfosReportedListener(WindowInfosListenerInvoker& invoker) : mInvoker(invoker) {} binder::Status onWindowInfosReported() override { - if (mListenerCb != nullptr) { - mListenerCb(); - } + mInvoker.windowInfosReported(); return binder::Status::ok(); } - std::function<void()> mListenerCb; + WindowInfosListenerInvoker& mInvoker; }; -WindowInfosListenerInvoker::WindowInfosListenerInvoker(const sp<SurfaceFlinger>& sf) : mSf(sf) { - mWindowInfosReportedListener = - new WindowInfosReportedListener([&]() { windowInfosReported(); }); -} +WindowInfosListenerInvoker::WindowInfosListenerInvoker(SurfaceFlinger& flinger) + : mFlinger(flinger), + mWindowInfosReportedListener(sp<WindowInfosReportedListener>::make(*this)) {} void WindowInfosListenerInvoker::addWindowInfosListener( const sp<IWindowInfosListener>& windowInfosListener) { @@ -91,8 +88,8 @@ void WindowInfosListenerInvoker::windowInfosChanged(const std::vector<WindowInfo void WindowInfosListenerInvoker::windowInfosReported() { mCallbacksPending--; if (mCallbacksPending == 0) { - mSf->windowInfosReported(); + mFlinger.windowInfosReported(); } } -} // namespace android
\ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.h b/services/surfaceflinger/WindowInfosListenerInvoker.h index 4e08393940..2eabf481c8 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.h +++ b/services/surfaceflinger/WindowInfosListenerInvoker.h @@ -31,7 +31,8 @@ class SurfaceFlinger; class WindowInfosListenerInvoker : public IBinder::DeathRecipient { public: - WindowInfosListenerInvoker(const sp<SurfaceFlinger>& sf); + explicit WindowInfosListenerInvoker(SurfaceFlinger&); + void addWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener); void removeWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener); @@ -42,13 +43,15 @@ protected: void binderDied(const wp<IBinder>& who) override; private: + struct WindowInfosReportedListener; void windowInfosReported(); - const sp<SurfaceFlinger> mSf; + SurfaceFlinger& mFlinger; std::mutex mListenersMutex; std::unordered_map<wp<IBinder>, const sp<gui::IWindowInfosListener>, WpHash> mWindowInfosListeners GUARDED_BY(mListenersMutex); sp<gui::IWindowInfosReportedListener> mWindowInfosReportedListener; std::atomic<size_t> mCallbacksPending{0}; }; -} // namespace android
\ No newline at end of file + +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 6c96d5ff03..dee2358ab7 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -143,11 +143,10 @@ public: .WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD)); EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0)); - constexpr scheduler::ISchedulerCallback* kCallback = nullptr; - constexpr bool kHasMultipleConfigs = true; mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker), - std::move(eventThread), std::move(sfEventThread), kCallback, - kHasMultipleConfigs); + std::move(eventThread), std::move(sfEventThread), + TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp, + TestableSurfaceFlinger::kTwoDisplayModes); } void setupForceGeometryDirty() { diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp index 5a0033ea7e..40a9b1aad0 100644 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp @@ -44,8 +44,8 @@ public: mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED); mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) - .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120}) - .setActiveMode(kDisplayModeId60) + .setDisplayModes({kDisplayMode60, kDisplayMode90, kDisplayMode120}, + kDisplayModeId60) .inject(); } diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index c318e28ffe..2425862383 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -77,7 +77,8 @@ void DisplayTransactionTest::injectMockScheduler() { mFlinger.setupScheduler(std::unique_ptr<scheduler::VsyncController>(mVsyncController), std::unique_ptr<scheduler::VSyncTracker>(mVSyncTracker), std::unique_ptr<EventThread>(mEventThread), - std::unique_ptr<EventThread>(mSFEventThread), &mSchedulerCallback); + std::unique_ptr<EventThread>(mSFEventThread), + TestableSurfaceFlinger::SchedulerCallbackImpl::kMock); } void DisplayTransactionTest::injectMockComposer(int virtualDisplayCount) { diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h index 69ac26e997..45eceff96d 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h @@ -48,7 +48,6 @@ #include "mock/DisplayHardware/MockPowerAdvisor.h" #include "mock/MockEventThread.h" #include "mock/MockNativeWindowSurface.h" -#include "mock/MockSchedulerCallback.h" #include "mock/MockSurfaceInterceptor.h" #include "mock/MockVsyncController.h" #include "mock/system/window/MockNativeWindow.h" @@ -121,7 +120,6 @@ public: mock::VsyncController* mVsyncController = new mock::VsyncController; mock::VSyncTracker* mVSyncTracker = new mock::VSyncTracker; - scheduler::mock::SchedulerCallback mSchedulerCallback; mock::EventThread* mEventThread = new mock::EventThread; mock::EventThread* mSFEventThread = new mock::EventThread; diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index fe5f9e0717..2b69f13126 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -164,8 +164,9 @@ void SetFrameRateTest::setupScheduler() { .WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD)); EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0)); mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker), - std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr, - /*hasMultipleModes*/ true); + std::move(eventThread), std::move(sfEventThread), + TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp, + TestableSurfaceFlinger::kTwoDisplayModes); } namespace { diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 56a050648d..3205952f5a 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -42,13 +42,18 @@ public: mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED); - mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) - .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120, - kDisplayMode90DifferentResolution}) - .setActiveMode(kDisplayModeId60) - .inject(); + { + DisplayModes modes = {kDisplayMode60, kDisplayMode90, kDisplayMode120, + kDisplayMode90DifferentResolution}; + const DisplayModeId activeModeId = kDisplayModeId60; + auto configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, activeModeId); - setupScheduler(); + mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) + .setDisplayModes(modes, activeModeId, std::move(configs)) + .inject(); + } + + setupScheduler(mDisplay->holdRefreshRateConfigs()); // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy // will call setActiveConfig instead of setActiveConfigWithConstraints. @@ -57,7 +62,7 @@ public: } protected: - void setupScheduler(); + void setupScheduler(std::shared_ptr<scheduler::RefreshRateConfigs>); void testChangeRefreshRate(bool isDisplayActive, bool isRefreshRequired); sp<DisplayDevice> mDisplay; @@ -108,7 +113,8 @@ protected: .build(); }; -void DisplayModeSwitchingTest::setupScheduler() { +void DisplayModeSwitchingTest::setupScheduler( + std::shared_ptr<scheduler::RefreshRateConfigs> configs) { auto eventThread = std::make_unique<mock::EventThread>(); mAppEventThread = eventThread.get(); auto sfEventThread = std::make_unique<mock::EventThread>(); @@ -132,8 +138,9 @@ void DisplayModeSwitchingTest::setupScheduler() { Return(TestableSurfaceFlinger::FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD)); EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0)); mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker), - std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr, - /*hasMultipleModes*/ true); + std::move(eventThread), std::move(sfEventThread), + TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp, + std::move(configs)); } TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) { diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp index b57feffe83..7948e60e14 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp @@ -61,7 +61,7 @@ struct DozeNotSupportedVariant { struct EventThreadBaseSupportedVariant { static void setupVsyncAndEventThreadNoCallExpectations(DisplayTransactionTest* test) { // The callback should not be notified to toggle VSYNC. - EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(_)).Times(0); + EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(_)).Times(0); // The event thread should not be notified. EXPECT_CALL(*test->mEventThread, onScreenReleased()).Times(0); @@ -88,7 +88,7 @@ struct EventThreadNotSupportedVariant : public EventThreadBaseSupportedVariant { struct EventThreadIsSupportedVariant : public EventThreadBaseSupportedVariant { static void setupAcquireAndEnableVsyncCallExpectations(DisplayTransactionTest* test) { // The callback should be notified to enable VSYNC. - EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(true)).Times(1); + EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(true)).Times(1); // The event thread should be notified that the screen was acquired. EXPECT_CALL(*test->mEventThread, onScreenAcquired()).Times(1); @@ -96,7 +96,7 @@ struct EventThreadIsSupportedVariant : public EventThreadBaseSupportedVariant { static void setupReleaseAndDisableVsyncCallExpectations(DisplayTransactionTest* test) { // The callback should be notified to disable VSYNC. - EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(false)).Times(1); + EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(false)).Times(1); // The event thread should not be notified that the screen was released. EXPECT_CALL(*test->mEventThread, onScreenReleased()).Times(1); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 361d629f1e..d292e0879d 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -16,6 +16,9 @@ #pragma once +#include <algorithm> +#include <variant> + #include <compositionengine/Display.h> #include <compositionengine/LayerFECompositionState.h> #include <compositionengine/OutputLayer.h> @@ -24,7 +27,6 @@ #include <compositionengine/impl/OutputLayerCompositionState.h> #include <compositionengine/mock/DisplaySurface.h> #include <gui/ScreenCaptureResults.h> -#include <algorithm> #include "BufferQueueLayer.h" #include "BufferStateLayer.h" @@ -45,6 +47,7 @@ #include "mock/DisplayHardware/MockComposer.h" #include "mock/MockFrameTimeline.h" #include "mock/MockFrameTracer.h" +#include "mock/MockSchedulerCallback.h" namespace android { @@ -170,7 +173,7 @@ public: } // namespace surfaceflinger::test -class TestableSurfaceFlinger final : private scheduler::ISchedulerCallback { +class TestableSurfaceFlinger { public: using HotplugEvent = SurfaceFlinger::HotplugEvent; @@ -193,42 +196,64 @@ public: mFlinger->mCompositionEngine->setTimeStats(timeStats); } - // The ISchedulerCallback argument can be nullptr for a no-op implementation. + enum class SchedulerCallbackImpl { kNoOp, kMock }; + + static constexpr struct OneDisplayMode { + } kOneDisplayMode; + + static constexpr struct TwoDisplayModes { + } kTwoDisplayModes; + + using RefreshRateConfigsPtr = std::shared_ptr<scheduler::RefreshRateConfigs>; + + using DisplayModesVariant = + std::variant<OneDisplayMode, TwoDisplayModes, RefreshRateConfigsPtr>; + void setupScheduler(std::unique_ptr<scheduler::VsyncController> vsyncController, std::unique_ptr<scheduler::VSyncTracker> vsyncTracker, std::unique_ptr<EventThread> appEventThread, std::unique_ptr<EventThread> sfEventThread, - scheduler::ISchedulerCallback* callback = nullptr, - bool hasMultipleModes = false) { - DisplayModes modes{DisplayMode::Builder(0) - .setId(DisplayModeId(0)) - .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) - .setVsyncPeriod(16'666'667) - .setGroup(0) - .build()}; - - if (hasMultipleModes) { - modes.emplace_back(DisplayMode::Builder(1) - .setId(DisplayModeId(1)) - .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) - .setVsyncPeriod(11'111'111) - .setGroup(0) - .build()); + SchedulerCallbackImpl callbackImpl = SchedulerCallbackImpl::kNoOp, + DisplayModesVariant modesVariant = kOneDisplayMode) { + RefreshRateConfigsPtr configs; + if (std::holds_alternative<RefreshRateConfigsPtr>(modesVariant)) { + configs = std::move(std::get<RefreshRateConfigsPtr>(modesVariant)); + } else { + DisplayModes modes = {DisplayMode::Builder(0) + .setId(DisplayModeId(0)) + .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) + .setVsyncPeriod(16'666'667) + .setGroup(0) + .build()}; + + if (std::holds_alternative<TwoDisplayModes>(modesVariant)) { + modes.emplace_back(DisplayMode::Builder(1) + .setId(DisplayModeId(1)) + .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) + .setVsyncPeriod(11'111'111) + .setGroup(0) + .build()); + } + + configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, DisplayModeId(0)); } - const auto currMode = DisplayModeId(0); - mRefreshRateConfigs = std::make_shared<scheduler::RefreshRateConfigs>(modes, currMode); - const auto currFps = mRefreshRateConfigs->getCurrentRefreshRate().getFps(); + const auto currFps = configs->getCurrentRefreshRate().getFps(); mFlinger->mVsyncConfiguration = mFactory.createVsyncConfiguration(currFps); mFlinger->mVsyncModulator = sp<scheduler::VsyncModulator>::make( mFlinger->mVsyncConfiguration->getCurrentConfigs()); mFlinger->mRefreshRateStats = std::make_unique<scheduler::RefreshRateStats>(*mFlinger->mTimeStats, currFps, - /*powerMode=*/hal::PowerMode::OFF); + hal::PowerMode::OFF); + + using Callback = scheduler::ISchedulerCallback; + Callback& callback = callbackImpl == SchedulerCallbackImpl::kNoOp + ? static_cast<Callback&>(mNoOpSchedulerCallback) + : static_cast<Callback&>(mSchedulerCallback); mScheduler = new scheduler::TestableScheduler(std::move(vsyncController), - std::move(vsyncTracker), mRefreshRateConfigs, - *(callback ?: this)); + std::move(vsyncTracker), std::move(configs), + callback); mFlinger->mAppConnectionHandle = mScheduler->createConnection(std::move(appEventThread)); mFlinger->mSfConnectionHandle = mScheduler->createConnection(std::move(sfEventThread)); @@ -237,7 +262,8 @@ public: void resetScheduler(scheduler::Scheduler* scheduler) { mFlinger->mScheduler.reset(scheduler); } - scheduler::TestableScheduler& mutableScheduler() const { return *mScheduler; } + scheduler::TestableScheduler& mutableScheduler() { return *mScheduler; } + scheduler::mock::SchedulerCallback& mockSchedulerCallback() { return mSchedulerCallback; } using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction; void setCreateBufferQueueFunction(CreateBufferQueueFunction f) { @@ -662,23 +688,6 @@ public: mHwcDisplayId(hwcDisplayId) { mCreationArgs.connectionType = connectionType; mCreationArgs.isPrimary = isPrimary; - - mCreationArgs.activeModeId = DisplayModeId(0); - DisplayModePtr activeMode = - DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG) - .setId(mCreationArgs.activeModeId) - .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) - .setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH) - .setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT) - .setVsyncPeriod(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD) - .setDpiX(FakeHwcDisplayInjector::DEFAULT_DPI) - .setDpiY(FakeHwcDisplayInjector::DEFAULT_DPI) - .setGroup(0) - .build(); - - DisplayModes modes{activeMode}; - mCreationArgs.supportedModes = modes; - mCreationArgs.refreshRateConfigs = flinger.mRefreshRateConfigs; } sp<IBinder> token() const { return mDisplayToken; } @@ -701,13 +710,16 @@ public: auto& mutableDisplayDevice() { return mFlinger.mutableDisplays()[mDisplayToken]; } - auto& setActiveMode(DisplayModeId mode) { - mCreationArgs.activeModeId = mode; - return *this; - } - - auto& setSupportedModes(DisplayModes mode) { - mCreationArgs.supportedModes = mode; + // If `configs` is nullptr, the injector creates RefreshRateConfigs from the `modes`. + // Otherwise, it uses `configs`, which the caller must create using the same `modes`. + // + // TODO(b/182939859): Once `modes` can be retrieved from RefreshRateConfigs, remove + // the `configs` parameter in favor of an alternative setRefreshRateConfigs API. + auto& setDisplayModes(DisplayModes modes, DisplayModeId activeModeId, + std::shared_ptr<scheduler::RefreshRateConfigs> configs = nullptr) { + mCreationArgs.supportedModes = std::move(modes); + mCreationArgs.activeModeId = activeModeId; + mCreationArgs.refreshRateConfigs = std::move(configs); return *this; } @@ -749,39 +761,58 @@ public: } sp<DisplayDevice> inject() NO_THREAD_SAFETY_ANALYSIS { - const auto displayId = mCreationArgs.compositionDisplay->getDisplayId(); + auto& modes = mCreationArgs.supportedModes; + auto& activeModeId = mCreationArgs.activeModeId; + + if (!mCreationArgs.refreshRateConfigs) { + if (modes.empty()) { + activeModeId = DisplayModeId(0); + modes.emplace_back( + DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG) + .setId(activeModeId) + .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0)) + .setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH) + .setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT) + .setVsyncPeriod(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD) + .setDpiX(FakeHwcDisplayInjector::DEFAULT_DPI) + .setDpiY(FakeHwcDisplayInjector::DEFAULT_DPI) + .setGroup(0) + .build()); + } + + mCreationArgs.refreshRateConfigs = + std::make_shared<scheduler::RefreshRateConfigs>(modes, activeModeId); + } DisplayDeviceState state; if (const auto type = mCreationArgs.connectionType) { + const auto displayId = mCreationArgs.compositionDisplay->getDisplayId(); LOG_ALWAYS_FATAL_IF(!displayId); const auto physicalId = PhysicalDisplayId::tryCast(*displayId); LOG_ALWAYS_FATAL_IF(!physicalId); LOG_ALWAYS_FATAL_IF(!mHwcDisplayId); - const DisplayModePtr activeModePtr = - *std::find_if(mCreationArgs.supportedModes.begin(), - mCreationArgs.supportedModes.end(), [&](DisplayModePtr mode) { - return mode->getId() == mCreationArgs.activeModeId; - }); + const auto it = std::find_if(modes.begin(), modes.end(), + [&activeModeId](const DisplayModePtr& mode) { + return mode->getId() == activeModeId; + }); + LOG_ALWAYS_FATAL_IF(it == modes.end()); + state.physical = {.id = *physicalId, .type = *type, .hwcDisplayId = *mHwcDisplayId, .deviceProductInfo = {}, - .supportedModes = mCreationArgs.supportedModes, - .activeMode = activeModePtr}; + .supportedModes = modes, + .activeMode = *it}; } state.isSecure = mCreationArgs.isSecure; - mCreationArgs.refreshRateConfigs = - std::make_shared<scheduler::RefreshRateConfigs>(mCreationArgs.supportedModes, - mCreationArgs.activeModeId); - - sp<DisplayDevice> device = new DisplayDevice(mCreationArgs); - if (!device->isVirtual()) { - device->setActiveMode(mCreationArgs.activeModeId); + sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs); + if (!display->isVirtual()) { + display->setActiveMode(activeModeId); } - mFlinger.mutableDisplays().emplace(mDisplayToken, device); + mFlinger.mutableDisplays().emplace(mDisplayToken, display); mFlinger.mutableCurrentState().displays.add(mDisplayToken, state); mFlinger.mutableDrawingState().displays.add(mDisplayToken, state); @@ -789,7 +820,7 @@ public: mFlinger.mutablePhysicalDisplayTokens()[physical->id] = mDisplayToken; } - return device; + return display; } private: @@ -800,16 +831,12 @@ public: }; private: - void scheduleComposite(FrameHint) override {} - void setVsyncEnabled(bool) override {} - void changeRefreshRate(const RefreshRate&, DisplayModeEvent) override {} - void kernelTimerChanged(bool) override {} - void triggerOnFrameRateOverridesChanged() {} - surfaceflinger::test::Factory mFactory; sp<SurfaceFlinger> mFlinger = new SurfaceFlinger(mFactory, SurfaceFlinger::SkipInitialization); + + scheduler::mock::SchedulerCallback mSchedulerCallback; + scheduler::mock::NoOpSchedulerCallback mNoOpSchedulerCallback; scheduler::TestableScheduler* mScheduler = nullptr; - std::shared_ptr<scheduler::RefreshRateConfigs> mRefreshRateConfigs; }; } // namespace android diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h index 849e3083c4..c90b8ed6c7 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h @@ -35,7 +35,7 @@ struct NoOpSchedulerCallback final : ISchedulerCallback { void setVsyncEnabled(bool) override {} void changeRefreshRate(const RefreshRate&, DisplayModeEvent) override {} void kernelTimerChanged(bool) override {} - void triggerOnFrameRateOverridesChanged() {} + void triggerOnFrameRateOverridesChanged() override {} }; } // namespace android::scheduler::mock |