From a92a56868ea8c5cd1866ca1f6c057f31c5b0b9d9 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Tue, 7 Jun 2022 23:35:38 +0000 Subject: SurfaceFlinger: add logging when changing refresh rate policy Add a log message when refresh rate policy is changed to help debug issues when SF changes refresh rate unexpectedly. Bug: 230590870 Test: change policy over adb and observe logs Change-Id: I31cdc3ad47ea25c0f700ebad897e7efbc986bd24 --- services/surfaceflinger/DisplayDevice.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'services/surfaceflinger/DisplayDevice.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index a915b615d9..86809007b4 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -211,6 +211,7 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info, to_string(getId()).c_str()); return BAD_VALUE; } + mNumModeSwitchesInPolicy++; mUpcomingActiveMode = info; ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), info.mode->getFps().getIntValue()); return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), info.mode->getHwcId(), @@ -537,6 +538,27 @@ void DisplayDevice::clearDesiredActiveModeState() { mDesiredActiveModeChanged = false; } +status_t DisplayDevice::setRefreshRatePolicy( + const std::optional& policy, bool overridePolicy) { + const auto oldPolicy = mRefreshRateConfigs->getCurrentPolicy(); + const status_t setPolicyResult = overridePolicy + ? mRefreshRateConfigs->setOverridePolicy(policy) + : mRefreshRateConfigs->setDisplayManagerPolicy(*policy); + + if (setPolicyResult == OK) { + const int numModeChanges = mNumModeSwitchesInPolicy.exchange(0); + + ALOGI("Display %s policy changed\n" + "Previous: {%s}\n" + "Current: {%s}\n" + "%d mode changes were performed under the previous policy", + to_string(getId()).c_str(), oldPolicy.toString().c_str(), + policy ? policy->toString().c_str() : "null", numModeChanges); + } + + return setPolicyResult; +} + std::atomic DisplayDeviceState::sNextSequenceId(1); } // namespace android -- cgit v1.2.3-59-g8ed1b From e80593238c3af1dea4885ca0e32d44392f3d42ee Mon Sep 17 00:00:00 2001 From: linpeter Date: Wed, 13 Jul 2022 19:20:48 +0800 Subject: Allow first power mode as off state Bug: 237745199 test: atest libsurfaceflinger_unittest:OnInitializeDisplaysTest atest libsurfaceflinger_unittest:SetPowerModeInternalTest Merged-In: I3a2eae4efc4a5c6113700a9ca9e9b261e364a878 Change-Id: I3a2eae4efc4a5c6113700a9ca9e9b261e364a878 --- services/surfaceflinger/DisplayDevice.cpp | 12 +++++++----- services/surfaceflinger/DisplayDevice.h | 9 ++++----- services/surfaceflinger/SurfaceFlinger.cpp | 13 +++++++------ .../surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 1 + 4 files changed, 19 insertions(+), 16 deletions(-) (limited to 'services/surfaceflinger/DisplayDevice.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 86809007b4..f3b6254b52 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -104,7 +104,7 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) mCompositionDisplay->getRenderSurface()->initialize(); - setPowerMode(args.initialPowerMode); + if (args.initialPowerMode.has_value()) setPowerMode(args.initialPowerMode.value()); // initialize the display orientation transform. setProjection(ui::ROTATION_0, Rect::INVALID_RECT, Rect::INVALID_RECT); @@ -174,19 +174,21 @@ auto DisplayDevice::getInputInfo() const -> InputInfo { void DisplayDevice::setPowerMode(hal::PowerMode mode) { mPowerMode = mode; - getCompositionDisplay()->setCompositionEnabled(mPowerMode != hal::PowerMode::OFF); + + getCompositionDisplay()->setCompositionEnabled(mPowerMode.has_value() && + *mPowerMode != hal::PowerMode::OFF); } void DisplayDevice::enableLayerCaching(bool enable) { getCompositionDisplay()->setLayerCachingEnabled(enable); } -hal::PowerMode DisplayDevice::getPowerMode() const { +std::optional DisplayDevice::getPowerMode() const { return mPowerMode; } bool DisplayDevice::isPoweredOn() const { - return mPowerMode != hal::PowerMode::OFF; + return mPowerMode && *mPowerMode != hal::PowerMode::OFF; } void DisplayDevice::setActiveMode(DisplayModeId id) { @@ -373,7 +375,7 @@ void DisplayDevice::dump(std::string& result) const { } result += "\n powerMode="s; - result += to_string(mPowerMode); + result += mPowerMode.has_value() ? to_string(mPowerMode.value()) : "OFF(reset)"; result += '\n'; if (mRefreshRateConfigs) { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 2161436d44..fc24a9ce49 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -181,7 +181,7 @@ public: /* ------------------------------------------------------------------------ * Display power mode management. */ - hardware::graphics::composer::hal::PowerMode getPowerMode() const; + std::optional getPowerMode() const; void setPowerMode(hardware::graphics::composer::hal::PowerMode mode); bool isPoweredOn() const; @@ -281,8 +281,8 @@ private: static ui::Transform::RotationFlags sPrimaryDisplayRotationFlags; - hardware::graphics::composer::hal::PowerMode mPowerMode = - hardware::graphics::composer::hal::PowerMode::OFF; + // allow initial power mode as null. + std::optional mPowerMode; DisplayModePtr mActiveMode; std::optional mStagedBrightness = std::nullopt; float mBrightness = -1.f; @@ -366,8 +366,7 @@ struct DisplayDeviceCreationArgs { HdrCapabilities hdrCapabilities; int32_t supportedPerFrameMetadata{0}; std::unordered_map> hwcColorModes; - hardware::graphics::composer::hal::PowerMode initialPowerMode{ - hardware::graphics::composer::hal::PowerMode::ON}; + std::optional initialPowerMode; bool isPrimary{false}; DisplayModes supportedModes; DisplayModeId activeModeId; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index fd824dc502..db2447c40b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2889,7 +2889,8 @@ sp SurfaceFlinger::setupNewDisplayDeviceInternal( ALOGV("Display Orientation: %s", toCString(creationArgs.physicalOrientation)); // virtual displays are always considered enabled - creationArgs.initialPowerMode = state.isVirtual() ? hal::PowerMode::ON : hal::PowerMode::OFF; + creationArgs.initialPowerMode = + state.isVirtual() ? std::make_optional(hal::PowerMode::ON) : std::nullopt; sp display = getFactory().createDisplayDevice(creationArgs); @@ -4870,8 +4871,8 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: const auto displayId = display->getPhysicalId(); ALOGD("Setting power mode %d on display %s", mode, to_string(displayId).c_str()); - const hal::PowerMode currentMode = display->getPowerMode(); - if (mode == currentMode) { + std::optional currentMode = display->getPowerMode(); + if (currentMode.has_value() && mode == *currentMode) { return; } @@ -4887,7 +4888,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: mInterceptor->savePowerModeUpdate(display->getSequenceId(), static_cast(mode)); } const auto refreshRate = display->refreshRateConfigs().getActiveMode()->getFps(); - if (currentMode == hal::PowerMode::OFF) { + if (*currentMode == hal::PowerMode::OFF) { // Turn on the display if (display->isInternal() && (!activeDisplay || !activeDisplay->isPoweredOn())) { onActiveDisplayChangedLocked(display); @@ -4918,7 +4919,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: if (SurfaceFlinger::setSchedAttr(false) != NO_ERROR) { ALOGW("Couldn't set uclamp.min on display off: %s\n", strerror(errno)); } - if (isDisplayActiveLocked(display) && currentMode != hal::PowerMode::DOZE_SUSPEND) { + if (isDisplayActiveLocked(display) && *currentMode != hal::PowerMode::DOZE_SUSPEND) { mScheduler->disableHardwareVsync(true); mScheduler->onScreenReleased(mAppConnectionHandle); } @@ -4932,7 +4933,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: } else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) { // Update display while dozing getHwComposer().setPowerMode(displayId, mode); - if (isDisplayActiveLocked(display) && currentMode == hal::PowerMode::DOZE_SUSPEND) { + if (isDisplayActiveLocked(display) && *currentMode == hal::PowerMode::DOZE_SUSPEND) { mScheduler->onScreenAcquired(mAppConnectionHandle); mScheduler->resyncToHardwareVsync(true, refreshRate); } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index b70fdcd93e..283f9ca77b 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -745,6 +745,7 @@ public: mHwcDisplayId(hwcDisplayId) { mCreationArgs.connectionType = connectionType; mCreationArgs.isPrimary = isPrimary; + mCreationArgs.initialPowerMode = hal::PowerMode::ON; } sp token() const { return mDisplayToken; } -- cgit v1.2.3-59-g8ed1b From e72ba5e27de8677448a1666c1fb122e5f8c1685f Mon Sep 17 00:00:00 2001 From: joenchen Date: Wed, 24 Aug 2022 13:08:58 +0000 Subject: CE: flush the staged brightness to HWC before power off After SurfaceFlinger receives the brightness commands, the commands are sent to HWC until the next frame presents. However, no following frames are present after the power mode is set to off, and HWC cannot receive the last brightness commands. This change forces SurfaceFlinger to flush the staged brightness to HWC prior to execution of power off. Bug: 239908529 Test: suspend and resume repeatedly Change-Id: Ia8fb04399e757de16e06e6516d86bdfcfabd5a2e --- .../include/compositionengine/Display.h | 3 +++ .../include/compositionengine/impl/Display.h | 1 + .../include/compositionengine/mock/Display.h | 1 + .../CompositionEngine/src/Display.cpp | 25 +++++++++++++--------- services/surfaceflinger/DisplayDevice.cpp | 9 ++++++++ 5 files changed, 29 insertions(+), 10 deletions(-) (limited to 'services/surfaceflinger/DisplayDevice.cpp') diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h index 16cb41b024..5e84be1841 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Display.h @@ -56,6 +56,9 @@ public: // similar requests if needed. virtual void createClientCompositionCache(uint32_t cacheSize) = 0; + // Sends the brightness setting to HWC + virtual void applyDisplayBrightness(const bool applyImmediately) = 0; + protected: ~Display() = default; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h index fa7bc5da7f..33a10a36a7 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h @@ -72,6 +72,7 @@ public: const compositionengine::DisplayColorProfileCreationArgs&) override; void createRenderSurface(const compositionengine::RenderSurfaceCreationArgs&) override; void createClientCompositionCache(uint32_t cacheSize) override; + void applyDisplayBrightness(const bool applyImmediately) override; // Internal helpers used by chooseCompositionStrategy() using ChangedTypes = android::HWComposer::DeviceRequestedChanges::ChangedTypes; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Display.h index 72e6f3bdbb..7e99ec2f5a 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Display.h @@ -41,6 +41,7 @@ public: MOCK_METHOD1(createDisplayColorProfile, void(const DisplayColorProfileCreationArgs&)); MOCK_METHOD1(createRenderSurface, void(const RenderSurfaceCreationArgs&)); MOCK_METHOD1(createClientCompositionCache, void(uint32_t)); + MOCK_METHOD1(applyDisplayBrightness, void(const bool)); MOCK_METHOD1(setPredictCompositionStrategy, void(bool)); }; diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index ea856e4859..1ec6449ed0 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -203,23 +203,16 @@ void Display::setReleasedLayers(const compositionengine::CompositionRefreshArgs& setReleasedLayers(std::move(releasedLayers)); } -void Display::beginFrame() { - Output::beginFrame(); - - // If we don't have a HWC display, then we are done. - const auto halDisplayId = HalDisplayId::tryCast(mId); - if (!halDisplayId) { - return; - } - +void Display::applyDisplayBrightness(const bool applyImmediately) { auto& hwc = getCompositionEngine().getHwComposer(); + const auto halDisplayId = HalDisplayId::tryCast(*getDisplayId()); if (const auto physicalDisplayId = PhysicalDisplayId::tryCast(*halDisplayId); physicalDisplayId && getState().displayBrightness) { const status_t result = hwc.setDisplayBrightness(*physicalDisplayId, *getState().displayBrightness, getState().displayBrightnessNits, Hwc2::Composer::DisplayBrightnessOptions{ - .applyImmediately = false}) + .applyImmediately = applyImmediately}) .get(); ALOGE_IF(result != NO_ERROR, "setDisplayBrightness failed for %s: %d, (%s)", getName().c_str(), result, strerror(-result)); @@ -228,6 +221,18 @@ void Display::beginFrame() { editState().displayBrightness.reset(); } +void Display::beginFrame() { + Output::beginFrame(); + + // If we don't have a HWC display, then we are done. + const auto halDisplayId = HalDisplayId::tryCast(mId); + if (!halDisplayId) { + return; + } + + applyDisplayBrightness(false); +} + bool Display::chooseCompositionStrategy( std::optional* outChanges) { ATRACE_CALL(); diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 86809007b4..3bc3ae54b3 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -173,6 +173,15 @@ auto DisplayDevice::getInputInfo() const -> InputInfo { } void DisplayDevice::setPowerMode(hal::PowerMode mode) { + if (mode == hal::PowerMode::OFF || mode == hal::PowerMode::ON) { + if (mStagedBrightness && mBrightness != *mStagedBrightness) { + getCompositionDisplay()->setNextBrightness(*mStagedBrightness); + mBrightness = *mStagedBrightness; + } + mStagedBrightness = std::nullopt; + getCompositionDisplay()->applyDisplayBrightness(true); + } + mPowerMode = mode; getCompositionDisplay()->setCompositionEnabled(mPowerMode != hal::PowerMode::OFF); } -- cgit v1.2.3-59-g8ed1b From 4292810fbe008693390696ae0b9e91e9ffded231 Mon Sep 17 00:00:00 2001 From: joenchen Date: Tue, 6 Sep 2022 18:03:57 +0000 Subject: Set mBrightness when needsComposite is zero When needsComposite is zero in persistBrightness(), the mBrightness needs to be updated to avoid unexpected skipping. Bug: 244268674 Test: adb shell cmd device_state state 0 or 1 repeatly Change-Id: Iccf925c1407e4be489da2dba521b93eee1b1c4b5 --- services/surfaceflinger/DisplayDevice.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'services/surfaceflinger/DisplayDevice.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 86809007b4..269a1fa17b 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -325,8 +325,10 @@ void DisplayDevice::stageBrightness(float brightness) { } void DisplayDevice::persistBrightness(bool needsComposite) { - if (needsComposite && mStagedBrightness && mBrightness != *mStagedBrightness) { - getCompositionDisplay()->setNextBrightness(*mStagedBrightness); + if (mStagedBrightness && mBrightness != *mStagedBrightness) { + if (needsComposite) { + getCompositionDisplay()->setNextBrightness(*mStagedBrightness); + } mBrightness = *mStagedBrightness; } mStagedBrightness = std::nullopt; -- cgit v1.2.3-59-g8ed1b