From 0f0ddc1055f2c3c5b976bcb0255937a81313e416 Mon Sep 17 00:00:00 2001 From: Daniel Solomon Date: Mon, 19 Aug 2019 19:31:09 -0700 Subject: SurfaceFlinger: Query Scheduler when updating allowed display configs Currently two entities in SurfaceFlinger can set a new display refresh rate: (1) SurfaceFlinger core, and (2) Scheduler. It's possible for these two entities to get out of sync in the following way: 1) Scheduler updates the refresh rate to some rate 2) Upper layers call into SurfaceFlinger to update allowed display configs 3) SurfaceFlinger always sets display rate to max If the refresh rate from #1 and #3 don't match, it can leave the system in an inconsistent state, potentially causing visual and power issues. This change fixes this problem by changing step #3: Instead of always choosing the max refresh rate, SurfaceFlinger queries the optimal refresh rate from Scheduler. If that rate isn't available, only then does SurfaceFlinger default to the maximum rate. Bug: 139557239 Test: atest libsurfaceflinger_unittest Test: Manual: 1) Start with SurfaceFlinger idling (Scheduler selected RefreshRateType::DEFAULT) 2) Trigger a change in allowed display configs from DisplayModeDirector 3) Make sure the RefreshRateType SurfaceFlinger sets is DEFAULT instead of PERFORMANCE Change-Id: Ia85a60fde55afaed5106462942e0bb77652ec737 --- services/surfaceflinger/Scheduler/Scheduler.cpp | 6 +++++ services/surfaceflinger/Scheduler/Scheduler.h | 3 +++ services/surfaceflinger/SurfaceFlinger.cpp | 30 ++++++++++++++++--------- services/surfaceflinger/SurfaceFlinger.h | 3 +++ 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 8d9357a681..eb52d3fce6 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -512,6 +512,7 @@ Scheduler::RefreshRateType Scheduler::calculateRefreshRateType() { } // Content detection is on, find the appropriate refresh rate with minimal error + // TODO(b/139751853): Scan allowed refresh rates only (SurfaceFlinger::mAllowedDisplayConfigs) const float rate = static_cast(mFeatures.contentRefreshRate); auto iter = min_element(mRefreshRateConfigs.getRefreshRates().cbegin(), mRefreshRateConfigs.getRefreshRates().cend(), @@ -541,6 +542,11 @@ Scheduler::RefreshRateType Scheduler::calculateRefreshRateType() { return currRefreshRateType; } +Scheduler::RefreshRateType Scheduler::getPreferredRefreshRateType() { + std::lock_guard lock(mFeatureStateLock); + return mFeatures.refreshRateType; +} + void Scheduler::changeRefreshRate(RefreshRateType refreshRateType, ConfigEvent configEvent) { std::lock_guard lock(mCallbackLock); if (mChangeRefreshRateCallback) { diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index c15f793b1c..34e527cdc6 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -127,6 +127,9 @@ public: void dump(std::string&) const; void dump(ConnectionHandle, std::string&) const; + // Get the appropriate refresh type for current conditions. + RefreshRateType getPreferredRefreshRateType(); + private: friend class TestableScheduler; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ba734d2937..31299638fc 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5634,6 +5634,25 @@ void SurfaceFlinger::traverseLayersInDisplay(const sp& disp } } +void SurfaceFlinger::setPreferredDisplayConfig() { + const auto& type = mScheduler->getPreferredRefreshRateType(); + const auto& config = mRefreshRateConfigs.getRefreshRate(type); + if (config && isDisplayConfigAllowed(config->configId)) { + ALOGV("switching to Scheduler preferred config %d", config->configId); + setDesiredActiveConfig({type, config->configId, Scheduler::ConfigEvent::Changed}); + } else { + // Set the highest allowed config by iterating backwards on available refresh rates + const auto& refreshRates = mRefreshRateConfigs.getRefreshRates(); + for (auto iter = refreshRates.crbegin(); iter != refreshRates.crend(); ++iter) { + if (iter->second && isDisplayConfigAllowed(iter->second->configId)) { + ALOGV("switching to allowed config %d", iter->second->configId); + setDesiredActiveConfig({iter->first, iter->second->configId, + Scheduler::ConfigEvent::Changed}); + } + } + } +} + void SurfaceFlinger::setAllowedDisplayConfigsInternal(const sp& display, const std::vector& allowedConfigs) { if (!display->isPrimary()) { @@ -5655,16 +5674,7 @@ void SurfaceFlinger::setAllowedDisplayConfigsInternal(const sp& d mScheduler->onConfigChanged(mAppConnectionHandle, display->getId()->value, display->getActiveConfig()); - // Set the highest allowed config by iterating backwards on available refresh rates - const auto& refreshRates = mRefreshRateConfigs.getRefreshRates(); - for (auto iter = refreshRates.crbegin(); iter != refreshRates.crend(); ++iter) { - if (iter->second && isDisplayConfigAllowed(iter->second->configId)) { - ALOGV("switching to config %d", iter->second->configId); - setDesiredActiveConfig( - {iter->first, iter->second->configId, Scheduler::ConfigEvent::Changed}); - break; - } - } + setPreferredDisplayConfig(); } status_t SurfaceFlinger::setAllowedDisplayConfigs(const sp& displayToken, diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index f6df33a999..316170623c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -528,6 +528,9 @@ private: // called on the main thread in response to setPowerMode() void setPowerModeInternal(const sp& display, int mode) REQUIRES(mStateLock); + // Query the Scheduler or allowed display configs list for a matching config, and set it + void setPreferredDisplayConfig() REQUIRES(mStateLock); + // called on the main thread in response to setAllowedDisplayConfigs() void setAllowedDisplayConfigsInternal(const sp& display, const std::vector& allowedConfigs) -- cgit v1.2.3-59-g8ed1b