Merge "Turn off synthetic VSYNC when adjusting thread scheduling for performance" into main

Author: Treehugger Robot

services/surfaceflinger/DisplayDevice.cpp

@@ -50,17 +50,6 @@ 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)

services/surfaceflinger/DisplayDevice.h

@@ -67,6 +67,17 @@ 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;

services/surfaceflinger/SurfaceFlinger.cpp

@@ -5679,7 +5679,15 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
         incRefreshableDisplays();
     }
 
+    const bool shouldApplyOptimizationPolicy =
+            FlagManager::getInstance().disable_synthetic_vsync_for_performance() &&
+            FlagManager::getInstance().correct_virtual_display_power_state();
+    if (isInternalDisplay && shouldApplyOptimizationPolicy) {
+        applyOptimizationPolicy(__func__);
+    }
+
     const auto activeMode = display->refreshRateSelector().getActiveMode().modePtr;
+    using OptimizationPolicy = gui::ISurfaceComposer::OptimizationPolicy;
     if (currentMode == hal::PowerMode::OFF) {
         // Turn on the display
 
@@ -5694,12 +5702,9 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
             onActiveDisplayChangedLocked(activeDisplay.get(), *display);
         }
 
-        if (displayId == mActiveDisplayId) {
-            if (FlagManager::getInstance().correct_virtual_display_power_state()) {
-                applyOptimizationPolicy("setPhysicalDisplayPowerMode(ON)");
-            } else {
-                disablePowerOptimizations("setPhysicalDisplayPowerMode(ON)");
-            }
+        if (displayId == mActiveDisplayId && !shouldApplyOptimizationPolicy) {
+            optimizeThreadScheduling("setPhysicalDisplayPowerMode(ON/DOZE)",
+                                     OptimizationPolicy::optimizeForPerformance);
         }
 
         getHwComposer().setPowerMode(displayId, mode);
@@ -5708,7 +5713,7 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
                     mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState();
             requestHardwareVsync(displayId, enable);
 
-            if (displayId == mActiveDisplayId) {
+            if (displayId == mActiveDisplayId && !shouldApplyOptimizationPolicy) {
                 mScheduler->enableSyntheticVsync(false);
             }
 
@@ -5725,13 +5730,12 @@ 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()) {
-                    applyOptimizationPolicy("setPhysicalDisplayPowerMode(OFF)");
-                } else {
-                    enablePowerOptimizations("setPhysicalDisplayPowerMode(OFF)");
+                if (!shouldApplyOptimizationPolicy) {
+                    optimizeThreadScheduling("setPhysicalDisplayPowerMode(OFF)",
+                                             OptimizationPolicy::optimizeForPower);
                 }
 
-                if (currentModeNotDozeSuspend) {
+                if (currentModeNotDozeSuspend && !shouldApplyOptimizationPolicy) {
                     mScheduler->enableSyntheticVsync();
                 }
             }
@@ -5759,7 +5763,9 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
                 ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON.");
                 mVisibleRegionsDirty = true;
                 scheduleRepaint();
-                mScheduler->enableSyntheticVsync(false);
+                if (!shouldApplyOptimizationPolicy) {
+                    mScheduler->enableSyntheticVsync(false);
+                }
             }
             constexpr bool kAllowToEnable = true;
             mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, activeMode.get());
@@ -5769,7 +5775,7 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa
         constexpr bool kDisallow = true;
         mScheduler->disableHardwareVsync(displayId, kDisallow);
 
-        if (displayId == mActiveDisplayId) {
+        if (displayId == mActiveDisplayId && !shouldApplyOptimizationPolicy) {
             mScheduler->enableSyntheticVsync();
         }
         getHwComposer().setPowerMode(displayId, mode);
@@ -5808,43 +5814,44 @@ void SurfaceFlinger::setVirtualDisplayPowerMode(const sp<DisplayDevice>& display
           to_string(displayId).c_str());
 }
 
-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);
+void SurfaceFlinger::optimizeThreadScheduling(
+        const char* whence, gui::ISurfaceComposer::OptimizationPolicy optimizationPolicy) {
+    ALOGD("%s: Optimizing thread scheduling: %s", whence, to_string(optimizationPolicy));
 
+    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(true, whence);
-    setSchedFifo(true, whence);
+    setSchedAttr(optimizeForPerformance, whence);
+    setSchedFifo(optimizeForPerformance, whence);
 }
 
 void SurfaceFlinger::applyOptimizationPolicy(const char* whence) {
-    if (shouldOptimizeForPerformance()) {
-        disablePowerOptimizations(whence);
-    } else {
-        enablePowerOptimizations(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);
     }
 }
 

services/surfaceflinger/SurfaceFlinger.h

@@ -733,19 +733,14 @@ class SurfaceFlinger : public BnSurfaceComposer,
     void setVirtualDisplayPowerMode(const sp<DisplayDevice>& display, hal::PowerMode mode)
             REQUIRES(mStateLock, kMainThreadContext);
 
-    // 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);
+    // Adjusts thread scheduling according to the optimization policy
+    static void optimizeThreadScheduling(
+            const char* whence, gui::ISurfaceComposer::OptimizationPolicy optimizationPolicy);
 
     // Enables or disables power optimizations depending on whether there are displays that should
     // be optimized for performance.
-    void applyOptimizationPolicy(const char* whence) REQUIRES(mStateLock);
+    void applyOptimizationPolicy(const char* whence) REQUIRES(kMainThreadContext)
+            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.

services/surfaceflinger/common/FlagManager.cpp

@@ -147,6 +147,7 @@ void FlagManager::dump(std::string& result) const {
     DUMP_ACONFIG_FLAG(deprecate_frame_tracker);
     DUMP_ACONFIG_FLAG(deprecate_vsync_sf);
     DUMP_ACONFIG_FLAG(detached_mirror);
+    DUMP_ACONFIG_FLAG(disable_synthetic_vsync_for_performance);
     DUMP_ACONFIG_FLAG(display_config_error_hal);
     DUMP_ACONFIG_FLAG(display_protected);
     DUMP_ACONFIG_FLAG(dont_skip_on_early_ro);
@@ -284,6 +285,7 @@ FLAG_MANAGER_ACONFIG_FLAG(latch_unsignaled_with_auto_refresh_changed, "");
 FLAG_MANAGER_ACONFIG_FLAG(deprecate_vsync_sf, "");
 FLAG_MANAGER_ACONFIG_FLAG(allow_n_vsyncs_in_targeter, "");
 FLAG_MANAGER_ACONFIG_FLAG(detached_mirror, "");
+FLAG_MANAGER_ACONFIG_FLAG(disable_synthetic_vsync_for_performance, "");
 FLAG_MANAGER_ACONFIG_FLAG(commit_not_composited, "");
 FLAG_MANAGER_ACONFIG_FLAG(correct_dpi_with_display_size, "");
 FLAG_MANAGER_ACONFIG_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_screenshots");

services/surfaceflinger/common/include/common/FlagManager.h

@@ -81,6 +81,7 @@ class FlagManager {
     bool deprecate_frame_tracker() const;
     bool deprecate_vsync_sf() const;
     bool detached_mirror() const;
+    bool disable_synthetic_vsync_for_performance() const;
     bool display_config_error_hal() const;
     bool display_protected() const;
     bool dont_skip_on_early_ro() const;

services/surfaceflinger/surfaceflinger_flags_new.aconfig

@@ -133,6 +133,16 @@ flag {
   }
 } # detached_mirror
 
+flag {
+  name: "disable_synthetic_vsync_for_performance"
+  namespace: "core_graphics"
+  description: "Turn off synthetic VSYNC when adjusting thread scheduling for performance"
+  bug: "406147398"
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+} # disable_synthetic_vsync_for_performance
+
 flag {
   name: "display_config_error_hal"
   namespace: "core_graphics"

services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp

@@ -17,6 +17,7 @@
 #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"
@@ -78,11 +79,19 @@ 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);
     }
 };
@@ -91,12 +100,20 @@ 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);
     }
 };
@@ -151,7 +168,7 @@ struct TransitionOffToDozeSuspendVariant
     template <typename Case>
     static void setupCallExpectations(DisplayTransactionTest* test) {
         Case::setupComposerCallExpectations(test, Case::Doze::ACTUAL_POWER_MODE_FOR_DOZE_SUSPEND);
-        Case::EventThread::setupVsyncNoCallExpectations(test);
+        Case::EventThread::setupEnableSyntheticVsyncCallExpectations(test);
         Case::setupRepaintEverythingCallExpectations(test);
     }
 
@@ -176,7 +193,7 @@ struct TransitionDozeSuspendToOffVariant
       : public TransitionVariantCommon<PowerMode::DOZE_SUSPEND, PowerMode::OFF> {
     template <typename Case>
     static void setupCallExpectations(DisplayTransactionTest* test) {
-        Case::EventThread::setupVsyncNoCallExpectations(test);
+        Case::EventThread::setupEnableSyntheticVsyncCallExpectations(test);
         Case::setupComposerCallExpectations(test, IComposerClient::PowerMode::OFF);
     }
 
@@ -188,7 +205,7 @@ struct TransitionDozeSuspendToOffVariant
 struct TransitionOnToDozeVariant : public TransitionVariantCommon<PowerMode::ON, PowerMode::DOZE> {
     template <typename Case>
     static void setupCallExpectations(DisplayTransactionTest* test) {
-        Case::EventThread::setupVsyncNoCallExpectations(test);
+        Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
         Case::setupComposerCallExpectations(test, Case::Doze::ACTUAL_POWER_MODE_FOR_DOZE);
     }
 };
@@ -206,7 +223,7 @@ struct TransitionDozeSuspendToDozeVariant
 struct TransitionDozeToOnVariant : public TransitionVariantCommon<PowerMode::DOZE, PowerMode::ON> {
     template <typename Case>
     static void setupCallExpectations(DisplayTransactionTest* test) {
-        Case::EventThread::setupVsyncNoCallExpectations(test);
+        Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
         Case::setupComposerCallExpectations(test, IComposerClient::PowerMode::ON);
     }
 };
@@ -234,7 +251,7 @@ struct TransitionOnToUnknownVariant
       : public TransitionVariantCommon<PowerMode::ON, static_cast<PowerMode>(POWER_MODE_LEET)> {
     template <typename Case>
     static void setupCallExpectations(DisplayTransactionTest* test) {
-        Case::EventThread::setupVsyncNoCallExpectations(test);
+        Case::EventThread::setupDisableSyntheticVsyncCallExpectations(test);
         Case::setupNoComposerPowerModeCallExpectations(test);
     }
 };
@@ -335,6 +352,10 @@ void SetPhysicalDisplayPowerModeTest::transitionDisplayCommon() {
     // --------------------------------------------------------------------
     // Preconditions
 
+    SET_FLAG_FOR_TEST(android::companion::virtualdevice::flags::correct_virtual_display_power_state,
+                      true);
+    SET_FLAG_FOR_TEST(flags::disable_synthetic_vsync_for_performance, true);
+
     Case::Doze::setupComposerCallExpectations(this);
     auto display =
             Case::injectDisplayWithInitialPowerMode(this, Case::Transition::INITIAL_POWER_MODE);