diff options
43 files changed, 582 insertions, 436 deletions
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp index aa42541a66..484231225d 100644 --- a/cmds/dumpstate/DumpstateUtil.cpp +++ b/cmds/dumpstate/DumpstateUtil.cpp @@ -207,6 +207,9 @@ std::string PropertiesHelper::build_type_ = ""; int PropertiesHelper::dry_run_ = -1; int PropertiesHelper::unroot_ = -1; int PropertiesHelper::parallel_run_ = -1; +#if !defined(__ANDROID_VNDK__) +int PropertiesHelper::strict_run_ = -1; +#endif bool PropertiesHelper::IsUserBuild() { if (build_type_.empty()) { @@ -237,6 +240,16 @@ bool PropertiesHelper::IsParallelRun() { return parallel_run_ == 1; } +#if !defined(__ANDROID_VNDK__) +bool PropertiesHelper::IsStrictRun() { + if (strict_run_ == -1) { + // Defaults to using stricter timeouts. + strict_run_ = android::base::GetBoolProperty("dumpstate.strict_run", true) ? 1 : 0; + } + return strict_run_ == 1; +} +#endif + int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) { android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC))); if (fd.get() < 0) { diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h index b00c46e0db..6049e3e19d 100644 --- a/cmds/dumpstate/DumpstateUtil.h +++ b/cmds/dumpstate/DumpstateUtil.h @@ -193,11 +193,23 @@ class PropertiesHelper { */ static bool IsParallelRun(); + /* + * Strict-run mode is determined by the `dumpstate.strict_run` sysprop which + * will default to true. This results in shortened timeouts for flaky + * sections. + */ +#if !defined(__ANDROID_VNDK__) + static bool IsStrictRun(); +#endif + private: static std::string build_type_; static int dry_run_; static int unroot_; static int parallel_run_; +#if !defined(__ANDROID_VNDK__) + static int strict_run_; +#endif }; /* diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 8a337569c4..e93b46c666 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -822,9 +822,12 @@ void Dumpstate::PrintHeader() const { RunCommandToFd(STDOUT_FILENO, "", {"uptime", "-p"}, CommandOptions::WithTimeout(1).Always().Build()); printf("Bugreport format version: %s\n", version_.c_str()); - printf("Dumpstate info: id=%d pid=%d dry_run=%d parallel_run=%d args=%s bugreport_mode=%s\n", - id_, pid_, PropertiesHelper::IsDryRun(), PropertiesHelper::IsParallelRun(), - options_->args.c_str(), options_->bugreport_mode_string.c_str()); + printf( + "Dumpstate info: id=%d pid=%d dry_run=%d parallel_run=%d strict_run=%d args=%s " + "bugreport_mode=%s\n", + id_, pid_, PropertiesHelper::IsDryRun(), PropertiesHelper::IsParallelRun(), + PropertiesHelper::IsStrictRun(), options_->args.c_str(), + options_->bugreport_mode_string.c_str()); printf("\n"); } @@ -1046,7 +1049,8 @@ static void DumpIncidentReport() { MYLOGE("Could not open %s to dump incident report.\n", path.c_str()); return; } - RunCommandToFd(fd, "", {"incident", "-u"}, CommandOptions::WithTimeout(20).Build()); + RunCommandToFd(fd, "", {"incident", "-u"}, + CommandOptions::WithTimeout(PropertiesHelper::IsStrictRun() ? 20 : 120).Build()); bool empty = 0 == lseek(fd, 0, SEEK_END); if (!empty) { // Use a different name from "incident.proto" @@ -3127,6 +3131,12 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n"); } + if (PropertiesHelper::IsStrictRun()) { + MYLOGI( + "Running on strict-run mode, which has shorter timeouts " + "(to disable, call 'setprop dumpstate.strict_run false')\n"); + } + MYLOGI("dumpstate info: id=%d, args='%s', bugreport_mode= %s bugreport format version: %s\n", id_, options_->args.c_str(), options_->bugreport_mode_string.c_str(), version_.c_str()); diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 3c8df2bb29..bf34987b9e 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -73,7 +73,6 @@ filegroup { "android/gui/FocusRequest.aidl", "android/gui/InputApplicationInfo.aidl", "android/gui/IWindowInfosListener.aidl", - "android/gui/IWindowInfosPublisher.aidl", "android/gui/IWindowInfosReportedListener.aidl", "android/gui/WindowInfo.aidl", "android/gui/WindowInfosUpdate.aidl", @@ -91,7 +90,6 @@ cc_library_static { "android/gui/FocusRequest.aidl", "android/gui/InputApplicationInfo.aidl", "android/gui/IWindowInfosListener.aidl", - "android/gui/IWindowInfosPublisher.aidl", "android/gui/IWindowInfosReportedListener.aidl", "android/gui/WindowInfosUpdate.aidl", "android/gui/WindowInfo.aidl", diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 9a2343bffb..43ff54f3c8 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -418,6 +418,9 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou EGLSyncKHR eglFence = EGL_NO_SYNC_KHR; bool attachedByConsumer = false; + sp<IConsumerListener> listener; + bool callOnFrameDequeued = false; + uint64_t bufferId = 0; // Only used if callOnFrameDequeued == true { // Autolock scope std::unique_lock<std::mutex> lock(mCore->mMutex); @@ -561,10 +564,11 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou } if (!(returnFlags & BUFFER_NEEDS_REALLOCATION)) { - if (mCore->mConsumerListener != nullptr) { - mCore->mConsumerListener->onFrameDequeued(mSlots[*outSlot].mGraphicBuffer->getId()); - } + callOnFrameDequeued = true; + bufferId = mSlots[*outSlot].mGraphicBuffer->getId(); } + + listener = mCore->mConsumerListener; } // Autolock scope if (returnFlags & BUFFER_NEEDS_REALLOCATION) { @@ -581,10 +585,8 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou if (error == NO_ERROR && !mCore->mIsAbandoned) { graphicBuffer->setGenerationNumber(mCore->mGenerationNumber); mSlots[*outSlot].mGraphicBuffer = graphicBuffer; - if (mCore->mConsumerListener != nullptr) { - mCore->mConsumerListener->onFrameDequeued( - mSlots[*outSlot].mGraphicBuffer->getId()); - } + callOnFrameDequeued = true; + bufferId = mSlots[*outSlot].mGraphicBuffer->getId(); } mCore->mIsAllocating = false; @@ -608,6 +610,10 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou } // Autolock scope } + if (listener != nullptr && callOnFrameDequeued) { + listener->onFrameDequeued(bufferId); + } + if (attachedByConsumer) { returnFlags |= BUFFER_NEEDS_REALLOCATION; } @@ -646,6 +652,8 @@ status_t BufferQueueProducer::detachBuffer(int slot) { BQ_LOGV("detachBuffer: slot %d", slot); sp<IConsumerListener> listener; + bool callOnFrameDetached = false; + uint64_t bufferId = 0; // Only used if callOnFrameDetached is true { std::lock_guard<std::mutex> lock(mCore->mMutex); @@ -683,8 +691,9 @@ status_t BufferQueueProducer::detachBuffer(int slot) { listener = mCore->mConsumerListener; auto gb = mSlots[slot].mGraphicBuffer; - if (listener != nullptr && gb != nullptr) { - listener->onFrameDetached(gb->getId()); + if (gb != nullptr) { + callOnFrameDetached = true; + bufferId = gb->getId(); } mSlots[slot].mBufferState.detachProducer(); mCore->mActiveBuffers.erase(slot); @@ -694,6 +703,10 @@ status_t BufferQueueProducer::detachBuffer(int slot) { VALIDATE_CONSISTENCY(); } + if (listener != nullptr && callOnFrameDetached) { + listener->onFrameDetached(bufferId); + } + if (listener != nullptr) { listener->onBuffersReleased(); } @@ -1104,57 +1117,70 @@ status_t BufferQueueProducer::queueBuffer(int slot, status_t BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) { ATRACE_CALL(); BQ_LOGV("cancelBuffer: slot %d", slot); - std::lock_guard<std::mutex> lock(mCore->mMutex); - if (mCore->mIsAbandoned) { - BQ_LOGE("cancelBuffer: BufferQueue has been abandoned"); - return NO_INIT; - } + sp<IConsumerListener> listener; + bool callOnFrameCancelled = false; + uint64_t bufferId = 0; // Only used if callOnFrameCancelled == true + { + std::lock_guard<std::mutex> lock(mCore->mMutex); - if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) { - BQ_LOGE("cancelBuffer: BufferQueue has no connected producer"); - return NO_INIT; - } + if (mCore->mIsAbandoned) { + BQ_LOGE("cancelBuffer: BufferQueue has been abandoned"); + return NO_INIT; + } - if (mCore->mSharedBufferMode) { - BQ_LOGE("cancelBuffer: cannot cancel a buffer in shared buffer mode"); - return BAD_VALUE; - } + if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) { + BQ_LOGE("cancelBuffer: BufferQueue has no connected producer"); + return NO_INIT; + } - if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) { - BQ_LOGE("cancelBuffer: slot index %d out of range [0, %d)", - slot, BufferQueueDefs::NUM_BUFFER_SLOTS); - return BAD_VALUE; - } else if (!mSlots[slot].mBufferState.isDequeued()) { - BQ_LOGE("cancelBuffer: slot %d is not owned by the producer " - "(state = %s)", slot, mSlots[slot].mBufferState.string()); - return BAD_VALUE; - } else if (fence == nullptr) { - BQ_LOGE("cancelBuffer: fence is NULL"); - return BAD_VALUE; - } + if (mCore->mSharedBufferMode) { + BQ_LOGE("cancelBuffer: cannot cancel a buffer in shared buffer mode"); + return BAD_VALUE; + } + + if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) { + BQ_LOGE("cancelBuffer: slot index %d out of range [0, %d)", slot, + BufferQueueDefs::NUM_BUFFER_SLOTS); + return BAD_VALUE; + } else if (!mSlots[slot].mBufferState.isDequeued()) { + BQ_LOGE("cancelBuffer: slot %d is not owned by the producer " + "(state = %s)", + slot, mSlots[slot].mBufferState.string()); + return BAD_VALUE; + } else if (fence == nullptr) { + BQ_LOGE("cancelBuffer: fence is NULL"); + return BAD_VALUE; + } - mSlots[slot].mBufferState.cancel(); + mSlots[slot].mBufferState.cancel(); - // After leaving shared buffer mode, the shared buffer will still be around. - // Mark it as no longer shared if this operation causes it to be free. - if (!mCore->mSharedBufferMode && mSlots[slot].mBufferState.isFree()) { - mSlots[slot].mBufferState.mShared = false; - } + // After leaving shared buffer mode, the shared buffer will still be around. + // Mark it as no longer shared if this operation causes it to be free. + if (!mCore->mSharedBufferMode && mSlots[slot].mBufferState.isFree()) { + mSlots[slot].mBufferState.mShared = false; + } - // Don't put the shared buffer on the free list. - if (!mSlots[slot].mBufferState.isShared()) { - mCore->mActiveBuffers.erase(slot); - mCore->mFreeBuffers.push_back(slot); + // Don't put the shared buffer on the free list. + if (!mSlots[slot].mBufferState.isShared()) { + mCore->mActiveBuffers.erase(slot); + mCore->mFreeBuffers.push_back(slot); + } + + auto gb = mSlots[slot].mGraphicBuffer; + if (gb != nullptr) { + callOnFrameCancelled = true; + bufferId = gb->getId(); + } + mSlots[slot].mFence = fence; + mCore->mDequeueCondition.notify_all(); + listener = mCore->mConsumerListener; + VALIDATE_CONSISTENCY(); } - auto gb = mSlots[slot].mGraphicBuffer; - if (mCore->mConsumerListener != nullptr && gb != nullptr) { - mCore->mConsumerListener->onFrameCancelled(gb->getId()); + if (listener != nullptr && callOnFrameCancelled) { + listener->onFrameCancelled(bufferId); } - mSlots[slot].mFence = fence; - mCore->mDequeueCondition.notify_all(); - VALIDATE_CONSISTENCY(); return NO_ERROR; } diff --git a/libs/gui/WindowInfosListenerReporter.cpp b/libs/gui/WindowInfosListenerReporter.cpp index 0929b8e120..76e7b6e162 100644 --- a/libs/gui/WindowInfosListenerReporter.cpp +++ b/libs/gui/WindowInfosListenerReporter.cpp @@ -22,6 +22,7 @@ namespace android { using gui::DisplayInfo; +using gui::IWindowInfosReportedListener; using gui::WindowInfo; using gui::WindowInfosListener; using gui::aidl_utils::statusTFromBinderStatus; @@ -39,13 +40,8 @@ status_t WindowInfosListenerReporter::addWindowInfosListener( { std::scoped_lock lock(mListenersMutex); if (mWindowInfosListeners.empty()) { - gui::WindowInfosListenerInfo listenerInfo; - binder::Status s = surfaceComposer->addWindowInfosListener(this, &listenerInfo); + binder::Status s = surfaceComposer->addWindowInfosListener(this); status = statusTFromBinderStatus(s); - if (status == OK) { - mWindowInfosPublisher = std::move(listenerInfo.windowInfosPublisher); - mListenerId = listenerInfo.listenerId; - } } if (status == OK) { @@ -89,7 +85,8 @@ status_t WindowInfosListenerReporter::removeWindowInfosListener( } binder::Status WindowInfosListenerReporter::onWindowInfosChanged( - const gui::WindowInfosUpdate& update) { + const gui::WindowInfosUpdate& update, + const sp<IWindowInfosReportedListener>& windowInfosReportedListener) { std::unordered_set<sp<WindowInfosListener>, gui::SpHash<WindowInfosListener>> windowInfosListeners; @@ -107,7 +104,9 @@ binder::Status WindowInfosListenerReporter::onWindowInfosChanged( listener->onWindowInfosChanged(update); } - mWindowInfosPublisher->ackWindowInfosReceived(update.vsyncId, mListenerId); + if (windowInfosReportedListener) { + windowInfosReportedListener->onWindowInfosReported(); + } return binder::Status::ok(); } @@ -115,10 +114,7 @@ binder::Status WindowInfosListenerReporter::onWindowInfosChanged( void WindowInfosListenerReporter::reconnect(const sp<gui::ISurfaceComposer>& composerService) { std::scoped_lock lock(mListenersMutex); if (!mWindowInfosListeners.empty()) { - gui::WindowInfosListenerInfo listenerInfo; - composerService->addWindowInfosListener(this, &listenerInfo); - mWindowInfosPublisher = std::move(listenerInfo.windowInfosPublisher); - mListenerId = listenerInfo.listenerId; + composerService->addWindowInfosListener(this); } } diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index 539a1c140e..ec3266ca83 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -40,14 +40,12 @@ import android.gui.IScreenCaptureListener; import android.gui.ISurfaceComposerClient; import android.gui.ITunnelModeEnabledListener; import android.gui.IWindowInfosListener; -import android.gui.IWindowInfosPublisher; import android.gui.LayerCaptureArgs; import android.gui.LayerDebugInfo; import android.gui.OverlayProperties; import android.gui.PullAtomData; import android.gui.ARect; import android.gui.StaticDisplayInfo; -import android.gui.WindowInfosListenerInfo; /** @hide */ interface ISurfaceComposer { @@ -502,7 +500,7 @@ interface ISurfaceComposer { */ int getMaxAcquiredBufferCount(); - WindowInfosListenerInfo addWindowInfosListener(IWindowInfosListener windowInfosListener); + void addWindowInfosListener(IWindowInfosListener windowInfosListener); void removeWindowInfosListener(IWindowInfosListener windowInfosListener); diff --git a/libs/gui/aidl/android/gui/WindowInfosListenerInfo.aidl b/libs/gui/aidl/android/gui/WindowInfosListenerInfo.aidl deleted file mode 100644 index 0ca13b768a..0000000000 --- a/libs/gui/aidl/android/gui/WindowInfosListenerInfo.aidl +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Copyright (c) 2023, 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. - */ - -package android.gui; - -import android.gui.IWindowInfosPublisher; - -/** @hide */ -parcelable WindowInfosListenerInfo { - long listenerId; - IWindowInfosPublisher windowInfosPublisher; -}
\ No newline at end of file diff --git a/libs/gui/android/gui/IWindowInfosListener.aidl b/libs/gui/android/gui/IWindowInfosListener.aidl index 07cb5ed0e6..400229d99f 100644 --- a/libs/gui/android/gui/IWindowInfosListener.aidl +++ b/libs/gui/android/gui/IWindowInfosListener.aidl @@ -16,9 +16,11 @@ package android.gui; +import android.gui.IWindowInfosReportedListener; import android.gui.WindowInfosUpdate; /** @hide */ oneway interface IWindowInfosListener { - void onWindowInfosChanged(in WindowInfosUpdate update); + void onWindowInfosChanged( + in WindowInfosUpdate update, in @nullable IWindowInfosReportedListener windowInfosReportedListener); } diff --git a/libs/gui/android/gui/IWindowInfosPublisher.aidl b/libs/gui/android/gui/IWindowInfosPublisher.aidl deleted file mode 100644 index 5a9c32845e..0000000000 --- a/libs/gui/android/gui/IWindowInfosPublisher.aidl +++ /dev/null @@ -1,23 +0,0 @@ -/** - * Copyright (c) 2023, 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. - */ - -package android.gui; - -/** @hide */ -oneway interface IWindowInfosPublisher -{ - void ackWindowInfosReceived(long vsyncId, long listenerId); -} diff --git a/libs/gui/fuzzer/libgui_fuzzer_utils.h b/libs/gui/fuzzer/libgui_fuzzer_utils.h index 4c7d0562af..8c003d8ad4 100644 --- a/libs/gui/fuzzer/libgui_fuzzer_utils.h +++ b/libs/gui/fuzzer/libgui_fuzzer_utils.h @@ -153,8 +153,8 @@ public: MOCK_METHOD(binder::Status, setOverrideFrameRate, (int32_t, float), (override)); MOCK_METHOD(binder::Status, getGpuContextPriority, (int32_t*), (override)); MOCK_METHOD(binder::Status, getMaxAcquiredBufferCount, (int32_t*), (override)); - MOCK_METHOD(binder::Status, addWindowInfosListener, - (const sp<gui::IWindowInfosListener>&, gui::WindowInfosListenerInfo*), (override)); + MOCK_METHOD(binder::Status, addWindowInfosListener, (const sp<gui::IWindowInfosListener>&), + (override)); MOCK_METHOD(binder::Status, removeWindowInfosListener, (const sp<gui::IWindowInfosListener>&), (override)); MOCK_METHOD(binder::Status, getOverlaySupport, (gui::OverlayProperties*), (override)); diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 2790167191..4a3cb33973 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -26,7 +26,6 @@ #include <android/gui/IScreenCaptureListener.h> #include <android/gui/ITunnelModeEnabledListener.h> #include <android/gui/IWindowInfosListener.h> -#include <android/gui/IWindowInfosPublisher.h> #include <binder/IBinder.h> #include <binder/IInterface.h> #include <gui/ITransactionCompletedListener.h> diff --git a/libs/gui/include/gui/WindowInfosListenerReporter.h b/libs/gui/include/gui/WindowInfosListenerReporter.h index 684e21ad96..38cb108912 100644 --- a/libs/gui/include/gui/WindowInfosListenerReporter.h +++ b/libs/gui/include/gui/WindowInfosListenerReporter.h @@ -18,7 +18,7 @@ #include <android/gui/BnWindowInfosListener.h> #include <android/gui/ISurfaceComposer.h> -#include <android/gui/IWindowInfosPublisher.h> +#include <android/gui/IWindowInfosReportedListener.h> #include <binder/IBinder.h> #include <gui/SpHash.h> #include <gui/WindowInfosListener.h> @@ -30,7 +30,8 @@ namespace android { class WindowInfosListenerReporter : public gui::BnWindowInfosListener { public: static sp<WindowInfosListenerReporter> getInstance(); - binder::Status onWindowInfosChanged(const gui::WindowInfosUpdate& update) override; + binder::Status onWindowInfosChanged(const gui::WindowInfosUpdate& update, + const sp<gui::IWindowInfosReportedListener>&) override; status_t addWindowInfosListener( const sp<gui::WindowInfosListener>& windowInfosListener, const sp<gui::ISurfaceComposer>&, @@ -46,8 +47,5 @@ private: std::vector<gui::WindowInfo> mLastWindowInfos GUARDED_BY(mListenersMutex); std::vector<gui::DisplayInfo> mLastDisplayInfos GUARDED_BY(mListenersMutex); - - sp<gui::IWindowInfosPublisher> mWindowInfosPublisher; - int64_t mListenerId; }; } // namespace android diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index d351e28ca9..d853f603d9 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -998,8 +998,7 @@ public: } binder::Status addWindowInfosListener( - const sp<gui::IWindowInfosListener>& /*windowInfosListener*/, - gui::WindowInfosListenerInfo* /*outInfo*/) override { + const sp<gui::IWindowInfosListener>& /*windowInfosListener*/) override { return binder::Status::ok(); } diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index edf734271a..2cd5255649 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -507,7 +507,8 @@ sk_sp<SkShader> SkiaRenderEngine::createRuntimeEffectShader( auto effect = shaders::LinearEffect{.inputDataspace = parameters.layer.sourceDataspace, .outputDataspace = parameters.outputDataSpace, - .undoPremultipliedAlpha = parameters.undoPremultipliedAlpha}; + .undoPremultipliedAlpha = parameters.undoPremultipliedAlpha, + .fakeOutputDataspace = parameters.fakeOutputDataspace}; auto effectIter = mRuntimeEffects.find(effect); sk_sp<SkRuntimeEffect> runtimeEffect = nullptr; @@ -901,12 +902,14 @@ void SkiaRenderEngine::drawLayersInternal( (display.outputDataspace & ui::Dataspace::TRANSFER_MASK) == static_cast<int32_t>(ui::Dataspace::TRANSFER_SRGB); - const ui::Dataspace runtimeEffectDataspace = !dimInLinearSpace && isExtendedHdr + const bool useFakeOutputDataspaceForRuntimeEffect = !dimInLinearSpace && isExtendedHdr; + + const ui::Dataspace fakeDataspace = useFakeOutputDataspaceForRuntimeEffect ? static_cast<ui::Dataspace>( (display.outputDataspace & ui::Dataspace::STANDARD_MASK) | ui::Dataspace::TRANSFER_GAMMA2_2 | (display.outputDataspace & ui::Dataspace::RANGE_MASK)) - : display.outputDataspace; + : ui::Dataspace::UNKNOWN; // If the input dataspace is range extended, the output dataspace transfer is sRGB // and dimmingStage is GAMMA_OETF, dim in linear space instead, and @@ -1013,7 +1016,8 @@ void SkiaRenderEngine::drawLayersInternal( .layerDimmingRatio = dimInLinearSpace ? layerDimmingRatio : 1.f, - .outputDataSpace = runtimeEffectDataspace})); + .outputDataSpace = display.outputDataspace, + .fakeOutputDataspace = fakeDataspace})); // Turn on dithering when dimming beyond this (arbitrary) threshold... static constexpr float kDimmingThreshold = 0.2f; @@ -1077,7 +1081,8 @@ void SkiaRenderEngine::drawLayersInternal( .undoPremultipliedAlpha = false, .requiresLinearEffect = requiresLinearEffect, .layerDimmingRatio = layerDimmingRatio, - .outputDataSpace = runtimeEffectDataspace})); + .outputDataSpace = display.outputDataspace, + .fakeOutputDataspace = fakeDataspace})); } if (layer.disableBlending) { diff --git a/libs/renderengine/skia/SkiaRenderEngine.h b/libs/renderengine/skia/SkiaRenderEngine.h index 6457bfa9eb..723e73c29e 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.h +++ b/libs/renderengine/skia/SkiaRenderEngine.h @@ -157,6 +157,7 @@ private: bool requiresLinearEffect; float layerDimmingRatio; const ui::Dataspace outputDataSpace; + const ui::Dataspace fakeOutputDataspace; }; sk_sp<SkShader> createRuntimeEffectShader(const RuntimeEffectShaderParameters&); diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index c85517a976..ef039e5c36 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -168,8 +168,8 @@ void generateOOTF(ui::Dataspace inputDataspace, ui::Dataspace outputDataspace, void generateOETF(std::string& shader) { // Only support gamma 2.2 for now shader.append(R"( - float OETF(float3 linear) { - return sign(linear) * pow(abs(linear), (1.0 / 2.2)); + float3 OETF(float3 linear) { + return sign(linear) * pow(abs(linear), float3(1.0 / 2.2)); } )"); } diff --git a/libs/shaders/tests/Android.bp b/libs/shaders/tests/Android.bp index 1e4f45ac45..5639d744df 100644 --- a/libs/shaders/tests/Android.bp +++ b/libs/shaders/tests/Android.bp @@ -37,6 +37,7 @@ cc_test { shared_libs: [ "android.hardware.graphics.common@1.2", "libnativewindow", + "libbase", ], static_libs: [ "libarect", diff --git a/libs/tonemap/tests/Android.bp b/libs/tonemap/tests/Android.bp index 2abf51563c..5c5fc6c8b3 100644 --- a/libs/tonemap/tests/Android.bp +++ b/libs/tonemap/tests/Android.bp @@ -36,6 +36,7 @@ cc_test { ], shared_libs: [ "libnativewindow", + "libbase", ], static_libs: [ "libmath", diff --git a/services/sensorservice/AidlSensorHalWrapper.cpp b/services/sensorservice/AidlSensorHalWrapper.cpp index f5b360f3b6..e60db93431 100644 --- a/services/sensorservice/AidlSensorHalWrapper.cpp +++ b/services/sensorservice/AidlSensorHalWrapper.cpp @@ -308,8 +308,12 @@ status_t AidlSensorHalWrapper::configureDirectChannel(int32_t sensorHandle, int3 } int32_t token; - mSensors->configDirectReport(sensorHandle, channelHandle, rate, &token); - return token; + status_t status = convertToStatus( + mSensors->configDirectReport(sensorHandle, channelHandle, rate, &token)); + if (status == OK && rate != ISensors::RateLevel::STOP) { + status = static_cast<status_t>(token); + } + return status; } void AidlSensorHalWrapper::writeWakeLockHandled(uint32_t count) { diff --git a/services/surfaceflinger/BackgroundExecutor.cpp b/services/surfaceflinger/BackgroundExecutor.cpp index 5a1ec6f501..6ddf790d47 100644 --- a/services/surfaceflinger/BackgroundExecutor.cpp +++ b/services/surfaceflinger/BackgroundExecutor.cpp @@ -20,7 +20,6 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS #include <utils/Log.h> -#include <mutex> #include "BackgroundExecutor.h" @@ -61,17 +60,4 @@ void BackgroundExecutor::sendCallbacks(Callbacks&& tasks) { LOG_ALWAYS_FATAL_IF(sem_post(&mSemaphore), "sem_post failed"); } -void BackgroundExecutor::flushQueue() { - std::mutex mutex; - std::condition_variable cv; - bool flushComplete = false; - sendCallbacks({[&]() { - std::scoped_lock lock{mutex}; - flushComplete = true; - cv.notify_one(); - }}); - std::unique_lock<std::mutex> lock{mutex}; - cv.wait(lock, [&]() { return flushComplete; }); -} - } // namespace android diff --git a/services/surfaceflinger/BackgroundExecutor.h b/services/surfaceflinger/BackgroundExecutor.h index 66b7d7a1fc..0fae5a5c93 100644 --- a/services/surfaceflinger/BackgroundExecutor.h +++ b/services/surfaceflinger/BackgroundExecutor.h @@ -34,7 +34,6 @@ public: // Queues callbacks onto a work queue to be executed by a background thread. // This is safe to call from multiple threads. void sendCallbacks(Callbacks&& tasks); - void flushQueue(); private: sem_t mSemaphore; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index d93e25e4ca..09bc46747a 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -37,6 +37,15 @@ struct BorderRenderInfo { half4 color; std::vector<int32_t> layerIds; }; + +// Interface of composition engine power hint callback. +struct ICEPowerCallback { + virtual void notifyCpuLoadUp() = 0; + +protected: + ~ICEPowerCallback() = default; +}; + /** * A parameter object for refreshing a set of outputs */ @@ -96,6 +105,8 @@ struct CompositionRefreshArgs { std::vector<BorderRenderInfo> borderInfoList; bool hasTrustedPresentationListener = false; + + ICEPowerCallback* powerCallback = nullptr; }; } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index a3fda61ecb..28c6e92b06 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -32,6 +32,7 @@ // TODO(b/129481165): remove the #pragma below and fix conversion issues #pragma clang diagnostic pop // ignored "-Wconversion -Wextra" +#include <compositionengine/CompositionRefreshArgs.h> #include <compositionengine/ProjectionSpace.h> #include <renderengine/BorderRenderInfo.h> #include <ui/LayerStack.h> @@ -167,6 +168,8 @@ struct OutputCompositionState { uint64_t lastOutputLayerHash = 0; uint64_t outputLayerHash = 0; + ICEPowerCallback* powerCallback = nullptr; + // Debugging void dump(std::string& result) const; }; diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 793959cea6..1205a2ce71 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -843,6 +843,7 @@ void Output::writeCompositionState(const compositionengine::CompositionRefreshAr editState().earliestPresentTime = refreshArgs.earliestPresentTime; editState().expectedPresentTime = refreshArgs.expectedPresentTime; + editState().powerCallback = refreshArgs.powerCallback; compositionengine::OutputLayer* peekThroughLayer = nullptr; sp<GraphicBuffer> previousOverride = nullptr; diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp index c512a1e97f..9713e79fe3 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp @@ -67,7 +67,7 @@ void OutputCompositionState::dump(std::string& out) const { out.append("\n "); out.append("\n "); - dumpVal(out, "treate170mAsSrgb", treat170mAsSrgb); + dumpVal(out, "treat170mAsSrgb", treat170mAsSrgb); out.append("\n"); for (const auto& borderRenderInfo : borderInfoList) { diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp index 8ced0aca36..7547be94e3 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -162,6 +162,9 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, TexturePool& te const OutputCompositionState& outputState, bool deviceHandlesColorTransform) { ATRACE_CALL(); + if (outputState.powerCallback) { + outputState.powerCallback->notifyCpuLoadUp(); + } const Rect& viewport = outputState.layerStackSpace.getContent(); const ui::Dataspace& outputDataspace = outputState.dataspace; const ui::Transform::RotationFlags orientation = @@ -177,18 +180,19 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, TexturePool& te .targetLuminanceNits = outputState.displayBrightnessNits, }; - LayerFE::ClientCompositionTargetSettings targetSettings{ - .clip = Region(viewport), - .needsFiltering = false, - .isSecure = outputState.isSecure, - .supportsProtectedContent = false, - .viewport = viewport, - .dataspace = outputDataspace, - .realContentIsVisible = true, - .clearContent = false, - .blurSetting = LayerFE::ClientCompositionTargetSettings::BlurSetting::Enabled, - .whitePointNits = outputState.displayBrightnessNits, - }; + LayerFE::ClientCompositionTargetSettings + targetSettings{.clip = Region(viewport), + .needsFiltering = false, + .isSecure = outputState.isSecure, + .supportsProtectedContent = false, + .viewport = viewport, + .dataspace = outputDataspace, + .realContentIsVisible = true, + .clearContent = false, + .blurSetting = + LayerFE::ClientCompositionTargetSettings::BlurSetting::Enabled, + .whitePointNits = outputState.displayBrightnessNits, + .treat170mAsSrgb = outputState.treat170mAsSrgb}; std::vector<renderengine::LayerSettings> layerSettings; renderengine::LayerSettings highlight; diff --git a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h index 961ec808e8..f74ef4c60e 100644 --- a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h +++ b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h @@ -34,6 +34,7 @@ public: MOCK_METHOD(void, setExpensiveRenderingExpected, (DisplayId displayId, bool expected), (override)); MOCK_METHOD(bool, isUsingExpensiveRendering, (), (override)); + MOCK_METHOD(void, notifyCpuLoadUp, (), (override)); MOCK_METHOD(void, notifyDisplayUpdateImminentAndCpuReset, (), (override)); MOCK_METHOD(bool, usePowerHintSession, (), (override)); MOCK_METHOD(bool, supportsPowerHintSession, (), (override)); diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp index f8b466c93c..9c7576e76d 100644 --- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp +++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp @@ -138,6 +138,21 @@ void PowerAdvisor::setExpensiveRenderingExpected(DisplayId displayId, bool expec } } +void PowerAdvisor::notifyCpuLoadUp() { + // Only start sending this notification once the system has booted so we don't introduce an + // early-boot dependency on Power HAL + if (!mBootFinished.load()) { + return; + } + if (usePowerHintSession() && ensurePowerHintSessionRunning()) { + std::lock_guard lock(mHintSessionMutex); + auto ret = mHintSession->sendHint(SessionHint::CPU_LOAD_UP); + if (!ret.isOk()) { + mHintSessionRunning = false; + } + } +} + void PowerAdvisor::notifyDisplayUpdateImminentAndCpuReset() { // Only start sending this notification once the system has booted so we don't introduce an // early-boot dependency on Power HAL diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h index f0d3fd8518..cfaa135299 100644 --- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h +++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h @@ -49,6 +49,7 @@ public: virtual void onBootFinished() = 0; virtual void setExpensiveRenderingExpected(DisplayId displayId, bool expected) = 0; virtual bool isUsingExpensiveRendering() = 0; + virtual void notifyCpuLoadUp() = 0; virtual void notifyDisplayUpdateImminentAndCpuReset() = 0; // Checks both if it supports and if it's enabled virtual bool usePowerHintSession() = 0; @@ -108,6 +109,7 @@ public: void onBootFinished() override; void setExpensiveRenderingExpected(DisplayId displayId, bool expected) override; bool isUsingExpensiveRendering() override { return mNotifiedExpensiveRendering; }; + void notifyCpuLoadUp() override; void notifyDisplayUpdateImminentAndCpuReset() override; bool usePowerHintSession() override; bool supportsPowerHintSession() override; diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp index f136e9f9df..80a7a42de3 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp @@ -148,8 +148,8 @@ std::string toString(const RefreshRateSelector::PolicyVariant& policy) { } // namespace auto RefreshRateSelector::createFrameRateModes( - std::function<bool(const DisplayMode&)>&& filterModes, const FpsRange& renderRange) const - -> std::vector<FrameRateMode> { + const Policy& policy, std::function<bool(const DisplayMode&)>&& filterModes, + const FpsRange& renderRange) const -> std::vector<FrameRateMode> { struct Key { Fps fps; int32_t group; @@ -202,11 +202,25 @@ auto RefreshRateSelector::createFrameRateModes( ALOGV("%s: including %s (%s)", __func__, to_string(fps).c_str(), to_string(mode->getFps()).c_str()); } else { - // We might need to update the map as we found a lower refresh rate - if (isStrictlyLess(mode->getFps(), existingIter->second->second->getFps())) { + // If the primary physical range is a single rate, prefer to stay in that rate + // even if there is a lower physical refresh rate available. This would cause more + // cases to stay within the primary physical range + const Fps existingModeFps = existingIter->second->second->getFps(); + const bool existingModeIsPrimaryRange = policy.primaryRangeIsSingleRate() && + policy.primaryRanges.physical.includes(existingModeFps); + const bool newModeIsPrimaryRange = policy.primaryRangeIsSingleRate() && + policy.primaryRanges.physical.includes(mode->getFps()); + if (newModeIsPrimaryRange == existingModeIsPrimaryRange) { + // We might need to update the map as we found a lower refresh rate + if (isStrictlyLess(mode->getFps(), existingModeFps)) { + existingIter->second = it; + ALOGV("%s: changing %s (%s) as we found a lower physical rate", __func__, + to_string(fps).c_str(), to_string(mode->getFps()).c_str()); + } + } else if (newModeIsPrimaryRange) { existingIter->second = it; - ALOGV("%s: changing %s (%s)", __func__, to_string(fps).c_str(), - to_string(mode->getFps()).c_str()); + ALOGV("%s: changing %s (%s) to stay in the primary range", __func__, + to_string(fps).c_str(), to_string(mode->getFps()).c_str()); } } } @@ -487,10 +501,8 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi // If the primary range consists of a single refresh rate then we can only // move out the of range if layers explicitly request a different refresh // rate. - const bool primaryRangeIsSingleRate = - isApproxEqual(policy->primaryRanges.physical.min, policy->primaryRanges.physical.max); - - if (!signals.touch && signals.idle && !(primaryRangeIsSingleRate && hasExplicitVoteLayers)) { + if (!signals.touch && signals.idle && + !(policy->primaryRangeIsSingleRate() && hasExplicitVoteLayers)) { ALOGV("Idle"); const auto ranking = rankFrameRates(activeMode.getGroup(), RefreshRateOrder::Ascending); ATRACE_FORMAT_INSTANT("%s (Idle)", to_string(ranking.front().frameRateMode.fps).c_str()); @@ -564,8 +576,11 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi continue; } - const bool inPrimaryRange = policy->primaryRanges.render.includes(fps); - if ((primaryRangeIsSingleRate || !inPrimaryRange) && + const bool inPrimaryPhysicalRange = + policy->primaryRanges.physical.includes(modePtr->getFps()); + const bool inPrimaryRenderRange = policy->primaryRanges.render.includes(fps); + if (((policy->primaryRangeIsSingleRate() && !inPrimaryPhysicalRange) || + !inPrimaryRenderRange) && !(layer.focused && (layer.vote == LayerVoteType::ExplicitDefault || layer.vote == LayerVoteType::ExplicitExact))) { @@ -676,7 +691,7 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi return score.overallScore == 0; }); - if (primaryRangeIsSingleRate) { + if (policy->primaryRangeIsSingleRate()) { // If we never scored any layers, then choose the rate from the primary // range instead of picking a random score from the app range. if (noLayerScore) { @@ -1221,10 +1236,19 @@ void RefreshRateSelector::constructAvailableRefreshRates() { (supportsFrameRateOverride() || ranges.render.includes(mode.getFps())); }; - const auto frameRateModes = createFrameRateModes(filterModes, ranges.render); + auto frameRateModes = createFrameRateModes(*policy, filterModes, ranges.render); + if (frameRateModes.empty()) { + ALOGW("No matching frame rate modes for %s range. policy: %s", rangeName, + policy->toString().c_str()); + // TODO(b/292105422): Ideally DisplayManager should not send render ranges smaller than + // the min supported. See b/292047939. + // For not we just ignore the render ranges. + frameRateModes = createFrameRateModes(*policy, filterModes, {}); + } LOG_ALWAYS_FATAL_IF(frameRateModes.empty(), - "No matching frame rate modes for %s range. policy: %s", rangeName, - policy->toString().c_str()); + "No matching frame rate modes for %s range even after ignoring the " + "render range. policy: %s", + rangeName, policy->toString().c_str()); const auto stringifyModes = [&] { std::string str; diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h index 5052e6e257..3f70c66c6e 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h @@ -100,6 +100,11 @@ public: } bool operator!=(const Policy& other) const { return !(*this == other); } + + bool primaryRangeIsSingleRate() const { + return isApproxEqual(primaryRanges.physical.min, primaryRanges.physical.max); + } + std::string toString() const; }; @@ -467,8 +472,8 @@ private: } std::vector<FrameRateMode> createFrameRateModes( - std::function<bool(const DisplayMode&)>&& filterModes, const FpsRange&) const - REQUIRES(mLock); + const Policy&, std::function<bool(const DisplayMode&)>&& filterModes, + const FpsRange&) const REQUIRES(mLock); // The display modes of the active display. The DisplayModeIterators below are pointers into // this container, so must be invalidated whenever the DisplayModes change. The Policy below diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index 09dac23410..ee87687eea 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -37,6 +37,7 @@ std::shared_ptr<ScreenCaptureOutput> createScreenCaptureOutput(ScreenCaptureOutp output->setRenderSurface(std::make_unique<ScreenCaptureRenderSurface>(std::move(args.buffer))); output->setDisplayBrightness(args.sdrWhitePointNits, args.displayBrightnessNits); output->editState().clientTargetBrightness = args.targetBrightness; + output->editState().treat170mAsSrgb = args.treat170mAsSrgb; output->setDisplayColorProfile(std::make_unique<compositionengine::impl::DisplayColorProfile>( compositionengine::DisplayColorProfileCreationArgsBuilder() diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h index 3c307b0733..159c2bf903 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.h +++ b/services/surfaceflinger/ScreenCaptureOutput.h @@ -36,6 +36,7 @@ struct ScreenCaptureOutputArgs { // Counterintuitively, when targetBrightness > 1.0 then dim the scene. float targetBrightness; bool regionSampling; + bool treat170mAsSrgb; }; // ScreenCaptureOutput is used to compose a set of layers into a preallocated buffer. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e90b66ec8d..3fa93c4fda 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2568,6 +2568,7 @@ void SurfaceFlinger::composite(TimePoint frameTime, VsyncId vsyncId) ATRACE_FORMAT("%s %" PRId64, __func__, vsyncId.value); compositionengine::CompositionRefreshArgs refreshArgs; + refreshArgs.powerCallback = this; const auto& displays = FTL_FAKE_GUARD(mStateLock, mDisplays); refreshArgs.outputs.reserve(displays.size()); std::vector<DisplayId> displayIds; @@ -3926,6 +3927,10 @@ void SurfaceFlinger::triggerOnFrameRateOverridesChanged() { mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId); } +void SurfaceFlinger::notifyCpuLoadUp() { + mPowerAdvisor->notifyCpuLoadUp(); +} + void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { using namespace scheduler; @@ -6158,7 +6163,8 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& comp windowInfosDebug.maxSendDelayVsyncId.value); StringAppendF(&result, " max send delay (ns): %" PRId64 " ns\n", windowInfosDebug.maxSendDelayDuration); - StringAppendF(&result, " unsent messages: %zu\n", windowInfosDebug.pendingMessageCount); + StringAppendF(&result, " unsent messages: %" PRIu32 "\n", + windowInfosDebug.pendingMessageCount); result.append("\n"); } @@ -7418,7 +7424,10 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( renderArea->getHintForSeamlessTransition()); sdrWhitePointNits = state.sdrWhitePointNits; displayBrightnessNits = state.displayBrightnessNits; - if (sdrWhitePointNits > 1.0f) { + // Only clamp the display brightness if this is not a seamless transition. Otherwise + // for seamless transitions it's important to match the current display state as the + // buffer will be shown under these same conditions, and we want to avoid any flickers + if (sdrWhitePointNits > 1.0f && !renderArea->getHintForSeamlessTransition()) { // Restrict the amount of HDR "headroom" in the screenshot to avoid over-dimming // the SDR portion. 2.0 chosen by experimentation constexpr float kMaxScreenshotHeadroom = 2.0f; @@ -7479,7 +7488,8 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( .sdrWhitePointNits = sdrWhitePointNits, .displayBrightnessNits = displayBrightnessNits, .targetBrightness = targetBrightness, - .regionSampling = regionSampling}); + .regionSampling = regionSampling, + .treat170mAsSrgb = mTreat170mAsSrgb}); const float colorSaturation = grayscale ? 0 : 1; compositionengine::CompositionRefreshArgs refreshArgs{ @@ -7991,9 +8001,9 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD forceApplyPolicy); } -status_t SurfaceFlinger::addWindowInfosListener(const sp<IWindowInfosListener>& windowInfosListener, - gui::WindowInfosListenerInfo* outInfo) { - mWindowInfosListenerInvoker->addWindowInfosListener(windowInfosListener, outInfo); +status_t SurfaceFlinger::addWindowInfosListener( + const sp<IWindowInfosListener>& windowInfosListener) { + mWindowInfosListenerInvoker->addWindowInfosListener(windowInfosListener); setTransactionFlags(eInputInfoUpdateNeeded); return NO_ERROR; } @@ -9075,8 +9085,7 @@ binder::Status SurfaceComposerAIDL::getMaxAcquiredBufferCount(int32_t* buffers) } binder::Status SurfaceComposerAIDL::addWindowInfosListener( - const sp<gui::IWindowInfosListener>& windowInfosListener, - gui::WindowInfosListenerInfo* outInfo) { + const sp<gui::IWindowInfosListener>& windowInfosListener) { status_t status; const int pid = IPCThreadState::self()->getCallingPid(); const int uid = IPCThreadState::self()->getCallingUid(); @@ -9084,7 +9093,7 @@ binder::Status SurfaceComposerAIDL::addWindowInfosListener( // WindowInfosListeners if (uid == AID_SYSTEM || uid == AID_GRAPHICS || checkPermission(sAccessSurfaceFlinger, pid, uid)) { - status = mFlinger->addWindowInfosListener(windowInfosListener, outInfo); + status = mFlinger->addWindowInfosListener(windowInfosListener); } else { status = PERMISSION_DENIED; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index aa2f074191..c196285e7e 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -194,7 +194,8 @@ class SurfaceFlinger : public BnSurfaceComposer, private IBinder::DeathRecipient, private HWC2::ComposerCallback, private ICompositor, - private scheduler::ISchedulerCallback { + private scheduler::ISchedulerCallback, + private compositionengine::ICEPowerCallback { public: struct SkipInitializationTag {}; @@ -612,8 +613,7 @@ private: status_t getMaxAcquiredBufferCount(int* buffers) const; - status_t addWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener, - gui::WindowInfosListenerInfo* outResult); + status_t addWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener); status_t removeWindowInfosListener( const sp<gui::IWindowInfosListener>& windowInfosListener) const; @@ -646,6 +646,9 @@ private: void kernelTimerChanged(bool expired) override; void triggerOnFrameRateOverridesChanged() override; + // ICEPowerCallback overrides: + void notifyCpuLoadUp() override; + // Toggles the kernel idle timer on or off depending the policy decisions around refresh rates. void toggleKernelIdleTimer() REQUIRES(mStateLock); @@ -1557,8 +1560,8 @@ public: binder::Status setOverrideFrameRate(int32_t uid, float frameRate) override; binder::Status getGpuContextPriority(int32_t* outPriority) override; binder::Status getMaxAcquiredBufferCount(int32_t* buffers) override; - binder::Status addWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener, - gui::WindowInfosListenerInfo* outInfo) override; + binder::Status addWindowInfosListener( + const sp<gui::IWindowInfosListener>& windowInfosListener) override; binder::Status removeWindowInfosListener( const sp<gui::IWindowInfosListener>& windowInfosListener) override; diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.cpp b/services/surfaceflinger/WindowInfosListenerInvoker.cpp index 7062a4e3a7..20699ef123 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.cpp +++ b/services/surfaceflinger/WindowInfosListenerInvoker.cpp @@ -14,9 +14,7 @@ * limitations under the License. */ -#include <android/gui/BnWindowInfosPublisher.h> -#include <android/gui/IWindowInfosPublisher.h> -#include <android/gui/WindowInfosListenerInfo.h> +#include <ftl/small_vector.h> #include <gui/ISurfaceComposer.h> #include <gui/TraceUtils.h> #include <gui/WindowInfosUpdate.h> @@ -25,130 +23,162 @@ #include "BackgroundExecutor.h" #include "WindowInfosListenerInvoker.h" -#undef ATRACE_TAG -#define ATRACE_TAG ATRACE_TAG_GRAPHICS - namespace android { using gui::DisplayInfo; using gui::IWindowInfosListener; using gui::WindowInfo; -void WindowInfosListenerInvoker::addWindowInfosListener(sp<IWindowInfosListener> listener, - gui::WindowInfosListenerInfo* outInfo) { - int64_t listenerId = mNextListenerId++; - outInfo->listenerId = listenerId; - outInfo->windowInfosPublisher = sp<gui::IWindowInfosPublisher>::fromExisting(this); +using WindowInfosListenerVector = ftl::SmallVector<const sp<gui::IWindowInfosListener>, 3>; + +struct WindowInfosReportedListenerInvoker : gui::BnWindowInfosReportedListener, + IBinder::DeathRecipient { + WindowInfosReportedListenerInvoker(WindowInfosListenerVector windowInfosListeners, + WindowInfosReportedListenerSet windowInfosReportedListeners) + : mCallbacksPending(windowInfosListeners.size()), + mWindowInfosListeners(std::move(windowInfosListeners)), + mWindowInfosReportedListeners(std::move(windowInfosReportedListeners)) {} - BackgroundExecutor::getInstance().sendCallbacks( - {[this, listener = std::move(listener), listenerId]() { - ATRACE_NAME("WindowInfosListenerInvoker::addWindowInfosListener"); + binder::Status onWindowInfosReported() override { + if (--mCallbacksPending == 0) { + for (const auto& listener : mWindowInfosReportedListeners) { sp<IBinder> asBinder = IInterface::asBinder(listener); - asBinder->linkToDeath(sp<DeathRecipient>::fromExisting(this)); - mWindowInfosListeners.try_emplace(asBinder, - std::make_pair(listenerId, std::move(listener))); - }}); + if (asBinder->isBinderAlive()) { + listener->onWindowInfosReported(); + } + } + + auto wpThis = wp<WindowInfosReportedListenerInvoker>::fromExisting(this); + for (const auto& listener : mWindowInfosListeners) { + sp<IBinder> binder = IInterface::asBinder(listener); + binder->unlinkToDeath(wpThis); + } + } + return binder::Status::ok(); + } + + void binderDied(const wp<IBinder>&) { onWindowInfosReported(); } + +private: + std::atomic<size_t> mCallbacksPending; + static constexpr size_t kStaticCapacity = 3; + const WindowInfosListenerVector mWindowInfosListeners; + WindowInfosReportedListenerSet mWindowInfosReportedListeners; +}; + +void WindowInfosListenerInvoker::addWindowInfosListener(sp<IWindowInfosListener> listener) { + sp<IBinder> asBinder = IInterface::asBinder(listener); + asBinder->linkToDeath(sp<DeathRecipient>::fromExisting(this)); + + std::scoped_lock lock(mListenersMutex); + mWindowInfosListeners.try_emplace(asBinder, std::move(listener)); } void WindowInfosListenerInvoker::removeWindowInfosListener( const sp<IWindowInfosListener>& listener) { - BackgroundExecutor::getInstance().sendCallbacks({[this, listener]() { - ATRACE_NAME("WindowInfosListenerInvoker::removeWindowInfosListener"); - sp<IBinder> asBinder = IInterface::asBinder(listener); - asBinder->unlinkToDeath(sp<DeathRecipient>::fromExisting(this)); - mWindowInfosListeners.erase(asBinder); - }}); + sp<IBinder> asBinder = IInterface::asBinder(listener); + + std::scoped_lock lock(mListenersMutex); + asBinder->unlinkToDeath(sp<DeathRecipient>::fromExisting(this)); + mWindowInfosListeners.erase(asBinder); } void WindowInfosListenerInvoker::binderDied(const wp<IBinder>& who) { - BackgroundExecutor::getInstance().sendCallbacks({[this, who]() { - ATRACE_NAME("WindowInfosListenerInvoker::binderDied"); - auto it = mWindowInfosListeners.find(who); - int64_t listenerId = it->second.first; - mWindowInfosListeners.erase(who); - - std::vector<int64_t> vsyncIds; - for (auto& [vsyncId, state] : mUnackedState) { - if (std::find(state.unackedListenerIds.begin(), state.unackedListenerIds.end(), - listenerId) != state.unackedListenerIds.end()) { - vsyncIds.push_back(vsyncId); - } - } - - for (int64_t vsyncId : vsyncIds) { - ackWindowInfosReceived(vsyncId, listenerId); - } - }}); + std::scoped_lock lock(mListenersMutex); + mWindowInfosListeners.erase(who); } void WindowInfosListenerInvoker::windowInfosChanged( gui::WindowInfosUpdate update, WindowInfosReportedListenerSet reportedListeners, bool forceImmediateCall) { - if (!mDelayInfo) { - mDelayInfo = DelayInfo{ - .vsyncId = update.vsyncId, - .frameTime = update.timestamp, - }; - } + WindowInfosListenerVector listeners; + { + std::scoped_lock lock{mMessagesMutex}; + + if (!mDelayInfo) { + mDelayInfo = DelayInfo{ + .vsyncId = update.vsyncId, + .frameTime = update.timestamp, + }; + } - // If there are unacked messages and this isn't a forced call, then return immediately. - // If a forced window infos change doesn't happen first, the update will be sent after - // the WindowInfosReportedListeners are called. If a forced window infos change happens or - // if there are subsequent delayed messages before this update is sent, then this message - // will be dropped and the listeners will only be called with the latest info. This is done - // to reduce the amount of binder memory used. - if (!mUnackedState.empty() && !forceImmediateCall) { - mDelayedUpdate = std::move(update); - mReportedListeners.merge(reportedListeners); - return; - } + // If there are unacked messages and this isn't a forced call, then return immediately. + // If a forced window infos change doesn't happen first, the update will be sent after + // the WindowInfosReportedListeners are called. If a forced window infos change happens or + // if there are subsequent delayed messages before this update is sent, then this message + // will be dropped and the listeners will only be called with the latest info. This is done + // to reduce the amount of binder memory used. + if (mActiveMessageCount > 0 && !forceImmediateCall) { + mDelayedUpdate = std::move(update); + mReportedListeners.merge(reportedListeners); + return; + } - if (mDelayedUpdate) { - mDelayedUpdate.reset(); - } + if (mDelayedUpdate) { + mDelayedUpdate.reset(); + } + + { + std::scoped_lock lock{mListenersMutex}; + for (const auto& [_, listener] : mWindowInfosListeners) { + listeners.push_back(listener); + } + } + if (CC_UNLIKELY(listeners.empty())) { + mReportedListeners.merge(reportedListeners); + mDelayInfo.reset(); + return; + } + + reportedListeners.insert(sp<WindowInfosListenerInvoker>::fromExisting(this)); + reportedListeners.merge(mReportedListeners); + mReportedListeners.clear(); - if (CC_UNLIKELY(mWindowInfosListeners.empty())) { - mReportedListeners.merge(reportedListeners); + mActiveMessageCount++; + updateMaxSendDelay(); mDelayInfo.reset(); - return; } - reportedListeners.merge(mReportedListeners); - mReportedListeners.clear(); - - // Update mUnackedState to include the message we're about to send - auto [it, _] = mUnackedState.try_emplace(update.vsyncId, - UnackedState{.reportedListeners = - std::move(reportedListeners)}); - auto& unackedState = it->second; - for (auto& pair : mWindowInfosListeners) { - int64_t listenerId = pair.second.first; - unackedState.unackedListenerIds.push_back(listenerId); - } + auto reportedInvoker = + sp<WindowInfosReportedListenerInvoker>::make(listeners, std::move(reportedListeners)); - mDelayInfo.reset(); - updateMaxSendDelay(); + for (const auto& listener : listeners) { + sp<IBinder> asBinder = IInterface::asBinder(listener); - // Call the listeners - for (auto& pair : mWindowInfosListeners) { - auto& [listenerId, listener] = pair.second; - auto status = listener->onWindowInfosChanged(update); + // linkToDeath is used here to ensure that the windowInfosReportedListeners + // are called even if one of the windowInfosListeners dies before + // calling onWindowInfosReported. + asBinder->linkToDeath(reportedInvoker); + + auto status = listener->onWindowInfosChanged(update, reportedInvoker); if (!status.isOk()) { - ackWindowInfosReceived(update.vsyncId, listenerId); + reportedInvoker->onWindowInfosReported(); } } } -WindowInfosListenerInvoker::DebugInfo WindowInfosListenerInvoker::getDebugInfo() { - DebugInfo result; - BackgroundExecutor::getInstance().sendCallbacks({[&, this]() { - ATRACE_NAME("WindowInfosListenerInvoker::getDebugInfo"); - updateMaxSendDelay(); - result = mDebugInfo; - result.pendingMessageCount = mUnackedState.size(); +binder::Status WindowInfosListenerInvoker::onWindowInfosReported() { + BackgroundExecutor::getInstance().sendCallbacks({[this]() { + gui::WindowInfosUpdate update; + { + std::scoped_lock lock{mMessagesMutex}; + mActiveMessageCount--; + if (!mDelayedUpdate || mActiveMessageCount > 0) { + return; + } + update = std::move(*mDelayedUpdate); + mDelayedUpdate.reset(); + } + windowInfosChanged(std::move(update), {}, false); }}); - BackgroundExecutor::getInstance().flushQueue(); - return result; + return binder::Status::ok(); +} + +WindowInfosListenerInvoker::DebugInfo WindowInfosListenerInvoker::getDebugInfo() { + std::scoped_lock lock{mMessagesMutex}; + updateMaxSendDelay(); + mDebugInfo.pendingMessageCount = mActiveMessageCount; + return mDebugInfo; } void WindowInfosListenerInvoker::updateMaxSendDelay() { @@ -162,41 +192,4 @@ void WindowInfosListenerInvoker::updateMaxSendDelay() { } } -binder::Status WindowInfosListenerInvoker::ackWindowInfosReceived(int64_t vsyncId, - int64_t listenerId) { - BackgroundExecutor::getInstance().sendCallbacks({[this, vsyncId, listenerId]() { - ATRACE_NAME("WindowInfosListenerInvoker::ackWindowInfosReceived"); - auto it = mUnackedState.find(vsyncId); - if (it == mUnackedState.end()) { - return; - } - - auto& state = it->second; - state.unackedListenerIds.unstable_erase(std::find(state.unackedListenerIds.begin(), - state.unackedListenerIds.end(), - listenerId)); - if (!state.unackedListenerIds.empty()) { - return; - } - - WindowInfosReportedListenerSet reportedListeners{std::move(state.reportedListeners)}; - mUnackedState.erase(vsyncId); - - for (const auto& reportedListener : reportedListeners) { - sp<IBinder> asBinder = IInterface::asBinder(reportedListener); - if (asBinder->isBinderAlive()) { - reportedListener->onWindowInfosReported(); - } - } - - if (!mDelayedUpdate || !mUnackedState.empty()) { - return; - } - gui::WindowInfosUpdate update{std::move(*mDelayedUpdate)}; - mDelayedUpdate.reset(); - windowInfosChanged(std::move(update), {}, false); - }}); - return binder::Status::ok(); -} - } // namespace android diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.h b/services/surfaceflinger/WindowInfosListenerInvoker.h index f36b0edd7d..bc465a3a2b 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.h +++ b/services/surfaceflinger/WindowInfosListenerInvoker.h @@ -19,12 +19,11 @@ #include <optional> #include <unordered_set> -#include <android/gui/BnWindowInfosPublisher.h> +#include <android/gui/BnWindowInfosReportedListener.h> #include <android/gui/IWindowInfosListener.h> #include <android/gui/IWindowInfosReportedListener.h> #include <binder/IBinder.h> #include <ftl/small_map.h> -#include <ftl/small_vector.h> #include <gui/SpHash.h> #include <utils/Mutex.h> @@ -36,22 +35,22 @@ using WindowInfosReportedListenerSet = std::unordered_set<sp<gui::IWindowInfosReportedListener>, gui::SpHash<gui::IWindowInfosReportedListener>>; -class WindowInfosListenerInvoker : public gui::BnWindowInfosPublisher, +class WindowInfosListenerInvoker : public gui::BnWindowInfosReportedListener, public IBinder::DeathRecipient { public: - void addWindowInfosListener(sp<gui::IWindowInfosListener>, gui::WindowInfosListenerInfo*); + void addWindowInfosListener(sp<gui::IWindowInfosListener>); void removeWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener); void windowInfosChanged(gui::WindowInfosUpdate update, WindowInfosReportedListenerSet windowInfosReportedListeners, bool forceImmediateCall); - binder::Status ackWindowInfosReceived(int64_t, int64_t) override; + binder::Status onWindowInfosReported() override; struct DebugInfo { VsyncId maxSendDelayVsyncId; nsecs_t maxSendDelayDuration; - size_t pendingMessageCount; + uint32_t pendingMessageCount; }; DebugInfo getDebugInfo(); @@ -59,28 +58,24 @@ protected: void binderDied(const wp<IBinder>& who) override; private: + std::mutex mListenersMutex; + static constexpr size_t kStaticCapacity = 3; - std::atomic<int64_t> mNextListenerId{0}; - ftl::SmallMap<wp<IBinder>, const std::pair<int64_t, sp<gui::IWindowInfosListener>>, - kStaticCapacity> - mWindowInfosListeners; + ftl::SmallMap<wp<IBinder>, const sp<gui::IWindowInfosListener>, kStaticCapacity> + mWindowInfosListeners GUARDED_BY(mListenersMutex); - std::optional<gui::WindowInfosUpdate> mDelayedUpdate; + std::mutex mMessagesMutex; + uint32_t mActiveMessageCount GUARDED_BY(mMessagesMutex) = 0; + std::optional<gui::WindowInfosUpdate> mDelayedUpdate GUARDED_BY(mMessagesMutex); WindowInfosReportedListenerSet mReportedListeners; - struct UnackedState { - ftl::SmallVector<int64_t, kStaticCapacity> unackedListenerIds; - WindowInfosReportedListenerSet reportedListeners; - }; - ftl::SmallMap<int64_t /* vsyncId */, UnackedState, 5> mUnackedState; - - DebugInfo mDebugInfo; + DebugInfo mDebugInfo GUARDED_BY(mMessagesMutex); struct DelayInfo { int64_t vsyncId; nsecs_t frameTime; }; - std::optional<DelayInfo> mDelayInfo; - void updateMaxSendDelay(); + std::optional<DelayInfo> mDelayInfo GUARDED_BY(mMessagesMutex); + void updateMaxSendDelay() REQUIRES(mMessagesMutex); }; } // namespace android diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp index 60ad7a3a03..2d87ddd488 100644 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp @@ -78,7 +78,7 @@ TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) { mDisplay->setDesiredActiveMode( {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None})); ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode()); - EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt); + EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt); EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event); // Setting another mode should be cached but return None @@ -86,7 +86,7 @@ TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) { mDisplay->setDesiredActiveMode( {scheduler::FrameRateMode{120_Hz, kMode120}, Event::None})); ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode()); - EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, mDisplay->getDesiredActiveMode()->modeOpt); + EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt); EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event); } @@ -105,7 +105,7 @@ TEST_F(InitiateModeChangeTest, initiateModeChange) NO_THREAD_SAFETY_ANALYSIS { mDisplay->setDesiredActiveMode( {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None})); ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode()); - EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt); + EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt); EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event); hal::VsyncPeriodChangeConstraints constraints{ @@ -136,7 +136,7 @@ NO_THREAD_SAFETY_ANALYSIS { mDisplay->setDesiredActiveMode( {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None})); ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode()); - EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt); + EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt); EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event); hal::VsyncPeriodChangeConstraints constraints{ @@ -154,7 +154,7 @@ NO_THREAD_SAFETY_ANALYSIS { mDisplay->setDesiredActiveMode( {scheduler::FrameRateMode{120_Hz, kMode120}, Event::None})); ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode()); - EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, mDisplay->getDesiredActiveMode()->modeOpt); + EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt); EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event); EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getUpcomingActiveMode().modeOpt); diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp index d63e187ac4..49e7a4ef14 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp @@ -3042,5 +3042,84 @@ TEST_P(RefreshRateSelectorTest, frameRateNotInRange) { EXPECT_FRAME_RATE_MODE(kMode60, 60_Hz, selector.getBestScoredFrameRate(layers).frameRateMode); } +TEST_P(RefreshRateSelectorTest, frameRateIsLowerThanMinSupported) { + if (GetParam() != Config::FrameRateOverride::Enabled) { + return; + } + + auto selector = createSelector(kModes_60_90, kModeId60); + + constexpr Fps kMin = RefreshRateSelector::kMinSupportedFrameRate; + constexpr FpsRanges kLowerThanMin = {{60_Hz, 90_Hz}, {kMin / 2, kMin / 2}}; + + EXPECT_EQ(SetPolicyResult::Changed, + selector.setDisplayManagerPolicy( + {DisplayModeId(kModeId60), kLowerThanMin, kLowerThanMin})); +} + +// b/296079213 +TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_120) { + auto selector = createSelector(kModes_60_120, kModeId120); + + const FpsRange only120 = {120_Hz, 120_Hz}; + const FpsRange allRange = {0_Hz, 120_Hz}; + EXPECT_EQ(SetPolicyResult::Changed, + selector.setDisplayManagerPolicy( + {kModeId120, {only120, allRange}, {allRange, allRange}})); + + std::vector<LayerRequirement> layers = {{.weight = 1.f}}; + layers[0].name = "30Hz ExplicitExactOrMultiple"; + layers[0].desiredRefreshRate = 30_Hz; + layers[0].vote = LayerVoteType::ExplicitExactOrMultiple; + + if (GetParam() != Config::FrameRateOverride::Enabled) { + EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, + selector.getBestScoredFrameRate(layers).frameRateMode); + } else { + EXPECT_FRAME_RATE_MODE(kMode120, 30_Hz, + selector.getBestScoredFrameRate(layers).frameRateMode); + } +} + +TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_90) { + auto selector = createSelector(kModes_60_90, kModeId90); + + const FpsRange only90 = {90_Hz, 90_Hz}; + const FpsRange allRange = {0_Hz, 90_Hz}; + EXPECT_EQ(SetPolicyResult::Changed, + selector.setDisplayManagerPolicy( + {kModeId90, {only90, allRange}, {allRange, allRange}})); + + std::vector<LayerRequirement> layers = {{.weight = 1.f}}; + layers[0].name = "30Hz ExplicitExactOrMultiple"; + layers[0].desiredRefreshRate = 30_Hz; + layers[0].vote = LayerVoteType::ExplicitExactOrMultiple; + + if (GetParam() != Config::FrameRateOverride::Enabled) { + EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, + selector.getBestScoredFrameRate(layers).frameRateMode); + } else { + EXPECT_FRAME_RATE_MODE(kMode90, 30_Hz, + selector.getBestScoredFrameRate(layers).frameRateMode); + } +} + +TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_90_NonDivisor) { + auto selector = createSelector(kModes_60_90, kModeId90); + + const FpsRange only90 = {90_Hz, 90_Hz}; + const FpsRange allRange = {0_Hz, 90_Hz}; + EXPECT_EQ(SetPolicyResult::Changed, + selector.setDisplayManagerPolicy( + {kModeId90, {only90, allRange}, {allRange, allRange}})); + + std::vector<LayerRequirement> layers = {{.weight = 1.f}}; + layers[0].name = "60Hz ExplicitExactOrMultiple"; + layers[0].desiredRefreshRate = 60_Hz; + layers[0].vote = LayerVoteType::ExplicitExactOrMultiple; + + EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, selector.getBestScoredFrameRate(layers).frameRateMode); +} + } // namespace } // namespace android::scheduler diff --git a/services/surfaceflinger/tests/unittests/WindowInfosListenerInvokerTest.cpp b/services/surfaceflinger/tests/unittests/WindowInfosListenerInvokerTest.cpp index c7b845e668..af4971b063 100644 --- a/services/surfaceflinger/tests/unittests/WindowInfosListenerInvokerTest.cpp +++ b/services/surfaceflinger/tests/unittests/WindowInfosListenerInvokerTest.cpp @@ -15,23 +15,35 @@ protected: WindowInfosListenerInvokerTest() : mInvoker(sp<WindowInfosListenerInvoker>::make()) {} ~WindowInfosListenerInvokerTest() { + std::mutex mutex; + std::condition_variable cv; + bool flushComplete = false; // Flush the BackgroundExecutor thread to ensure any scheduled tasks are complete. // Otherwise, references those tasks hold may go out of scope before they are done // executing. - BackgroundExecutor::getInstance().flushQueue(); + BackgroundExecutor::getInstance().sendCallbacks({[&]() { + std::scoped_lock lock{mutex}; + flushComplete = true; + cv.notify_one(); + }}); + std::unique_lock<std::mutex> lock{mutex}; + cv.wait(lock, [&]() { return flushComplete; }); } sp<WindowInfosListenerInvoker> mInvoker; }; -using WindowInfosUpdateConsumer = std::function<void(const gui::WindowInfosUpdate&)>; +using WindowInfosUpdateConsumer = std::function<void(const gui::WindowInfosUpdate&, + const sp<gui::IWindowInfosReportedListener>&)>; class Listener : public gui::BnWindowInfosListener { public: Listener(WindowInfosUpdateConsumer consumer) : mConsumer(std::move(consumer)) {} - binder::Status onWindowInfosChanged(const gui::WindowInfosUpdate& update) override { - mConsumer(update); + binder::Status onWindowInfosChanged( + const gui::WindowInfosUpdate& update, + const sp<gui::IWindowInfosReportedListener>& reportedListener) override { + mConsumer(update, reportedListener); return binder::Status::ok(); } @@ -46,17 +58,15 @@ TEST_F(WindowInfosListenerInvokerTest, callsSingleListener) { int callCount = 0; - gui::WindowInfosListenerInfo listenerInfo; - mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate& update) { - std::scoped_lock lock{mutex}; - callCount++; - cv.notify_one(); + mInvoker->addWindowInfosListener( + sp<Listener>::make([&](const gui::WindowInfosUpdate&, + const sp<gui::IWindowInfosReportedListener>& reportedListener) { + std::scoped_lock lock{mutex}; + callCount++; + cv.notify_one(); - listenerInfo.windowInfosPublisher - ->ackWindowInfosReceived(update.vsyncId, - listenerInfo.listenerId); - }), - &listenerInfo); + reportedListener->onWindowInfosReported(); + })); BackgroundExecutor::getInstance().sendCallbacks( {[this]() { mInvoker->windowInfosChanged({}, {}, false); }}); @@ -71,27 +81,21 @@ TEST_F(WindowInfosListenerInvokerTest, callsMultipleListeners) { std::mutex mutex; std::condition_variable cv; - size_t callCount = 0; - const size_t expectedCallCount = 3; - std::vector<gui::WindowInfosListenerInfo> listenerInfos{expectedCallCount, - gui::WindowInfosListenerInfo{}}; - - for (size_t i = 0; i < expectedCallCount; i++) { - mInvoker->addWindowInfosListener(sp<Listener>::make([&, &listenerInfo = listenerInfos[i]]( - const gui::WindowInfosUpdate& - update) { - std::scoped_lock lock{mutex}; - callCount++; - if (callCount == expectedCallCount) { - cv.notify_one(); - } - - listenerInfo.windowInfosPublisher - ->ackWindowInfosReceived(update.vsyncId, - listenerInfo - .listenerId); - }), - &listenerInfos[i]); + int callCount = 0; + const int expectedCallCount = 3; + + for (int i = 0; i < expectedCallCount; i++) { + mInvoker->addWindowInfosListener(sp<Listener>::make( + [&](const gui::WindowInfosUpdate&, + const sp<gui::IWindowInfosReportedListener>& reportedListener) { + std::scoped_lock lock{mutex}; + callCount++; + if (callCount == expectedCallCount) { + cv.notify_one(); + } + + reportedListener->onWindowInfosReported(); + })); } BackgroundExecutor::getInstance().sendCallbacks( @@ -110,20 +114,17 @@ TEST_F(WindowInfosListenerInvokerTest, delaysUnackedCall) { int callCount = 0; - // Simulate a slow ack by not calling IWindowInfosPublisher.ackWindowInfosReceived - gui::WindowInfosListenerInfo listenerInfo; - mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate&) { - std::scoped_lock lock{mutex}; - callCount++; - cv.notify_one(); - }), - &listenerInfo); + // Simulate a slow ack by not calling the WindowInfosReportedListener. + mInvoker->addWindowInfosListener(sp<Listener>::make( + [&](const gui::WindowInfosUpdate&, const sp<gui::IWindowInfosReportedListener>&) { + std::scoped_lock lock{mutex}; + callCount++; + cv.notify_one(); + })); BackgroundExecutor::getInstance().sendCallbacks({[&]() { - mInvoker->windowInfosChanged(gui::WindowInfosUpdate{{}, {}, /* vsyncId= */ 0, 0}, {}, - false); - mInvoker->windowInfosChanged(gui::WindowInfosUpdate{{}, {}, /* vsyncId= */ 1, 0}, {}, - false); + mInvoker->windowInfosChanged({}, {}, false); + mInvoker->windowInfosChanged({}, {}, false); }}); { @@ -133,7 +134,7 @@ TEST_F(WindowInfosListenerInvokerTest, delaysUnackedCall) { EXPECT_EQ(callCount, 1); // Ack the first message. - listenerInfo.windowInfosPublisher->ackWindowInfosReceived(0, listenerInfo.listenerId); + mInvoker->onWindowInfosReported(); { std::unique_lock lock{mutex}; @@ -151,21 +152,19 @@ TEST_F(WindowInfosListenerInvokerTest, sendsForcedMessage) { int callCount = 0; const int expectedCallCount = 2; - // Simulate a slow ack by not calling IWindowInfosPublisher.ackWindowInfosReceived - gui::WindowInfosListenerInfo listenerInfo; - mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate&) { - std::scoped_lock lock{mutex}; - callCount++; - if (callCount == expectedCallCount) { - cv.notify_one(); - } - }), - &listenerInfo); + // Simulate a slow ack by not calling the WindowInfosReportedListener. + mInvoker->addWindowInfosListener(sp<Listener>::make( + [&](const gui::WindowInfosUpdate&, const sp<gui::IWindowInfosReportedListener>&) { + std::scoped_lock lock{mutex}; + callCount++; + if (callCount == expectedCallCount) { + cv.notify_one(); + } + })); BackgroundExecutor::getInstance().sendCallbacks({[&]() { - mInvoker->windowInfosChanged(gui::WindowInfosUpdate{{}, {}, /* vsyncId= */ 0, 0}, {}, - false); - mInvoker->windowInfosChanged(gui::WindowInfosUpdate{{}, {}, /* vsyncId= */ 1, 0}, {}, true); + mInvoker->windowInfosChanged({}, {}, false); + mInvoker->windowInfosChanged({}, {}, true); }}); { @@ -183,14 +182,14 @@ TEST_F(WindowInfosListenerInvokerTest, skipsDelayedMessage) { int64_t lastUpdateId = -1; - // Simulate a slow ack by not calling IWindowInfosPublisher.ackWindowInfosReceived - gui::WindowInfosListenerInfo listenerInfo; - mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate& update) { - std::scoped_lock lock{mutex}; - lastUpdateId = update.vsyncId; - cv.notify_one(); - }), - &listenerInfo); + // Simulate a slow ack by not calling the WindowInfosReportedListener. + mInvoker->addWindowInfosListener( + sp<Listener>::make([&](const gui::WindowInfosUpdate& update, + const sp<gui::IWindowInfosReportedListener>&) { + std::scoped_lock lock{mutex}; + lastUpdateId = update.vsyncId; + cv.notify_one(); + })); BackgroundExecutor::getInstance().sendCallbacks({[&]() { mInvoker->windowInfosChanged({{}, {}, /* vsyncId= */ 1, 0}, {}, false); @@ -205,7 +204,7 @@ TEST_F(WindowInfosListenerInvokerTest, skipsDelayedMessage) { EXPECT_EQ(lastUpdateId, 1); // Ack the first message. The third update should be sent. - listenerInfo.windowInfosPublisher->ackWindowInfosReceived(1, listenerInfo.listenerId); + mInvoker->onWindowInfosReported(); { std::unique_lock lock{mutex}; @@ -226,17 +225,14 @@ TEST_F(WindowInfosListenerInvokerTest, noListeners) { // delayed. BackgroundExecutor::getInstance().sendCallbacks({[&]() { mInvoker->windowInfosChanged({}, {}, false); - gui::WindowInfosListenerInfo listenerInfo; - mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate&) { - std::scoped_lock lock{mutex}; - callCount++; - cv.notify_one(); - }), - &listenerInfo); + mInvoker->addWindowInfosListener(sp<Listener>::make( + [&](const gui::WindowInfosUpdate&, const sp<gui::IWindowInfosReportedListener>&) { + std::scoped_lock lock{mutex}; + callCount++; + cv.notify_one(); + })); + mInvoker->windowInfosChanged({}, {}, false); }}); - BackgroundExecutor::getInstance().flushQueue(); - BackgroundExecutor::getInstance().sendCallbacks( - {[&]() { mInvoker->windowInfosChanged({}, {}, false); }}); { std::unique_lock lock{mutex}; diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h index 3caa2b9847..d635508b5e 100644 --- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h +++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h @@ -32,6 +32,7 @@ public: MOCK_METHOD(void, setExpensiveRenderingExpected, (DisplayId displayId, bool expected), (override)); MOCK_METHOD(bool, isUsingExpensiveRendering, (), (override)); + MOCK_METHOD(void, notifyCpuLoadUp, (), (override)); MOCK_METHOD(void, notifyDisplayUpdateImminentAndCpuReset, (), (override)); MOCK_METHOD(bool, usePowerHintSession, (), (override)); MOCK_METHOD(bool, supportsPowerHintSession, (), (override)); diff --git a/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h b/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h index ef9cd9bc43..4cfdd58c70 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h +++ b/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h @@ -19,5 +19,7 @@ #include <scheduler/FrameRateMode.h> // Use a C style macro to keep the line numbers printed in gtest -#define EXPECT_FRAME_RATE_MODE(modePtr, fps, mode) \ - EXPECT_EQ((scheduler::FrameRateMode{(fps), (modePtr)}), (mode)) +#define EXPECT_FRAME_RATE_MODE(_modePtr, _fps, _mode) \ + EXPECT_EQ((scheduler::FrameRateMode{(_fps), (_modePtr)}), (_mode)) \ + << "Expected " << (_fps) << " (" << (_modePtr)->getFps() << ") but was " \ + << (_mode).fps << " (" << (_mode).modePtr->getFps() << ")" |