Revert "Turn off synthetic VSYNC when adjusting thread scheduling for performance"
This reverts commit bac7071aa3b1ae72f90b904b9af61b5c74ba9265.
Reason for revert: Droidmonitor created revert due to Jank regression b/404073995.
Change-Id: Id57f5cda7a34f4598a82d86d7290d5b1c57f1315
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index de7d455..bad5e2e 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -50,6 +50,17 @@
namespace hal = hardware::graphics::composer::hal;
+namespace gui {
+inline std::string_view to_string(ISurfaceComposer::OptimizationPolicy optimizationPolicy) {
+ switch (optimizationPolicy) {
+ case ISurfaceComposer::OptimizationPolicy::optimizeForPower:
+ return "optimizeForPower";
+ case ISurfaceComposer::OptimizationPolicy::optimizeForPerformance:
+ return "optimizeForPerformance";
+ }
+}
+} // namespace gui
+
DisplayDeviceCreationArgs::DisplayDeviceCreationArgs(
const sp<SurfaceFlinger>& flinger, HWComposer& hwComposer, const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay)
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 1b14145..7d7c8ad 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -67,17 +67,6 @@
class DisplaySnapshot;
} // namespace display
-namespace gui {
-inline const char* to_string(ISurfaceComposer::OptimizationPolicy optimizationPolicy) {
- switch (optimizationPolicy) {
- case ISurfaceComposer::OptimizationPolicy::optimizeForPower:
- return "optimizeForPower";
- case ISurfaceComposer::OptimizationPolicy::optimizeForPerformance:
- return "optimizeForPerformance";
- }
-}
-} // namespace gui
-
class DisplayDevice : public RefBase {
public:
constexpr static float sDefaultMinLumiance = 0.0;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index aa933ee..940374b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5719,13 +5719,7 @@
incRefreshableDisplays();
}
- if (displayId == mActiveDisplayId &&
- FlagManager::getInstance().correct_virtual_display_power_state()) {
- applyOptimizationPolicy(__func__);
- }
-
const auto activeMode = display->refreshRateSelector().getActiveMode().modePtr;
- using OptimizationPolicy = gui::ISurfaceComposer::OptimizationPolicy;
if (currentMode == hal::PowerMode::OFF) {
// Turn on the display
@@ -5740,10 +5734,12 @@
onActiveDisplayChangedLocked(activeDisplay.get(), *display);
}
- if (displayId == mActiveDisplayId &&
- !FlagManager::getInstance().correct_virtual_display_power_state()) {
- optimizeThreadScheduling("setPhysicalDisplayPowerMode(ON/DOZE)",
- OptimizationPolicy::optimizeForPerformance);
+ if (displayId == mActiveDisplayId) {
+ if (FlagManager::getInstance().correct_virtual_display_power_state()) {
+ applyOptimizationPolicy("setPhysicalDisplayPowerMode(ON)");
+ } else {
+ disablePowerOptimizations("setPhysicalDisplayPowerMode(ON)");
+ }
}
getHwComposer().setPowerMode(displayId, mode);
@@ -5752,8 +5748,7 @@
mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState();
requestHardwareVsync(displayId, enable);
- if (displayId == mActiveDisplayId &&
- !FlagManager::getInstance().correct_virtual_display_power_state()) {
+ if (displayId == mActiveDisplayId) {
mScheduler->enableSyntheticVsync(false);
}
@@ -5770,13 +5765,13 @@
if (const auto display = getActivatableDisplay()) {
onActiveDisplayChangedLocked(activeDisplay.get(), *display);
} else {
- if (!FlagManager::getInstance().correct_virtual_display_power_state()) {
- optimizeThreadScheduling("setPhysicalDisplayPowerMode(OFF)",
- OptimizationPolicy::optimizeForPower);
+ if (FlagManager::getInstance().correct_virtual_display_power_state()) {
+ applyOptimizationPolicy("setPhysicalDisplayPowerMode(OFF)");
+ } else {
+ enablePowerOptimizations("setPhysicalDisplayPowerMode(OFF)");
}
- if (currentModeNotDozeSuspend &&
- !FlagManager::getInstance().correct_virtual_display_power_state()) {
+ if (currentModeNotDozeSuspend) {
mScheduler->enableSyntheticVsync();
}
}
@@ -5804,9 +5799,7 @@
ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");
mVisibleRegionsDirty = true;
scheduleRepaint();
- if (!FlagManager::getInstance().correct_virtual_display_power_state()) {
- mScheduler->enableSyntheticVsync(false);
- }
+ mScheduler->enableSyntheticVsync(false);
}
constexpr bool kAllowToEnable = true;
mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, activeMode.get());
@@ -5816,8 +5809,7 @@
constexpr bool kDisallow = true;
mScheduler->disableHardwareVsync(displayId, kDisallow);
- if (displayId == mActiveDisplayId &&
- !FlagManager::getInstance().correct_virtual_display_power_state()) {
+ if (displayId == mActiveDisplayId) {
mScheduler->enableSyntheticVsync();
}
getHwComposer().setPowerMode(displayId, mode);
@@ -5856,44 +5848,43 @@
to_string(displayId).c_str());
}
-void SurfaceFlinger::optimizeThreadScheduling(
- const char* whence, gui::ISurfaceComposer::OptimizationPolicy optimizationPolicy) {
- ALOGD("%s: Optimizing thread scheduling: %s", whence, to_string(optimizationPolicy));
+bool SurfaceFlinger::shouldOptimizeForPerformance() {
+ for (const auto& [_, display] : mDisplays) {
+ // Displays that are optimized for power are always powered on and should not influence
+ // whether there is an active display for the purpose of power optimization, etc. If these
+ // displays are being shown somewhere, a different (physical or virtual) display that is
+ // optimized for performance will be powered on in addition. Displays optimized for
+ // performance will change power mode, so if they are off then they are not active.
+ if (display->isPoweredOn() &&
+ display->getOptimizationPolicy() ==
+ gui::ISurfaceComposer::OptimizationPolicy::optimizeForPerformance) {
+ return true;
+ }
+ }
+ return false;
+}
- const bool optimizeForPerformance =
- optimizationPolicy == gui::ISurfaceComposer::OptimizationPolicy::optimizeForPerformance;
+void SurfaceFlinger::enablePowerOptimizations(const char* whence) {
+ ALOGD("%s: Enabling power optimizations", whence);
+
+ setSchedAttr(false, whence);
+ setSchedFifo(false, whence);
+}
+
+void SurfaceFlinger::disablePowerOptimizations(const char* whence) {
+ ALOGD("%s: Disabling power optimizations", whence);
+
// TODO: b/281692563 - Merge the syscalls. For now, keep uclamp in a separate syscall
// and set it before SCHED_FIFO due to b/190237315.
- setSchedAttr(optimizeForPerformance, whence);
- setSchedFifo(optimizeForPerformance, whence);
+ setSchedAttr(true, whence);
+ setSchedFifo(true, whence);
}
void SurfaceFlinger::applyOptimizationPolicy(const char* whence) {
- using OptimizationPolicy = gui::ISurfaceComposer::OptimizationPolicy;
-
- const bool optimizeForPerformance =
- std::any_of(mDisplays.begin(), mDisplays.end(), [](const auto& pair) {
- const auto& display = pair.second;
- return display->isPoweredOn() &&
- display->getOptimizationPolicy() ==
- OptimizationPolicy::optimizeForPerformance;
- });
-
- optimizeThreadScheduling(whence,
- optimizeForPerformance ? OptimizationPolicy::optimizeForPerformance
- : OptimizationPolicy::optimizeForPower);
-
- if (mScheduler) {
- const bool disableSyntheticVsync =
- std::any_of(mDisplays.begin(), mDisplays.end(), [](const auto& pair) {
- const auto& display = pair.second;
- const hal::PowerMode powerMode = display->getPowerMode();
- return powerMode != hal::PowerMode::OFF &&
- powerMode != hal::PowerMode::DOZE_SUSPEND &&
- display->getOptimizationPolicy() ==
- OptimizationPolicy::optimizeForPerformance;
- });
- mScheduler->enableSyntheticVsync(!disableSyntheticVsync);
+ if (shouldOptimizeForPerformance()) {
+ disablePowerOptimizations(whence);
+ } else {
+ enablePowerOptimizations(whence);
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index f61214c..9cf0c6a 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -733,14 +733,19 @@
void setVirtualDisplayPowerMode(const sp<DisplayDevice>& display, hal::PowerMode mode)
REQUIRES(mStateLock, kMainThreadContext);
- // Adjusts thread scheduling according to the optimization policy
- static void optimizeThreadScheduling(
- const char* whence, gui::ISurfaceComposer::OptimizationPolicy optimizationPolicy);
+ // Returns whether to optimize globally for performance instead of power.
+ bool shouldOptimizeForPerformance() REQUIRES(mStateLock);
+
+ // Turns on power optimizations, for example when there are no displays to be optimized for
+ // performance.
+ static void enablePowerOptimizations(const char* whence);
+
+ // Turns off power optimizations.
+ static void disablePowerOptimizations(const char* whence);
// Enables or disables power optimizations depending on whether there are displays that should
// be optimized for performance.
- void applyOptimizationPolicy(const char* whence) REQUIRES(kMainThreadContext)
- REQUIRES(mStateLock);
+ void applyOptimizationPolicy(const char* whence) REQUIRES(mStateLock);
// Returns the preferred mode for PhysicalDisplayId if the Scheduler has selected one for that
// display. Falls back to the display's defaultModeId otherwise.
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
index 2332bf6..d5c22a9 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp
@@ -17,7 +17,6 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
-#include <android_companion_virtualdevice_flags.h>
#include <com_android_graphics_surfaceflinger_flags.h>
#include <common/test/FlagUtils.h>
#include "DisplayTransactionTestHelpers.h"
@@ -79,19 +78,11 @@
struct EventThreadNotSupportedVariant : public EventThreadBaseSupportedVariant {
static void setupEnableVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(test->mFlinger.scheduler()->mockRequestHardwareVsync, Call(_, true)).Times(1);
- setupDisableSyntheticVsyncCallExpectations(test);
- }
-
- static void setupDisableSyntheticVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(*test->mEventThread, enableSyntheticVsync(_)).Times(0);
}
static void setupDisableVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(test->mFlinger.scheduler()->mockRequestHardwareVsync, Call(_, false)).Times(1);
- setupEnableSyntheticVsyncCallExpectations(test);
- }
-
- static void setupEnableSyntheticVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(*test->mEventThread, enableSyntheticVsync(_)).Times(0);
}
};
@@ -100,20 +91,12 @@
static void setupEnableVsyncCallExpectations(DisplayTransactionTest* test) {
// Expect to enable hardware VSYNC and disable synthetic VSYNC.
EXPECT_CALL(test->mFlinger.scheduler()->mockRequestHardwareVsync, Call(_, true)).Times(1);
- setupDisableSyntheticVsyncCallExpectations(test);
- }
-
- static void setupDisableSyntheticVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(*test->mEventThread, enableSyntheticVsync(false)).Times(1);
}
static void setupDisableVsyncCallExpectations(DisplayTransactionTest* test) {
// Expect to disable hardware VSYNC and enable synthetic VSYNC.
EXPECT_CALL(test->mFlinger.scheduler()->mockRequestHardwareVsync, Call(_, false)).Times(1);
- setupEnableSyntheticVsyncCallExpectations(test);
- }
-
- static void setupEnableSyntheticVsyncCallExpectations(DisplayTransactionTest* test) {
EXPECT_CALL(*test->mEventThread, enableSyntheticVsync(true)).Times(1);
}
};
@@ -168,7 +151,7 @@
template <typename Case>
static void setupCallExpectations(DisplayTransactionTest* test) {
Case::setupComposerCallExpectations(test, Case::Doze::ACTUAL_POWER_MODE_FOR_DOZE_SUSPEND);
- Case::EventThread::setupEnableSyntheticVsyncCallExpectations(test);
+ Case::EventThread::setupVsyncNoCallExpectations(test);
Case::setupRepaintEverythingCallExpectations(test);
}
@@ -193,7 +176,7 @@
: public TransitionVariantCommon<PowerMode::DOZE_SUSPEND, PowerMode::OFF> {
template <typename Case>
static void setupCallExpectations(DisplayTransactionTest* test) {
- Case::EventThread::setupEnableSyntheticVsyncCallExpectations(test);
+ Case::EventThread::setupVsyncNoCallExpectations(test);
Case::setupComposerCallExpectations(test, IComposerClient::PowerMode::OFF);
}
@@ -205,7 +188,7 @@
struct TransitionOnToDozeVariant : public TransitionVariantCommon<PowerMode::ON, PowerMode::DOZE> {
template <typename Case>
static void setupCallExpectations(DisplayTransactionTest* test) {
- Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
+ Case::EventThread::setupVsyncNoCallExpectations(test);
Case::setupComposerCallExpectations(test, Case::Doze::ACTUAL_POWER_MODE_FOR_DOZE);
}
};
@@ -223,7 +206,7 @@
struct TransitionDozeToOnVariant : public TransitionVariantCommon<PowerMode::DOZE, PowerMode::ON> {
template <typename Case>
static void setupCallExpectations(DisplayTransactionTest* test) {
- Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
+ Case::EventThread::setupVsyncNoCallExpectations(test);
Case::setupComposerCallExpectations(test, IComposerClient::PowerMode::ON);
}
};
@@ -251,7 +234,7 @@
: public TransitionVariantCommon<PowerMode::ON, static_cast<PowerMode>(POWER_MODE_LEET)> {
template <typename Case>
static void setupCallExpectations(DisplayTransactionTest* test) {
- Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
+ Case::EventThread::setupVsyncNoCallExpectations(test);
Case::setupNoComposerPowerModeCallExpectations(test);
}
};
@@ -352,9 +335,6 @@
// --------------------------------------------------------------------
// Preconditions
- SET_FLAG_FOR_TEST(android::companion::virtualdevice::flags::correct_virtual_display_power_state,
- true);
-
Case::Doze::setupComposerCallExpectations(this);
auto display =
Case::injectDisplayWithInitialPowerMode(this, Case::Transition::INITIAL_POWER_MODE);