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

5 files changed, 71 insertions, 95 deletions

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index de7d455fa4..bad5e2e3b5 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -50,6 +50,17 @@ namespace android {
 
 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 1b14145147..7d7c8adb7b 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -67,17 +67,6 @@ namespace display {
 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 aa933ee8a7..940374b256 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5719,13 +5719,7 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
         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 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
             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 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
                     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 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
             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 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
                 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 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
         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 @@ void SurfaceFlinger::setVirtualDisplayPowerMode(const sp<DisplayDevice>& display
           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;
+}
+
+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);
 
-    const bool optimizeForPerformance =
-            optimizationPolicy == gui::ISurfaceComposer::OptimizationPolicy::optimizeForPerformance;
     // 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 f61214cc65..9cf0c6aaa0 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -733,14 +733,19 @@ private:
     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 2332bf62da..d5c22a9601 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 EventThreadBaseSupportedVariant {
 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 @@ struct EventThreadIsSupportedVariant : public EventThreadBaseSupportedVariant {
     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 @@ struct TransitionOffToDozeSuspendVariant
     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 @@ struct TransitionDozeSuspendToOffVariant
       : 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 TransitionDozeSuspendToOffVariant
 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 TransitionDozeSuspendToDozeVariant
 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 @@ struct TransitionOnToUnknownVariant
       : 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 @@ void SetPhysicalDisplayPowerModeTest::transitionDisplayCommon() {
     // --------------------------------------------------------------------
     // 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);