Handle virtual display power state in SurfaceFlinger

If the state changes while there's no surface, the new power state is lost. Even though there's no DisplayDevice, there's a proper DisplayDeviceState for that virtual display. Storing the power state there and respecting it when creating the display will work.

This is tied to android::companion::virtualdevice::flags::correct_virtual_display_power_state since that flag controls whether or not to set the power mode of the SurfaceControl for the virtual display (which ultimately feeds into SurfaceFlinger::setPowerMode).

With this change, virtual displays whose PowerMode is OFF will no longer be composited since the correct power mode is now set in SurfaceFlinger (instead of assuming all virtual displays are ON).

Bug: 342681202

Flag: android.companion.virtualdevice.flags.correct_virtual_display_power_state

Test: manually tested with flag on/off with Android Auto Projected

Change-Id: Ib10514da05e277a8ed574c92ff71d2a4d52975b2

6 files changed, 59 insertions, 19 deletions

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 07770f12ba..b39654437f 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -180,8 +180,7 @@ auto DisplayDevice::getFrontEndInfo() const -> frontend::DisplayInfo {
 }
 
 void DisplayDevice::setPowerMode(hal::PowerMode mode) {
-    // TODO(b/241285876): Skip this for virtual displays.
-    if (mode == hal::PowerMode::OFF || mode == hal::PowerMode::ON) {
+    if (!isVirtual() && (mode == hal::PowerMode::OFF || mode == hal::PowerMode::ON)) {
         if (mStagedBrightness && mBrightness != mStagedBrightness) {
             getCompositionDisplay()->setNextBrightness(*mStagedBrightness);
             mBrightness = *mStagedBrightness;
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index be374299f7..02522a3deb 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -299,6 +299,8 @@ struct DisplayDeviceState {
     Fps requestedRefreshRate;
     int32_t maxLayerPictureProfiles = 0;
     bool hasPictureProcessing = false;
+    hardware::graphics::composer::hal::PowerMode initialPowerMode{
+            hardware::graphics::composer::hal::PowerMode::OFF};
 
 private:
     static std::atomic<int32_t> sNextSequenceId;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 806740b37f..ae41ed971b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -589,6 +589,8 @@ sp<IBinder> SurfaceFlinger::createVirtualDisplay(
         return nullptr;
     }
 
+    ALOGD("Creating virtual display: %s", displayName.c_str());
+
     class DisplayToken : public BBinder {
         sp<SurfaceFlinger> flinger;
         virtual ~DisplayToken() {
@@ -613,6 +615,8 @@ sp<IBinder> SurfaceFlinger::createVirtualDisplay(
     // TODO (b/314820005): separate as a different arg when creating the display.
     state.isProtected = isSecure;
     state.optimizationPolicy = optimizationPolicy;
+    // Virtual displays start in ON mode.
+    state.initialPowerMode = hal::PowerMode::ON;
     state.displayName = displayName;
     state.uniqueId = uniqueId;
     state.requestedRefreshRate = Fps::fromValue(requestedRefreshRate);
@@ -634,6 +638,9 @@ status_t SurfaceFlinger::destroyVirtualDisplay(const sp<IBinder>& displayToken)
         ALOGE("%s: Invalid operation on physical display", __func__);
         return INVALID_OPERATION;
     }
+
+    ALOGD("Destroying virtual display: %s", state.displayName.c_str());
+
     mCurrentState.displays.removeItemsAt(index);
     setTransactionFlags(eDisplayTransactionNeeded);
     return NO_ERROR;
@@ -3901,7 +3908,12 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
             getPhysicalDisplayOrientation(compositionDisplay->getId(), creationArgs.isPrimary);
     ALOGV("Display Orientation: %s", toCString(creationArgs.physicalOrientation));
 
-    creationArgs.initialPowerMode = state.isVirtual() ? hal::PowerMode::ON : hal::PowerMode::OFF;
+    if (FlagManager::getInstance().correct_virtual_display_power_state()) {
+        creationArgs.initialPowerMode = state.initialPowerMode;
+    } else {
+        creationArgs.initialPowerMode =
+                state.isVirtual() ? hal::PowerMode::ON : hal::PowerMode::OFF;
+    }
 
     creationArgs.requestedRefreshRate = state.requestedRefreshRate;
 
@@ -3975,6 +3987,8 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken,
         // Virtual displays without a surface are dormant:
         // they have external state (layer stack, projection,
         // etc.) but no internal state (i.e. a DisplayDevice).
+        ALOGD("Not adding dormant virtual display with token %p: %s", displayToken.unsafe_get(),
+              state.displayName.c_str());
         return;
     }
 
@@ -5806,16 +5820,32 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
 }
 
 void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) {
-    auto future = mScheduler->schedule([=, this]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(
-                                               kMainThreadContext) {
+    auto future = mScheduler->schedule([=, this]() FTL_FAKE_GUARD(kMainThreadContext) {
         mSkipPowerOnForQuiescent = false;
-        const auto display = getDisplayDeviceLocked(displayToken);
+        const auto display = FTL_FAKE_GUARD(mStateLock, getDisplayDeviceLocked(displayToken));
         if (!display) {
-            ALOGE("Attempt to set power mode %d for invalid display token %p", mode,
-                  displayToken.get());
+            Mutex::Autolock lock(mStateLock);
+            const ssize_t index = mCurrentState.displays.indexOfKey(displayToken);
+            if (index >= 0) {
+                auto& state = mCurrentState.displays.editValueFor(displayToken);
+                if (state.isVirtual()) {
+                    ALOGD("Setting power mode %d for a dormant virtual display with token %p", mode,
+                          displayToken.get());
+                    state.initialPowerMode = static_cast<hal::PowerMode>(mode);
+                    return;
+                }
+            }
+            ALOGE("Failed to set power mode %d for display token %p", mode, displayToken.get());
         } else if (display->isVirtual()) {
-            ALOGW("Attempt to set power mode %d for virtual display", mode);
+            if (FlagManager::getInstance().correct_virtual_display_power_state()) {
+                ALOGD("Setting power mode %d on virtual display %s", mode,
+                      display->getDisplayName().c_str());
+                display->setPowerMode(static_cast<hal::PowerMode>(mode));
+            } else {
+                ALOGW("Attempt to set power mode %d for virtual display", mode);
+            }
         } else {
+            ftl::FakeGuard guard(mStateLock);
             setPowerModeInternal(display, static_cast<hal::PowerMode>(mode));
         }
     });
diff --git a/services/surfaceflinger/common/Android.bp b/services/surfaceflinger/common/Android.bp
index 8786f6e592..13f65770cd 100644
--- a/services/surfaceflinger/common/Android.bp
+++ b/services/surfaceflinger/common/Android.bp
@@ -16,9 +16,9 @@ cc_defaults {
     ],
     shared_libs: [
         "libSurfaceFlingerProp",
-        "server_configurable_flags",
         "libaconfig_storage_read_api_cc",
         "libtracing_perfetto",
+        "server_configurable_flags",
     ],
     static_libs: [
         "librenderengine_includes",
@@ -37,11 +37,12 @@ cc_library_static {
         "libsurfaceflinger_common_defaults",
     ],
     static_libs: [
-        "libsurfaceflingerflags",
         "aconfig_hardware_flags_c_lib",
+        "android.companion.virtualdevice.flags-aconfig-cc",
         "android.os.flags-aconfig-cc",
         "android.server.display.flags-aconfig-cc",
         "libguiflags_no_apex",
+        "libsurfaceflingerflags",
     ],
 }
 
@@ -51,44 +52,47 @@ cc_library_static {
         "libsurfaceflinger_common_defaults",
     ],
     static_libs: [
-        "libsurfaceflingerflags_test",
         "aconfig_hardware_flags_c_lib",
+        "android.companion.virtualdevice.flags-aconfig-cc",
         "android.os.flags-aconfig-cc-test",
         "android.server.display.flags-aconfig-cc",
         "libguiflags_no_apex",
+        "libsurfaceflingerflags_test",
     ],
 }
 
 cc_defaults {
     name: "libsurfaceflinger_common_deps",
     shared_libs: [
-        "server_configurable_flags",
         "libaconfig_storage_read_api_cc",
         "libtracing_perfetto",
+        "server_configurable_flags",
     ],
     static_libs: [
-        "libsurfaceflinger_common",
-        "libsurfaceflingerflags",
         "aconfig_hardware_flags_c_lib",
+        "android.companion.virtualdevice.flags-aconfig-cc",
         "android.os.flags-aconfig-cc",
         "android.server.display.flags-aconfig-cc",
         "libguiflags_no_apex",
+        "libsurfaceflinger_common",
+        "libsurfaceflingerflags",
     ],
 }
 
 cc_defaults {
     name: "libsurfaceflinger_common_test_deps",
     shared_libs: [
-        "server_configurable_flags",
         "libaconfig_storage_read_api_cc",
         "libtracing_perfetto",
+        "server_configurable_flags",
     ],
     static_libs: [
-        "libsurfaceflinger_common_test",
-        "libsurfaceflingerflags_test",
         "aconfig_hardware_flags_c_lib",
+        "android.companion.virtualdevice.flags-aconfig-cc",
         "android.os.flags-aconfig-cc-test",
         "android.server.display.flags-aconfig-cc",
         "libguiflags_no_apex",
+        "libsurfaceflinger_common_test",
+        "libsurfaceflingerflags_test",
     ],
 }
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index abde524214..d894f40398 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -26,8 +26,9 @@
 #include <server_configurable_flags/get_flags.h>
 #include <cinttypes>
 
-#include <android_os.h>
+#include <android_companion_virtualdevice_flags.h>
 #include <android_hardware_flags.h>
+#include <android_os.h>
 #include <com_android_graphics_libgui_flags.h>
 #include <com_android_graphics_surfaceflinger_flags.h>
 #include <com_android_server_display_feature_flags.h>
@@ -125,6 +126,7 @@ void FlagManager::dump(std::string& result) const {
     DUMP_ACONFIG_FLAG(adpf_gpu_sf);
     DUMP_ACONFIG_FLAG(adpf_native_session_manager);
     DUMP_ACONFIG_FLAG(adpf_use_fmq_channel);
+    DUMP_ACONFIG_FLAG(correct_virtual_display_power_state);
     DUMP_ACONFIG_FLAG(graphite_renderengine_preview_rollout);
     DUMP_ACONFIG_FLAG(increase_missed_frame_jank_threshold);
     DUMP_ACONFIG_FLAG(refresh_rate_overlay_on_external_display);
@@ -309,6 +311,8 @@ FLAG_MANAGER_ACONFIG_FLAG(vsync_predictor_recovery, "");
 
 /// Trunk stable server (R/W) flags from outside SurfaceFlinger ///
 FLAG_MANAGER_ACONFIG_FLAG_IMPORTED(adpf_use_fmq_channel, "", android::os)
+FLAG_MANAGER_ACONFIG_FLAG_IMPORTED(correct_virtual_display_power_state, "",
+                                   android::companion::virtualdevice::flags)
 
 /// Trunk stable readonly flags from outside SurfaceFlinger ///
 FLAG_MANAGER_ACONFIG_FLAG_IMPORTED(idle_screen_refresh_rate_timeout, "",
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 6295c5b17f..baa8318d52 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -60,6 +60,7 @@ public:
     bool adpf_native_session_manager() const;
     bool adpf_use_fmq_channel() const;
     bool adpf_use_fmq_channel_fixed() const;
+    bool correct_virtual_display_power_state() const;
     bool graphite_renderengine_preview_rollout() const;
     bool increase_missed_frame_jank_threshold() const;
     bool refresh_rate_overlay_on_external_display() const;