From dd5c2148e97dcf68577a84d6162618e0e5ce4936 Mon Sep 17 00:00:00 2001 From: ramindani Date: Tue, 14 Jan 2025 09:33:08 -0800 Subject: [SF] Use render rate for the compositorTiming Vsync Period does not represent the actual composition rate in the framework anymore, render rate is the more accurate composition rate. BUG: 383938139 BUG: 355342197 Flag: EXEMPT bug fix Test: atest CtsDeqpTestCases --test-filter '*dEQP-EGL.functional.get_frame_timestamps*' Test: Before fix http://ab/I51300010353712246 Test: After the fix http://ab/I22500010354940353 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:6b6d542c5c002e2247aacdb1aec79868c0c4a809) Merged-In: I33574b3ff347264a19e2e0bcfd2236639aad01a5 Change-Id: I33574b3ff347264a19e2e0bcfd2236639aad01a5 --- services/surfaceflinger/SurfaceFlinger.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d7d567c6c3..896d13d31d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3171,12 +3171,12 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, const auto schedule = mScheduler->getVsyncSchedule(); const TimePoint vsyncDeadline = schedule->vsyncDeadlineAfter(presentTime); - const Period vsyncPeriod = schedule->period(); + const Fps renderRate = pacesetterDisplay->refreshRateSelector().getActiveMode().fps; const nsecs_t vsyncPhase = mScheduler->getVsyncConfiguration().getCurrentConfigs().late.sfOffset; - const CompositorTiming compositorTiming(vsyncDeadline.ns(), vsyncPeriod.ns(), vsyncPhase, - presentLatency.ns()); + const CompositorTiming compositorTiming(vsyncDeadline.ns(), renderRate.getPeriodNsecs(), + vsyncPhase, presentLatency.ns()); ui::DisplayMap layerStackToDisplay; { -- cgit v1.2.3-59-g8ed1b From e0ab7b97975c18603ec4cd92806932393bd749e1 Mon Sep 17 00:00:00 2001 From: Vladimir Komsiyski Date: Fri, 7 Mar 2025 01:10:15 -0800 Subject: 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 --- services/surfaceflinger/DisplayDevice.cpp | 3 +- services/surfaceflinger/DisplayDevice.h | 2 + services/surfaceflinger/SurfaceFlinger.cpp | 44 ++++++++++++++++++---- services/surfaceflinger/common/Android.bp | 22 ++++++----- services/surfaceflinger/common/FlagManager.cpp | 6 ++- .../common/include/common/FlagManager.h | 1 + 6 files changed, 59 insertions(+), 19 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') 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 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 SurfaceFlinger::createVirtualDisplay( return nullptr; } + ALOGD("Creating virtual display: %s", displayName.c_str()); + class DisplayToken : public BBinder { sp flinger; virtual ~DisplayToken() { @@ -613,6 +615,8 @@ sp 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& 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 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& 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& display, hal: } void SurfaceFlinger::setPowerMode(const sp& 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(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(mode)); + } else { + ALOGW("Attempt to set power mode %d for virtual display", mode); + } } else { + ftl::FakeGuard guard(mStateLock); setPowerModeInternal(display, static_cast(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 #include -#include +#include #include +#include #include #include #include @@ -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; -- cgit v1.2.3-59-g8ed1b