diff options
7 files changed, 79 insertions, 70 deletions
diff --git a/cmds/bugreportz/readme.md b/cmds/bugreportz/readme.md index eb0d898be1..3606827a4d 100644 --- a/cmds/bugreportz/readme.md +++ b/cmds/bugreportz/readme.md @@ -1,6 +1,6 @@ # bugreportz protocol -`bugreportz` is used to generate a zippped bugreport whose path is passed back to `adb`, using +`bugreportz` is used to generate a zipped bugreport whose path is passed back to `adb`, using the simple protocol defined below. # Version 1.1 diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index b823e06e0e..a25296c86d 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -192,17 +192,16 @@ bool DisplayDevice::isPoweredOn() const { } void DisplayDevice::setActiveMode(DisplayModeId modeId, const display::DisplaySnapshot& snapshot) { - const auto modeOpt = snapshot.displayModes().get(modeId); - LOG_ALWAYS_FATAL_IF(!modeOpt, "Unknown mode"); + const auto fpsOpt = snapshot.displayModes().get(modeId).transform( + [](const DisplayModePtr& mode) { return mode->getFps(); }); - mActiveMode = modeOpt->get(); - const Fps fps = mActiveMode->getFps(); + LOG_ALWAYS_FATAL_IF(!fpsOpt, "Unknown mode"); + const Fps fps = *fpsOpt; ATRACE_INT(mActiveModeFPSTrace.c_str(), fps.getIntValue()); - if (mRefreshRateConfigs) { - mRefreshRateConfigs->setActiveModeId(modeId); - } + mRefreshRateConfigs->setActiveModeId(modeId); + if (mRefreshRateOverlay) { mRefreshRateOverlay->changeRefreshRate(fps); } @@ -224,10 +223,6 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info, constraints, outTimeline); } -const DisplayModePtr& DisplayDevice::getActiveMode() const { - return mActiveMode; -} - nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { const auto physicalId = getPhysicalId(); if (!mHwComposer.isConnected(physicalId)) { @@ -240,7 +235,7 @@ nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { return vsyncPeriod; } - return getActiveMode()->getFps().getPeriodNsecs(); + return refreshRateConfigs().getActiveModePtr()->getVsyncPeriod(); } nsecs_t DisplayDevice::getRefreshTimestamp() const { @@ -451,7 +446,7 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool showSpinnner) { mRefreshRateOverlay = std::make_unique<RefreshRateOverlay>(fpsRange, showSpinnner); mRefreshRateOverlay->setLayerStack(getLayerStack()); mRefreshRateOverlay->setViewport(getSize()); - mRefreshRateOverlay->changeRefreshRate(getActiveMode()->getFps()); + mRefreshRateOverlay->changeRefreshRate(getActiveMode().getFps()); } bool DisplayDevice::onKernelTimerChanged(std::optional<DisplayModeId> desiredModeId, @@ -492,7 +487,7 @@ bool DisplayDevice::setDesiredActiveMode(const ActiveModeInfo& info) { } // Check if we are already at the desired mode - if (getActiveMode()->getId() == info.mode->getId()) { + if (refreshRateConfigs().getActiveModePtr()->getId() == info.mode->getId()) { return false; } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index d79a6b5f3d..0f52aff4f3 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -189,8 +189,6 @@ public: /* ------------------------------------------------------------------------ * Display mode management. */ - const DisplayModePtr& getActiveMode() const; - struct ActiveModeInfo { DisplayModePtr mode; scheduler::DisplayModeEvent event = scheduler::DisplayModeEvent::None; @@ -207,6 +205,10 @@ public: return mUpcomingActiveMode; } + const DisplayMode& getActiveMode() const REQUIRES(kMainThreadContext) { + return mRefreshRateConfigs->getActiveMode(); + } + // Precondition: DisplaySnapshot must contain a mode with DisplayModeId. void setActiveMode(DisplayModeId, const display::DisplaySnapshot&) REQUIRES(kMainThreadContext); @@ -226,7 +228,7 @@ public: } // Enables an overlay to be displayed with the current refresh rate - void enableRefreshRateOverlay(bool enable, bool showSpinner); + void enableRefreshRateOverlay(bool enable, bool showSpinner) REQUIRES(kMainThreadContext); bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; } bool onKernelTimerChanged(std::optional<DisplayModeId>, bool timerExpired); void animateRefreshRateOverlay(); @@ -265,10 +267,10 @@ private: static ui::Transform::RotationFlags sPrimaryDisplayRotationFlags; - // allow initial power mode as null. + // Allow nullopt as initial power mode. std::optional<hardware::graphics::composer::hal::PowerMode> mPowerMode; - DisplayModePtr mActiveMode; - std::optional<float> mStagedBrightness = std::nullopt; + + std::optional<float> mStagedBrightness; float mBrightness = -1.f; std::atomic<nsecs_t> mLastHwVsync = 0; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 9a19f0a6c0..f3551ae080 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -680,7 +680,7 @@ void SurfaceFlinger::bootFinished() { sp<IBinder> input(defaultServiceManager()->getService(String16("inputflinger"))); - static_cast<void>(mScheduler->schedule([=] { + static_cast<void>(mScheduler->schedule([=]() FTL_FAKE_GUARD(kMainThreadContext) { if (input == nullptr) { ALOGE("Failed to link to input service"); } else { @@ -708,8 +708,9 @@ void SurfaceFlinger::bootFinished() { mBootStage = BootStage::FINISHED; - if (property_get_bool("sf.debug.show_refresh_rate_overlay", false)) { - FTL_FAKE_GUARD(mStateLock, enableRefreshRateOverlay(true)); + if (base::GetBoolProperty("sf.debug.show_refresh_rate_overlay"s, false)) { + ftl::FakeGuard guard(mStateLock); + enableRefreshRateOverlay(true); } })); } @@ -1059,7 +1060,7 @@ status_t SurfaceFlinger::getDynamicDisplayInfo(const sp<IBinder>& displayToken, const PhysicalDisplayId displayId = snapshot.displayId(); - info->activeDisplayModeId = display->getActiveMode()->getId().value(); + info->activeDisplayModeId = display->refreshRateConfigs().getActiveModePtr()->getId().value(); info->activeColorMode = display->getCompositionDisplay()->getState().colorMode; info->supportedColorModes = getDisplayColorModes(displayId); info->hdrCapabilities = display->getHdrCapabilities(); @@ -1183,7 +1184,7 @@ void SurfaceFlinger::updateInternalStateWithChangedMode() { return; } - if (display->getActiveMode()->getResolution() != upcomingModeInfo.mode->getResolution()) { + if (display->getActiveMode().getResolution() != upcomingModeInfo.mode->getResolution()) { auto& state = mCurrentState.displays.editValueFor(display->getDisplayToken()); // We need to generate new sequenceId in order to recreate the display (and this // way the framebuffer). @@ -1269,7 +1270,7 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { ALOGV("%s changing active mode to %d(%s) for display %s", __func__, desiredModeId.value(), to_string(*refreshRateOpt).c_str(), to_string(display->getId()).c_str()); - if (display->getActiveMode()->getId() == desiredModeId) { + if (display->getActiveMode().getId() == desiredModeId) { // we are already in the requested mode, there is nothing left to do desiredActiveModeChangeDone(display); continue; @@ -1318,7 +1319,7 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately); const auto desiredActiveMode = display->getDesiredActiveMode(); if (desiredActiveMode && - display->getActiveMode()->getId() == desiredActiveMode->mode->getId()) { + display->getActiveMode().getId() == desiredActiveMode->mode->getId()) { desiredActiveModeChangeDone(display); } } @@ -2107,7 +2108,7 @@ bool SurfaceFlinger::commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expe activeDisplay->getPowerMode() == hal::PowerMode::ON; if (mPowerHintSessionEnabled) { const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get(); - const Period vsyncPeriod = Period::fromNs(display->getActiveMode()->getVsyncPeriod()); + const Period vsyncPeriod = Period::fromNs(display->getActiveMode().getVsyncPeriod()); mPowerAdvisor->setCommitStart(frameTime); mPowerAdvisor->setExpectedPresentTime(mExpectedPresentTime); @@ -3084,7 +3085,7 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, void SurfaceFlinger::updateInternalDisplayVsyncLocked(const sp<DisplayDevice>& activeDisplay) { mVsyncConfiguration->reset(); - const Fps refreshRate = activeDisplay->refreshRateConfigs().getActiveMode().getFps(); + const Fps refreshRate = activeDisplay->getActiveMode().getFps(); updatePhaseConfiguration(refreshRate); mRefreshRateStats->setRefreshRate(refreshRate); } @@ -3394,11 +3395,13 @@ void SurfaceFlinger::triggerOnFrameRateOverridesChanged() { void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { LOG_ALWAYS_FATAL_IF(mScheduler); - const auto currRefreshRate = display->getActiveMode()->getFps(); - mRefreshRateStats = std::make_unique<scheduler::RefreshRateStats>(*mTimeStats, currRefreshRate, - hal::PowerMode::OFF); + const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr(); + const Fps activeRefreshRate = activeModePtr->getFps(); + mRefreshRateStats = + std::make_unique<scheduler::RefreshRateStats>(*mTimeStats, activeRefreshRate, + hal::PowerMode::OFF); - mVsyncConfiguration = getFactory().createVsyncConfiguration(currRefreshRate); + mVsyncConfiguration = getFactory().createVsyncConfiguration(activeRefreshRate); mVsyncModulator = sp<VsyncModulator>::make(mVsyncConfiguration->getCurrentConfigs()); using Feature = scheduler::Feature; @@ -3431,7 +3434,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { mScheduler->startTimers(); const auto configs = mVsyncConfiguration->getCurrentConfigs(); - const nsecs_t vsyncPeriod = currRefreshRate.getPeriodNsecs(); + const nsecs_t vsyncPeriod = activeRefreshRate.getPeriodNsecs(); mAppConnectionHandle = mScheduler->createConnection("app", mFrameTimeline->getTokenManager(), /*workDuration=*/configs.late.appWorkDuration, @@ -3459,7 +3462,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { // This is a bit hacky, but this avoids a back-pointer into the main SF // classes from EventThread, and there should be no run-time binder cost // anyway since there are no connected apps at this point. - mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, display->getActiveMode()); + mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr); } void SurfaceFlinger::updatePhaseConfiguration(const Fps& refreshRate) { @@ -5348,10 +5351,10 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co if (const auto display = getDefaultDisplayDeviceLocked()) { std::string fps, xDpi, yDpi; - if (const auto activeMode = display->getActiveMode()) { - fps = to_string(activeMode->getFps()); + if (const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr()) { + fps = to_string(activeModePtr->getFps()); - const auto dpi = activeMode->getDpi(); + const auto dpi = activeModePtr->getDpi(); xDpi = base::StringPrintf("%.2f", dpi.x); yDpi = base::StringPrintf("%.2f", dpi.y); } else { @@ -5851,19 +5854,17 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r return NO_ERROR; } case 1034: { - auto future = mScheduler->schedule([&] { - switch (n = data.readInt32()) { - case 0: - case 1: - FTL_FAKE_GUARD(mStateLock, - enableRefreshRateOverlay(static_cast<bool>(n))); - break; - default: { - reply->writeBool( - FTL_FAKE_GUARD(mStateLock, isRefreshRateOverlayEnabled())); - } - } - }); + auto future = mScheduler->schedule( + [&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) { + switch (n = data.readInt32()) { + case 0: + case 1: + enableRefreshRateOverlay(static_cast<bool>(n)); + break; + default: + reply->writeBool(isRefreshRateOverlayEnabled()); + } + }); future.wait(); return NO_ERROR; @@ -6804,12 +6805,12 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might // be depending in this callback. - const auto activeMode = display->getActiveMode(); + const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr(); if (isDisplayActiveLocked(display)) { - mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode); + mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr); toggleKernelIdleTimer(); } else { - mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode); + mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr); } auto preferredModeOpt = diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 1f8874df5f..fdaacf074c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1340,7 +1340,7 @@ private: std::unique_ptr<Hwc2::PowerAdvisor> mPowerAdvisor; - void enableRefreshRateOverlay(bool enable) REQUIRES(mStateLock); + void enableRefreshRateOverlay(bool enable) REQUIRES(mStateLock, kMainThreadContext); // Flag used to set override desired display mode from backdoor bool mDebugDisplayModeSetByBackdoor = false; diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 57937dcaea..6b7e3533a5 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -20,6 +20,7 @@ #include "DisplayTransactionTestHelpers.h" +#include <ftl/fake_guard.h> #include <scheduler/Fps.h> namespace android { @@ -111,8 +112,10 @@ void DisplayModeSwitchingTest::setupScheduler( } TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) { + ftl::FakeGuard guard(kMainThreadContext); + ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); mFlinger.onActiveDisplayChanged(mDisplay); @@ -121,7 +124,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value()); ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; @@ -134,7 +137,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe Mock::VerifyAndClearExpectations(mComposer); ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); // Verify that the next commit will complete the mode change and send // a onModeChanged event to the framework. @@ -144,10 +147,12 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe Mock::VerifyAndClearExpectations(mAppEventThread); ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefreshRequired) { + ftl::FakeGuard guard(kMainThreadContext); + ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); mFlinger.onActiveDisplayChanged(mDisplay); @@ -157,7 +162,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefres ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value()); ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. @@ -172,15 +177,17 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefres mFlinger.commit(); ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { + ftl::FakeGuard guard(kMainThreadContext); + // Test that if we call setDesiredDisplayModeSpecs while a previous mode change // is still being processed the later call will be respected. ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); mFlinger.onActiveDisplayChanged(mDisplay); @@ -214,12 +221,14 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { mFlinger.commit(); ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId120); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId120); } TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefreshRequired) { + ftl::FakeGuard guard(kMainThreadContext); + ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); mFlinger.onActiveDisplayChanged(mDisplay); @@ -228,7 +237,7 @@ TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefresh ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value()); ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90_4K); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. @@ -263,7 +272,7 @@ TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefresh mDisplay = mFlinger.getDisplay(displayToken); ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value()); - ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90_4K); + ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90_4K); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp index ec2c2b4358..073c459ea3 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp @@ -17,6 +17,8 @@ #undef LOG_TAG #define LOG_TAG "LibSurfaceFlingerUnittests" +#include <ftl/fake_guard.h> + #include "DisplayHardware/DisplayMode.h" #include "DisplayTransactionTestHelpers.h" @@ -265,7 +267,7 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { // -------------------------------------------------------------------- // Postconditions - ASSERT_TRUE(device != nullptr); + ASSERT_NE(nullptr, device); EXPECT_EQ(Case::Display::DISPLAY_ID::get(), device->getId()); EXPECT_EQ(static_cast<bool>(Case::Display::VIRTUAL), device->isVirtual()); EXPECT_EQ(static_cast<bool>(Case::Display::SECURE), device->isSecure()); @@ -282,8 +284,8 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { device->receivesInput()); if constexpr (Case::Display::CONNECTION_TYPE::value) { - EXPECT_NE(nullptr, device->getActiveMode()); - EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode()->getHwcId()); + ftl::FakeGuard guard(kMainThreadContext); + EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode().getHwcId()); } } |