From 48a619f8332e06ea1cd96d82719cdf5e05c69630 Mon Sep 17 00:00:00 2001 From: Yi Kong Date: Tue, 5 Jun 2018 16:34:59 -0700 Subject: Replace NULL/0 with nullptr Fixes -Wzero-as-null-pointer-constant warning. clang-tidy -checks=modernize-use-nullptr -p compile_commands.json -fix ... Test: m Bug: 68236239 Change-Id: I3a8e982ba40f9b029bafef78437b146a878f56a9 --- libs/gui/BufferQueueConsumer.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'libs/gui/BufferQueueConsumer.cpp') diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index d70e1422b0..3837c3e11a 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -255,7 +255,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, // mGraphicBuffer to NULL to avoid unnecessarily remapping this buffer // on the consumer side if (outBuffer->mAcquireCalled) { - outBuffer->mGraphicBuffer = NULL; + outBuffer->mGraphicBuffer = nullptr; } mCore->mQueue.erase(front); @@ -272,7 +272,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, VALIDATE_CONSISTENCY(); } - if (listener != NULL) { + if (listener != nullptr) { for (int i = 0; i < numDroppedBuffers; ++i) { listener->onBufferReleased(); } @@ -321,10 +321,10 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot, const sp& buffer) { ATRACE_CALL(); - if (outSlot == NULL) { + if (outSlot == nullptr) { BQ_LOGE("attachBuffer: outSlot must not be NULL"); return BAD_VALUE; - } else if (buffer == NULL) { + } else if (buffer == nullptr) { BQ_LOGE("attachBuffer: cannot attach NULL buffer"); return BAD_VALUE; } @@ -413,7 +413,7 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, ATRACE_BUFFER_INDEX(slot); if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS || - releaseFence == NULL) { + releaseFence == nullptr) { BQ_LOGE("releaseBuffer: slot %d out of range or fence %p NULL", slot, releaseFence.get()); return BAD_VALUE; @@ -465,7 +465,7 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, } // Autolock scope // Call back without lock held - if (listener != NULL) { + if (listener != nullptr) { listener->onBufferReleased(); } @@ -476,7 +476,7 @@ status_t BufferQueueConsumer::connect( const sp& consumerListener, bool controlledByApp) { ATRACE_CALL(); - if (consumerListener == NULL) { + if (consumerListener == nullptr) { BQ_LOGE("connect: consumerListener may not be NULL"); return BAD_VALUE; } @@ -504,13 +504,13 @@ status_t BufferQueueConsumer::disconnect() { Mutex::Autolock lock(mCore->mMutex); - if (mCore->mConsumerListener == NULL) { + if (mCore->mConsumerListener == nullptr) { BQ_LOGE("disconnect: no consumer is connected"); return BAD_VALUE; } mCore->mIsAbandoned = true; - mCore->mConsumerListener = NULL; + mCore->mConsumerListener = nullptr; mCore->mQueue.clear(); mCore->freeAllBuffersLocked(); mCore->mSharedBufferSlot = BufferQueueCore::INVALID_BUFFER_SLOT; @@ -521,7 +521,7 @@ status_t BufferQueueConsumer::disconnect() { status_t BufferQueueConsumer::getReleasedBuffers(uint64_t *outSlotMask) { ATRACE_CALL(); - if (outSlotMask == NULL) { + if (outSlotMask == nullptr) { BQ_LOGE("getReleasedBuffers: outSlotMask may not be NULL"); return BAD_VALUE; } @@ -673,7 +673,7 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount( } } // Call back without lock held - if (listener != NULL) { + if (listener != nullptr) { listener->onBuffersReleased(); } @@ -772,7 +772,7 @@ status_t BufferQueueConsumer::dumpState(const String8& prefix, String8* outResul if (uid != shellUid) { #endif android_errorWriteWithInfoLog(0x534e4554, "27046057", - static_cast(uid), NULL, 0); + static_cast(uid), nullptr, 0); return PERMISSION_DENIED; } -- cgit v1.2.3-59-g8ed1b From ad9fe277ea967b27ce7b748bc51ff15ff011f488 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Thu, 7 Mar 2019 22:36:06 -0800 Subject: BufferQueueProducer: use the correct IPCThreadState. The system variant of libgui maybe be double loaded. Also IGraphicBufferProducer functions may be called from hwbinder threads due to the presence of TWGraphicBufferProducer wrappers (hybrid interfaces). Therefore, we should use the correct IPCThreadState/ hardware::IPCThreadState to query callingPids. This also avoids access to /dev/binder in vendor processes, in case the system variant of the library is loaded, for eg: in libmediandk. Bug: 124128212 Test: Selinux denials realted to/dev/binder acccess are not present when AImageReader from libmediandk is used in a vendor process. Test: Play Youtube movies on Chrome, use camera to take pictures/ record videos (sanity). Change-Id: I27d78e30e16b7df5e3dfbb130121f3d7078671a3 Signed-off-by: Jayant Chowdhary --- libs/gui/Android.bp | 10 ++++-- libs/gui/BufferQueueConsumer.cpp | 29 ++++++++++++----- libs/gui/BufferQueueProducer.cpp | 5 +-- libs/gui/BufferQueueThreadState.cpp | 38 ++++++++++++++++++++++ .../include/private/gui/BufferQueueThreadState.h | 30 +++++++++++++++++ 5 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 libs/gui/BufferQueueThreadState.cpp create mode 100644 libs/gui/include/private/gui/BufferQueueThreadState.h (limited to 'libs/gui/BufferQueueConsumer.cpp') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 0510492803..8b663bc8ec 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -91,6 +91,7 @@ cc_library_shared { "BufferQueueConsumer.cpp", "BufferQueueCore.cpp", "BufferQueueProducer.cpp", + "BufferQueueThreadState.cpp", "BufferSlot.cpp", "ConsumerBase.cpp", "CpuConsumer.cpp", @@ -128,6 +129,7 @@ cc_library_shared { "libbase", "libsync", "libbinder", + "libhwbinder", "libbufferhub", "libbufferhubqueue", // TODO(b/70046255): Remove this once BufferHub is integrated into libgui. "libpdx_default_transport", @@ -143,12 +145,16 @@ cc_library_shared { "libhidltransport", "android.hidl.token@1.0-utils", "android.hardware.graphics.bufferqueue@1.0", + "libvndksupport", ], // bufferhub is not used when building libgui for vendors target: { vendor: { - cflags: ["-DNO_BUFFERHUB", "-DNO_INPUT"], + cflags: [ + "-DNO_BUFFERHUB", + "-DNO_INPUT", + ], exclude_srcs: [ "BufferHubConsumer.cpp", "BufferHubProducer.cpp", @@ -158,7 +164,7 @@ cc_library_shared { "libbufferhub", "libbufferhubqueue", "libpdx_default_transport", - "libinput" + "libinput", ], }, }, diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index 3837c3e11a..f2d5c8edd8 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -34,9 +34,10 @@ #include #include -#include +#include #ifndef __ANDROID_VNDK__ #include +#include #endif #include @@ -758,19 +759,29 @@ status_t BufferQueueConsumer::dumpState(const String8& prefix, String8* outResul return savedErrno ? -savedErrno : UNKNOWN_ERROR; } - const IPCThreadState* ipc = IPCThreadState::self(); - const uid_t uid = ipc->getCallingUid(); + bool denied = false; + const uid_t uid = BufferQueueThreadState::getCallingUid(); #ifndef __ANDROID_VNDK__ // permission check can't be done for vendors as vendors have no access to - // the PermissionController - const pid_t pid = ipc->getCallingPid(); - if ((uid != shellUid) && - !PermissionCache::checkPermission(String16("android.permission.DUMP"), pid, uid)) { - outResult->appendFormat("Permission Denial: can't dump BufferQueueConsumer " - "from pid=%d, uid=%d\n", pid, uid); + // the PermissionController. We need to do a runtime check as well, since + // the system variant of libgui can be loaded in a vendor process. For eg: + // if a HAL uses an llndk library that depends on libgui (libmediandk etc). + if (!android_is_in_vendor_process()) { + const pid_t pid = BufferQueueThreadState::getCallingPid(); + if ((uid != shellUid) && + !PermissionCache::checkPermission(String16("android.permission.DUMP"), pid, uid)) { + outResult->appendFormat("Permission Denial: can't dump BufferQueueConsumer " + "from pid=%d, uid=%d\n", + pid, uid); + denied = true; + } + } #else if (uid != shellUid) { + denied = true; + } #endif + if (denied) { android_errorWriteWithInfoLog(0x534e4554, "27046057", static_cast(uid), nullptr, 0); return PERMISSION_DENIED; diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 5e250a4185..a462b0362f 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -1210,7 +1211,7 @@ status_t BufferQueueProducer::connect(const sp& listener, status = BAD_VALUE; break; } - mCore->mConnectedPid = IPCThreadState::self()->getCallingPid(); + mCore->mConnectedPid = BufferQueueThreadState::getCallingPid(); mCore->mBufferHasBeenQueued = false; mCore->mDequeueBufferCannotBlock = false; if (mDequeueTimeout < 0) { @@ -1233,7 +1234,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) { Mutex::Autolock lock(mCore->mMutex); if (mode == DisconnectMode::AllLocal) { - if (IPCThreadState::self()->getCallingPid() != mCore->mConnectedPid) { + if (BufferQueueThreadState::getCallingPid() != mCore->mConnectedPid) { return NO_ERROR; } api = BufferQueueCore::CURRENTLY_CONNECTED_API; diff --git a/libs/gui/BufferQueueThreadState.cpp b/libs/gui/BufferQueueThreadState.cpp new file mode 100644 index 0000000000..3b531ec752 --- /dev/null +++ b/libs/gui/BufferQueueThreadState.cpp @@ -0,0 +1,38 @@ +/* + * Copyright 2019 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. + */ + +#include +#include +#include +#include + +namespace android { + +uid_t BufferQueueThreadState::getCallingUid() { + if (hardware::IPCThreadState::self()->isServingCall()) { + return hardware::IPCThreadState::self()->getCallingUid(); + } + return IPCThreadState::self()->getCallingUid(); +} + +pid_t BufferQueueThreadState::getCallingPid() { + if (hardware::IPCThreadState::self()->isServingCall()) { + return hardware::IPCThreadState::self()->getCallingPid(); + } + return IPCThreadState::self()->getCallingPid(); +} + +} // namespace android diff --git a/libs/gui/include/private/gui/BufferQueueThreadState.h b/libs/gui/include/private/gui/BufferQueueThreadState.h new file mode 100644 index 0000000000..67dcf621ce --- /dev/null +++ b/libs/gui/include/private/gui/BufferQueueThreadState.h @@ -0,0 +1,30 @@ +/* + * Copyright 2019 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. + */ + +#include +#include +#include + +namespace android { + +// TODO: Replace this with b/127962003 +class BufferQueueThreadState { +public: + static pid_t getCallingPid(); + static uid_t getCallingUid(); +}; + +} // namespace android -- cgit v1.2.3-59-g8ed1b From 6ae550323264bacc45da2d8fca90534d02815308 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Tue, 2 Apr 2019 02:27:44 +0200 Subject: Convert Mutex to std::mutex in BufferQueueCore/Producer/Consumer Bug: 111517695 Change-Id: I1226ed7396d0d768a26f4145af5fe48e2c70e8d2 --- libs/gui/BufferQueueConsumer.cpp | 50 ++++++------- libs/gui/BufferQueueCore.cpp | 6 +- libs/gui/BufferQueueProducer.cpp | 109 ++++++++++++++--------------- libs/gui/include/gui/BufferQueueCore.h | 12 ++-- libs/gui/include/gui/BufferQueueProducer.h | 7 +- 5 files changed, 92 insertions(+), 92 deletions(-) (limited to 'libs/gui/BufferQueueConsumer.cpp') diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index f2d5c8edd8..776c6e6c11 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -58,7 +58,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, int numDroppedBuffers = 0; sp listener; { - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); // Check that the consumer doesn't currently have the maximum number of // buffers acquired. We allow the max buffer count to be exceeded by one @@ -203,7 +203,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, if (sharedBufferAvailable && mCore->mQueue.empty()) { // make sure the buffer has finished allocating before acquiring it - mCore->waitWhileAllocatingLocked(); + mCore->waitWhileAllocatingLocked(lock); slot = mCore->mSharedBufferSlot; @@ -264,7 +264,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, // We might have freed a slot while dropping old buffers, or the producer // may be blocked waiting for the number of buffers in the queue to // decrease. - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); ATRACE_INT(mCore->mConsumerName.string(), static_cast(mCore->mQueue.size())); @@ -286,7 +286,7 @@ status_t BufferQueueConsumer::detachBuffer(int slot) { ATRACE_CALL(); ATRACE_BUFFER_INDEX(slot); BQ_LOGV("detachBuffer: slot %d", slot); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("detachBuffer: BufferQueue has been abandoned"); @@ -312,7 +312,7 @@ status_t BufferQueueConsumer::detachBuffer(int slot) { mCore->mActiveBuffers.erase(slot); mCore->mFreeSlots.insert(slot); mCore->clearBufferSlotLocked(slot); - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); VALIDATE_CONSISTENCY(); return NO_ERROR; @@ -330,7 +330,7 @@ status_t BufferQueueConsumer::attachBuffer(int* outSlot, return BAD_VALUE; } - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mSharedBufferMode) { BQ_LOGE("attachBuffer: cannot attach a buffer in shared buffer mode"); @@ -422,7 +422,7 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, sp listener; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); // If the frame number has changed because the buffer has been reallocated, // we can ignore this releaseBuffer for the old buffer. @@ -461,7 +461,7 @@ status_t BufferQueueConsumer::releaseBuffer(int slot, uint64_t frameNumber, listener = mCore->mConnectedProducerListener; BQ_LOGV("releaseBuffer: releasing slot %d", slot); - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); VALIDATE_CONSISTENCY(); } // Autolock scope @@ -485,7 +485,7 @@ status_t BufferQueueConsumer::connect( BQ_LOGV("connect: controlledByApp=%s", controlledByApp ? "true" : "false"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("connect: BufferQueue has been abandoned"); @@ -503,7 +503,7 @@ status_t BufferQueueConsumer::disconnect() { BQ_LOGV("disconnect"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mConsumerListener == nullptr) { BQ_LOGE("disconnect: no consumer is connected"); @@ -515,7 +515,7 @@ status_t BufferQueueConsumer::disconnect() { mCore->mQueue.clear(); mCore->freeAllBuffersLocked(); mCore->mSharedBufferSlot = BufferQueueCore::INVALID_BUFFER_SLOT; - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); return NO_ERROR; } @@ -527,7 +527,7 @@ status_t BufferQueueConsumer::getReleasedBuffers(uint64_t *outSlotMask) { return BAD_VALUE; } - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("getReleasedBuffers: BufferQueue has been abandoned"); @@ -569,7 +569,7 @@ status_t BufferQueueConsumer::setDefaultBufferSize(uint32_t width, BQ_LOGV("setDefaultBufferSize: width=%u height=%u", width, height); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mDefaultWidth = width; mCore->mDefaultHeight = height; return NO_ERROR; @@ -583,7 +583,7 @@ status_t BufferQueueConsumer::setMaxBufferCount(int bufferCount) { return BAD_VALUE; } - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) { BQ_LOGE("setMaxBufferCount: producer is already connected"); @@ -623,8 +623,8 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount( sp listener; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); - mCore->waitWhileAllocatingLocked(); + std::unique_lock lock(mCore->mMutex); + mCore->waitWhileAllocatingLocked(lock); if (mCore->mIsAbandoned) { BQ_LOGE("setMaxAcquiredBufferCount: consumer is abandoned"); @@ -684,7 +684,7 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount( status_t BufferQueueConsumer::setConsumerName(const String8& name) { ATRACE_CALL(); BQ_LOGV("setConsumerName: '%s'", name.string()); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mConsumerName = name; mConsumerName = name; return NO_ERROR; @@ -693,7 +693,7 @@ status_t BufferQueueConsumer::setConsumerName(const String8& name) { status_t BufferQueueConsumer::setDefaultBufferFormat(PixelFormat defaultFormat) { ATRACE_CALL(); BQ_LOGV("setDefaultBufferFormat: %u", defaultFormat); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mDefaultBufferFormat = defaultFormat; return NO_ERROR; } @@ -702,7 +702,7 @@ status_t BufferQueueConsumer::setDefaultBufferDataSpace( android_dataspace defaultDataSpace) { ATRACE_CALL(); BQ_LOGV("setDefaultBufferDataSpace: %u", defaultDataSpace); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mDefaultBufferDataSpace = defaultDataSpace; return NO_ERROR; } @@ -710,7 +710,7 @@ status_t BufferQueueConsumer::setDefaultBufferDataSpace( status_t BufferQueueConsumer::setConsumerUsageBits(uint64_t usage) { ATRACE_CALL(); BQ_LOGV("setConsumerUsageBits: %#" PRIx64, usage); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mConsumerUsageBits = usage; return NO_ERROR; } @@ -718,7 +718,7 @@ status_t BufferQueueConsumer::setConsumerUsageBits(uint64_t usage) { status_t BufferQueueConsumer::setConsumerIsProtected(bool isProtected) { ATRACE_CALL(); BQ_LOGV("setConsumerIsProtected: %s", isProtected ? "true" : "false"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mConsumerIsProtected = isProtected; return NO_ERROR; } @@ -726,26 +726,26 @@ status_t BufferQueueConsumer::setConsumerIsProtected(bool isProtected) { status_t BufferQueueConsumer::setTransformHint(uint32_t hint) { ATRACE_CALL(); BQ_LOGV("setTransformHint: %#x", hint); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mTransformHint = hint; return NO_ERROR; } status_t BufferQueueConsumer::getSidebandStream(sp* outStream) const { - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); *outStream = mCore->mSidebandStream; return NO_ERROR; } status_t BufferQueueConsumer::getOccupancyHistory(bool forceFlush, std::vector* outHistory) { - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); *outHistory = mCore->mOccupancyTracker.getSegmentHistory(forceFlush); return NO_ERROR; } status_t BufferQueueConsumer::discardFreeBuffers() { - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->discardFreeBuffersLocked(); return NO_ERROR; } diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index 960b1948c2..96c55acdb0 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -110,7 +110,7 @@ BufferQueueCore::BufferQueueCore() : BufferQueueCore::~BufferQueueCore() {} void BufferQueueCore::dumpState(const String8& prefix, String8* outResult) const { - Mutex::Autolock lock(mMutex); + std::lock_guard lock(mMutex); outResult->appendFormat("%s- BufferQueue ", prefix.string()); outResult->appendFormat("mMaxAcquiredBufferCount=%d mMaxDequeuedBufferCount=%d\n", @@ -306,10 +306,10 @@ bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) { return true; } -void BufferQueueCore::waitWhileAllocatingLocked() const { +void BufferQueueCore::waitWhileAllocatingLocked(std::unique_lock& lock) const { ATRACE_CALL(); while (mIsAllocating) { - mIsAllocatingCondition.wait(mMutex); + mIsAllocatingCondition.wait(lock); } } diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index a462b0362f..c17a525cb2 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -66,7 +66,7 @@ BufferQueueProducer::~BufferQueueProducer() {} status_t BufferQueueProducer::requestBuffer(int slot, sp* buf) { ATRACE_CALL(); BQ_LOGV("requestBuffer: slot %d", slot); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("requestBuffer: BufferQueue has been abandoned"); @@ -101,8 +101,8 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount( sp listener; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); - mCore->waitWhileAllocatingLocked(); + std::unique_lock lock(mCore->mMutex); + mCore->waitWhileAllocatingLocked(lock); if (mCore->mIsAbandoned) { BQ_LOGE("setMaxDequeuedBufferCount: BufferQueue has been " @@ -163,7 +163,7 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount( if (delta < 0) { listener = mCore->mConsumerListener; } - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); } // Autolock scope // Call back without lock held @@ -180,8 +180,8 @@ status_t BufferQueueProducer::setAsyncMode(bool async) { sp listener; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); - mCore->waitWhileAllocatingLocked(); + std::unique_lock lock(mCore->mMutex); + mCore->waitWhileAllocatingLocked(lock); if (mCore->mIsAbandoned) { BQ_LOGE("setAsyncMode: BufferQueue has been abandoned"); @@ -215,7 +215,7 @@ status_t BufferQueueProducer::setAsyncMode(bool async) { } mCore->mAsyncMode = async; VALIDATE_CONSISTENCY(); - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); if (delta < 0) { listener = mCore->mConsumerListener; } @@ -247,7 +247,7 @@ int BufferQueueProducer::getFreeSlotLocked() const { } status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller, - int* found) const { + std::unique_lock& lock, int* found) const { auto callerString = (caller == FreeSlotCaller::Dequeue) ? "dequeueBuffer" : "attachBuffer"; bool tryAgain = true; @@ -334,13 +334,13 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller, return WOULD_BLOCK; } if (mDequeueTimeout >= 0) { - status_t result = mCore->mDequeueCondition.waitRelative( - mCore->mMutex, mDequeueTimeout); - if (result == TIMED_OUT) { - return result; + std::cv_status result = mCore->mDequeueCondition.wait_for(lock, + std::chrono::nanoseconds(mDequeueTimeout)); + if (result == std::cv_status::timeout) { + return TIMED_OUT; } } else { - mCore->mDequeueCondition.wait(mCore->mMutex); + mCore->mDequeueCondition.wait(lock); } } } // while (tryAgain) @@ -354,7 +354,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou FrameEventHistoryDelta* outTimestamps) { ATRACE_CALL(); { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mConsumerName = mCore->mConsumerName; if (mCore->mIsAbandoned) { @@ -381,7 +381,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou bool attachedByConsumer = false; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); if (format == 0) { format = mCore->mDefaultBufferFormat; @@ -398,8 +398,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou int found = BufferItem::INVALID_BUFFER_SLOT; while (found == BufferItem::INVALID_BUFFER_SLOT) { - status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue, - &found); + status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue, lock, &found); if (status != NO_ERROR) { return status; } @@ -506,7 +505,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou status_t error = graphicBuffer->initCheck(); { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (error == NO_ERROR && !mCore->mIsAbandoned) { graphicBuffer->setGenerationNumber(mCore->mGenerationNumber); @@ -514,7 +513,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou } mCore->mIsAllocating = false; - mCore->mIsAllocatingCondition.broadcast(); + mCore->mIsAllocatingCondition.notify_all(); if (error != NO_ERROR) { mCore->mFreeSlots.insert(*outSlot); @@ -573,7 +572,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) { sp listener; { - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("detachBuffer: BufferQueue has been abandoned"); @@ -608,7 +607,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) { mCore->mActiveBuffers.erase(slot); mCore->mFreeSlots.insert(slot); mCore->clearBufferSlotLocked(slot); - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); VALIDATE_CONSISTENCY(); listener = mCore->mConsumerListener; } @@ -634,7 +633,7 @@ status_t BufferQueueProducer::detachNextBuffer(sp* outBuffer, sp listener; { - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned"); @@ -652,7 +651,7 @@ status_t BufferQueueProducer::detachNextBuffer(sp* outBuffer, return BAD_VALUE; } - mCore->waitWhileAllocatingLocked(); + mCore->waitWhileAllocatingLocked(lock); if (mCore->mFreeBuffers.empty()) { return NO_MEMORY; @@ -690,7 +689,7 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot, return BAD_VALUE; } - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("attachBuffer: BufferQueue has been abandoned"); @@ -714,11 +713,11 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot, return BAD_VALUE; } - mCore->waitWhileAllocatingLocked(); + mCore->waitWhileAllocatingLocked(lock); status_t returnFlags = NO_ERROR; int found; - status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, &found); + status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, lock, &found); if (status != NO_ERROR) { return status; } @@ -791,7 +790,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, uint64_t currentFrameNumber = 0; BufferItem item; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("queueBuffer: BufferQueue has been abandoned"); @@ -932,7 +931,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, } mCore->mBufferHasBeenQueued = true; - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); mCore->mLastQueuedSlot = slot; output->width = mCore->mDefaultWidth; @@ -968,9 +967,9 @@ status_t BufferQueueProducer::queueBuffer(int slot, sp lastQueuedFence; { // scope for the lock - Mutex::Autolock lock(mCallbackMutex); + std::unique_lock lock(mCallbackMutex); while (callbackTicket != mCurrentCallbackTicket) { - mCallbackCondition.wait(mCallbackMutex); + mCallbackCondition.wait(lock); } if (frameAvailableListener != nullptr) { @@ -987,7 +986,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, mLastQueuedTransform = item.mTransform; ++mCurrentCallbackTicket; - mCallbackCondition.broadcast(); + mCallbackCondition.notify_all(); } // Wait without lock held @@ -1015,7 +1014,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, status_t BufferQueueProducer::cancelBuffer(int slot, const sp& fence) { ATRACE_CALL(); BQ_LOGV("cancelBuffer: slot %d", slot); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mIsAbandoned) { BQ_LOGE("cancelBuffer: BufferQueue has been abandoned"); @@ -1060,7 +1059,7 @@ status_t BufferQueueProducer::cancelBuffer(int slot, const sp& fence) { } mSlots[slot].mFence = fence; - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); VALIDATE_CONSISTENCY(); return NO_ERROR; @@ -1068,7 +1067,7 @@ status_t BufferQueueProducer::cancelBuffer(int slot, const sp& fence) { int BufferQueueProducer::query(int what, int *outValue) { ATRACE_CALL(); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (outValue == nullptr) { BQ_LOGE("query: outValue was NULL"); @@ -1136,7 +1135,7 @@ int BufferQueueProducer::query(int what, int *outValue) { status_t BufferQueueProducer::connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput *output) { ATRACE_CALL(); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mConsumerName = mCore->mConsumerName; BQ_LOGV("connect: api=%d producerControlledByApp=%s", api, producerControlledByApp ? "true" : "false"); @@ -1231,7 +1230,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) { int status = NO_ERROR; sp listener; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); if (mode == DisconnectMode::AllLocal) { if (BufferQueueThreadState::getCallingPid() != mCore->mConnectedPid) { @@ -1240,7 +1239,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) { api = BufferQueueCore::CURRENTLY_CONNECTED_API; } - mCore->waitWhileAllocatingLocked(); + mCore->waitWhileAllocatingLocked(lock); if (mCore->mIsAbandoned) { // It's not really an error to disconnect after the surface has @@ -1284,7 +1283,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) { mCore->mConnectedApi = BufferQueueCore::NO_CONNECTED_API; mCore->mConnectedPid = -1; mCore->mSidebandStream.clear(); - mCore->mDequeueCondition.broadcast(); + mCore->mDequeueCondition.notify_all(); listener = mCore->mConsumerListener; } else if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) { BQ_LOGE("disconnect: not connected (req=%d)", api); @@ -1314,7 +1313,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) { status_t BufferQueueProducer::setSidebandStream(const sp& stream) { sp listener; { // Autolock scope - Mutex::Autolock _l(mCore->mMutex); + std::lock_guard _l(mCore->mMutex); mCore->mSidebandStream = stream; listener = mCore->mConsumerListener; } // Autolock scope @@ -1336,8 +1335,8 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, uint64_t allocUsage = 0; std::string allocName; { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); - mCore->waitWhileAllocatingLocked(); + std::unique_lock lock(mCore->mMutex); + mCore->waitWhileAllocatingLocked(lock); if (!mCore->mAllowAllocation) { BQ_LOGE("allocateBuffers: allocation is not allowed for this " @@ -1372,16 +1371,16 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, if (result != NO_ERROR) { BQ_LOGE("allocateBuffers: failed to allocate buffer (%u x %u, format" " %u, usage %#" PRIx64 ")", width, height, format, usage); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mIsAllocating = false; - mCore->mIsAllocatingCondition.broadcast(); + mCore->mIsAllocatingCondition.notify_all(); return; } buffers.push_back(graphicBuffer); } { // Autolock scope - Mutex::Autolock lock(mCore->mMutex); + std::unique_lock lock(mCore->mMutex); uint32_t checkWidth = width > 0 ? width : mCore->mDefaultWidth; uint32_t checkHeight = height > 0 ? height : mCore->mDefaultHeight; PixelFormat checkFormat = format != 0 ? @@ -1392,7 +1391,7 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, // Something changed while we released the lock. Retry. BQ_LOGV("allocateBuffers: size/format/usage changed while allocating. Retrying."); mCore->mIsAllocating = false; - mCore->mIsAllocatingCondition.broadcast(); + mCore->mIsAllocatingCondition.notify_all(); continue; } @@ -1420,7 +1419,7 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, } mCore->mIsAllocating = false; - mCore->mIsAllocatingCondition.broadcast(); + mCore->mIsAllocatingCondition.notify_all(); VALIDATE_CONSISTENCY(); } // Autolock scope } @@ -1430,7 +1429,7 @@ status_t BufferQueueProducer::allowAllocation(bool allow) { ATRACE_CALL(); BQ_LOGV("allowAllocation: %s", allow ? "true" : "false"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mAllowAllocation = allow; return NO_ERROR; } @@ -1439,14 +1438,14 @@ status_t BufferQueueProducer::setGenerationNumber(uint32_t generationNumber) { ATRACE_CALL(); BQ_LOGV("setGenerationNumber: %u", generationNumber); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mGenerationNumber = generationNumber; return NO_ERROR; } String8 BufferQueueProducer::getConsumerName() const { ATRACE_CALL(); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); BQ_LOGV("getConsumerName: %s", mConsumerName.string()); return mConsumerName; } @@ -1455,7 +1454,7 @@ status_t BufferQueueProducer::setSharedBufferMode(bool sharedBufferMode) { ATRACE_CALL(); BQ_LOGV("setSharedBufferMode: %d", sharedBufferMode); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (!sharedBufferMode) { mCore->mSharedBufferSlot = BufferQueueCore::INVALID_BUFFER_SLOT; } @@ -1467,7 +1466,7 @@ status_t BufferQueueProducer::setAutoRefresh(bool autoRefresh) { ATRACE_CALL(); BQ_LOGV("setAutoRefresh: %d", autoRefresh); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); mCore->mAutoRefresh = autoRefresh; return NO_ERROR; @@ -1477,7 +1476,7 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) { ATRACE_CALL(); BQ_LOGV("setDequeueTimeout: %" PRId64, timeout); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode, false, mCore->mMaxBufferCount) - mCore->getMaxBufferCountLocked(); if (!mCore->adjustAvailableSlotsLocked(delta)) { @@ -1498,7 +1497,7 @@ status_t BufferQueueProducer::getLastQueuedBuffer(sp* outBuffer, ATRACE_CALL(); BQ_LOGV("getLastQueuedBuffer"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT) { *outBuffer = nullptr; *outFence = Fence::NO_FENCE; @@ -1534,7 +1533,7 @@ void BufferQueueProducer::addAndGetFrameTimestamps( BQ_LOGV("addAndGetFrameTimestamps"); sp listener; { - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); listener = mCore->mConsumerListener; } if (listener != nullptr) { @@ -1561,7 +1560,7 @@ status_t BufferQueueProducer::getUniqueId(uint64_t* outId) const { status_t BufferQueueProducer::getConsumerUsage(uint64_t* outUsage) const { BQ_LOGV("getConsumerUsage"); - Mutex::Autolock lock(mCore->mMutex); + std::lock_guard lock(mCore->mMutex); *outUsage = mCore->mConsumerUsageBits; return NO_ERROR; } diff --git a/libs/gui/include/gui/BufferQueueCore.h b/libs/gui/include/gui/BufferQueueCore.h index b377a412bc..0e80283804 100644 --- a/libs/gui/include/gui/BufferQueueCore.h +++ b/libs/gui/include/gui/BufferQueueCore.h @@ -22,8 +22,6 @@ #include #include -#include -#include #include #include #include @@ -33,6 +31,8 @@ #include #include +#include +#include #define BQ_LOGV(x, ...) ALOGV("[%s] " x, mConsumerName.string(), ##__VA_ARGS__) #define BQ_LOGD(x, ...) ALOGD("[%s] " x, mConsumerName.string(), ##__VA_ARGS__) @@ -134,7 +134,7 @@ private: bool adjustAvailableSlotsLocked(int delta); // waitWhileAllocatingLocked blocks until mIsAllocating is false. - void waitWhileAllocatingLocked() const; + void waitWhileAllocatingLocked(std::unique_lock& lock) const; #if DEBUG_ONLY_CODE // validateConsistencyLocked ensures that the free lists are in sync with @@ -145,7 +145,7 @@ private: // mMutex is the mutex used to prevent concurrent access to the member // variables of BufferQueueCore objects. It must be locked whenever any // member variable is accessed. - mutable Mutex mMutex; + mutable std::mutex mMutex; // mIsAbandoned indicates that the BufferQueue will no longer be used to // consume image buffers pushed to it using the IGraphicBufferProducer @@ -219,7 +219,7 @@ private: // mDequeueCondition is a condition variable used for dequeueBuffer in // synchronous mode. - mutable Condition mDequeueCondition; + mutable std::condition_variable mDequeueCondition; // mDequeueBufferCannotBlock indicates whether dequeueBuffer is allowed to // block. This flag is set during connect when both the producer and @@ -282,7 +282,7 @@ private: // mIsAllocatingCondition is a condition variable used by producers to wait until mIsAllocating // becomes false. - mutable Condition mIsAllocatingCondition; + mutable std::condition_variable mIsAllocatingCondition; // mAllowAllocation determines whether dequeueBuffer is allowed to allocate // new buffers diff --git a/libs/gui/include/gui/BufferQueueProducer.h b/libs/gui/include/gui/BufferQueueProducer.h index 73bc5dd318..b6e98862f7 100644 --- a/libs/gui/include/gui/BufferQueueProducer.h +++ b/libs/gui/include/gui/BufferQueueProducer.h @@ -211,7 +211,8 @@ private: Dequeue, Attach, }; - status_t waitForFreeSlotThenRelock(FreeSlotCaller caller, int* found) const; + status_t waitForFreeSlotThenRelock(FreeSlotCaller caller, std::unique_lock& lock, + int* found) const; sp mCore; @@ -243,10 +244,10 @@ private: // (mCore->mMutex) is held, a ticket is retained by the producer. After // dropping the BufferQueue lock, the producer must wait on the condition // variable until the current callback ticket matches its retained ticket. - Mutex mCallbackMutex; + std::mutex mCallbackMutex; int mNextCallbackTicket; // Protected by mCore->mMutex int mCurrentCallbackTicket; // Protected by mCallbackMutex - Condition mCallbackCondition; + std::condition_variable mCallbackCondition; // Sets how long dequeueBuffer or attachBuffer will block if a buffer or // slot is not yet available. -- cgit v1.2.3-59-g8ed1b From 09bd3920155f0961b303d1cdd0f6027135aff36d Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 8 Apr 2019 10:44:56 -0700 Subject: SF: Updating content FPS tracking 1) Each time SF creates a layer, register it with Scheduler and return handle 2) BufferQueueLayer and BufferStateLayer can now send information about buffers for given layers via layer handle. Algorithm for detecting content fps: 1) Keep the refresh rate per layer (explicit timestamp, or 0). 2) Keep information about last 10 present or update timestamps. This will be an indicator for precedence. 3) Choose the MAX refresh rate among last updated layers. For more info see go/surface-flinger-scheduler and go/content-fps-detection-in-scheduler Test: Updating unit tests. Systrace. Change-Id: I988a7a79e9a9f0f61674c9b637c5142db3336177 Bug: 127727337 --- libs/gui/BufferQueueConsumer.cpp | 1 + services/surfaceflinger/Android.bp | 1 + services/surfaceflinger/BufferLayer.cpp | 1 + services/surfaceflinger/BufferQueueLayer.cpp | 4 +- services/surfaceflinger/BufferStateLayer.cpp | 20 ++- services/surfaceflinger/BufferStateLayer.h | 5 +- services/surfaceflinger/Layer.cpp | 2 + services/surfaceflinger/Layer.h | 11 +- services/surfaceflinger/Scheduler/LayerHistory.cpp | 118 +++++++++++-- services/surfaceflinger/Scheduler/LayerHistory.h | 62 ++++--- services/surfaceflinger/Scheduler/LayerInfo.cpp | 50 ++++++ services/surfaceflinger/Scheduler/LayerInfo.h | 156 +++++++++++++++++ .../surfaceflinger/Scheduler/RefreshRateConfigs.h | 2 +- services/surfaceflinger/Scheduler/Scheduler.cpp | 109 +++++++----- services/surfaceflinger/Scheduler/Scheduler.h | 42 +++-- services/surfaceflinger/Scheduler/SchedulerUtils.h | 13 +- services/surfaceflinger/SurfaceFlinger.cpp | 12 +- services/surfaceflinger/SurfaceFlingerFactory.cpp | 6 +- services/surfaceflinger/SurfaceFlingerFactory.h | 4 +- .../tests/unittests/CompositionTest.cpp | 2 +- .../tests/unittests/DisplayTransactionTest.cpp | 2 +- .../tests/unittests/LayerHistoryTest.cpp | 194 +++++++-------------- .../tests/unittests/SchedulerTest.cpp | 9 +- .../tests/unittests/TestableScheduler.h | 4 +- .../tests/unittests/TestableSurfaceFlinger.h | 4 +- 25 files changed, 576 insertions(+), 258 deletions(-) create mode 100644 services/surfaceflinger/Scheduler/LayerInfo.cpp create mode 100644 services/surfaceflinger/Scheduler/LayerInfo.h (limited to 'libs/gui/BufferQueueConsumer.cpp') diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index 776c6e6c11..e2261360b6 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -190,6 +190,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, desiredPresent - expectedPresent, systemTime(CLOCK_MONOTONIC), front->mFrameNumber, maxFrameNumber); + ATRACE_NAME("PRESENT_LATER"); return PRESENT_LATER; } diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index 52c68df814..9080d29194 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -147,6 +147,7 @@ filegroup { "Scheduler/EventThread.cpp", "Scheduler/IdleTimer.cpp", "Scheduler/LayerHistory.cpp", + "Scheduler/LayerInfo.cpp", "Scheduler/MessageQueue.cpp", "Scheduler/Scheduler.cpp", "Scheduler/SchedulerUtils.cpp", diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index 1b2b180c52..a08ae8c7e6 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -413,6 +413,7 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) // If the head buffer's acquire fence hasn't signaled yet, return and // try again later if (!fenceHasSignaled()) { + ATRACE_NAME("!fenceHasSignaled()"); mFlinger->signalLayerUpdate(); return false; } diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index b623991742..ff5f271960 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -398,8 +398,8 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) { // Add this buffer from our internal queue tracker { // Autolock scope if (mFlinger->mUseSmart90ForVideo) { - // Report mApi ID for each layer. - mFlinger->mScheduler->addNativeWindowApi(item.mApi); + const nsecs_t presentTime = item.mIsAutoTimestamp ? 0 : item.mTimestamp; + mFlinger->mScheduler->addLayerPresentTime(mSchedulerLayerHandle, presentTime); } Mutex::Autolock lock(mQueueItemLock); diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 64dfdc3d5e..2cbb917478 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -209,7 +209,8 @@ bool BufferStateLayer::setFrame(const Rect& frame) { return true; } -bool BufferStateLayer::setBuffer(const sp& buffer) { +bool BufferStateLayer::setBuffer(const sp& buffer, nsecs_t postTime, + nsecs_t desiredPresentTime) { if (mCurrentState.buffer) { mReleasePreviousBuffer = true; } @@ -217,6 +218,15 @@ bool BufferStateLayer::setBuffer(const sp& buffer) { mCurrentState.buffer = buffer; mCurrentState.modified = true; setTransactionFlags(eTransactionNeeded); + + mFlinger->mTimeStats->setPostTime(getSequence(), getFrameNumber(), getName().c_str(), postTime); + mDesiredPresentTime = desiredPresentTime; + + if (mFlinger->mUseSmart90ForVideo) { + const nsecs_t presentTime = (mDesiredPresentTime == -1) ? 0 : mDesiredPresentTime; + mFlinger->mScheduler->addLayerPresentTime(mSchedulerLayerHandle, presentTime); + } + return true; } @@ -348,14 +358,6 @@ FloatRect BufferStateLayer::computeSourceBounds(const FloatRect& parentBounds) c return parentBounds; } -void BufferStateLayer::setPostTime(nsecs_t postTime) { - mFlinger->mTimeStats->setPostTime(getSequence(), getFrameNumber(), getName().c_str(), postTime); -} - -void BufferStateLayer::setDesiredPresentTime(nsecs_t desiredPresentTime) { - mDesiredPresentTime = desiredPresentTime; -} - // ----------------------------------------------------------------------- // ----------------------------------------------------------------------- diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 668830a3fe..864a15db1f 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -63,7 +63,8 @@ public: bool setTransformToDisplayInverse(bool transformToDisplayInverse) override; bool setCrop(const Rect& crop) override; bool setFrame(const Rect& frame) override; - bool setBuffer(const sp& buffer) override; + bool setBuffer(const sp& buffer, nsecs_t postTime, + nsecs_t desiredPresentTime) override; bool setAcquireFence(const sp& fence) override; bool setDataspace(ui::Dataspace dataspace) override; bool setHdrMetadata(const HdrMetadata& hdrMetadata) override; @@ -90,8 +91,6 @@ public: Rect getBufferSize(const State& s) const override; FloatRect computeSourceBounds(const FloatRect& parentBounds) const override; - void setPostTime(nsecs_t postTime) override; - void setDesiredPresentTime(nsecs_t desiredPresentTime) override; // ----------------------------------------------------------------------- // ----------------------------------------------------------------------- diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 1142df8460..de05f74a9b 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -115,6 +115,8 @@ Layer::Layer(const LayerCreationArgs& args) mFrameEventHistory.initializeCompositorTiming(compositorTiming); mFrameTracker.setDisplayRefreshPeriod(compositorTiming.interval); + mSchedulerLayerHandle = mFlinger->mScheduler->registerLayer(mName.c_str()); + mFlinger->onLayerCreated(); } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 8348ce1f38..66e35b6136 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -311,7 +311,10 @@ public: virtual bool setTransformToDisplayInverse(bool /*transformToDisplayInverse*/) { return false; }; virtual bool setCrop(const Rect& /*crop*/) { return false; }; virtual bool setFrame(const Rect& /*frame*/) { return false; }; - virtual bool setBuffer(const sp& /*buffer*/) { return false; }; + virtual bool setBuffer(const sp& /*buffer*/, nsecs_t /*postTime*/, + nsecs_t /*desiredPresentTime*/) { + return false; + } virtual bool setAcquireFence(const sp& /*fence*/) { return false; }; virtual bool setDataspace(ui::Dataspace /*dataspace*/) { return false; }; virtual bool setHdrMetadata(const HdrMetadata& /*hdrMetadata*/) { return false; }; @@ -450,9 +453,6 @@ public: } virtual Rect getCrop(const Layer::State& s) const { return s.crop_legacy; } - virtual void setPostTime(nsecs_t /*postTime*/) {} - virtual void setDesiredPresentTime(nsecs_t /*desiredPresentTime*/) {} - protected: virtual bool prepareClientLayer(const RenderArea& renderArea, const Region& clip, bool useIdentityTransform, Region& clearRegion, @@ -885,6 +885,9 @@ protected: // Can only be accessed with the SF state lock held. bool mChildrenChanged{false}; + // This is populated if the layer is registered with Scheduler for tracking purposes. + std::unique_ptr mSchedulerLayerHandle; + private: /** * Returns an unsorted vector of all layers that are part of this tree. diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp index d5ccbe186b..8e36ae9dc0 100644 --- a/services/surfaceflinger/Scheduler/LayerHistory.cpp +++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp @@ -14,14 +14,18 @@ * limitations under the License. */ +#define ATRACE_TAG ATRACE_TAG_GRAPHICS + #include "LayerHistory.h" #include #include +#include #include #include #include +#include #include #include #include @@ -29,28 +33,114 @@ #include "SchedulerUtils.h" namespace android { +namespace scheduler { + +std::atomic LayerHistory::sNextId = 0; -LayerHistory::LayerHistory() {} +LayerHistory::LayerHistory() { + char value[PROPERTY_VALUE_MAX]; + property_get("debug.sf.layer_history_trace", value, "0"); + mTraceEnabled = bool(atoi(value)); +} LayerHistory::~LayerHistory() = default; -void LayerHistory::insert(const std::string layerName, nsecs_t presentTime) { - mElements[mCounter].insert(std::make_pair(layerName, presentTime)); +std::unique_ptr LayerHistory::createLayer(const std::string name, + float maxRefreshRate) { + const int64_t id = sNextId++; + + std::lock_guard lock(mLock); + mInactiveLayerInfos.emplace(id, std::make_shared(name, maxRefreshRate)); + return std::make_unique(*this, id); +} + +void LayerHistory::destroyLayer(const int64_t id) { + std::lock_guard lock(mLock); + auto it = mActiveLayerInfos.find(id); + if (it != mActiveLayerInfos.end()) { + mActiveLayerInfos.erase(it); + } + + it = mInactiveLayerInfos.find(id); + if (it != mInactiveLayerInfos.end()) { + mInactiveLayerInfos.erase(it); + } +} + +void LayerHistory::insert(const std::unique_ptr& layerHandle, nsecs_t presentTime) { + std::shared_ptr layerInfo; + { + std::lock_guard lock(mLock); + auto layerInfoIterator = mInactiveLayerInfos.find(layerHandle->mId); + if (layerInfoIterator != mInactiveLayerInfos.end()) { + layerInfo = layerInfoIterator->second; + mInactiveLayerInfos.erase(layerInfoIterator); + mActiveLayerInfos.insert({layerHandle->mId, layerInfo}); + } else { + layerInfoIterator = mActiveLayerInfos.find(layerHandle->mId); + if (layerInfoIterator != mActiveLayerInfos.end()) { + layerInfo = layerInfoIterator->second; + } else { + ALOGW("Inserting information about layer that is not registered: %" PRId64, + layerHandle->mId); + return; + } + } + } + layerInfo->setLastPresentTime(presentTime); } -void LayerHistory::incrementCounter() { - mCounter++; - mCounter = mCounter % scheduler::ARRAY_SIZE; - // Clear all the previous data from the history. This is a ring buffer, so we are - // reusing memory. - mElements[mCounter].clear(); +float LayerHistory::getDesiredRefreshRate() { + float newRefreshRate = 0.f; + std::lock_guard lock(mLock); + + removeIrrelevantLayers(); + + // Iterate through all layers that have been recently updated, and find the max refresh rate. + for (const auto& [layerId, layerInfo] : mActiveLayerInfos) { + const float layerRefreshRate = layerInfo->getDesiredRefreshRate(); + if (mTraceEnabled) { + // Store the refresh rate in traces for easy debugging. + std::string layerName = "LFPS " + layerInfo->getName(); + ATRACE_INT(layerName.c_str(), std::round(layerRefreshRate)); + ALOGD("%s: %f", layerName.c_str(), std::round(layerRefreshRate)); + } + if (layerInfo->isRecentlyActive() && layerRefreshRate > newRefreshRate) { + newRefreshRate = layerRefreshRate; + } + } + if (mTraceEnabled) { + ALOGD("LayerHistory DesiredRefreshRate: %.2f", newRefreshRate); + } + + return newRefreshRate; } -const std::unordered_map& LayerHistory::get(size_t index) const { - // For the purposes of the layer history, the index = 0 always needs to start at the - // current counter, and then decrement to access the layers in correct historical order. - return mElements.at((scheduler::ARRAY_SIZE + (mCounter - (index % scheduler::ARRAY_SIZE))) % - scheduler::ARRAY_SIZE); +void LayerHistory::removeIrrelevantLayers() { + const int64_t obsoleteEpsilon = systemTime() - scheduler::OBSOLETE_TIME_EPSILON_NS.count(); + // Iterator pointing to first element in map + auto it = mActiveLayerInfos.begin(); + while (it != mActiveLayerInfos.end()) { + // If last updated was before the obsolete time, remove it. + if (it->second->getLastUpdatedTime() < obsoleteEpsilon) { + // erase() function returns the iterator of the next + // to last deleted element. + if (mTraceEnabled) { + ALOGD("Layer %s obsolete", it->second->getName().c_str()); + // Make sure to update systrace to indicate that the layer was erased. + std::string layerName = "LFPS " + it->second->getName(); + ATRACE_INT(layerName.c_str(), 0); + } + auto id = it->first; + auto layerInfo = it->second; + layerInfo->clearHistory(); + mInactiveLayerInfos.insert({id, layerInfo}); + it = mActiveLayerInfos.erase(it); + } else { + ++it; + } + } } +} // namespace scheduler } // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/Scheduler/LayerHistory.h b/services/surfaceflinger/Scheduler/LayerHistory.h index c6fab07c87..39061e7fdf 100644 --- a/services/surfaceflinger/Scheduler/LayerHistory.h +++ b/services/surfaceflinger/Scheduler/LayerHistory.h @@ -25,40 +25,60 @@ #include +#include "LayerInfo.h" #include "SchedulerUtils.h" namespace android { +namespace scheduler { /* - * This class represents a circular buffer in which we keep layer history for - * the past ARRAY_SIZE frames. Each time, a signal for new frame comes, the counter - * gets incremented and includes all the layers that are requested to draw in that - * frame. - * - * Once the buffer reaches the end of the array, it starts overriding the elements - * at the beginning of the array. + * This class represents information about layers that are considered current. We keep an + * unordered map between layer name and LayerInfo. */ class LayerHistory { public: + // Handle for each layer we keep track of. + class LayerHandle { + public: + LayerHandle(LayerHistory& lh, int64_t id) : mId(id), mLayerHistory(lh) {} + ~LayerHandle() { mLayerHistory.destroyLayer(mId); } + + const int64_t mId; + + private: + LayerHistory& mLayerHistory; + }; + LayerHistory(); ~LayerHistory(); - // Method for inserting layers and their requested present time into the ring buffer. - // The elements are going to be inserted into an unordered_map at the position 'now'. - void insert(const std::string layerName, nsecs_t presentTime); - // Method for incrementing the current slot in the ring buffer. It also clears the - // unordered_map, if it was created previously. - void incrementCounter(); - // Returns unordered_map at the given at index. The index is decremented from 'now'. For - // example, 0 is now, 1 is previous frame. - const std::unordered_map& get(size_t index) const; - // Returns the total size of the ring buffer. The value is always the same regardless - // of how many slots we filled in. - static constexpr size_t getSize() { return scheduler::ARRAY_SIZE; } + // When the layer is first created, register it. + std::unique_ptr createLayer(const std::string name, float maxRefreshRate); + + // Method for inserting layers and their requested present time into the unordered map. + void insert(const std::unique_ptr& layerHandle, nsecs_t presentTime); + // Returns the desired refresh rate, which is a max refresh rate of all the current + // layers. See go/content-fps-detection-in-scheduler for more information. + float getDesiredRefreshRate(); + + // Removes the handle and the object from the map. + void destroyLayer(const int64_t id); private: - size_t mCounter = 0; - std::array, scheduler::ARRAY_SIZE> mElements; + // Removes the layers that have been idle for a given amount of time from mLayerInfos. + void removeIrrelevantLayers() REQUIRES(mLock); + + // Information about currently active layers. + std::mutex mLock; + std::unordered_map> mActiveLayerInfos GUARDED_BY(mLock); + std::unordered_map> mInactiveLayerInfos GUARDED_BY(mLock); + + // Each layer has it's own ID. This variable keeps track of the count. + static std::atomic sNextId; + + // Flag whether to log layer FPS in systrace + bool mTraceEnabled = false; }; +} // namespace scheduler } // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/Scheduler/LayerInfo.cpp b/services/surfaceflinger/Scheduler/LayerInfo.cpp new file mode 100644 index 0000000000..95d7d31564 --- /dev/null +++ b/services/surfaceflinger/Scheduler/LayerInfo.cpp @@ -0,0 +1,50 @@ +/* + * Copyright 2019 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. + */ + +#include "LayerInfo.h" + +#include +#include +#include +#include + +namespace android { +namespace scheduler { + +LayerInfo::LayerInfo(const std::string name, float maxRefreshRate) + : mName(name), + mMinRefreshDuration(1e9f / maxRefreshRate), + mRefreshRateHistory(mMinRefreshDuration) {} + +LayerInfo::~LayerInfo() = default; + +void LayerInfo::setLastPresentTime(nsecs_t lastPresentTime) { + std::lock_guard lock(mLock); + + // Buffers can come with a present time far in the future. That keeps them relevant. + mLastUpdatedTime = std::max(lastPresentTime, systemTime()); + mPresentTimeHistory.insertPresentTime(mLastUpdatedTime); + + const nsecs_t timeDiff = lastPresentTime - mLastPresentTime; + mLastPresentTime = lastPresentTime; + // Ignore time diff that are too high - those are stale values + if (timeDiff > TIME_EPSILON_NS.count()) return; + const nsecs_t refreshDuration = (timeDiff > 0) ? timeDiff : mMinRefreshDuration; + mRefreshRateHistory.insertRefreshRate(refreshDuration); +} + +} // namespace scheduler +} // namespace android diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h new file mode 100644 index 0000000000..1970a47701 --- /dev/null +++ b/services/surfaceflinger/Scheduler/LayerInfo.h @@ -0,0 +1,156 @@ +/* + * Copyright 2019 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 +#include +#include +#include +#include +#include + +#include + +#include +#include + +#include "SchedulerUtils.h" + +namespace android { +namespace scheduler { + +/* + * This class represents information about individial layers. + */ +class LayerInfo { + /** + * Struct that keeps the information about the refresh rate for last + * HISTORY_SIZE frames. This is used to better determine the refresh rate + * for individual layers. + */ + class RefreshRateHistory { + public: + explicit RefreshRateHistory(nsecs_t minRefreshDuration) + : mMinRefreshDuration(minRefreshDuration) {} + void insertRefreshRate(nsecs_t refreshRate) { + mElements.push_back(refreshRate); + if (mElements.size() > HISTORY_SIZE) { + mElements.pop_front(); + } + } + + float getRefreshRateAvg() const { + nsecs_t refreshDuration = mMinRefreshDuration; + if (mElements.size() == HISTORY_SIZE) { + refreshDuration = scheduler::calculate_mean(mElements); + } + + return 1e9f / refreshDuration; + } + void clearHistory() { mElements.clear(); } + + private: + std::deque mElements; + static constexpr size_t HISTORY_SIZE = 30; + const nsecs_t mMinRefreshDuration; + }; + + /** + * Struct that keeps the information about the present time for last + * HISTORY_SIZE frames. This is used to better determine whether the given layer + * is still relevant and it's refresh rate should be considered. + */ + class PresentTimeHistory { + public: + void insertPresentTime(nsecs_t presentTime) { + mElements.push_back(presentTime); + if (mElements.size() > HISTORY_SIZE) { + mElements.pop_front(); + } + } + + // Checks whether the present time that was inserted HISTORY_SIZE ago is within a + // certain threshold: TIME_EPSILON_NS. + bool isRelevant() const { + const int64_t obsoleteEpsilon = systemTime() - scheduler::TIME_EPSILON_NS.count(); + // The layer had to publish at least HISTORY_SIZE of updates, and the first + // update should not be older than TIME_EPSILON_NS nanoseconds. + if (mElements.size() == HISTORY_SIZE && + mElements.at(HISTORY_SIZE - 1) > obsoleteEpsilon) { + return true; + } + return false; + } + + void clearHistory() { mElements.clear(); } + + private: + std::deque mElements; + static constexpr size_t HISTORY_SIZE = 10; + }; + +public: + LayerInfo(const std::string name, float maxRefreshRate); + ~LayerInfo(); + + LayerInfo(const LayerInfo&) = delete; + LayerInfo& operator=(const LayerInfo&) = delete; + + // Records the last requested oresent time. It also stores information about when + // the layer was last updated. If the present time is farther in the future than the + // updated time, the updated time is the present time. + void setLastPresentTime(nsecs_t lastPresentTime); + + // Checks the present time history to see whether the layer is relevant. + bool isRecentlyActive() const { + std::lock_guard lock(mLock); + return mPresentTimeHistory.isRelevant(); + } + + // Calculate the average refresh rate. + float getDesiredRefreshRate() const { + std::lock_guard lock(mLock); + return mRefreshRateHistory.getRefreshRateAvg(); + } + + // Return the last updated time. If the present time is farther in the future than the + // updated time, the updated time is the present time. + nsecs_t getLastUpdatedTime() { + std::lock_guard lock(mLock); + return mLastUpdatedTime; + } + + std::string getName() const { return mName; } + + void clearHistory() { + std::lock_guard lock(mLock); + mRefreshRateHistory.clearHistory(); + mPresentTimeHistory.clearHistory(); + } + +private: + const std::string mName; + const nsecs_t mMinRefreshDuration; + mutable std::mutex mLock; + nsecs_t mLastUpdatedTime GUARDED_BY(mLock) = 0; + nsecs_t mLastPresentTime GUARDED_BY(mLock) = 0; + RefreshRateHistory mRefreshRateHistory GUARDED_BY(mLock); + PresentTimeHistory mPresentTimeHistory GUARDED_BY(mLock); +}; + +} // namespace scheduler +} // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h index 5874066aed..eb9ae35cec 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h @@ -54,7 +54,7 @@ public: const std::map>& getRefreshRates() const { return mRefreshRates; } - std::shared_ptr getRefreshRate(RefreshRateType type) { + std::shared_ptr getRefreshRate(RefreshRateType type) const { const auto& refreshRate = mRefreshRates.find(type); if (refreshRate != mRefreshRates.end()) { return refreshRate->second; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 0063c8a202..9f92d92ecf 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -39,6 +39,7 @@ #include "EventThread.h" #include "IdleTimer.h" #include "InjectVSyncSource.h" +#include "LayerInfo.h" #include "SchedulerUtils.h" #include "SurfaceFlingerProperties.h" @@ -55,11 +56,13 @@ using namespace android::sysprop; std::atomic Scheduler::sNextId = 0; -Scheduler::Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function) +Scheduler::Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function, + const scheduler::RefreshRateConfigs& refreshRateConfig) : mHasSyncFramework(running_without_sync_framework(true)), mDispSyncPresentTimeOffset(present_time_offset_from_vsync_ns(0)), mPrimaryHWVsyncEnabled(false), - mHWVsyncAvailable(false) { + mHWVsyncAvailable(false), + mRefreshRateConfigs(refreshRateConfig) { // Note: We create a local temporary with the real DispSync implementation // type temporarily so we can initialize it with the configured values, // before storing it for more generic use using the interface type. @@ -297,32 +300,32 @@ void Scheduler::dumpPrimaryDispSync(std::string& result) const { mPrimaryDispSync->dump(result); } -void Scheduler::addNativeWindowApi(int apiId) { - std::lock_guard lock(mWindowApiHistoryLock); - mWindowApiHistory[mApiHistoryCounter] = apiId; - mApiHistoryCounter++; - mApiHistoryCounter = mApiHistoryCounter % scheduler::ARRAY_SIZE; +std::unique_ptr Scheduler::registerLayer( + const std::string name) { + RefreshRateType refreshRateType = RefreshRateType::PERFORMANCE; + + const auto refreshRate = mRefreshRateConfigs.getRefreshRate(refreshRateType); + const uint32_t fps = (refreshRate) ? refreshRate->fps : 0; + return mLayerHistory.createLayer(name, fps); +} + +void Scheduler::addLayerPresentTime( + const std::unique_ptr& layerHandle, + nsecs_t presentTime) { + mLayerHistory.insert(layerHandle, presentTime); } void Scheduler::withPrimaryDispSync(std::function const& fn) { fn(*mPrimaryDispSync); } -void Scheduler::updateFpsBasedOnNativeWindowApi() { - int mode; - { - std::lock_guard lock(mWindowApiHistoryLock); - mode = scheduler::calculate_mode(mWindowApiHistory); - } - ATRACE_INT("NativeWindowApiMode", mode); - - if (mode == NATIVE_WINDOW_API_MEDIA) { - // TODO(b/127365162): These callback names are not accurate anymore. Update. - mediaChangeRefreshRate(MediaFeatureState::MEDIA_PLAYING); - ATRACE_INT("DetectedVideo", 1); +void Scheduler::updateFpsBasedOnContent() { + uint32_t refreshRate = std::round(mLayerHistory.getDesiredRefreshRate()); + ATRACE_INT("ContentFPS", refreshRate); + if (refreshRate > 0) { + contentChangeRefreshRate(ContentFeatureState::CONTENT_DETECTION_ON, refreshRate); } else { - mediaChangeRefreshRate(MediaFeatureState::MEDIA_OFF); - ATRACE_INT("DetectedVideo", 0); + contentChangeRefreshRate(ContentFeatureState::CONTENT_DETECTION_OFF, 0); } } @@ -365,39 +368,59 @@ std::string Scheduler::doDump() { return stream.str(); } -void Scheduler::mediaChangeRefreshRate(MediaFeatureState mediaFeatureState) { - // Default playback for media is DEFAULT when media is on. - RefreshRateType refreshRateType = RefreshRateType::DEFAULT; - ConfigEvent configEvent = ConfigEvent::None; - +void Scheduler::contentChangeRefreshRate(ContentFeatureState contentFeatureState, + uint32_t refreshRate) { + RefreshRateType newRefreshRateType; { std::lock_guard lock(mFeatureStateLock); - mCurrentMediaFeatureState = mediaFeatureState; - // Only switch to PERFORMANCE if idle timer was reset, when turning - // media off. If the timer is IDLE, stay at DEFAULT. - if (mediaFeatureState == MediaFeatureState::MEDIA_OFF && - mCurrentIdleTimerState == IdleTimerState::RESET) { - refreshRateType = RefreshRateType::PERFORMANCE; - } + mCurrentContentFeatureState = contentFeatureState; + mContentRefreshRate = refreshRate; + newRefreshRateType = calculateRefreshRateType(); } - changeRefreshRate(refreshRateType, configEvent); + changeRefreshRate(newRefreshRateType, ConfigEvent::Changed); } void Scheduler::timerChangeRefreshRate(IdleTimerState idleTimerState) { - RefreshRateType refreshRateType = RefreshRateType::DEFAULT; - ConfigEvent configEvent = ConfigEvent::None; - + RefreshRateType newRefreshRateType; { std::lock_guard lock(mFeatureStateLock); mCurrentIdleTimerState = idleTimerState; - // Only switch to PERFOMANCE if the idle timer was reset, and media is - // not playing. Otherwise, stay at DEFAULT. - if (idleTimerState == IdleTimerState::RESET && - mCurrentMediaFeatureState == MediaFeatureState::MEDIA_OFF) { - refreshRateType = RefreshRateType::PERFORMANCE; + newRefreshRateType = calculateRefreshRateType(); + } + changeRefreshRate(newRefreshRateType, ConfigEvent::None); +} + +Scheduler::RefreshRateType Scheduler::calculateRefreshRateType() { + // First check if timer has expired as it means there is no new content on the screen + if (mCurrentIdleTimerState == IdleTimerState::EXPIRED) { + return RefreshRateType::DEFAULT; + } + + // If content detection is off we choose performance as we don't know the content fps + if (mCurrentContentFeatureState == ContentFeatureState::CONTENT_DETECTION_OFF) { + return RefreshRateType::PERFORMANCE; + } + + // Content detection is on, find the appropriate refresh rate + // Start with the smallest refresh rate which is greater than the content + auto iter = mRefreshRateConfigs.getRefreshRates().cbegin(); + RefreshRateType currRefreshRateType = iter->first; + while (iter != mRefreshRateConfigs.getRefreshRates().cend()) { + if (iter->second->fps >= mContentRefreshRate) { + currRefreshRateType = iter->first; + break; } + ++iter; + } + + if (iter == mRefreshRateConfigs.getRefreshRates().cend()) { + return RefreshRateType::PERFORMANCE; } - changeRefreshRate(refreshRateType, configEvent); + + // TODO(b/129874336): This logic is sub-optimal for content refresh rate that aligns better + // with a higher refresh rate. For example for 45fps we should choose 90Hz config. + + return currRefreshRateType; } void Scheduler::changeRefreshRate(RefreshRateType refreshRateType, ConfigEvent configEvent) { diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 73896d5763..1d53252361 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -90,7 +90,8 @@ public: std::atomic lastResyncTime = 0; }; - explicit Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function); + explicit Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function, + const scheduler::RefreshRateConfigs& refreshRateConfig); virtual ~Scheduler(); @@ -145,16 +146,20 @@ public: void addPresentFence(const std::shared_ptr& fenceTime); void setIgnorePresentFences(bool ignore); nsecs_t expectedPresentTime(); - // apiId indicates the API (NATIVE_WINDOW_API_xxx) that queues the buffer. - // TODO(b/123956502): Remove this call with V1 go/content-fps-detection-in-scheduler. - void addNativeWindowApi(int apiId); - // Updates FPS based on the most occured request for Native Window API. - void updateFpsBasedOnNativeWindowApi(); + // Registers the layer in the scheduler, and returns the handle for future references. + std::unique_ptr registerLayer(const std::string name); + // Stores present time for a layer. + void addLayerPresentTime( + const std::unique_ptr& layerHandle, + nsecs_t presentTime); + // Updates FPS based on the most content presented. + void updateFpsBasedOnContent(); // Callback that gets invoked when Scheduler wants to change the refresh rate. void setChangeRefreshRateCallback(const ChangeRefreshRateCallback& changeRefreshRateCallback); // Returns whether idle timer is enabled or not bool isIdleTimerEnabled() { return mSetIdleTimerMs > 0; } + // Returns relevant information about Scheduler for dumpsys purposes. std::string doDump(); @@ -171,7 +176,7 @@ private: // In order to make sure that the features don't override themselves, we need a state machine // to keep track which feature requested the config change. - enum class MediaFeatureState { MEDIA_PLAYING, MEDIA_OFF }; + enum class ContentFeatureState { CONTENT_DETECTION_ON, CONTENT_DETECTION_OFF }; enum class IdleTimerState { EXPIRED, RESET }; // Creates a connection on the given EventThread and forwards the given callbacks. @@ -188,12 +193,17 @@ private: // Sets vsync period. void setVsyncPeriod(const nsecs_t period); // Media feature's function to change the refresh rate. - void mediaChangeRefreshRate(MediaFeatureState mediaFeatureState); + void contentChangeRefreshRate(ContentFeatureState contentFeatureState, uint32_t refreshRate); // Idle timer feature's function to change the refresh rate. void timerChangeRefreshRate(IdleTimerState idleTimerState); + // Calculate the new refresh rate type + RefreshRateType calculateRefreshRateType() REQUIRES(mFeatureStateLock); // Acquires a lock and calls the ChangeRefreshRateCallback() with given parameters. void changeRefreshRate(RefreshRateType refreshRateType, ConfigEvent configEvent); + // Helper function to calculate error frames + float getErrorFrames(float contentFps, float configFps); + // If fences from sync Framework are supported. const bool mHasSyncFramework; @@ -226,13 +236,8 @@ private: std::array mTimeDifferences{}; size_t mCounter = 0; - // The following few fields follow native window api bits that come with buffers. If there are - // more buffers with NATIVE_WINDOW_API_MEDIA we render at 60Hz, otherwise we render at 90Hz. - // There is not dependency on timestamp for V0. - // TODO(b/123956502): Remove this when more robust logic for content fps detection is developed. - std::mutex mWindowApiHistoryLock; - std::array mWindowApiHistory GUARDED_BY(mWindowApiHistoryLock); - int64_t mApiHistoryCounter = 0; + // Historical information about individual layers. Used for predicting the refresh rate. + scheduler::LayerHistory mLayerHistory; // Timer that records time between requests for next vsync. If the time is higher than a given // interval, a callback is fired. Set this variable to >0 to use this feature. @@ -245,9 +250,12 @@ private: // In order to make sure that the features don't override themselves, we need a state machine // to keep track which feature requested the config change. std::mutex mFeatureStateLock; - MediaFeatureState mCurrentMediaFeatureState GUARDED_BY(mFeatureStateLock) = - MediaFeatureState::MEDIA_OFF; + ContentFeatureState mCurrentContentFeatureState GUARDED_BY(mFeatureStateLock) = + ContentFeatureState::CONTENT_DETECTION_OFF; IdleTimerState mCurrentIdleTimerState GUARDED_BY(mFeatureStateLock) = IdleTimerState::RESET; + uint32_t mContentRefreshRate; + + const scheduler::RefreshRateConfigs& mRefreshRateConfigs; }; } // namespace android diff --git a/services/surfaceflinger/Scheduler/SchedulerUtils.h b/services/surfaceflinger/Scheduler/SchedulerUtils.h index 9e6e8c7af6..a935cac1eb 100644 --- a/services/surfaceflinger/Scheduler/SchedulerUtils.h +++ b/services/surfaceflinger/Scheduler/SchedulerUtils.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -23,6 +24,8 @@ namespace android { namespace scheduler { +using namespace std::chrono_literals; + // This number is used to set the size of the arrays in scheduler that hold information // about layers. static constexpr size_t ARRAY_SIZE = 30; @@ -32,12 +35,20 @@ static constexpr size_t ARRAY_SIZE = 30; // still like to keep track of time when the device is in this config. static constexpr int SCREEN_OFF_CONFIG_ID = -1; +// This number is used when we try to determine how long does a given layer stay relevant. +// Currently it is set to 100ms, because that would indicate 10Hz rendering. +static constexpr std::chrono::nanoseconds TIME_EPSILON_NS = 100ms; + +// This number is used when we try to determine how long do we keep layer information around +// before we remove it. Currently it is set to 100ms. +static constexpr std::chrono::nanoseconds OBSOLETE_TIME_EPSILON_NS = 100ms; + // Calculates the statistical mean (average) in the data structure (array, vector). The // function does not modify the contents of the array. template auto calculate_mean(const T& v) { using V = typename T::value_type; - V sum = std::accumulate(v.begin(), v.end(), 0); + V sum = std::accumulate(v.begin(), v.end(), static_cast(0)); return sum / static_cast(v.size()); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 9c2cef8cbd..7c88cd807d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -605,7 +605,8 @@ void SurfaceFlinger::init() { Mutex::Autolock _l(mStateLock); // start the EventThread mScheduler = - getFactory().createScheduler([this](bool enabled) { setPrimaryVsyncEnabled(enabled); }); + getFactory().createScheduler([this](bool enabled) { setPrimaryVsyncEnabled(enabled); }, + mRefreshRateConfigs); auto resyncCallback = mScheduler->makeResyncCallback(std::bind(&SurfaceFlinger::getVsyncPeriod, this)); @@ -702,6 +703,7 @@ void SurfaceFlinger::init() { setRefreshRateTo(type, event); }); } + mRefreshRateConfigs.populate(getHwComposer().getConfigs(*display->getId())); mRefreshRateStats.setConfigMode(getHwComposer().getActiveConfigIndex(*display->getId())); @@ -1607,7 +1609,7 @@ void SurfaceFlinger::onMessageReceived(int32_t what) NO_THREAD_SAFETY_ANALYSIS { if (mUseSmart90ForVideo) { // This call is made each time SF wakes up and creates a new frame. It is part // of video detection feature. - mScheduler->updateFpsBasedOnNativeWindowApi(); + mScheduler->updateFpsBasedOnContent(); } if (performSetActiveConfig()) { @@ -3115,6 +3117,7 @@ void SurfaceFlinger::invalidateLayerStack(const sp& layer, const Re bool SurfaceFlinger::handlePageFlip() { + ATRACE_CALL(); ALOGV("handlePageFlip"); nsecs_t latchTime = systemTime(); @@ -3140,6 +3143,7 @@ bool SurfaceFlinger::handlePageFlip() if (layer->shouldPresentNow(expectedPresentTime)) { mLayersWithQueuedFrames.push_back(layer); } else { + ATRACE_NAME("!layer->shouldPresentNow()"); layer->useEmptyDamage(); } } else { @@ -3979,10 +3983,8 @@ uint32_t SurfaceFlinger::setClientStateLocked( buffer = s.buffer; } if (buffer) { - if (layer->setBuffer(buffer)) { + if (layer->setBuffer(buffer, postTime, desiredPresentTime)) { flags |= eTraversalNeeded; - layer->setPostTime(postTime); - layer->setDesiredPresentTime(desiredPresentTime); } } if (layer->setTransactionCompletedListeners(callbackHandles)) flags |= eTraversalNeeded; diff --git a/services/surfaceflinger/SurfaceFlingerFactory.cpp b/services/surfaceflinger/SurfaceFlingerFactory.cpp index 26d2c21848..e425b2a953 100644 --- a/services/surfaceflinger/SurfaceFlingerFactory.cpp +++ b/services/surfaceflinger/SurfaceFlingerFactory.cpp @@ -73,8 +73,10 @@ sp createSurfaceFlinger() { return std::make_unique(); } - std::unique_ptr createScheduler(std::function callback) override { - return std::make_unique(callback); + std::unique_ptr createScheduler( + std::function callback, + const scheduler::RefreshRateConfigs& refreshRateConfig) override { + return std::make_unique(callback, refreshRateConfig); } std::unique_ptr createSurfaceInterceptor( diff --git a/services/surfaceflinger/SurfaceFlingerFactory.h b/services/surfaceflinger/SurfaceFlingerFactory.h index fc1d0f8b7f..c2bc808c8c 100644 --- a/services/surfaceflinger/SurfaceFlingerFactory.h +++ b/services/surfaceflinger/SurfaceFlingerFactory.h @@ -71,7 +71,9 @@ public: virtual std::unique_ptr createHWComposer(const std::string& serviceName) = 0; virtual std::unique_ptr createMessageQueue() = 0; virtual std::unique_ptr createPhaseOffsets() = 0; - virtual std::unique_ptr createScheduler(std::function callback) = 0; + virtual std::unique_ptr createScheduler( + std::function callback, + const scheduler::RefreshRateConfigs& refreshRateConfig) = 0; virtual std::unique_ptr createSurfaceInterceptor(SurfaceFlinger*) = 0; virtual sp createStartPropertySetThread( diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index ef3dfef38f..bebfa6c7ee 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -125,7 +125,7 @@ public: } void setupScheduler() { - mScheduler = new TestableScheduler(); + mScheduler = new TestableScheduler(mFlinger.mutableRefreshRateConfigs()); mScheduler->mutableEventControlThread().reset(mEventControlThread); mScheduler->mutablePrimaryDispSync().reset(mPrimaryDispSync); EXPECT_CALL(*mEventThread.get(), registerDisplayEventConnection(_)); diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index 9bf29a212f..d83f1bdf4f 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -179,7 +179,7 @@ DisplayTransactionTest::~DisplayTransactionTest() { } void DisplayTransactionTest::setupScheduler() { - mScheduler = new TestableScheduler(); + mScheduler = new TestableScheduler(mFlinger.mutableRefreshRateConfigs()); mScheduler->mutableEventControlThread().reset(mEventControlThread); mScheduler->mutablePrimaryDispSync().reset(mPrimaryDispSync); EXPECT_CALL(*mEventThread, registerDisplayEventConnection(_)); diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp index 3fb1a6e62a..e51121fcf7 100644 --- a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp @@ -7,6 +7,7 @@ #include #include +#include #include "Scheduler/LayerHistory.h" @@ -14,6 +15,7 @@ using testing::_; using testing::Return; namespace android { +namespace scheduler { class LayerHistoryTest : public testing::Test { public: @@ -22,6 +24,8 @@ public: protected: std::unique_ptr mLayerHistory; + + static constexpr float MAX_REFRESH_RATE = 90.f; }; LayerHistoryTest::LayerHistoryTest() { @@ -30,145 +34,79 @@ LayerHistoryTest::LayerHistoryTest() { LayerHistoryTest::~LayerHistoryTest() {} namespace { -TEST_F(LayerHistoryTest, simpleInsertAndGet) { - mLayerHistory->insert("TestLayer", 0); - - const std::unordered_map& testMap = mLayerHistory->get(0); - EXPECT_EQ(1, testMap.size()); - auto element = testMap.find("TestLayer"); - EXPECT_EQ("TestLayer", element->first); - EXPECT_EQ(0, element->second); - - // Testing accessing object at an empty container will return an empty map. - const std::unordered_map& emptyMap = mLayerHistory->get(1); - EXPECT_EQ(0, emptyMap.size()); +TEST_F(LayerHistoryTest, oneLayer) { + std::unique_ptr testLayer = + mLayerHistory->createLayer("TestLayer", MAX_REFRESH_RATE); + + mLayerHistory->insert(testLayer, 0); + EXPECT_FLOAT_EQ(0.f, mLayerHistory->getDesiredRefreshRate()); + + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + // This is still 0, because the layer is not considered recently active if it + // has been present in less than 10 frames. + EXPECT_FLOAT_EQ(0.f, mLayerHistory->getDesiredRefreshRate()); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + mLayerHistory->insert(testLayer, 0); + // This should be MAX_REFRESH_RATE as we have more than 10 samples + EXPECT_FLOAT_EQ(MAX_REFRESH_RATE, mLayerHistory->getDesiredRefreshRate()); } -TEST_F(LayerHistoryTest, multipleInserts) { - mLayerHistory->insert("TestLayer0", 0); - mLayerHistory->insert("TestLayer1", 1); - mLayerHistory->insert("TestLayer2", 2); - mLayerHistory->insert("TestLayer3", 3); - - const std::unordered_map& testMap = mLayerHistory->get(0); - // Because the counter was not incremented, all elements were inserted into the first - // container. - EXPECT_EQ(4, testMap.size()); - auto element = testMap.find("TestLayer0"); - EXPECT_EQ("TestLayer0", element->first); - EXPECT_EQ(0, element->second); - - element = testMap.find("TestLayer1"); - EXPECT_EQ("TestLayer1", element->first); - EXPECT_EQ(1, element->second); - - element = testMap.find("TestLayer2"); - EXPECT_EQ("TestLayer2", element->first); - EXPECT_EQ(2, element->second); - - element = testMap.find("TestLayer3"); - EXPECT_EQ("TestLayer3", element->first); - EXPECT_EQ(3, element->second); - - // Testing accessing object at an empty container will return an empty map. - const std::unordered_map& emptyMap = mLayerHistory->get(1); - EXPECT_EQ(0, emptyMap.size()); -} +TEST_F(LayerHistoryTest, explicitTimestamp) { + std::unique_ptr test30FpsLayer = + mLayerHistory->createLayer("30FpsLayer", MAX_REFRESH_RATE); -TEST_F(LayerHistoryTest, incrementingCounter) { - mLayerHistory->insert("TestLayer0", 0); - mLayerHistory->incrementCounter(); - mLayerHistory->insert("TestLayer1", 1); - mLayerHistory->insert("TestLayer2", 2); - mLayerHistory->incrementCounter(); - mLayerHistory->insert("TestLayer3", 3); - - // Because the counter was incremented, the elements were inserted into different - // containers. - // We expect the get method to access the slot at the current counter of the index - // is 0. - const std::unordered_map& testMap1 = mLayerHistory->get(0); - EXPECT_EQ(1, testMap1.size()); - auto element = testMap1.find("TestLayer3"); - EXPECT_EQ("TestLayer3", element->first); - EXPECT_EQ(3, element->second); - - const std::unordered_map& testMap2 = mLayerHistory->get(1); - EXPECT_EQ(2, testMap2.size()); - element = testMap2.find("TestLayer1"); - EXPECT_EQ("TestLayer1", element->first); - EXPECT_EQ(1, element->second); - element = testMap2.find("TestLayer2"); - EXPECT_EQ("TestLayer2", element->first); - EXPECT_EQ(2, element->second); - - const std::unordered_map& testMap3 = mLayerHistory->get(2); - EXPECT_EQ(1, testMap3.size()); - element = testMap3.find("TestLayer0"); - EXPECT_EQ("TestLayer0", element->first); - EXPECT_EQ(0, element->second); - - // Testing accessing object at an empty container will return an empty map. - const std::unordered_map& emptyMap = mLayerHistory->get(3); - EXPECT_EQ(0, emptyMap.size()); -} + nsecs_t startTime = systemTime(); + for (int i = 0; i < 31; i++) { + mLayerHistory->insert(test30FpsLayer, startTime + (i * 33333333)); + } -TEST_F(LayerHistoryTest, clearTheMap) { - mLayerHistory->insert("TestLayer0", 0); + EXPECT_FLOAT_EQ(30.f, mLayerHistory->getDesiredRefreshRate()); +} - const std::unordered_map& testMap1 = mLayerHistory->get(0); - EXPECT_EQ(1, testMap1.size()); - auto element = testMap1.find("TestLayer0"); - EXPECT_EQ("TestLayer0", element->first); - EXPECT_EQ(0, element->second); +TEST_F(LayerHistoryTest, multipleLayers) { + std::unique_ptr testLayer = + mLayerHistory->createLayer("TestLayer", MAX_REFRESH_RATE); + std::unique_ptr test30FpsLayer = + mLayerHistory->createLayer("30FpsLayer", MAX_REFRESH_RATE); + std::unique_ptr testLayer2 = + mLayerHistory->createLayer("TestLayer2", MAX_REFRESH_RATE); + + nsecs_t startTime = systemTime(); + for (int i = 0; i < 10; i++) { + mLayerHistory->insert(testLayer, 0); + } + EXPECT_FLOAT_EQ(MAX_REFRESH_RATE, mLayerHistory->getDesiredRefreshRate()); - mLayerHistory->incrementCounter(); - // The array currently contains 30 elements. - for (int i = 1; i < 30; ++i) { - mLayerHistory->insert("TestLayer0", i); - mLayerHistory->incrementCounter(); + startTime = systemTime(); + for (int i = 0; i < 10; i++) { + mLayerHistory->insert(test30FpsLayer, startTime + (i * 33333333)); } - // Expect the map to be cleared. - const std::unordered_map& testMap2 = mLayerHistory->get(0); - EXPECT_EQ(0, testMap2.size()); - - mLayerHistory->insert("TestLayer30", 30); - const std::unordered_map& testMap3 = mLayerHistory->get(0); - element = testMap3.find("TestLayer30"); - EXPECT_EQ("TestLayer30", element->first); - EXPECT_EQ(30, element->second); - // The original element in this location does not exist anymore. - element = testMap3.find("TestLayer0"); - EXPECT_EQ(testMap3.end(), element); -} + EXPECT_FLOAT_EQ(MAX_REFRESH_RATE, mLayerHistory->getDesiredRefreshRate()); -TEST_F(LayerHistoryTest, testingGet) { - // The array currently contains 30 elements. - for (int i = 0; i < 30; ++i) { - const auto name = "TestLayer" + std::to_string(i); - mLayerHistory->insert(name, i); - mLayerHistory->incrementCounter(); + for (int i = 10; i < 30; i++) { + mLayerHistory->insert(test30FpsLayer, startTime + (i * 33333333)); } + EXPECT_FLOAT_EQ(MAX_REFRESH_RATE, mLayerHistory->getDesiredRefreshRate()); - // The counter should be set to 0, and the map at 0 should be cleared. - const std::unordered_map& testMap1 = mLayerHistory->get(0); - EXPECT_EQ(0, testMap1.size()); - - // This should return (ARRAY_SIZE + (counter - 3)) % ARRAY_SIZE - const std::unordered_map& testMap2 = mLayerHistory->get(3); - EXPECT_EQ(1, testMap2.size()); - auto element = testMap2.find("TestLayer27"); - EXPECT_EQ("TestLayer27", element->first); - EXPECT_EQ(27, element->second); - - // If the user gives an out of bound index, we should mod it with ARRAY_SIZE first, - // so requesting element 40 would be the same as requesting element 10. - const std::unordered_map& testMap3 = mLayerHistory->get(40); - EXPECT_EQ(1, testMap3.size()); - element = testMap3.find("TestLayer20"); - EXPECT_EQ("TestLayer20", element->first); - EXPECT_EQ(20, element->second); + // This frame is only around for 9 occurrences, so it doesn't throw + // anything off. + for (int i = 0; i < 9; i++) { + mLayerHistory->insert(testLayer2, 0); + } + EXPECT_FLOAT_EQ(MAX_REFRESH_RATE, mLayerHistory->getDesiredRefreshRate()); + // After 100 ms frames become obsolete. + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + // Insert the 31st frame. + mLayerHistory->insert(test30FpsLayer, startTime + (30 * 33333333)); + EXPECT_FLOAT_EQ(30.f, mLayerHistory->getDesiredRefreshRate()); } } // namespace +} // namespace scheduler } // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index ec765382b8..b960bdf3fd 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -33,14 +33,17 @@ protected: MOCK_METHOD0(requestNextVsync, void()); }; + scheduler::RefreshRateConfigs mRefreshRateConfigs; + /** * This mock Scheduler class uses implementation of mock::EventThread but keeps everything else * the same. */ class MockScheduler : public android::Scheduler { public: - MockScheduler(std::unique_ptr eventThread) - : Scheduler([](bool) {}), mEventThread(std::move(eventThread)) {} + MockScheduler(const scheduler::RefreshRateConfigs& refreshRateConfigs, + std::unique_ptr eventThread) + : Scheduler([](bool) {}, refreshRateConfigs), mEventThread(std::move(eventThread)) {} std::unique_ptr makeEventThread( const char* /* connectionName */, DispSync* /* dispSync */, @@ -71,7 +74,7 @@ SchedulerTest::SchedulerTest() { std::unique_ptr eventThread = std::make_unique(); mEventThread = eventThread.get(); - mScheduler = std::make_unique(std::move(eventThread)); + mScheduler = std::make_unique(mRefreshRateConfigs, std::move(eventThread)); EXPECT_CALL(*mEventThread, registerDisplayEventConnection(_)).WillOnce(Return(0)); mEventThreadConnection = new MockEventThreadConnection(mEventThread); diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index dcbf973d2c..1c397d8443 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -19,13 +19,15 @@ #include #include "Scheduler/EventThread.h" +#include "Scheduler/RefreshRateConfigs.h" #include "Scheduler/Scheduler.h" namespace android { class TestableScheduler : public Scheduler { public: - TestableScheduler() : Scheduler([](bool) {}) {} + TestableScheduler(const scheduler::RefreshRateConfigs& refreshRateConfig) + : Scheduler([](bool) {}, refreshRateConfig) {} // Creates EventThreadConnection with the given eventThread. Creates Scheduler::Connection // and adds it to the list of connectins. Returns the ConnectionHandle for the diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 81235ba260..74a380a586 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -86,7 +86,8 @@ public: return std::make_unique(); } - std::unique_ptr createScheduler(std::function) override { + std::unique_ptr createScheduler(std::function, + const scheduler::RefreshRateConfigs&) override { // TODO: Use test-fixture controlled factory return nullptr; } @@ -339,6 +340,7 @@ public: auto& mutableScheduler() { return mFlinger->mScheduler; } auto& mutableAppConnectionHandle() { return mFlinger->mAppConnectionHandle; } auto& mutableSfConnectionHandle() { return mFlinger->mSfConnectionHandle; } + auto& mutableRefreshRateConfigs() { return mFlinger->mRefreshRateConfigs; } ~TestableSurfaceFlinger() { // All these pointer and container clears help ensure that GMock does -- cgit v1.2.3-59-g8ed1b From cd1580cb3fe40e225000a56c9b4ee43c0ce0a45d Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 29 Apr 2019 15:40:03 -0700 Subject: SurfaceFlinger: fix deferred transactions for buffers with timestamps A deferred transaction needs to wait until the buffer is ready to be latched. This means that the buffer needs to be: 1. Done with rendering (fence has signaled) 2. Present timestamp is within the boundary of the next vsync Test: Screen rotation with Chrome Bug: 130785247 Change-Id: I8def1f10ea3d5c253ab14fa3aa4445588fc2ba8b --- libs/gui/BufferQueueConsumer.cpp | 2 -- libs/gui/include/gui/BufferQueueConsumer.h | 3 ++ services/surfaceflinger/BufferLayer.cpp | 8 ++++-- services/surfaceflinger/BufferLayer.h | 1 + services/surfaceflinger/BufferQueueLayer.cpp | 42 +++++++++++++++++++++++++++- services/surfaceflinger/BufferQueueLayer.h | 1 + services/surfaceflinger/BufferStateLayer.cpp | 8 ++++++ services/surfaceflinger/BufferStateLayer.h | 1 + 8 files changed, 60 insertions(+), 6 deletions(-) (limited to 'libs/gui/BufferQueueConsumer.cpp') diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index e2261360b6..528bfb194e 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -94,8 +94,6 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, // Skip this if we're in shared buffer mode and the queue is empty, // since in that case we'll just return the shared buffer. if (expectedPresent != 0 && !mCore->mQueue.empty()) { - const int MAX_REASONABLE_NSEC = 1000000000ULL; // 1 second - // The 'expectedPresent' argument indicates when the buffer is expected // to be presented on-screen. If the buffer's desired present time is // earlier (less) than expectedPresent -- meaning it will be displayed diff --git a/libs/gui/include/gui/BufferQueueConsumer.h b/libs/gui/include/gui/BufferQueueConsumer.h index aa13c0cf13..7db69eca9d 100644 --- a/libs/gui/include/gui/BufferQueueConsumer.h +++ b/libs/gui/include/gui/BufferQueueConsumer.h @@ -171,6 +171,9 @@ public: // End functions required for backwards compatibility + // Value used to determine if present time is valid. + constexpr static int MAX_REASONABLE_NSEC = 1'000'000'000ULL; // 1 second + private: sp mCore; diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index 4ea587dbba..06caf1e2dc 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -534,11 +534,13 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime) // transaction void BufferLayer::notifyAvailableFrames() { - auto headFrameNumber = getHeadFrameNumber(); - bool headFenceSignaled = fenceHasSignaled(); + const auto headFrameNumber = getHeadFrameNumber(); + const bool headFenceSignaled = fenceHasSignaled(); + const bool presentTimeIsCurrent = framePresentTimeIsCurrent(); Mutex::Autolock lock(mLocalSyncPointMutex); for (auto& point : mLocalSyncPoints) { - if (headFrameNumber >= point->getFrameNumber() && headFenceSignaled) { + if (headFrameNumber >= point->getFrameNumber() && headFenceSignaled && + presentTimeIsCurrent) { point->setFrameAvailable(); } } diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h index dc103cbada..b679380b79 100644 --- a/services/surfaceflinger/BufferLayer.h +++ b/services/surfaceflinger/BufferLayer.h @@ -114,6 +114,7 @@ public: // ----------------------------------------------------------------------- private: virtual bool fenceHasSignaled() const = 0; + virtual bool framePresentTimeIsCurrent() const = 0; virtual nsecs_t getDesiredPresentTime() = 0; virtual std::shared_ptr getCurrentFenceTime() const = 0; diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index ff5f271960..5d729f5b4f 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "BufferQueueLayer.h" @@ -133,6 +134,15 @@ bool BufferQueueLayer::fenceHasSignaled() const { return mQueueItems[0].mFenceTime->getSignalTime() != Fence::SIGNAL_TIME_PENDING; } +bool BufferQueueLayer::framePresentTimeIsCurrent() const { + if (!hasFrameUpdate() || isRemovedFromCurrentState()) { + return true; + } + + Mutex::Autolock lock(mQueueItemLock); + return mQueueItems[0].mTimestamp <= mFlinger->mScheduler->expectedPresentTime(); +} + nsecs_t BufferQueueLayer::getDesiredPresentTime() { return mConsumer->getTimestamp(); } @@ -185,7 +195,37 @@ PixelFormat BufferQueueLayer::getPixelFormat() const { uint64_t BufferQueueLayer::getFrameNumber() const { Mutex::Autolock lock(mQueueItemLock); - return mQueueItems[0].mFrameNumber; + uint64_t frameNumber = mQueueItems[0].mFrameNumber; + + // The head of the queue will be dropped if there are signaled and timely frames behind it + nsecs_t expectedPresentTime = mFlinger->mScheduler->expectedPresentTime(); + + if (isRemovedFromCurrentState()) { + expectedPresentTime = 0; + } + + for (int i = 1; i < mQueueItems.size(); i++) { + const bool fenceSignaled = + mQueueItems[i].mFenceTime->getSignalTime() != Fence::SIGNAL_TIME_PENDING; + if (!fenceSignaled) { + break; + } + + // We don't drop frames without explicit timestamps + if (mQueueItems[i].mIsAutoTimestamp) { + break; + } + + const nsecs_t desiredPresent = mQueueItems[i].mTimestamp; + if (desiredPresent < expectedPresentTime - BufferQueueConsumer::MAX_REASONABLE_NSEC || + desiredPresent > expectedPresentTime) { + break; + } + + frameNumber = mQueueItems[i].mFrameNumber; + } + + return frameNumber; } bool BufferQueueLayer::getAutoRefresh() const { diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h index a2aad17497..7def33ad78 100644 --- a/services/surfaceflinger/BufferQueueLayer.h +++ b/services/surfaceflinger/BufferQueueLayer.h @@ -61,6 +61,7 @@ public: // ----------------------------------------------------------------------- public: bool fenceHasSignaled() const override; + bool framePresentTimeIsCurrent() const override; private: nsecs_t getDesiredPresentTime() override; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index dabc683aa9..30848d6eb5 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -374,6 +374,14 @@ bool BufferStateLayer::fenceHasSignaled() const { return getDrawingState().acquireFence->getStatus() == Fence::Status::Signaled; } +bool BufferStateLayer::framePresentTimeIsCurrent() const { + if (!hasFrameUpdate() || isRemovedFromCurrentState()) { + return true; + } + + return mDesiredPresentTime <= mFlinger->mScheduler->expectedPresentTime(); +} + nsecs_t BufferStateLayer::getDesiredPresentTime() { return mDesiredPresentTime; } diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 13186dd343..4e2bc45287 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -101,6 +101,7 @@ public: // Interface implementation for BufferLayer // ----------------------------------------------------------------------- bool fenceHasSignaled() const override; + bool framePresentTimeIsCurrent() const override; private: nsecs_t getDesiredPresentTime() override; -- cgit v1.2.3-59-g8ed1b