From 4f960d169f0dfbfaba43b0e1033843fcf55fd8a2 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Wed, 13 Oct 2021 16:59:49 -0700 Subject: SF: add HWVsync state to dumpsys Log debugging information about whether HWVsync callbacks where enabled with HWC. Bug: 201605862 Test: adb logcat -s SurfaceFlinger Change-Id: Iaa13a92d37fcfea1328a6e91b17372c3737ffff6 --- services/surfaceflinger/SurfaceFlinger.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 82bd398278..39d4d2a873 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1824,7 +1824,7 @@ void SurfaceFlinger::setVsyncEnabled(bool enabled) { if (const auto display = getDefaultDisplayDeviceLocked(); display && display->isPoweredOn()) { - getHwComposer().setVsyncEnabled(display->getPhysicalId(), mHWCVsyncPendingState); + setHWCVsyncEnabled(display->getPhysicalId(), mHWCVsyncPendingState); } })); } @@ -4585,6 +4585,12 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: return; } + const auto activeDisplay = getDisplayDeviceLocked(mActiveDisplayToken); + if (activeDisplay != display && display->isInternal() && activeDisplay && + activeDisplay->isPoweredOn()) { + ALOGW("Trying to change power mode on non active display while the active display is ON"); + } + display->setPowerMode(mode); if (mInterceptor->isEnabled()) { @@ -4592,7 +4598,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: } const auto vsyncPeriod = display->refreshRateConfigs().getCurrentRefreshRate().getVsyncPeriod(); if (currentMode == hal::PowerMode::OFF) { - const auto activeDisplay = getDisplayDeviceLocked(mActiveDisplayToken); + // Turn on the display if (display->isInternal() && (!activeDisplay || !activeDisplay->isPoweredOn())) { onActiveDisplayChangedLocked(display); } @@ -4606,7 +4612,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: } getHwComposer().setPowerMode(displayId, mode); if (display->isInternal() && mode != hal::PowerMode::DOZE_SUSPEND) { - getHwComposer().setVsyncEnabled(displayId, mHWCVsyncPendingState); + setHWCVsyncEnabled(displayId, mHWCVsyncPendingState); mScheduler->onScreenAcquired(mAppConnectionHandle); mScheduler->resyncToHardwareVsync(true, vsyncPeriod); } @@ -4628,7 +4634,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: } // Make sure HWVsync is disabled before turning off the display - getHwComposer().setVsyncEnabled(displayId, hal::Vsync::DISABLE); + setHWCVsyncEnabled(displayId, hal::Vsync::DISABLE); getHwComposer().setPowerMode(displayId, mode); mVisibleRegionsDirty = true; @@ -4831,6 +4837,8 @@ void SurfaceFlinger::dumpVSync(std::string& result) const { mScheduler->dump(mAppConnectionHandle, result); mScheduler->dumpVsync(result); + StringAppendF(&result, "mHWCVsyncPendingState=%s mLastHWCVsyncState=%s\n", + to_string(mHWCVsyncPendingState).c_str(), to_string(mLastHWCVsyncState).c_str()); } void SurfaceFlinger::dumpPlannerInfo(const DumpArgs& args, std::string& result) const { -- cgit v1.2.3-59-g8ed1b From adc914ccd7dd60754843e1f33c956c0f51430036 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Wed, 13 Oct 2021 17:04:27 -0700 Subject: SF: acquire/release screen for active display only SurfaceFlinger assumes that there is at most a single internal display powered on at a given time, and mark it as the active display. However, in order to be robust against rare race conditions where displays might be on together for a short period of time, we add a check to make sure that we tell the scheduler that the screen was acquired/released only for the active display. Bug: 201605862 Test: SF unit tests Change-Id: I25b3f807d9f5d93ae88ac8a6026cee76cb69f493 --- services/surfaceflinger/SurfaceFlinger.cpp | 10 +++++----- .../unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp | 5 +++++ .../surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 1 + 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 39d4d2a873..29d35c2a8b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4611,7 +4611,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: ALOGW("Couldn't set SCHED_FIFO on display on: %s\n", strerror(errno)); } getHwComposer().setPowerMode(displayId, mode); - if (display->isInternal() && mode != hal::PowerMode::DOZE_SUSPEND) { + if (isDisplayActiveLocked(display) && mode != hal::PowerMode::DOZE_SUSPEND) { setHWCVsyncEnabled(displayId, mHWCVsyncPendingState); mScheduler->onScreenAcquired(mAppConnectionHandle); mScheduler->resyncToHardwareVsync(true, vsyncPeriod); @@ -4628,7 +4628,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 (display->isInternal() && currentMode != hal::PowerMode::DOZE_SUSPEND) { + if (isDisplayActiveLocked(display) && currentMode != hal::PowerMode::DOZE_SUSPEND) { mScheduler->disableHardwareVsync(true); mScheduler->onScreenReleased(mAppConnectionHandle); } @@ -4642,13 +4642,13 @@ 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 (display->isInternal() && currentMode == hal::PowerMode::DOZE_SUSPEND) { + if (isDisplayActiveLocked(display) && currentMode == hal::PowerMode::DOZE_SUSPEND) { mScheduler->onScreenAcquired(mAppConnectionHandle); mScheduler->resyncToHardwareVsync(true, vsyncPeriod); } } else if (mode == hal::PowerMode::DOZE_SUSPEND) { // Leave display going to doze - if (display->isInternal()) { + if (isDisplayActiveLocked(display)) { mScheduler->disableHardwareVsync(true); mScheduler->onScreenReleased(mAppConnectionHandle); } @@ -4658,7 +4658,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: getHwComposer().setPowerMode(displayId, mode); } - if (display->isInternal()) { + if (isDisplayActiveLocked(display)) { mTimeStats->setPowerMode(mode); mRefreshRateStats->setPowerMode(mode); mScheduler->setDisplayPowerState(mode == hal::PowerMode::ON); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp index 65024202b8..eea1002236 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp @@ -260,6 +260,11 @@ struct DisplayPowerCase { auto display = Display::makeFakeExistingDisplayInjector(test); display.inject(); display.mutableDisplayDevice()->setPowerMode(mode); + if (display.mutableDisplayDevice()->isInternal()) { + test->mFlinger.mutableActiveDisplayToken() = + display.mutableDisplayDevice()->getDisplayToken(); + } + return display; } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 40c881e902..a23361e856 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -442,6 +442,7 @@ public: auto& mutableInternalHwcDisplayId() { return getHwComposer().mInternalHwcDisplayId; } auto& mutableExternalHwcDisplayId() { return getHwComposer().mExternalHwcDisplayId; } auto& mutableUseFrameRateApi() { return mFlinger->useFrameRateApi; } + auto& mutableActiveDisplayToken() { return mFlinger->mActiveDisplayToken; } auto fromHandle(const sp& handle) { return mFlinger->fromHandle(handle); -- cgit v1.2.3-59-g8ed1b