diff options
35 files changed, 525 insertions, 99 deletions
diff --git a/libs/binder/rust/rpcbinder/src/server/trusty.rs b/libs/binder/rust/rpcbinder/src/server/trusty.rs index fe45decf25..54d82d5bd0 100644 --- a/libs/binder/rust/rpcbinder/src/server/trusty.rs +++ b/libs/binder/rust/rpcbinder/src/server/trusty.rs @@ -106,6 +106,10 @@ pub struct RpcServerConnection { ctx: *mut c_void, } +// SAFETY: The opaque handle: `ctx` points into a dynamic allocation, +// and not tied to anything specific to the current thread. +unsafe impl Send for RpcServerConnection {} + impl Drop for RpcServerConnection { fn drop(&mut self) { // We do not need to close handle_fd since we do not own it. diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 6a8a69843a..771c65bf92 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -1160,6 +1160,12 @@ macro_rules! declare_binder_enum { pub const fn enum_values() -> [Self; $size] { [$(Self::$name),*] } + + #[inline(always)] + #[allow(missing_docs)] + pub const fn get(&self) -> $backing { + self.0 + } } impl std::fmt::Debug for $enum { diff --git a/libs/gui/libgui_flags.aconfig b/libs/gui/libgui_flags.aconfig index a893b841cc..ce1bc9512c 100644 --- a/libs/gui/libgui_flags.aconfig +++ b/libs/gui/libgui_flags.aconfig @@ -130,9 +130,6 @@ flag { description: "Remove BufferQueueProducer::dequeue's wait on this fence (or the fence entirely) to prevent deadlocks" bug: "339705065" is_fixed_read_only: true - metadata { - purpose: PURPOSE_BUGFIX - } } # bq_gl_fence_cleanup flag { diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index cf05fd4ba5..adf8fce472 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -222,8 +222,8 @@ public: ASSERT_EQ(InputEventType::MOTION, ev->getType()); MotionEvent* mev = static_cast<MotionEvent*>(ev); EXPECT_EQ(AMOTION_EVENT_ACTION_DOWN, mev->getAction()); - EXPECT_EQ(x, mev->getX(0)); - EXPECT_EQ(y, mev->getY(0)); + EXPECT_NEAR(x, mev->getX(0), EPSILON); + EXPECT_NEAR(y, mev->getY(0), EPSILON); EXPECT_EQ(flags, mev->getFlags() & flags); ev = consumeEvent(); @@ -241,8 +241,8 @@ public: MotionEvent* mev = static_cast<MotionEvent*>(ev); EXPECT_EQ(AMOTION_EVENT_ACTION_DOWN, mev->getAction()); const PointerCoords& coords = *mev->getRawPointerCoords(0 /*pointerIndex*/); - EXPECT_EQ(displayX, coords.getX()); - EXPECT_EQ(displayY, coords.getY()); + EXPECT_NEAR(displayX, coords.getX(), EPSILON); + EXPECT_NEAR(displayY, coords.getY(), EPSILON); EXPECT_EQ(0, mev->getFlags() & VERIFIED_MOTION_EVENT_FLAGS); ev = consumeEvent(); diff --git a/libs/renderengine/Android.bp b/libs/renderengine/Android.bp index 7f207f0670..f9b84fa948 100644 --- a/libs/renderengine/Android.bp +++ b/libs/renderengine/Android.bp @@ -25,6 +25,7 @@ cc_defaults { defaults: [ "android.hardware.graphics.composer3-ndk_shared", "renderengine_defaults", + "libsurfaceflinger_common_deps", ], cflags: [ "-DGL_GLEXT_PROTOTYPES", @@ -117,7 +118,10 @@ filegroup { // possible if libskia_renderengine is just pulled into librenderengine via whole_static_libs. cc_defaults { name: "librenderengine_deps", - defaults: ["skia_renderengine_deps"], + defaults: [ + "skia_renderengine_deps", + "libsurfaceflinger_common_deps", + ], static_libs: ["libskia_renderengine"], } diff --git a/libs/renderengine/benchmark/Android.bp b/libs/renderengine/benchmark/Android.bp index f84db0b04c..2d18ddb0aa 100644 --- a/libs/renderengine/benchmark/Android.bp +++ b/libs/renderengine/benchmark/Android.bp @@ -28,6 +28,7 @@ cc_benchmark { "android.hardware.graphics.composer3-ndk_shared", "librenderengine_deps", "surfaceflinger_defaults", + "libsurfaceflinger_common_deps", ], srcs: [ "main.cpp", @@ -38,7 +39,6 @@ cc_benchmark { static_libs: [ "librenderengine", "libshaders", - "libsurfaceflinger_common", "libtonemap", ], cflags: [ diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 25afc7b465..9e1c226371 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -1236,6 +1236,16 @@ void SkiaRenderEngine::drawLayersInternal( LOG_ALWAYS_FATAL_IF(activeSurface != dstSurface); auto drawFence = sp<Fence>::make(flushAndSubmit(context, dstSurface)); trace(drawFence); + FenceTimePtr fenceTime = FenceTime::makeValid(drawFence); + for (const auto& layer : layers) { + if (FlagManager::getInstance().monitor_buffer_fences()) { + if (layer.source.buffer.buffer) { + layer.source.buffer.buffer->getBuffer() + ->getDependencyMonitor() + .addAccessCompletion(fenceTime, "RE"); + } + } + } resultPromise->set_value(std::move(drawFence)); } diff --git a/libs/renderengine/skia/filters/LutShader.cpp b/libs/renderengine/skia/filters/LutShader.cpp index f262158f2c..6a577ff41f 100644 --- a/libs/renderengine/skia/filters/LutShader.cpp +++ b/libs/renderengine/skia/filters/LutShader.cpp @@ -24,7 +24,6 @@ #include <ui/ColorSpace.h> #include "include/core/SkColorSpace.h" -#include "src/core/SkColorFilterPriv.h" using aidl::android::hardware::graphics::composer3::LutProperties; @@ -116,7 +115,7 @@ static const SkString kShader = SkString(R"( linear = mix(c0, c1, linear.b); } } - return float4(linear, rgba.a); + return float4(fromLinearSrgb(linear), rgba.a); })"); // same as shader::toColorSpace function @@ -289,9 +288,7 @@ sk_sp<SkShader> LutShader::lutShader(sk_sp<SkShader>& input, lutProperties[i].samplingKey, srcDataspace); } - auto colorXformLutToDst = - SkColorFilterPriv::MakeColorSpaceXform(lutMathColorSpace, outColorSpace); - input = input->makeWithColorFilter(colorXformLutToDst); + input = input->makeWithWorkingColorSpace(outColorSpace); } return input; } diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp index 87e213e394..10cb992835 100644 --- a/libs/ui/Android.bp +++ b/libs/ui/Android.bp @@ -122,6 +122,7 @@ cc_library_shared { srcs: [ "DebugUtils.cpp", + "DependencyMonitor.cpp", "DeviceProductInfo.cpp", "DisplayIdentification.cpp", "DynamicDisplayInfo.cpp", diff --git a/libs/ui/DependencyMonitor.cpp b/libs/ui/DependencyMonitor.cpp new file mode 100644 index 0000000000..b7e490eba4 --- /dev/null +++ b/libs/ui/DependencyMonitor.cpp @@ -0,0 +1,144 @@ +/* + * Copyright 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// #define LOG_NDEBUG 0 +#undef LOG_TAG +#define LOG_TAG "DependencyMonitor" + +#include <ui/DependencyMonitor.h> +#include <ui/Fence.h> +#include <utils/Timers.h> + +#include <inttypes.h> + +namespace android { + +void DependencyMonitor::addIngress(FenceTimePtr fence, std::string annotation) { + std::lock_guard lock(mMutex); + resolveLocked(); + if (mDependencies.isFull() && !mDependencies.front().updateSignalTimes(true)) { + ALOGD("%s: Clobbering unresolved dependencies -- make me bigger!", mToken.c_str()); + } + + auto& entry = mDependencies.next(); + entry.reset(mToken.c_str()); + ALOGV("%" PRId64 "/%s: addIngress at CPU time %" PRId64 " (%s)", mDependencies.back().id, + mToken.c_str(), systemTime(), annotation.c_str()); + + mDependencies.back().ingress = {std::move(fence), std::move(annotation)}; +} + +void DependencyMonitor::addAccessCompletion(FenceTimePtr fence, std::string annotation) { + std::lock_guard lock(mMutex); + if (mDependencies.size() == 0) { + return; + } + ALOGV("%" PRId64 "/%s: addAccessCompletion at CPU time %" PRId64 " (%s)", + mDependencies.back().id, mToken.c_str(), systemTime(), annotation.c_str()); + mDependencies.back().accessCompletions.emplace_back(std::move(fence), std::move(annotation)); +} + +void DependencyMonitor::addEgress(FenceTimePtr fence, std::string annotation) { + std::lock_guard lock(mMutex); + if (mDependencies.size() == 0) { + return; + } + ALOGV("%" PRId64 "/%s: addEgress at CPU time %" PRId64 " (%s)", mDependencies.back().id, + mToken.c_str(), systemTime(), annotation.c_str()); + mDependencies.back().egress = {std::move(fence), std::move(annotation)}; +} + +void DependencyMonitor::resolveLocked() { + if (mDependencies.size() == 0) { + return; + } + + for (size_t i = mDependencies.size(); i > 0; i--) { + auto& dependencyBlock = mDependencies[i - 1]; + + if (dependencyBlock.validated) { + continue; + } + + if (!dependencyBlock.updateSignalTimes(false)) { + break; + } + + dependencyBlock.validated = true; + dependencyBlock.checkUnsafeAccess(); + } +} + +bool DependencyMonitor::DependencyBlock::updateSignalTimes(bool excludeIngress) { + if (egress.fence->getSignalTime() == Fence::SIGNAL_TIME_PENDING) { + return false; + } + + if (!excludeIngress && ingress.fence->getSignalTime() == Fence::SIGNAL_TIME_PENDING) { + return false; + } + + for (auto& accessCompletion : accessCompletions) { + if (accessCompletion.fence->getSignalTime() == Fence::SIGNAL_TIME_PENDING) { + return false; + } + } + + return true; +} + +void DependencyMonitor::DependencyBlock::checkUnsafeAccess() const { + const nsecs_t egressTime = egress.fence->getCachedSignalTime(); + const nsecs_t ingressTime = ingress.fence->getCachedSignalTime(); + + ALOGV_IF(egressTime != Fence::SIGNAL_TIME_INVALID, + "%" PRId64 "/%s: Egress time: %" PRId64 " (%s)", token, id, egressTime, + egress.annotation.c_str()); + ALOGV_IF(Fence::isValidTimestamp(egressTime) && Fence::isValidTimestamp(ingressTime) && + egressTime < ingressTime, + "%" PRId64 "/%s: Detected egress before ingress!: %" PRId64 " (%s) < %" PRId64 " (%s)", + id, token, egressTime, egress.annotation, ingressTime, ingress.annotation.c_str()); + + for (auto& accessCompletion : accessCompletions) { + const nsecs_t accessCompletionTime = accessCompletion.fence->getCachedSignalTime(); + if (!Fence::isValidTimestamp(accessCompletionTime)) { + ALOGI("%" PRId64 "/%s: Detected invalid access completion! <%s>", id, token, + accessCompletion.annotation.c_str()); + continue; + } else { + ALOGV("%" PRId64 "/%s: Access completion time: %" PRId64 " <%s>", id, token, + accessCompletionTime, accessCompletion.annotation.c_str()); + } + + ALOGI_IF(Fence::isValidTimestamp(egressTime) && accessCompletionTime > egressTime, + "%" PRId64 "/%s: Detected access completion after egress!: %" PRId64 + " (%s) > %" PRId64 " (%s)", + id, token, accessCompletionTime, accessCompletion.annotation.c_str(), egressTime, + egress.annotation.c_str()); + + ALOGI_IF(Fence::isValidTimestamp(ingressTime) && accessCompletionTime < ingressTime, + "%" PRId64 "/%s: Detected access completion prior to ingress!: %" PRId64 + " (%s) < %" PRId64 " (%s)", + id, token, accessCompletionTime, accessCompletion.annotation.c_str(), ingressTime, + ingress.annotation.c_str()); + } + + ALOGV_IF(ingressTime != Fence::SIGNAL_TIME_INVALID, + "%" PRId64 "/%s: Ingress time: %" PRId64 " (%s)", id, token, ingressTime, + ingress.annotation.c_str()); +} + +} // namespace android
\ No newline at end of file diff --git a/libs/ui/FenceTime.cpp b/libs/ui/FenceTime.cpp index 4246c40f64..81afe9ef0e 100644 --- a/libs/ui/FenceTime.cpp +++ b/libs/ui/FenceTime.cpp @@ -59,6 +59,14 @@ FenceTime::FenceTime(nsecs_t signalTime) } } +FenceTimePtr FenceTime::makeValid(const sp<Fence>& fence) { + if (fence && fence->isValid()) { + return std::make_shared<FenceTime>(fence); + } else { + return std::make_shared<FenceTime>(systemTime()); + } +} + void FenceTime::applyTrustedSnapshot(const Snapshot& src) { if (CC_UNLIKELY(src.state != Snapshot::State::SIGNAL_TIME)) { // Applying Snapshot::State::FENCE, could change the valid state of the @@ -289,9 +297,10 @@ status_t FenceTime::Snapshot::unflatten( // ============================================================================ void FenceTimeline::push(const std::shared_ptr<FenceTime>& fence) { std::lock_guard<std::mutex> lock(mMutex); - while (mQueue.size() >= MAX_ENTRIES) { + static constexpr size_t MAX_QUEUE_SIZE = 64; + while (mQueue.size() >= MAX_QUEUE_SIZE) { // This is a sanity check to make sure the queue doesn't grow unbounded. - // MAX_ENTRIES should be big enough not to trigger this path. + // MAX_QUEUE_SIZE should be big enough not to trigger this path. // In case this path is taken though, users of FenceTime must make sure // not to rely solely on FenceTimeline to get the final timestamp and // should eventually call Fence::getSignalTime on their own. diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 18c9a6bc48..f7c94005f1 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -27,6 +27,8 @@ #include <ui/GraphicBufferMapper.h> #include <utils/Trace.h> +#include <string> + namespace android { // =========================================================================== @@ -104,6 +106,7 @@ GraphicBuffer::GraphicBuffer() usage = 0; layerCount = 0; handle = nullptr; + mDependencyMonitor.setToken(std::to_string(mId)); } // deprecated @@ -155,6 +158,8 @@ GraphicBuffer::GraphicBuffer(const GraphicBufferAllocator::AllocationRequest& re layerCount = request.layerCount; usage = request.usage; usage_deprecated = int(usage); + std::string name = request.requestorName; + mDependencyMonitor.setToken(name.append(":").append(std::to_string(mId))); } } @@ -252,6 +257,7 @@ status_t GraphicBuffer::initWithSize(uint32_t inWidth, uint32_t inHeight, usage = inUsage; usage_deprecated = int(usage); stride = static_cast<int>(outStride); + mDependencyMonitor.setToken(requestorName.append(":").append(std::to_string(mId))); } return err; } @@ -609,6 +615,14 @@ status_t GraphicBuffer::unflatten(void const*& buffer, size_t& size, int const*& mBufferMapper.getTransportSize(handle, &mTransportNumFds, &mTransportNumInts); } + std::string name; + status_t err = mBufferMapper.getName(handle, &name); + if (err != NO_ERROR) { + name = "<Unknown>"; + } + + mDependencyMonitor.setToken(name.append(":").append(std::to_string(mId))); + buffer = static_cast<void const*>(static_cast<uint8_t const*>(buffer) + sizeNeeded); size -= sizeNeeded; fds += numFds; diff --git a/libs/ui/include/ui/DependencyMonitor.h b/libs/ui/include/ui/DependencyMonitor.h new file mode 100644 index 0000000000..5ad1fd9528 --- /dev/null +++ b/libs/ui/include/ui/DependencyMonitor.h @@ -0,0 +1,104 @@ +/* + * Copyright 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <ui/FatVector.h> +#include <ui/FenceTime.h> +#include <ui/RingBuffer.h> + +namespace android { + +// Debugging class for that tries to add userspace logging for fence depencencies. +// The model that a DependencyMonitor tries to follow is, for each access of some resource: +// 1. There is a single ingress fence, that guards whether a resource is now safe to read from +// another system. +// 2. There are multiple access fences, that are fired when a resource is read. +// 3. There is a single egress fence, that is fired when a resource is released and sent to another +// system. +// +// Note that there can be repeated ingress and egress of a resource, but the assumption is that +// there is exactly one egress for every ingress, unless the resource is destroyed rather than +// released. +// +// The DependencyMonitor will log if there is an anomaly in the fences tracked for some resource. +// This includes: +// * If (2) happens before (1) +// * If (2) happens after (3) +// +// Note that this class has no knowledge of the "other system". I.e., if the other system ignores +// the fence reported in (3), but still takes a long time to write to the resource and produce (1), +// then nothing will be logged. That other system must have its own DependencyMonitor. Conversely, +// this class has imperfect knowledge of the system it is monitoring. For example, this class does +// not know the precise start times of reading from a resource, the exact time that a read might +// occur from a hardware unit is not known to userspace. +// +// In other words, this class logs specific classes of fence violations, but is not sensitive to +// *all* violations. One property of this is that unless the system tracked by a DependencyMonitor +// is feeding in literally incorrect fences, then there is no chance of a false positive. +// +// This class is thread safe. +class DependencyMonitor { +public: + // Sets a debug token identifying the resource this monitor is tracking. + void setToken(std::string token) { mToken = std::move(token); } + + // Adds a fence that is fired when the resource ready to be ingested by the system using the + // DependencyMonitor. + void addIngress(FenceTimePtr fence, std::string annotation); + // Adds a fence that is fired when the resource is accessed. + void addAccessCompletion(FenceTimePtr fence, std::string annotation); + // Adds a fence that is fired when the resource is released to another system. + void addEgress(FenceTimePtr fence, std::string annotation); + +private: + struct AnnotatedFenceTime { + FenceTimePtr fence; + std::string annotation; + }; + + struct DependencyBlock { + int64_t id = -1; + AnnotatedFenceTime ingress = {FenceTime::NO_FENCE, ""}; + FatVector<AnnotatedFenceTime> accessCompletions; + AnnotatedFenceTime egress = {FenceTime::NO_FENCE, ""}; + bool validated = false; + const char* token = nullptr; + + void reset(const char* newToken) { + static std::atomic<int64_t> counter = 0; + id = counter++; + ingress = {FenceTime::NO_FENCE, ""}; + accessCompletions.clear(); + egress = {FenceTime::NO_FENCE, ""}; + validated = false; + token = newToken; + } + + // Returns true if all fences in this block have valid signal times. + bool updateSignalTimes(bool excludeIngress); + + void checkUnsafeAccess() const; + }; + + void resolveLocked() REQUIRES(mMutex); + + std::string mToken; + std::mutex mMutex; + ui::RingBuffer<DependencyBlock, 10> mDependencies GUARDED_BY(mMutex); +}; + +} // namespace android
\ No newline at end of file diff --git a/libs/ui/include/ui/FenceTime.h b/libs/ui/include/ui/FenceTime.h index 334106f0cf..3560d57cff 100644 --- a/libs/ui/include/ui/FenceTime.h +++ b/libs/ui/include/ui/FenceTime.h @@ -17,6 +17,7 @@ #ifndef ANDROID_FENCE_TIME_H #define ANDROID_FENCE_TIME_H +#include <stddef.h> #include <ui/Fence.h> #include <utils/Flattenable.h> #include <utils/Mutex.h> @@ -30,6 +31,8 @@ namespace android { class FenceToFenceTimeMap; +class FenceTime; +using FenceTimePtr = std::shared_ptr<FenceTime>; // A wrapper around fence that only implements isValid and getSignalTime. // It automatically closes the fence in a thread-safe manner once the signal @@ -95,6 +98,10 @@ public: FenceTime& operator=(const FenceTime&) = delete; FenceTime& operator=(FenceTime&&) = delete; + // Constructs a FenceTime, falling back to a timestamp if the fence is + // invalid. + static FenceTimePtr makeValid(const sp<Fence>& fence); + // This method should only be called when replacing the fence with // a signalTime. Since this is an indirect way of setting the signal time // of a fence, the snapshot should come from a trusted source. @@ -142,8 +149,6 @@ private: std::atomic<nsecs_t> mSignalTime{Fence::SIGNAL_TIME_INVALID}; }; -using FenceTimePtr = std::shared_ptr<FenceTime>; - // A queue of FenceTimes that are expected to signal in FIFO order. // Only maintains a queue of weak pointers so it doesn't keep references // to Fences on its own. @@ -162,8 +167,6 @@ using FenceTimePtr = std::shared_ptr<FenceTime>; // different threads. class FenceTimeline { public: - static constexpr size_t MAX_ENTRIES = 64; - void push(const std::shared_ptr<FenceTime>& fence); void updateSignalTimes(); diff --git a/libs/ui/include/ui/GraphicBuffer.h b/libs/ui/include/ui/GraphicBuffer.h index 936bf8f862..9305180ecb 100644 --- a/libs/ui/include/ui/GraphicBuffer.h +++ b/libs/ui/include/ui/GraphicBuffer.h @@ -23,6 +23,7 @@ #include <string> #include <utility> #include <vector> +#include "ui/DependencyMonitor.h" #include <android/hardware_buffer.h> #include <ui/ANativeObjectBase.h> @@ -229,6 +230,8 @@ public: void addDeathCallback(GraphicBufferDeathCallback deathCallback, void* context); + DependencyMonitor& getDependencyMonitor() { return mDependencyMonitor; } + private: ~GraphicBuffer(); @@ -295,6 +298,8 @@ private: // and informs SurfaceFlinger that it should drop its strong pointer reference to the buffer. std::vector<std::pair<GraphicBufferDeathCallback, void* /*mDeathCallbackContext*/>> mDeathCallbacks; + + DependencyMonitor mDependencyMonitor; }; } // namespace android diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 888fcfb592..5491ab78b4 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -7049,11 +7049,6 @@ void InputDispatcher::onWindowInfosChanged(const gui::WindowInfosUpdate& update) setInputWindowsLocked(handles, displayId); } - if (update.vsyncId < mWindowInfosVsyncId) { - ALOGE("Received out of order window infos update. Last update vsync id: %" PRId64 - ", current update vsync id: %" PRId64, - mWindowInfosVsyncId, update.vsyncId); - } mWindowInfosVsyncId = update.vsyncId; } // Wake up poll loop since it may need to make new input dispatching choices. diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h index 780758e2a3..e2ea0f1397 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h @@ -167,6 +167,8 @@ public: // Checks if the buffer's release fence has been set virtual LayerFE::ReleaseFencePromiseStatus getReleaseFencePromiseStatus() = 0; + virtual void setReleasedBuffer(sp<GraphicBuffer> buffer) = 0; + // Indicates that the picture profile request was applied to this layer. virtual void onPictureProfileCommitted() = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h index d2a5a2066c..f65a9083c5 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h @@ -52,6 +52,7 @@ public: MOCK_METHOD0(createReleaseFenceFuture, ftl::Future<FenceResult>()); MOCK_METHOD1(setReleaseFence, void(const FenceResult&)); + MOCK_METHOD1(setReleasedBuffer, void(sp<GraphicBuffer>)); MOCK_METHOD0(getReleaseFencePromiseStatus, LayerFE::ReleaseFencePromiseStatus()); MOCK_CONST_METHOD0(getDebugName, const char*()); MOCK_CONST_METHOD0(getSequence, int32_t()); diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 25f64368a2..00a61a5ab6 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1676,6 +1676,7 @@ void Output::presentFrameAndReleaseLayers(bool flushEvenWhenDisabled) { Fence::merge("LayerRelease", releaseFence, frame.clientTargetAcquireFence); } layer->getLayerFE().setReleaseFence(releaseFence); + layer->getLayerFE().setReleasedBuffer(layer->getLayerFE().getCompositionState()->buffer); } // We've got a list of layers needing fences, that are disjoint with diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp index 48d02423f8..34c09db6f8 100644 --- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp @@ -62,10 +62,15 @@ struct CompositionEngineTest : public testing::Test { void SetUp() override { EXPECT_CALL(*mOutput1, getDisplayId) .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId1))); + EXPECT_CALL(*mOutput1, getDisplayIdVariant).WillRepeatedly(Return(kDisplayId1)); + EXPECT_CALL(*mOutput2, getDisplayId) .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId2))); + EXPECT_CALL(*mOutput2, getDisplayIdVariant).WillRepeatedly(Return(kDisplayId2)); + EXPECT_CALL(*mOutput3, getDisplayId) .WillRepeatedly(Return(std::make_optional<DisplayId>(kDisplayId3))); + EXPECT_CALL(*mOutput3, getDisplayIdVariant).WillRepeatedly(Return(kDisplayId3)); // Most tests will depend on the outputs being enabled. for (auto& state : mOutputStates) { diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index b56147acd9..590626ace5 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -3314,6 +3314,17 @@ TEST_F(OutputPostFramebufferTest, releaseFencesAreSetInLayerFE) { sp<Fence> layer2Fence = sp<Fence>::make(); sp<Fence> layer3Fence = sp<Fence>::make(); + // Set up layerfe buffers + LayerFECompositionState layer1State; + layer1State.buffer = sp<GraphicBuffer>::make(); + LayerFECompositionState layer2State; + layer2State.buffer = sp<GraphicBuffer>::make(); + LayerFECompositionState layer3State; + layer3State.buffer = nullptr; + EXPECT_CALL(*mLayer1.layerFE, getCompositionState()).WillOnce(Return(&layer1State)); + EXPECT_CALL(*mLayer2.layerFE, getCompositionState()).WillOnce(Return(&layer2State)); + EXPECT_CALL(*mLayer3.layerFE, getCompositionState()).WillOnce(Return(&layer3State)); + Output::FrameFences frameFences; frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence); frameFences.layerFences.emplace(&mLayer2.hwc2Layer, layer2Fence); @@ -3330,14 +3341,23 @@ TEST_F(OutputPostFramebufferTest, releaseFencesAreSetInLayerFE) { .WillOnce([&layer1Fence](FenceResult releaseFence) { EXPECT_EQ(FenceResult(layer1Fence), releaseFence); }); + EXPECT_CALL(*mLayer1.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer1State.buffer, buffer); + }); EXPECT_CALL(*mLayer2.layerFE, setReleaseFence(_)) .WillOnce([&layer2Fence](FenceResult releaseFence) { EXPECT_EQ(FenceResult(layer2Fence), releaseFence); }); + EXPECT_CALL(*mLayer2.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer2State.buffer, buffer); + }); EXPECT_CALL(*mLayer3.layerFE, setReleaseFence(_)) .WillOnce([&layer3Fence](FenceResult releaseFence) { EXPECT_EQ(FenceResult(layer3Fence), releaseFence); }); + EXPECT_CALL(*mLayer3.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer3State.buffer, buffer); + }); constexpr bool kFlushEvenWhenDisabled = false; mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); @@ -3353,6 +3373,17 @@ TEST_F(OutputPostFramebufferTest, setReleaseFencesIncludeClientTargetAcquireFenc frameFences.layerFences.emplace(&mLayer2.hwc2Layer, sp<Fence>::make()); frameFences.layerFences.emplace(&mLayer3.hwc2Layer, sp<Fence>::make()); + // Set up layerfe buffers + LayerFECompositionState layer1State; + layer1State.buffer = sp<GraphicBuffer>::make(); + LayerFECompositionState layer2State; + layer2State.buffer = sp<GraphicBuffer>::make(); + LayerFECompositionState layer3State; + layer3State.buffer = nullptr; + EXPECT_CALL(*mLayer1.layerFE, getCompositionState()).WillOnce(Return(&layer1State)); + EXPECT_CALL(*mLayer2.layerFE, getCompositionState()).WillOnce(Return(&layer2State)); + EXPECT_CALL(*mLayer3.layerFE, getCompositionState()).WillOnce(Return(&layer3State)); + EXPECT_CALL(mOutput, presentFrame()).WillOnce(Return(frameFences)); EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted()); @@ -3362,6 +3393,15 @@ TEST_F(OutputPostFramebufferTest, setReleaseFencesIncludeClientTargetAcquireFenc EXPECT_CALL(*mLayer1.layerFE, setReleaseFence).WillOnce(Return()); EXPECT_CALL(*mLayer2.layerFE, setReleaseFence).WillOnce(Return()); EXPECT_CALL(*mLayer3.layerFE, setReleaseFence).WillOnce(Return()); + EXPECT_CALL(*mLayer1.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer1State.buffer, buffer); + }); + EXPECT_CALL(*mLayer2.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer2State.buffer, buffer); + }); + EXPECT_CALL(*mLayer3.layerFE, setReleasedBuffer(_)).WillOnce([&](sp<GraphicBuffer> buffer) { + EXPECT_EQ(layer3State.buffer, buffer); + }); constexpr bool kFlushEvenWhenDisabled = false; mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); } @@ -3404,7 +3444,6 @@ TEST_F(OutputPostFramebufferTest, setReleasedLayersSentPresentFence) { .WillOnce([&presentFence](FenceResult fenceResult) { EXPECT_EQ(FenceResult(presentFence), fenceResult); }); - constexpr bool kFlushEvenWhenDisabled = false; mOutput.presentFrameAndReleaseLayers(kFlushEvenWhenDisabled); diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index bad5e2e3b5..de7d455fa4 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/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) diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 7d7c8adb7b..1b14145147 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/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; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 3a884d6a0b..2e312827f2 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -722,6 +722,10 @@ void Layer::callReleaseBufferCallback(const sp<ITransactionCompletedListener>& l uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); + if (FlagManager::getInstance().monitor_buffer_fences()) { + buffer->getDependencyMonitor().addEgress(FenceTime::makeValid(fence), "Layer release"); + } + if (listener) { listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); } @@ -940,6 +944,7 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, std::max(mDrawingState.frameNumber, mDrawingState.barrierFrameNumber); mDrawingState.releaseBufferListener = bufferData.releaseBufferListener; + mDrawingState.previousBuffer = std::move(mDrawingState.buffer); mDrawingState.buffer = std::move(buffer); mDrawingState.acquireFence = bufferData.flags.test(BufferData::BufferDataChange::fenceChanged) ? bufferData.acquireFence @@ -1122,6 +1127,7 @@ bool Layer::setTransactionCompletedListeners(const std::vector<sp<CallbackHandle handle->acquireTimeOrFence = mCallbackHandleAcquireTimeOrFence; handle->frameNumber = mDrawingState.frameNumber; handle->previousFrameNumber = mDrawingState.previousFrameNumber; + handle->previousBuffer = mDrawingState.previousBuffer; if (mPreviousReleaseBufferEndpoint == handle->listener) { // Add fence from previous screenshot now so that it can be dispatched to the // client. diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 081bb22246..88754f9fa3 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -117,6 +117,7 @@ public: uint32_t bufferTransform; bool transformToDisplayInverse; Region transparentRegionHint; + std::shared_ptr<renderengine::ExternalTexture> previousBuffer; std::shared_ptr<renderengine::ExternalTexture> buffer; sp<Fence> acquireFence; std::shared_ptr<FenceTime> acquireFenceTime; diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp index b6192685ae..5e076bdae4 100644 --- a/services/surfaceflinger/LayerFE.cpp +++ b/services/surfaceflinger/LayerFE.cpp @@ -410,6 +410,15 @@ void LayerFE::setReleaseFence(const FenceResult& releaseFence) { if (mReleaseFencePromiseStatus == ReleaseFencePromiseStatus::FULFILLED) { return; } + + if (releaseFence.has_value()) { + if (FlagManager::getInstance().monitor_buffer_fences()) { + if (auto strongBuffer = mReleasedBuffer.promote()) { + strongBuffer->getDependencyMonitor() + .addAccessCompletion(FenceTime::makeValid(releaseFence.value()), "HWC"); + } + } + } mReleaseFence.set_value(releaseFence); mReleaseFencePromiseStatus = ReleaseFencePromiseStatus::FULFILLED; } @@ -428,6 +437,10 @@ LayerFE::ReleaseFencePromiseStatus LayerFE::getReleaseFencePromiseStatus() { return mReleaseFencePromiseStatus; } +void LayerFE::setReleasedBuffer(sp<GraphicBuffer> buffer) { + mReleasedBuffer = std::move(buffer); +} + void LayerFE::setLastHwcState(const LayerFE::HwcLayerDebugState &state) { mLastHwcState = state; } diff --git a/services/surfaceflinger/LayerFE.h b/services/surfaceflinger/LayerFE.h index a537456beb..b89b6b4b92 100644 --- a/services/surfaceflinger/LayerFE.h +++ b/services/surfaceflinger/LayerFE.h @@ -18,6 +18,7 @@ #include <android/gui/CachingHint.h> #include <gui/LayerMetadata.h> +#include <ui/GraphicBuffer.h> #include <ui/LayerStack.h> #include <ui/PictureProfileHandle.h> @@ -58,6 +59,7 @@ public: ftl::Future<FenceResult> createReleaseFenceFuture() override; void setReleaseFence(const FenceResult& releaseFence) override; LayerFE::ReleaseFencePromiseStatus getReleaseFencePromiseStatus() override; + void setReleasedBuffer(sp<GraphicBuffer> buffer) override; void onPictureProfileCommitted() override; // Used for debugging purposes, e.g. perfetto tracing, dumpsys. @@ -95,6 +97,7 @@ private: std::promise<FenceResult> mReleaseFence; ReleaseFencePromiseStatus mReleaseFencePromiseStatus = ReleaseFencePromiseStatus::UNINITIALIZED; HwcLayerDebugState mLastHwcState; + wp<GraphicBuffer> mReleasedBuffer; }; } // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 780b897aae..aa933ee8a7 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1269,7 +1269,17 @@ void SurfaceFlinger::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*& info ui::FrameRateCategoryRate frameRateCategoryRate(normal.getValue(), high.getValue()); info->frameRateCategoryRate = frameRateCategoryRate; - info->supportedRefreshRates = display->refreshRateSelector().getSupportedFrameRates(); + if (info->hasArrSupport) { + info->supportedRefreshRates = display->refreshRateSelector().getSupportedFrameRates(); + } else { + // On non-ARR devices, list the refresh rates same as the supported display modes. + std::vector<float> supportedFrameRates; + supportedFrameRates.reserve(info->supportedDisplayModes.size()); + std::transform(info->supportedDisplayModes.begin(), info->supportedDisplayModes.end(), + std::back_inserter(supportedFrameRates), + [](ui::DisplayMode mode) { return mode.peakRefreshRate; }); + info->supportedRefreshRates = supportedFrameRates; + } info->activeColorMode = display->getCompositionDisplay()->getState().colorMode; info->hdrCapabilities = filterOut4k30(display->getHdrCapabilities()); @@ -5088,6 +5098,12 @@ status_t SurfaceFlinger::setTransactionState(TransactionState&& transactionState layerName.c_str(), transactionState.getId()); if (resolvedState.externalTexture) { resolvedState.state.bufferData->buffer = resolvedState.externalTexture->getBuffer(); + if (FlagManager::getInstance().monitor_buffer_fences()) { + resolvedState.state.bufferData->buffer->getDependencyMonitor() + .addIngress(FenceTime::makeValid( + resolvedState.state.bufferData->acquireFence), + "Incoming txn"); + } } mBufferCountTracker.increment(resolvedState.layerId); } @@ -5703,7 +5719,13 @@ 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 @@ -5718,12 +5740,10 @@ 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 && + !FlagManager::getInstance().correct_virtual_display_power_state()) { + optimizeThreadScheduling("setPhysicalDisplayPowerMode(ON/DOZE)", + OptimizationPolicy::optimizeForPerformance); } getHwComposer().setPowerMode(displayId, mode); @@ -5732,7 +5752,8 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa mScheduler->getVsyncSchedule(displayId)->getPendingHardwareVsyncState(); requestHardwareVsync(displayId, enable); - if (displayId == mActiveDisplayId) { + if (displayId == mActiveDisplayId && + !FlagManager::getInstance().correct_virtual_display_power_state()) { mScheduler->enableSyntheticVsync(false); } @@ -5749,13 +5770,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()) { - applyOptimizationPolicy("setPhysicalDisplayPowerMode(OFF)"); - } else { - enablePowerOptimizations("setPhysicalDisplayPowerMode(OFF)"); + if (!FlagManager::getInstance().correct_virtual_display_power_state()) { + optimizeThreadScheduling("setPhysicalDisplayPowerMode(OFF)", + OptimizationPolicy::optimizeForPower); } - if (currentModeNotDozeSuspend) { + if (currentModeNotDozeSuspend && + !FlagManager::getInstance().correct_virtual_display_power_state()) { mScheduler->enableSyntheticVsync(); } } @@ -5783,7 +5804,9 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa ALOGI("Force repainting for DOZE_SUSPEND -> DOZE or ON."); mVisibleRegionsDirty = true; scheduleRepaint(); - mScheduler->enableSyntheticVsync(false); + if (!FlagManager::getInstance().correct_virtual_display_power_state()) { + mScheduler->enableSyntheticVsync(false); + } } constexpr bool kAllowToEnable = true; mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, activeMode.get()); @@ -5793,7 +5816,8 @@ void SurfaceFlinger::setPhysicalDisplayPowerMode(const sp<DisplayDevice>& displa constexpr bool kDisallow = true; mScheduler->disableHardwareVsync(displayId, kDisallow); - if (displayId == mActiveDisplayId) { + if (displayId == mActiveDisplayId && + !FlagManager::getInstance().correct_virtual_display_power_state()) { mScheduler->enableSyntheticVsync(); } getHwComposer().setPowerMode(displayId, mode); @@ -5832,43 +5856,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); } } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 9cf0c6aaa0..f61214cc65 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -733,19 +733,14 @@ private: 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. diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index b22ec66819..2e8c8c1111 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -18,7 +18,7 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" -//#define LOG_NDEBUG 0 +// #define LOG_NDEBUG 0 #undef LOG_TAG #define LOG_TAG "TransactionCallbackInvoker" #define ATRACE_TAG ATRACE_TAG_GRAPHICS @@ -28,6 +28,7 @@ #include "Utils/FenceUtils.h" #include <binder/IInterface.h> +#include <common/FlagManager.h> #include <common/trace.h> #include <utils/RefBase.h> @@ -142,8 +143,17 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); + if (handle->bufferReleaseChannel && handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { + if (FlagManager::getInstance().monitor_buffer_fences()) { + if (auto previousBuffer = handle->previousBuffer.lock()) { + previousBuffer->getBuffer() + ->getDependencyMonitor() + .addEgress(FenceTime::makeValid(handle->previousReleaseFence), + "Txn release"); + } + } mBufferReleases.emplace_back(handle->name, handle->bufferReleaseChannel, handle->previousReleaseCallbackId, handle->previousReleaseFence, diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 178ddbbe79..34f6ffc5da 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -25,6 +25,7 @@ #include <ftl/future.h> #include <gui/BufferReleaseChannel.h> #include <gui/ITransactionCompletedListener.h> +#include <renderengine/ExternalTexture.h> #include <ui/Fence.h> #include <ui/FenceResult.h> @@ -55,6 +56,7 @@ public: uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> bufferReleaseChannel; + std::weak_ptr<renderengine::ExternalTexture> previousBuffer; }; class TransactionCallbackInvoker { diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 5ff3d821f7..bf1035149b 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -129,6 +129,7 @@ void FlagManager::dump(std::string& result) const { 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(monitor_buffer_fences); DUMP_ACONFIG_FLAG(refresh_rate_overlay_on_external_display); DUMP_ACONFIG_FLAG(vsync_predictor_recovery); @@ -303,6 +304,7 @@ FLAG_MANAGER_ACONFIG_FLAG(adpf_gpu_sf, "") FLAG_MANAGER_ACONFIG_FLAG(adpf_native_session_manager, ""); FLAG_MANAGER_ACONFIG_FLAG(graphite_renderengine_preview_rollout, ""); FLAG_MANAGER_ACONFIG_FLAG(increase_missed_frame_jank_threshold, ""); +FLAG_MANAGER_ACONFIG_FLAG(monitor_buffer_fences, ""); FLAG_MANAGER_ACONFIG_FLAG(vsync_predictor_recovery, ""); /// Trunk stable server (R/W) flags from outside SurfaceFlinger /// diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 419e92b952..8f361ac610 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -63,6 +63,7 @@ public: bool correct_virtual_display_power_state() const; bool graphite_renderengine_preview_rollout() const; bool increase_missed_frame_jank_threshold() const; + bool monitor_buffer_fences() const; bool refresh_rate_overlay_on_external_display() const; bool vsync_predictor_recovery() const; diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig index d8f51fe7e4..d412a19f3c 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -216,6 +216,13 @@ flag { } # local_tonemap_screenshots flag { + name: "monitor_buffer_fences" + namespace: "core_graphics" + description: "Monitors fences for each buffer" + bug: "360932099" +} # monitor_buffer_fences + +flag { name: "no_vsyncs_on_screen_off" namespace: "core_graphics" description: "Stop vsync / Choreographer callbacks to apps when the screen is off" diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp index d5c22a9601..2332bf62da 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPhysicalDisplayPowerModeTest.cpp +++ b/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,9 @@ 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); |