diff options
29 files changed, 788 insertions, 565 deletions
diff --git a/cmds/cmd/Android.bp b/cmds/cmd/Android.bp index 27ef78854c..cf27e1974a 100644 --- a/cmds/cmd/Android.bp +++ b/cmds/cmd/Android.bp @@ -27,14 +27,10 @@ cc_library_static { "libselinux", "libbinder", ], - whole_static_libs: [ - "libc++fs", - ], cflags: [ "-Wall", "-Werror", - "-DXP_UNIX", ], } @@ -58,6 +54,5 @@ cc_binary { cflags: [ "-Wall", "-Werror", - "-DXP_UNIX", ], } diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index 61fe316225..f3e024c5dc 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -102,7 +102,6 @@ cc_defaults { "libziparchive", "liblog", "liblogwrap", - "libc++fs", ], product_variables: { arc: { diff --git a/cmds/service/Android.bp b/cmds/service/Android.bp index 21ac11b4cf..00fd249f56 100644 --- a/cmds/service/Android.bp +++ b/cmds/service/Android.bp @@ -27,7 +27,6 @@ cc_binary { ], cflags: [ - "-DXP_UNIX", "-Wall", "-Werror", ], @@ -46,7 +45,6 @@ cc_binary { ], cflags: [ - "-DXP_UNIX", "-DVENDORSERVICES", "-Wall", "-Werror", @@ -65,7 +63,6 @@ cc_binary_host { ], cflags: [ - "-DXP_UNIX", "-Wall", "-Werror", ], diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f317a2eea0..739c3c2a41 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -613,8 +613,19 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3); sp<Fence> fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; + + nsecs_t dequeueTime = -1; + { + std::lock_guard _lock{mTimestampMutex}; + auto dequeueTimeIt = mDequeueTimestamps.find(buffer->getId()); + if (dequeueTimeIt != mDequeueTimestamps.end()) { + dequeueTime = dequeueTimeIt->second; + mDequeueTimestamps.erase(dequeueTimeIt); + } + } + t->setBuffer(mSurfaceControl, buffer, fence, bufferItem.mFrameNumber, mProducerId, - releaseBufferCallback); + releaseBufferCallback, dequeueTime); t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace)); t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata); t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage); @@ -658,17 +669,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( mPendingFrameTimelines.pop(); } - { - std::lock_guard _lock{mTimestampMutex}; - auto dequeueTime = mDequeueTimestamps.find(buffer->getId()); - if (dequeueTime != mDequeueTimestamps.end()) { - Parcel p; - p.writeInt64(dequeueTime->second); - t->setMetadata(mSurfaceControl, gui::METADATA_DEQUEUE_TIME, p); - mDequeueTimestamps.erase(dequeueTime); - } - } - mergePendingTransactions(t, bufferItem.mFrameNumber); if (applyTransaction) { // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index c82bde9a83..3745805aa3 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -985,6 +985,7 @@ status_t BufferData::writeToParcel(Parcel* output) const { SAFE_PARCEL(output->writeBool, hasBarrier); SAFE_PARCEL(output->writeUint64, barrierFrameNumber); SAFE_PARCEL(output->writeUint32, producerId); + SAFE_PARCEL(output->writeInt64, dequeueTime); return NO_ERROR; } @@ -1024,6 +1025,7 @@ status_t BufferData::readFromParcel(const Parcel* input) { SAFE_PARCEL(input->readBool, &hasBarrier); SAFE_PARCEL(input->readUint64, &barrierFrameNumber); SAFE_PARCEL(input->readUint32, &producerId); + SAFE_PARCEL(input->readInt64, &dequeueTime); return NO_ERROR; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index b420aabb84..af91bb3ae2 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1702,7 +1702,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer( const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const std::optional<sp<Fence>>& fence, const std::optional<uint64_t>& optFrameNumber, - uint32_t producerId, ReleaseBufferCallback callback) { + uint32_t producerId, ReleaseBufferCallback callback, nsecs_t dequeueTime) { layer_state_t* s = getLayerState(sc); if (!s) { mStatus = BAD_INDEX; @@ -1718,6 +1718,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe bufferData->frameNumber = frameNumber; bufferData->producerId = producerId; bufferData->flags |= BufferData::BufferDataChange::frameNumberChanged; + bufferData->dequeueTime = dequeueTime; if (fence) { bufferData->acquireFence = *fence; bufferData->flags |= BufferData::BufferDataChange::fenceChanged; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 82889efe36..ba06101059 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -128,6 +128,8 @@ public: client_cache_t cachedBuffer; + nsecs_t dequeueTime; + // Generates the release callback id based on the buffer id and frame number. // This is used as an identifier when release callbacks are invoked. ReleaseCallbackId generateReleaseCallbackId() const; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 9712907396..0862e03c44 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -568,7 +568,8 @@ public: Transaction& setBuffer(const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const std::optional<sp<Fence>>& fence = std::nullopt, const std::optional<uint64_t>& frameNumber = std::nullopt, - uint32_t producerId = 0, ReleaseBufferCallback callback = nullptr); + uint32_t producerId = 0, ReleaseBufferCallback callback = nullptr, + nsecs_t dequeueTime = -1); Transaction& unsetBuffer(const sp<SurfaceControl>& sc); std::shared_ptr<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc); diff --git a/services/inputflinger/reader/Android.bp b/services/inputflinger/reader/Android.bp index ba586d7a18..e76b648ce5 100644 --- a/services/inputflinger/reader/Android.bp +++ b/services/inputflinger/reader/Android.bp @@ -91,7 +91,6 @@ cc_defaults { "libutils", ], static_libs: [ - "libc++fs", "libchrome-gestures", "libui-types", ], @@ -156,7 +155,6 @@ cc_library_shared { }, }, static_libs: [ - "libc++fs", "libchrome-gestures", ], } diff --git a/services/inputflinger/tests/Android.bp b/services/inputflinger/tests/Android.bp index 9b5db23151..cf0d46a75d 100644 --- a/services/inputflinger/tests/Android.bp +++ b/services/inputflinger/tests/Android.bp @@ -109,7 +109,6 @@ cc_test { }, static_libs: [ "libflagtest", - "libc++fs", "libgmock", ], require_root: true, diff --git a/services/surfaceflinger/Display/DisplayModeController.cpp b/services/surfaceflinger/Display/DisplayModeController.cpp index f093384921..a6a9bec3c3 100644 --- a/services/surfaceflinger/Display/DisplayModeController.cpp +++ b/services/surfaceflinger/Display/DisplayModeController.cpp @@ -20,30 +20,221 @@ #include "Display/DisplayModeController.h" #include "Display/DisplaySnapshot.h" +#include "DisplayHardware/HWComposer.h" +#include <common/FlagManager.h> +#include <ftl/concat.h> +#include <ftl/expected.h> #include <log/log.h> namespace android::display { +template <size_t N> +inline std::string DisplayModeController::Display::concatId(const char (&str)[N]) const { + return std::string(ftl::Concat(str, ' ', snapshot.get().displayId().value).str()); +} + +DisplayModeController::Display::Display(DisplaySnapshotRef snapshot, + RefreshRateSelectorPtr selectorPtr) + : snapshot(snapshot), + selectorPtr(std::move(selectorPtr)), + pendingModeFpsTrace(concatId("PendingModeFps")), + activeModeFpsTrace(concatId("ActiveModeFps")), + renderRateFpsTrace(concatId("RenderRateFps")), + hasDesiredModeTrace(concatId("HasDesiredMode"), false) {} + +void DisplayModeController::registerDisplay(PhysicalDisplayId displayId, + DisplaySnapshotRef snapshotRef, + RefreshRateSelectorPtr selectorPtr) { + std::lock_guard lock(mDisplayLock); + mDisplays.emplace_or_replace(displayId, std::make_unique<Display>(snapshotRef, selectorPtr)); +} + void DisplayModeController::registerDisplay(DisplaySnapshotRef snapshotRef, DisplayModeId activeModeId, scheduler::RefreshRateSelector::Config config) { const auto& snapshot = snapshotRef.get(); const auto displayId = snapshot.displayId(); - mDisplays.emplace_or_replace(displayId, snapshotRef, snapshot.displayModes(), activeModeId, - config); + std::lock_guard lock(mDisplayLock); + mDisplays.emplace_or_replace(displayId, + std::make_unique<Display>(snapshotRef, snapshot.displayModes(), + activeModeId, config)); } void DisplayModeController::unregisterDisplay(PhysicalDisplayId displayId) { + std::lock_guard lock(mDisplayLock); const bool ok = mDisplays.erase(displayId); ALOGE_IF(!ok, "%s: Unknown display %s", __func__, to_string(displayId).c_str()); } -auto DisplayModeController::selectorPtrFor(PhysicalDisplayId displayId) -> RefreshRateSelectorPtr { +auto DisplayModeController::selectorPtrFor(PhysicalDisplayId displayId) const + -> RefreshRateSelectorPtr { + std::lock_guard lock(mDisplayLock); return mDisplays.get(displayId) - .transform([](const Display& display) { return display.selectorPtr; }) + .transform([](const DisplayPtr& displayPtr) { return displayPtr->selectorPtr; }) .value_or(nullptr); } +auto DisplayModeController::setDesiredMode(PhysicalDisplayId displayId, + DisplayModeRequest&& desiredMode) -> DesiredModeAction { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DesiredModeAction::None)).get(); + + { + ATRACE_NAME(displayPtr->concatId(__func__).c_str()); + ALOGD("%s %s", displayPtr->concatId(__func__).c_str(), to_string(desiredMode).c_str()); + + std::scoped_lock lock(displayPtr->desiredModeLock); + + if (auto& desiredModeOpt = displayPtr->desiredModeOpt) { + // A mode transition was already scheduled, so just override the desired mode. + const bool emitEvent = desiredModeOpt->emitEvent; + const bool force = desiredModeOpt->force; + desiredModeOpt = std::move(desiredMode); + desiredModeOpt->emitEvent |= emitEvent; + if (FlagManager::getInstance().connected_display()) { + desiredModeOpt->force |= force; + } + return DesiredModeAction::None; + } + + // If the desired mode is already active... + const auto activeMode = displayPtr->selectorPtr->getActiveMode(); + if (const auto& desiredModePtr = desiredMode.mode.modePtr; + !desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) { + if (activeMode == desiredMode.mode) { + return DesiredModeAction::None; + } + + // ...but the render rate changed: + setActiveModeLocked(displayId, desiredModePtr->getId(), desiredModePtr->getVsyncRate(), + desiredMode.mode.fps); + return DesiredModeAction::InitiateRenderRateSwitch; + } + + // Restore peak render rate to schedule the next frame as soon as possible. + setActiveModeLocked(displayId, activeMode.modePtr->getId(), + activeMode.modePtr->getVsyncRate(), activeMode.modePtr->getPeakFps()); + + // Initiate a mode change. + displayPtr->desiredModeOpt = std::move(desiredMode); + displayPtr->hasDesiredModeTrace = true; + } + + return DesiredModeAction::InitiateDisplayModeSwitch; +} + +auto DisplayModeController::getDesiredMode(PhysicalDisplayId displayId) const + -> DisplayModeRequestOpt { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DisplayModeRequestOpt())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->desiredModeOpt; + } +} + +auto DisplayModeController::getPendingMode(PhysicalDisplayId displayId) const + -> DisplayModeRequestOpt { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = + FTL_EXPECT(mDisplays.get(displayId).ok_or(DisplayModeRequestOpt())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->pendingModeOpt; + } +} + +bool DisplayModeController::isModeSetPending(PhysicalDisplayId displayId) const { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + return displayPtr->isModeSetPending; + } +} + +scheduler::FrameRateMode DisplayModeController::getActiveMode(PhysicalDisplayId displayId) const { + return selectorPtrFor(displayId)->getActiveMode(); +} + +void DisplayModeController::clearDesiredMode(PhysicalDisplayId displayId) { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + + { + std::scoped_lock lock(displayPtr->desiredModeLock); + displayPtr->desiredModeOpt.reset(); + displayPtr->hasDesiredModeTrace = false; + } +} + +bool DisplayModeController::initiateModeChange(PhysicalDisplayId displayId, + DisplayModeRequest&& desiredMode, + const hal::VsyncPeriodChangeConstraints& constraints, + hal::VsyncPeriodChangeTimeline& outTimeline) { + std::lock_guard lock(mDisplayLock); + const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get(); + + // TODO: b/255635711 - Flow the DisplayModeRequest through the desired/pending/active states. + // For now, `desiredMode` and `desiredModeOpt` are one and the same, but the latter is not + // cleared until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been + // consumed at this point, so clear the `force` flag to prevent an endless loop of + // `initiateModeChange`. + if (FlagManager::getInstance().connected_display()) { + std::scoped_lock lock(displayPtr->desiredModeLock); + if (displayPtr->desiredModeOpt) { + displayPtr->desiredModeOpt->force = false; + } + } + + displayPtr->pendingModeOpt = std::move(desiredMode); + displayPtr->isModeSetPending = true; + + const auto& mode = *displayPtr->pendingModeOpt->mode.modePtr; + + if (mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(), constraints, + &outTimeline) != OK) { + return false; + } + + ATRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue()); + return true; +} + +void DisplayModeController::finalizeModeChange(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + std::lock_guard lock(mDisplayLock); + setActiveModeLocked(displayId, modeId, vsyncRate, renderFps); + + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + displayPtr->isModeSetPending = false; +} + +void DisplayModeController::setActiveMode(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + std::lock_guard lock(mDisplayLock); + setActiveModeLocked(displayId, modeId, vsyncRate, renderFps); +} + +void DisplayModeController::setActiveModeLocked(PhysicalDisplayId displayId, DisplayModeId modeId, + Fps vsyncRate, Fps renderFps) { + const auto& displayPtr = FTL_TRY(mDisplays.get(displayId).ok_or(ftl::Unit())).get(); + + ATRACE_INT(displayPtr->activeModeFpsTrace.c_str(), vsyncRate.getIntValue()); + ATRACE_INT(displayPtr->renderRateFpsTrace.c_str(), renderFps.getIntValue()); + + displayPtr->selectorPtr->setActiveMode(modeId, renderFps); + + if (mActiveModeListener) { + mActiveModeListener(displayId, vsyncRate, renderFps); + } +} + } // namespace android::display diff --git a/services/surfaceflinger/Display/DisplayModeController.h b/services/surfaceflinger/Display/DisplayModeController.h index b6a6bee714..258b04b876 100644 --- a/services/surfaceflinger/Display/DisplayModeController.h +++ b/services/surfaceflinger/Display/DisplayModeController.h @@ -17,16 +17,26 @@ #pragma once #include <memory> +#include <mutex> +#include <string> #include <utility> #include <android-base/thread_annotations.h> +#include <ftl/function.h> +#include <ftl/optional.h> #include <ui/DisplayId.h> #include <ui/DisplayMap.h> +#include "Display/DisplayModeRequest.h" #include "Display/DisplaySnapshotRef.h" #include "DisplayHardware/DisplayMode.h" #include "Scheduler/RefreshRateSelector.h" #include "ThreadContext.h" +#include "TracedOrdinal.h" + +namespace android { +class HWComposer; +} // namespace android namespace android::display { @@ -34,40 +44,97 @@ namespace android::display { // certain heuristic signals. class DisplayModeController { public: - // The referenced DisplaySnapshot must outlive the registration. - void registerDisplay(DisplaySnapshotRef, DisplayModeId, scheduler::RefreshRateSelector::Config) - REQUIRES(kMainThreadContext); - void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext); + using ActiveModeListener = ftl::Function<void(PhysicalDisplayId, Fps vsyncRate, Fps renderFps)>; + + DisplayModeController() = default; - // TODO(b/241285876): Remove once ownership is no longer shared with DisplayDevice. + void setHwComposer(HWComposer* composerPtr) { mComposerPtr = composerPtr; } + void setActiveModeListener(const ActiveModeListener& listener) { + mActiveModeListener = listener; + } + + // TODO: b/241285876 - Remove once ownership is no longer shared with DisplayDevice. using RefreshRateSelectorPtr = std::shared_ptr<scheduler::RefreshRateSelector>; + // Used by tests to inject an existing RefreshRateSelector. + // TODO: b/241285876 - Remove this. + void registerDisplay(PhysicalDisplayId, DisplaySnapshotRef, RefreshRateSelectorPtr) + EXCLUDES(mDisplayLock); + + // The referenced DisplaySnapshot must outlive the registration. + void registerDisplay(DisplaySnapshotRef, DisplayModeId, scheduler::RefreshRateSelector::Config) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + // Returns `nullptr` if the display is no longer registered (or never was). - RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) REQUIRES(kMainThreadContext); + RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) const EXCLUDES(mDisplayLock); - // Used by tests to inject an existing RefreshRateSelector. - // TODO(b/241285876): Remove this. - void registerDisplay(PhysicalDisplayId displayId, DisplaySnapshotRef snapshotRef, - RefreshRateSelectorPtr selectorPtr) { - mDisplays.emplace_or_replace(displayId, snapshotRef, selectorPtr); - } + enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch }; + + DesiredModeAction setDesiredMode(PhysicalDisplayId, DisplayModeRequest&&) + EXCLUDES(mDisplayLock); + + using DisplayModeRequestOpt = ftl::Optional<DisplayModeRequest>; + + DisplayModeRequestOpt getDesiredMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock); + void clearDesiredMode(PhysicalDisplayId) EXCLUDES(mDisplayLock); + + DisplayModeRequestOpt getPendingMode(PhysicalDisplayId) const REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + bool isModeSetPending(PhysicalDisplayId) const REQUIRES(kMainThreadContext) + EXCLUDES(mDisplayLock); + + scheduler::FrameRateMode getActiveMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock); + + bool initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&, + const hal::VsyncPeriodChangeConstraints&, + hal::VsyncPeriodChangeTimeline& outTimeline) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + + void finalizeModeChange(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock); + + void setActiveMode(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + EXCLUDES(mDisplayLock); private: struct Display { - Display(DisplaySnapshotRef snapshot, RefreshRateSelectorPtr selectorPtr) - : snapshot(snapshot), selectorPtr(std::move(selectorPtr)) {} + template <size_t N> + std::string concatId(const char (&)[N]) const; + Display(DisplaySnapshotRef, RefreshRateSelectorPtr); Display(DisplaySnapshotRef snapshot, DisplayModes modes, DisplayModeId activeModeId, scheduler::RefreshRateSelector::Config config) : Display(snapshot, std::make_shared<scheduler::RefreshRateSelector>(std::move(modes), activeModeId, config)) {} - const DisplaySnapshotRef snapshot; const RefreshRateSelectorPtr selectorPtr; + + const std::string pendingModeFpsTrace; + const std::string activeModeFpsTrace; + const std::string renderRateFpsTrace; + + std::mutex desiredModeLock; + DisplayModeRequestOpt desiredModeOpt GUARDED_BY(desiredModeLock); + TracedOrdinal<bool> hasDesiredModeTrace GUARDED_BY(desiredModeLock); + + DisplayModeRequestOpt pendingModeOpt GUARDED_BY(kMainThreadContext); + bool isModeSetPending GUARDED_BY(kMainThreadContext) = false; }; - ui::PhysicalDisplayMap<PhysicalDisplayId, Display> mDisplays; + using DisplayPtr = std::unique_ptr<Display>; + + void setActiveModeLocked(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps) + REQUIRES(mDisplayLock); + + // Set once when initializing the DisplayModeController, which the HWComposer must outlive. + HWComposer* mComposerPtr = nullptr; + + ActiveModeListener mActiveModeListener; + + mutable std::mutex mDisplayLock; + ui::PhysicalDisplayMap<PhysicalDisplayId, DisplayPtr> mDisplays GUARDED_BY(mDisplayLock); }; } // namespace android::display diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index a57e626224..27ea4a9865 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -24,7 +24,6 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include <common/FlagManager.h> #include <compositionengine/CompositionEngine.h> #include <compositionengine/Display.h> #include <compositionengine/DisplayColorProfile.h> @@ -36,6 +35,7 @@ #include <compositionengine/RenderSurfaceCreationArgs.h> #include <compositionengine/impl/OutputCompositionState.h> #include <configstore/Utils.h> +#include <ftl/concat.h> #include <log/log.h> #include <system/window.h> @@ -64,15 +64,11 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) mDisplayToken(args.displayToken), mSequenceId(args.sequenceId), mCompositionDisplay{args.compositionDisplay}, - mPendingModeFpsTrace(concatId("PendingModeFps")), - mActiveModeFpsTrace(concatId("ActiveModeFps")), - mRenderRateFpsTrace(concatId("RenderRateFps")), mPhysicalOrientation(args.physicalOrientation), mPowerMode(ftl::Concat("PowerMode ", getId().value).c_str(), args.initialPowerMode), mIsPrimary(args.isPrimary), mRequestedRefreshRate(args.requestedRefreshRate), - mRefreshRateSelector(std::move(args.refreshRateSelector)), - mHasDesiredModeTrace(concatId("HasDesiredMode"), false) { + mRefreshRateSelector(std::move(args.refreshRateSelector)) { mCompositionDisplay->editState().isSecure = args.isSecure; mCompositionDisplay->editState().isProtected = args.isProtected; mCompositionDisplay->createRenderSurface( @@ -204,47 +200,6 @@ bool DisplayDevice::isPoweredOn() const { return mPowerMode != hal::PowerMode::OFF; } -void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) { - ATRACE_INT(mActiveModeFpsTrace.c_str(), vsyncRate.getIntValue()); - ATRACE_INT(mRenderRateFpsTrace.c_str(), renderFps.getIntValue()); - - mRefreshRateSelector->setActiveMode(modeId, renderFps); - updateRefreshRateOverlayRate(vsyncRate, renderFps); -} - -bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode, - const hal::VsyncPeriodChangeConstraints& constraints, - hal::VsyncPeriodChangeTimeline& outTimeline) { - // TODO(b/255635711): Flow the DisplayModeRequest through the desired/pending/active states. For - // now, `desiredMode` and `mDesiredModeOpt` are one and the same, but the latter is not cleared - // until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been consumed - // at this point, so clear the `force` flag to prevent an endless loop of `initiateModeChange`. - if (FlagManager::getInstance().connected_display()) { - std::scoped_lock lock(mDesiredModeLock); - if (mDesiredModeOpt) { - mDesiredModeOpt->force = false; - } - } - - mPendingModeOpt = std::move(desiredMode); - mIsModeSetPending = true; - - const auto& mode = *mPendingModeOpt->mode.modePtr; - - if (mHwComposer.setActiveModeWithConstraints(getPhysicalId(), mode.getHwcId(), constraints, - &outTimeline) != OK) { - return false; - } - - ATRACE_INT(mPendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue()); - return true; -} - -void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) { - setActiveMode(modeId, vsyncRate, renderFps); - mIsModeSetPending = false; -} - nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { const auto physicalId = getPhysicalId(); if (!mHwComposer.isConnected(physicalId)) { @@ -450,8 +405,9 @@ void DisplayDevice::updateHdrSdrRatioOverlayRatio(float currentHdrSdrRatio) { } } -void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, bool showSpinner, - bool showRenderRate, bool showInMiddle) { +void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, Fps refreshRate, + Fps renderFps, bool showSpinner, bool showRenderRate, + bool showInMiddle) { if (!enable) { mRefreshRateOverlay.reset(); return; @@ -479,8 +435,7 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, bool sh if (mRefreshRateOverlay) { mRefreshRateOverlay->setLayerStack(getLayerStack()); mRefreshRateOverlay->setViewport(getSize()); - updateRefreshRateOverlayRate(getActiveMode().modePtr->getVsyncRate(), getActiveMode().fps, - setByHwc); + updateRefreshRateOverlayRate(refreshRate, renderFps, setByHwc); } } @@ -531,57 +486,6 @@ void DisplayDevice::animateOverlay() { } } -auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode) -> DesiredModeAction { - ATRACE_NAME(concatId(__func__).c_str()); - ALOGD("%s %s", concatId(__func__).c_str(), to_string(desiredMode).c_str()); - - std::scoped_lock lock(mDesiredModeLock); - if (mDesiredModeOpt) { - // A mode transition was already scheduled, so just override the desired mode. - const bool emitEvent = mDesiredModeOpt->emitEvent; - const bool force = mDesiredModeOpt->force; - mDesiredModeOpt = std::move(desiredMode); - mDesiredModeOpt->emitEvent |= emitEvent; - if (FlagManager::getInstance().connected_display()) { - mDesiredModeOpt->force |= force; - } - return DesiredModeAction::None; - } - - // If the desired mode is already active... - const auto activeMode = refreshRateSelector().getActiveMode(); - if (const auto& desiredModePtr = desiredMode.mode.modePtr; - !desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) { - if (activeMode == desiredMode.mode) { - return DesiredModeAction::None; - } - - // ...but the render rate changed: - setActiveMode(desiredModePtr->getId(), desiredModePtr->getVsyncRate(), - desiredMode.mode.fps); - return DesiredModeAction::InitiateRenderRateSwitch; - } - - setActiveMode(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(), - activeMode.modePtr->getPeakFps()); - - // Initiate a mode change. - mDesiredModeOpt = std::move(desiredMode); - mHasDesiredModeTrace = true; - return DesiredModeAction::InitiateDisplayModeSwitch; -} - -auto DisplayDevice::getDesiredMode() const -> DisplayModeRequestOpt { - std::scoped_lock lock(mDesiredModeLock); - return mDesiredModeOpt; -} - -void DisplayDevice::clearDesiredMode() { - std::scoped_lock lock(mDesiredModeLock); - mDesiredModeOpt.reset(); - mHasDesiredModeTrace = false; -} - void DisplayDevice::adjustRefreshRate(Fps pacesetterDisplayRefreshRate) { using fps_approx_ops::operator<=; if (mRequestedRefreshRate <= 0_Hz) { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index a21559fb45..3cc8cf5d63 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -23,8 +23,6 @@ #include <android-base/thread_annotations.h> #include <android/native_window.h> #include <binder/IBinder.h> -#include <ftl/concat.h> -#include <ftl/optional.h> #include <gui/LayerState.h> #include <math/mat4.h> #include <renderengine/RenderEngine.h> @@ -42,7 +40,6 @@ #include <utils/RefBase.h> #include <utils/Timers.h> -#include "Display/DisplayModeRequest.h" #include "DisplayHardware/DisplayMode.h" #include "DisplayHardware/Hal.h" #include "DisplayHardware/PowerAdvisor.h" @@ -183,37 +180,6 @@ public: ui::Dataspace getCompositionDataSpace() const; - /* ------------------------------------------------------------------------ - * Display mode management. - */ - - enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch }; - - DesiredModeAction setDesiredMode(display::DisplayModeRequest&&) EXCLUDES(mDesiredModeLock); - - using DisplayModeRequestOpt = ftl::Optional<display::DisplayModeRequest>; - - DisplayModeRequestOpt getDesiredMode() const EXCLUDES(mDesiredModeLock); - void clearDesiredMode() EXCLUDES(mDesiredModeLock); - - DisplayModeRequestOpt getPendingMode() const REQUIRES(kMainThreadContext) { - return mPendingModeOpt; - } - bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; } - - scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) { - return mRefreshRateSelector->getActiveMode(); - } - - void setActiveMode(DisplayModeId, Fps vsyncRate, Fps renderFps); - - bool initiateModeChange(display::DisplayModeRequest&&, const hal::VsyncPeriodChangeConstraints&, - hal::VsyncPeriodChangeTimeline& outTimeline) - REQUIRES(kMainThreadContext); - - void finalizeModeChange(DisplayModeId, Fps vsyncRate, Fps renderFps) - REQUIRES(kMainThreadContext); - scheduler::RefreshRateSelector& refreshRateSelector() const { return *mRefreshRateSelector; } // Extends the lifetime of the RefreshRateSelector, so it can outlive this DisplayDevice. @@ -221,13 +187,14 @@ public: return mRefreshRateSelector; } - void animateOverlay(); - // Enables an overlay to be displayed with the current refresh rate - void enableRefreshRateOverlay(bool enable, bool setByHwc, bool showSpinner, bool showRenderRate, - bool showInMiddle) REQUIRES(kMainThreadContext); + // TODO(b/241285876): Move overlay to DisplayModeController. + void enableRefreshRateOverlay(bool enable, bool setByHwc, Fps refreshRate, Fps renderFps, + bool showSpinner, bool showRenderRate, bool showInMiddle) + REQUIRES(kMainThreadContext); void updateRefreshRateOverlayRate(Fps refreshRate, Fps renderFps, bool setByHwc = false); bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; } + void animateOverlay(); bool onKernelTimerChanged(std::optional<DisplayModeId>, bool timerExpired); // Enables an overlay to be display with the hdr/sdr ratio @@ -249,11 +216,6 @@ public: void dump(utils::Dumper&) const; private: - template <size_t N> - inline std::string concatId(const char (&str)[N]) const { - return std::string(ftl::Concat(str, ' ', getId().value).str()); - } - const sp<SurfaceFlinger> mFlinger; HWComposer& mHwComposer; const wp<IBinder> mDisplayToken; @@ -262,9 +224,6 @@ private: const std::shared_ptr<compositionengine::Display> mCompositionDisplay; std::string mDisplayName; - std::string mPendingModeFpsTrace; - std::string mActiveModeFpsTrace; - std::string mRenderRateFpsTrace; const ui::Rotation mPhysicalOrientation; ui::Rotation mOrientation = ui::ROTATION_0; @@ -296,13 +255,6 @@ private: std::unique_ptr<HdrSdrRatioOverlay> mHdrSdrRatioOverlay; // This parameter is only used for hdr/sdr ratio overlay float mHdrSdrRatio = 1.0f; - - mutable std::mutex mDesiredModeLock; - DisplayModeRequestOpt mDesiredModeOpt GUARDED_BY(mDesiredModeLock); - TracedOrdinal<bool> mHasDesiredModeTrace GUARDED_BY(mDesiredModeLock); - - DisplayModeRequestOpt mPendingModeOpt GUARDED_BY(kMainThreadContext); - bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false; }; struct DisplayDeviceState { diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index 6d4b0b5c54..ca53a0dfa6 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -256,6 +256,9 @@ auto getBlendMode(const LayerSnapshot& snapshot, const RequestedLayerState& requ } void updateVisibility(LayerSnapshot& snapshot, bool visible) { + if (snapshot.isVisible != visible) { + snapshot.changes |= RequestedLayerState::Changes::Visibility; + } snapshot.isVisible = visible; // TODO(b/238781169) we are ignoring this compat for now, since we will have diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 6b97e2f799..d27bfd29ef 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -3149,8 +3149,7 @@ void Layer::resetDrawingStateBufferInfo() { bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, const BufferData& bufferData, nsecs_t postTime, nsecs_t desiredPresentTime, - bool isAutoTimestamp, std::optional<nsecs_t> dequeueTime, - const FrameTimelineInfo& info) { + bool isAutoTimestamp, const FrameTimelineInfo& info) { ATRACE_FORMAT("setBuffer %s - hasBuffer=%s", getDebugName(), (buffer ? "true" : "false")); const bool frameNumberChanged = @@ -3224,10 +3223,11 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, setFrameTimelineVsyncForBufferTransaction(info, postTime); - if (dequeueTime && *dequeueTime != 0) { + if (bufferData.dequeueTime > 0) { const uint64_t bufferId = mDrawingState.buffer->getId(); mFlinger->mFrameTracer->traceNewLayer(layerId, getName().c_str()); - mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, frameNumber, *dequeueTime, + mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, frameNumber, + bufferData.dequeueTime, FrameTracer::FrameEvent::DEQUEUE); mFlinger->mFrameTracer->traceTimestamp(layerId, bufferId, frameNumber, postTime, FrameTracer::FrameEvent::QUEUE); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 9db7664aa2..b9fcd5c333 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -312,7 +312,7 @@ public: bool setBuffer(std::shared_ptr<renderengine::ExternalTexture>& /* buffer */, const BufferData& /* bufferData */, nsecs_t /* postTime */, nsecs_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/, - std::optional<nsecs_t> /* dequeueTime */, const FrameTimelineInfo& /*info*/); + const FrameTimelineInfo& /*info*/); void setDesiredPresentTime(nsecs_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/); bool setDataspace(ui::Dataspace /*dataspace*/); bool setExtendedRangeBrightness(float currentBufferRatio, float desiredRatio); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 59345db67c..596ec12d9e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -891,8 +891,12 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { } mCompositionEngine->setTimeStats(mTimeStats); + mCompositionEngine->setHwComposer(getFactory().createHWComposer(mHwcServiceName)); - mCompositionEngine->getHwComposer().setCallback(*this); + auto& composer = mCompositionEngine->getHwComposer(); + composer.setCallback(*this); + mDisplayModeController.setHwComposer(&composer); + ClientCache::getInstance().setRenderEngine(&getRenderEngine()); mHasReliablePresentFences = @@ -931,6 +935,20 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { // initializing the Scheduler after configureLocked, once decoupled from DisplayDevice. initScheduler(display); + // Start listening after creating the Scheduler, since the listener calls into it. + mDisplayModeController.setActiveModeListener( + display::DisplayModeController::ActiveModeListener::make( + [this](PhysicalDisplayId displayId, Fps vsyncRate, Fps renderRate) { + // This callback cannot lock mStateLock, as some callers already lock it. + // Instead, switch context to the main thread. + static_cast<void>(mScheduler->schedule([=, + this]() FTL_FAKE_GUARD(mStateLock) { + if (const auto display = getDisplayDeviceLocked(displayId)) { + display->updateRefreshRateOverlayRate(vsyncRate, renderRate); + } + })); + })); + mLayerTracing.setTakeLayersSnapshotProtoFunction([&](uint32_t traceFlags) { auto snapshot = perfetto::protos::LayersSnapshotProto{}; mScheduler @@ -1296,19 +1314,19 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); - const auto display = getDisplayDeviceLocked(displayId); - if (!display) { - ALOGW("%s: display is no longer valid", __func__); - return; - } - const bool emitEvent = desiredMode.emitEvent; - switch (display->setDesiredMode(std::move(desiredMode))) { - case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch: - // DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler. - mScheduler->setRenderRate(displayId, display->refreshRateSelector().getActiveMode().fps, - /*applyImmediately*/ true); + using DesiredModeAction = display::DisplayModeController::DesiredModeAction; + + switch (mDisplayModeController.setDesiredMode(displayId, std::move(desiredMode))) { + case DesiredModeAction::InitiateDisplayModeSwitch: { + const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId); + if (!selectorPtr) break; + + const Fps renderRate = selectorPtr->getActiveMode().fps; + + // DisplayModeController::setDesiredMode updated the render rate, so inform Scheduler. + mScheduler->setRenderRate(displayId, renderRate, true /* applyImmediately */); // Schedule a new frame to initiate the display mode switch. scheduleComposite(FrameHint::kNone); @@ -1328,7 +1346,8 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { mScheduler->setModeChangePending(true); break; - case DisplayDevice::DesiredModeAction::InitiateRenderRateSwitch: + } + case DesiredModeAction::InitiateRenderRateSwitch: mScheduler->setRenderRate(displayId, mode.fps, /*applyImmediately*/ false); if (displayId == mActiveDisplayId) { @@ -1339,7 +1358,7 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { dispatchDisplayModeChangeEvent(displayId, mode); } break; - case DisplayDevice::DesiredModeAction::None: + case DesiredModeAction::None: break; } } @@ -1395,11 +1414,10 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const sp<display::DisplayToke return future.get(); } -void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { - const auto displayId = display.getPhysicalId(); +void SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) { ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); - const auto pendingModeOpt = display.getPendingMode(); + const auto pendingModeOpt = mDisplayModeController.getPendingMode(displayId); if (!pendingModeOpt) { // There is no pending mode change. This can happen if the active // display changed and the mode change happened on a different display. @@ -1408,8 +1426,12 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { const auto& activeMode = pendingModeOpt->mode; - if (display.getActiveMode().modePtr->getResolution() != activeMode.modePtr->getResolution()) { - auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken()); + if (const auto oldResolution = + mDisplayModeController.getActiveMode(displayId).modePtr->getResolution(); + oldResolution != activeMode.modePtr->getResolution()) { + Mutex::Autolock lock(mStateLock); + + auto& state = mCurrentState.displays.editValueFor(getPhysicalDisplayTokenLocked(displayId)); // We need to generate new sequenceId in order to recreate the display (and this // way the framebuffer). state.sequenceId = DisplayDeviceState{}.sequenceId; @@ -1420,8 +1442,8 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { return; } - display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(), - activeMode.fps); + mDisplayModeController.finalizeModeChange(displayId, activeMode.modePtr->getId(), + activeMode.modePtr->getVsyncRate(), activeMode.fps); if (displayId == mActiveDisplayId) { mScheduler->updatePhaseConfiguration(activeMode.fps); @@ -1432,21 +1454,20 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) { } } -void SurfaceFlinger::dropModeRequest(const sp<DisplayDevice>& display) { - display->clearDesiredMode(); - if (display->getPhysicalId() == mActiveDisplayId) { +void SurfaceFlinger::dropModeRequest(PhysicalDisplayId displayId) { + mDisplayModeController.clearDesiredMode(displayId); + if (displayId == mActiveDisplayId) { // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); } } -void SurfaceFlinger::applyActiveMode(const sp<DisplayDevice>& display) { - const auto activeModeOpt = display->getDesiredMode(); +void SurfaceFlinger::applyActiveMode(PhysicalDisplayId displayId) { + const auto activeModeOpt = mDisplayModeController.getDesiredMode(displayId); auto activeModePtr = activeModeOpt->mode.modePtr; - const auto displayId = activeModePtr->getPhysicalDisplayId(); const auto renderFps = activeModeOpt->mode.fps; - dropModeRequest(display); + dropModeRequest(displayId); constexpr bool kAllowToEnable = true; mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, std::move(activeModePtr).take()); @@ -1462,11 +1483,8 @@ void SurfaceFlinger::initiateDisplayModeChanges() { std::optional<PhysicalDisplayId> displayToUpdateImmediately; - for (const auto& [id, physical] : mPhysicalDisplays) { - const auto display = getDisplayDeviceLocked(id); - if (!display) continue; - - auto desiredModeOpt = display->getDesiredMode(); + for (const auto& [displayId, physical] : FTL_FAKE_GUARD(mStateLock, mPhysicalDisplays)) { + auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId); if (!desiredModeOpt) { continue; } @@ -1483,19 +1501,21 @@ void SurfaceFlinger::initiateDisplayModeChanges() { ALOGV("%s changing active mode to %d(%s) for display %s", __func__, ftl::to_underlying(desiredModeId), to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(), - to_string(display->getId()).c_str()); + to_string(displayId).c_str()); if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) && - display->getActiveMode() == desiredModeOpt->mode) { - applyActiveMode(display); + mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) { + applyActiveMode(displayId); continue; } + const auto selectorPtr = mDisplayModeController.selectorPtrFor(displayId); + // Desired active mode was set, it is different than the mode currently in use, however // allowed modes might have changed by the time we process the refresh. // Make sure the desired mode is still allowed - if (!display->refreshRateSelector().isModeAllowed(desiredModeOpt->mode)) { - dropModeRequest(display); + if (!selectorPtr->isModeAllowed(desiredModeOpt->mode)) { + dropModeRequest(displayId); continue; } @@ -1505,11 +1525,12 @@ void SurfaceFlinger::initiateDisplayModeChanges() { constraints.seamlessRequired = false; hal::VsyncPeriodChangeTimeline outTimeline; - if (!display->initiateModeChange(std::move(*desiredModeOpt), constraints, outTimeline)) { + if (!mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt), + constraints, outTimeline)) { continue; } - display->refreshRateSelector().onModeChangeInitiated(); + selectorPtr->onModeChangeInitiated(); mScheduler->onNewVsyncPeriodChangeTimeline(outTimeline); if (outTimeline.refreshRequired) { @@ -1518,17 +1539,18 @@ void SurfaceFlinger::initiateDisplayModeChanges() { // TODO(b/255635711): Remove `displayToUpdateImmediately` to `finalizeDisplayModeChange` // for all displays. This was only needed when the loop iterated over `mDisplays` rather // than `mPhysicalDisplays`. - displayToUpdateImmediately = display->getPhysicalId(); + displayToUpdateImmediately = displayId; } } if (displayToUpdateImmediately) { - const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately); - finalizeDisplayModeChange(*display); + const auto displayId = *displayToUpdateImmediately; + finalizeDisplayModeChange(displayId); - const auto desiredModeOpt = display->getDesiredMode(); - if (desiredModeOpt && display->getActiveMode() == desiredModeOpt->mode) { - applyActiveMode(display); + const auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId); + if (desiredModeOpt && + mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) { + applyActiveMode(displayId); } } } @@ -2276,8 +2298,10 @@ void SurfaceFlinger::onRefreshRateChangedDebug(const RefreshRateChangedDebugData getHwComposer().getComposer()->isVrrSupported() ? data.refreshPeriodNanos : data.vsyncPeriodNanos); ATRACE_FORMAT("%s refresh rate = %d", whence, refreshRate.getIntValue()); - display->updateRefreshRateOverlayRate(refreshRate, display->getActiveMode().fps, - /* showRefreshRate */ true); + + const auto renderRate = mDisplayModeController.getActiveMode(*displayIdOpt).fps; + constexpr bool kSetByHwc = true; + display->updateRefreshRateOverlayRate(refreshRate, renderRate, kSetByHwc); } } })); @@ -2585,33 +2609,18 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, // If a mode set is pending and the fence hasn't fired yet, wait for the next commit. if (std::any_of(frameTargets.begin(), frameTargets.end(), - [this](const auto& pair) FTL_FAKE_GUARD(mStateLock) - FTL_FAKE_GUARD(kMainThreadContext) { - if (!pair.second->isFramePending()) return false; - - if (const auto display = getDisplayDeviceLocked(pair.first)) { - return display->isModeSetPending(); - } - - return false; - })) { + [this](const auto& pair) FTL_FAKE_GUARD(kMainThreadContext) { + const auto [displayId, target] = pair; + return target->isFramePending() && + mDisplayModeController.isModeSetPending(displayId); + })) { mScheduler->scheduleFrame(); return false; } - { - Mutex::Autolock lock(mStateLock); - - for (const auto [id, target] : frameTargets) { - // TODO(b/241285876): This is `nullptr` when the DisplayDevice is about to be removed in - // this commit, since the PhysicalDisplay has already been removed. Rather than checking - // for `nullptr` below, change Scheduler::onFrameSignal to filter out the FrameTarget of - // the removed display. - const auto display = getDisplayDeviceLocked(id); - - if (display && display->isModeSetPending()) { - finalizeDisplayModeChange(*display); - } + for (const auto [displayId, _] : frameTargets) { + if (mDisplayModeController.isModeSetPending(displayId)) { + finalizeDisplayModeChange(displayId); } } @@ -2646,8 +2655,8 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, mPowerAdvisor->setFrameDelay(frameDelay); mPowerAdvisor->setTotalFrameTargetWorkDuration(idealSfWorkDuration); - const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get(); - const Period idealVsyncPeriod = display->getActiveMode().fps.getPeriod(); + const Period idealVsyncPeriod = + mDisplayModeController.getActiveMode(pacesetterId).fps.getPeriod(); mPowerAdvisor->updateTargetWorkDuration(idealVsyncPeriod); } @@ -2710,9 +2719,10 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, ? &mLayerHierarchyBuilder.getHierarchy() : nullptr, updateAttachedChoreographer); - initiateDisplayModeChanges(); } + initiateDisplayModeChanges(); + updateCursorAsync(); if (!mustComposite) { updateInputFlinger(vsyncId, pacesetterFrameTarget.frameBeginTime()); @@ -3758,7 +3768,8 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( if (const auto& physical = state.physical) { const auto& mode = *physical->activeMode; - display->setActiveMode(mode.getId(), mode.getVsyncRate(), mode.getVsyncRate()); + mDisplayModeController.setActiveMode(physical->id, mode.getId(), mode.getVsyncRate(), + mode.getVsyncRate()); } display->setLayerFilter(makeLayerFilterForDisplay(display->getId(), state.layerStack)); @@ -3937,7 +3948,9 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, // TODO(b/175678251) Call a listener instead. if (currentState.physical->hwcDisplayId == getHwComposer().getPrimaryHwcDisplayId()) { - mScheduler->resetPhaseConfiguration(display->getActiveMode().fps); + const Fps refreshRate = + mDisplayModeController.getActiveMode(display->getPhysicalId()).fps; + mScheduler->resetPhaseConfiguration(refreshRate); } } return; @@ -5635,10 +5648,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime layer->setInputInfo(*s.windowInfoHandle->getInfo()); flags |= eTraversalNeeded; } - std::optional<nsecs_t> dequeueBufferTimestamp; if (what & layer_state_t::eMetadataChanged) { - dequeueBufferTimestamp = s.metadata.getInt64(gui::METADATA_DEQUEUE_TIME); - if (const int32_t gameMode = s.metadata.getInt32(gui::METADATA_GAME_MODE, -1); gameMode != -1) { // The transaction will be received on the Task layer and needs to be applied to all @@ -5780,8 +5790,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime if (what & layer_state_t::eBufferChanged) { if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime, - desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, - frameTimelineInfo)) { + desiredPresentTime, isAutoTimestamp, frameTimelineInfo)) { flags |= eTraversalNeeded; } } else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) { @@ -5867,10 +5876,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f if (what & layer_state_t::eProducerDisconnect) { layer->onDisconnect(); } - std::optional<nsecs_t> dequeueBufferTimestamp; - if (what & layer_state_t::eMetadataChanged) { - dequeueBufferTimestamp = s.metadata.getInt64(gui::METADATA_DEQUEUE_TIME); - } std::vector<sp<CallbackHandle>> callbackHandles; if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) { @@ -5917,8 +5922,7 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } layer->setTransformHint(transformHint); if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime, - desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, - frameTimelineInfo)) { + desiredPresentTime, isAutoTimestamp, frameTimelineInfo)) { flags |= eTraversalNeeded; } mLayersWithQueuedFrames.emplace(layer); @@ -7684,9 +7688,10 @@ void SurfaceFlinger::kernelTimerChanged(bool expired) { if (!display->isRefreshRateOverlayEnabled()) return; const auto desiredModeIdOpt = - display->getDesiredMode().transform([](const display::DisplayModeRequest& request) { - return request.mode.modePtr->getId(); - }); + mDisplayModeController.getDesiredMode(display->getPhysicalId()) + .transform([](const display::DisplayModeRequest& request) { + return request.mode.modePtr->getId(); + }); const bool timerExpired = mKernelIdleTimerEnabled && expired; @@ -8921,20 +8926,26 @@ status_t SurfaceFlinger::setSmallAreaDetectionThreshold(int32_t appId, float thr void SurfaceFlinger::enableRefreshRateOverlay(bool enable) { bool setByHwc = getHwComposer().hasCapability(Capability::REFRESH_RATE_CHANGED_CALLBACK_DEBUG); - for (const auto& [id, display] : mPhysicalDisplays) { - if (display.snapshot().connectionType() == ui::DisplayConnectionType::Internal || + for (const auto& [displayId, physical] : mPhysicalDisplays) { + if (physical.snapshot().connectionType() == ui::DisplayConnectionType::Internal || FlagManager::getInstance().refresh_rate_overlay_on_external_display()) { - if (const auto device = getDisplayDeviceLocked(id)) { - const auto enableOverlay = [&](const bool setByHwc) FTL_FAKE_GUARD( - kMainThreadContext) { - device->enableRefreshRateOverlay(enable, setByHwc, mRefreshRateOverlaySpinner, - mRefreshRateOverlayRenderRate, - mRefreshRateOverlayShowInMiddle); + if (const auto display = getDisplayDeviceLocked(displayId)) { + const auto enableOverlay = [&](bool setByHwc) FTL_FAKE_GUARD(kMainThreadContext) { + const auto activeMode = mDisplayModeController.getActiveMode(displayId); + const Fps refreshRate = activeMode.modePtr->getVsyncRate(); + const Fps renderFps = activeMode.fps; + + display->enableRefreshRateOverlay(enable, setByHwc, refreshRate, renderFps, + mRefreshRateOverlaySpinner, + mRefreshRateOverlayRenderRate, + mRefreshRateOverlayShowInMiddle); }; + enableOverlay(setByHwc); if (setByHwc) { const auto status = - getHwComposer().setRefreshRateChangedCallbackDebugEnabled(id, enable); + getHwComposer().setRefreshRateChangedCallbackDebugEnabled(displayId, + enable); if (status != NO_ERROR) { ALOGE("Error %s refresh rate changed callback debug", enable ? "enabling" : "disabling"); @@ -9090,7 +9101,7 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD mActiveDisplayId = activeDisplay.getPhysicalId(); activeDisplay.getCompositionDisplay()->setLayerCachingTexturePoolEnabled(true); - mScheduler->resetPhaseConfiguration(activeDisplay.getActiveMode().fps); + mScheduler->resetPhaseConfiguration(mDisplayModeController.getActiveMode(mActiveDisplayId).fps); // TODO(b/255635711): Check for pending mode changes on other displays. mScheduler->setModeChangePending(false); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 12307172f7..ee541c4364 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -737,12 +737,12 @@ private: status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps, Fps maxFps); - void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext); - void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext); + void initiateDisplayModeChanges() REQUIRES(kMainThreadContext) EXCLUDES(mStateLock); + void finalizeDisplayModeChange(PhysicalDisplayId) REQUIRES(kMainThreadContext) + EXCLUDES(mStateLock); - // TODO(b/241285191): Replace DisplayDevice with DisplayModeRequest, and move to Scheduler. - void dropModeRequest(const sp<DisplayDevice>&) REQUIRES(mStateLock); - void applyActiveMode(const sp<DisplayDevice>&) REQUIRES(mStateLock); + void dropModeRequest(PhysicalDisplayId) REQUIRES(kMainThreadContext); + void applyActiveMode(PhysicalDisplayId) REQUIRES(kMainThreadContext); // Called on the main thread in response to setPowerMode() void setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) @@ -1098,8 +1098,7 @@ private: const DisplayDeviceState& drawingState) REQUIRES(mStateLock, kMainThreadContext); - void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&) - REQUIRES(mStateLock); + void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&); /* * VSYNC @@ -1339,9 +1338,7 @@ private: display::PhysicalDisplays mPhysicalDisplays GUARDED_BY(mStateLock); // The inner or outer display for foldables, assuming they have mutually exclusive power states. - // Atomic because writes from onActiveDisplayChangedLocked are not always under mStateLock, but - // reads from ISchedulerCallback::requestDisplayModes may happen concurrently. - std::atomic<PhysicalDisplayId> mActiveDisplayId GUARDED_BY(mStateLock); + std::atomic<PhysicalDisplayId> mActiveDisplayId; display::DisplayModeController mDisplayModeController; diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp index fc496b2982..0bafb71f3f 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp @@ -436,6 +436,7 @@ void TransactionProtoParser::fromProto(const perfetto::protos::LayerState& proto layer.bufferData->flags = ftl::Flags<BufferData::BufferDataChange>(bufferProto.flags()); layer.bufferData->cachedBuffer.id = bufferProto.cached_buffer_id(); layer.bufferData->acquireFence = Fence::NO_FENCE; + layer.bufferData->dequeueTime = -1; } if (proto.what() & layer_state_t::eApiChanged) { diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 5145e112d6..98d5754271 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -70,9 +70,9 @@ cc_test { "DisplayIdGeneratorTest.cpp", "DisplayTransactionTest.cpp", "DisplayDevice_GetBestColorModeTest.cpp", - "DisplayDevice_InitiateModeChange.cpp", "DisplayDevice_SetDisplayBrightnessTest.cpp", "DisplayDevice_SetProjectionTest.cpp", + "DisplayModeControllerTest.cpp", "EventThreadTest.cpp", "FlagManagerTest.cpp", "FpsReporterTest.cpp", diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp deleted file mode 100644 index c463a9271b..0000000000 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2021 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. - */ - -#undef LOG_TAG -#define LOG_TAG "LibSurfaceFlingerUnittests" - -#include "DisplayTransactionTestHelpers.h" -#include "mock/MockFrameRateMode.h" - -#include <gmock/gmock.h> -#include <gtest/gtest.h> - -#define EXPECT_DISPLAY_MODE_REQUEST(expected, requestOpt) \ - ASSERT_TRUE(requestOpt); \ - EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, requestOpt->mode); \ - EXPECT_EQ(expected.emitEvent, requestOpt->emitEvent) - -namespace android { -namespace { - -using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector; -using DisplayModeRequest = display::DisplayModeRequest; - -class InitiateModeChangeTest : public DisplayTransactionTest { -public: - using Action = DisplayDevice::DesiredModeAction; - void SetUp() override { - injectFakeBufferQueueFactory(); - injectFakeNativeWindowSurfaceFactory(); - - PrimaryDisplayVariant::setupHwcHotplugCallExpectations(this); - PrimaryDisplayVariant::setupFramebufferConsumerBufferQueueCallExpectations(this); - PrimaryDisplayVariant::setupFramebufferProducerBufferQueueCallExpectations(this); - PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this); - PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this); - - mFlinger.onComposerHalHotplugEvent(PrimaryDisplayVariant::HWC_DISPLAY_ID, - DisplayHotplugEvent::CONNECTED); - mFlinger.configureAndCommit(); - - mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) - .setDisplayModes(makeModes(kMode60, kMode90, kMode120), kModeId60) - .inject(); - } - -protected: - sp<DisplayDevice> mDisplay; - - static constexpr DisplayModeId kModeId60{0}; - static constexpr DisplayModeId kModeId90{1}; - static constexpr DisplayModeId kModeId120{2}; - - static inline const ftl::NonNull<DisplayModePtr> kMode60 = - ftl::as_non_null(createDisplayMode(kModeId60, 60_Hz)); - static inline const ftl::NonNull<DisplayModePtr> kMode90 = - ftl::as_non_null(createDisplayMode(kModeId90, 90_Hz)); - static inline const ftl::NonNull<DisplayModePtr> kMode120 = - ftl::as_non_null(createDisplayMode(kModeId120, 120_Hz)); - - static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false}; - static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true}; - static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false}; - static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true}; -}; - -TEST_F(InitiateModeChangeTest, setDesiredModeToActiveMode) { - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode60))); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, setDesiredMode) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, clearDesiredMode) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_TRUE(mDisplay->getDesiredMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - const hal::VsyncPeriodChangeConstraints constraints{ - .desiredTimeNanos = systemTime(), - .seamlessRequired = false, - }; - hal::VsyncPeriodChangeTimeline timeline; - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) { - EXPECT_EQ(Action::InitiateRenderRateSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode30))); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) { - EXPECT_EQ(Action::InitiateDisplayModeSwitch, - mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90))); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode()); - - const hal::VsyncPeriodChangeConstraints constraints{ - .desiredTimeNanos = systemTime(), - .seamlessRequired = false, - }; - hal::VsyncPeriodChangeTimeline timeline; - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120))); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode()); - - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode()); - - EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline)); - EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getPendingMode()); - - mDisplay->clearDesiredMode(); - EXPECT_FALSE(mDisplay->getDesiredMode()); -} - -} // namespace -} // namespace android diff --git a/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp new file mode 100644 index 0000000000..d971150d42 --- /dev/null +++ b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp @@ -0,0 +1,234 @@ +/* + * Copyright 2024 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. + */ + +#undef LOG_TAG +#define LOG_TAG "LibSurfaceFlingerUnittests" + +#include "Display/DisplayModeController.h" +#include "Display/DisplaySnapshot.h" +#include "DisplayHardware/HWComposer.h" +#include "DisplayIdentificationTestHelpers.h" +#include "FpsOps.h" +#include "mock/DisplayHardware/MockComposer.h" +#include "mock/DisplayHardware/MockDisplayMode.h" +#include "mock/MockFrameRateMode.h" + +#include <ftl/fake_guard.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> + +#define EXPECT_DISPLAY_MODE_REQUEST(expected, requestOpt) \ + ASSERT_TRUE(requestOpt); \ + EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, requestOpt->mode); \ + EXPECT_EQ(expected.emitEvent, requestOpt->emitEvent) + +namespace android::display { +namespace { + +namespace hal = android::hardware::graphics::composer::hal; + +using testing::_; +using testing::DoAll; +using testing::Return; +using testing::SetArgPointee; + +class DisplayModeControllerTest : public testing::Test { +public: + using Action = DisplayModeController::DesiredModeAction; + + void SetUp() override { + mDmc.setHwComposer(mComposer.get()); + mDmc.setActiveModeListener( + [this](PhysicalDisplayId displayId, Fps vsyncRate, Fps renderFps) { + mActiveModeListener.Call(displayId, vsyncRate, renderFps); + }); + + constexpr uint8_t kPort = 111; + EXPECT_CALL(*mComposerHal, getDisplayIdentificationData(kHwcDisplayId, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kPort), SetArgPointee<2>(getInternalEdid()), + Return(hal::Error::NONE))); + + EXPECT_CALL(*mComposerHal, setClientTargetSlotCount(kHwcDisplayId)); + EXPECT_CALL(*mComposerHal, + setVsyncEnabled(kHwcDisplayId, hal::IComposerClient::Vsync::DISABLE)); + EXPECT_CALL(*mComposerHal, onHotplugConnect(kHwcDisplayId)); + + const auto infoOpt = mComposer->onHotplug(kHwcDisplayId, hal::Connection::CONNECTED); + ASSERT_TRUE(infoOpt); + + mDisplayId = infoOpt->id; + mDisplaySnapshotOpt.emplace(mDisplayId, ui::DisplayConnectionType::Internal, + makeModes(kMode60, kMode90, kMode120), ui::ColorModes{}, + std::nullopt); + + ftl::FakeGuard guard(kMainThreadContext); + mDmc.registerDisplay(*mDisplaySnapshotOpt, kModeId60, + scheduler::RefreshRateSelector::Config{}); + } + +protected: + hal::VsyncPeriodChangeConstraints expectModeSet(const DisplayModeRequest& request, + hal::VsyncPeriodChangeTimeline& timeline, + bool subsequent = false) { + EXPECT_CALL(*mComposerHal, + isSupported(Hwc2::Composer::OptionalFeature::RefreshRateSwitching)) + .WillOnce(Return(true)); + + if (!subsequent) { + EXPECT_CALL(*mComposerHal, getDisplayConnectionType(kHwcDisplayId, _)) + .WillOnce(DoAll(SetArgPointee<1>( + hal::IComposerClient::DisplayConnectionType::INTERNAL), + Return(hal::V2_4::Error::NONE))); + } + + const hal::VsyncPeriodChangeConstraints constraints{ + .desiredTimeNanos = systemTime(), + .seamlessRequired = false, + }; + + const hal::HWConfigId hwcModeId = request.mode.modePtr->getHwcId(); + + EXPECT_CALL(*mComposerHal, + setActiveConfigWithConstraints(kHwcDisplayId, hwcModeId, constraints, _)) + .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::V2_4::Error::NONE))); + + return constraints; + } + + static constexpr hal::HWDisplayId kHwcDisplayId = 1234; + + Hwc2::mock::Composer* mComposerHal = new testing::StrictMock<Hwc2::mock::Composer>(); + const std::unique_ptr<HWComposer> mComposer{ + std::make_unique<impl::HWComposer>(std::unique_ptr<Hwc2::Composer>(mComposerHal))}; + + testing::MockFunction<void(PhysicalDisplayId, Fps, Fps)> mActiveModeListener; + + DisplayModeController mDmc; + + PhysicalDisplayId mDisplayId; + std::optional<DisplaySnapshot> mDisplaySnapshotOpt; + + static constexpr DisplayModeId kModeId60{0}; + static constexpr DisplayModeId kModeId90{1}; + static constexpr DisplayModeId kModeId120{2}; + + static inline const ftl::NonNull<DisplayModePtr> kMode60 = + ftl::as_non_null(mock::createDisplayMode(kModeId60, 60_Hz)); + static inline const ftl::NonNull<DisplayModePtr> kMode90 = + ftl::as_non_null(mock::createDisplayMode(kModeId90, 90_Hz)); + static inline const ftl::NonNull<DisplayModePtr> kMode120 = + ftl::as_non_null(mock::createDisplayMode(kModeId120, 120_Hz)); + + static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false}; + static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true}; + static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false}; + static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true}; +}; + +TEST_F(DisplayModeControllerTest, setDesiredModeToActiveMode) { + EXPECT_CALL(mActiveModeListener, Call(_, _, _)).Times(0); + + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode60))); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, setDesiredMode) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + + // No action since a mode switch has already been initiated. + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, clearDesiredMode) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_TRUE(mDmc.getDesiredMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateModeChange) REQUIRES(kMainThreadContext) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + auto modeRequest = kDesiredMode90; + + hal::VsyncPeriodChangeTimeline timeline; + const auto constraints = expectModeSet(modeRequest, timeline); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateRenderRateSwitch) { + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 30_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateRenderRateSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode30))); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +TEST_F(DisplayModeControllerTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) { + // Called because setDesiredMode resets the render rate to the active refresh rate. + EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1); + + EXPECT_EQ(Action::InitiateDisplayModeSwitch, + mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90))); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId)); + auto modeRequest = kDesiredMode90; + + hal::VsyncPeriodChangeTimeline timeline; + auto constraints = expectModeSet(modeRequest, timeline); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + + // No action since a mode switch has already been initiated. + EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120))); + + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId)); + modeRequest = kDesiredMode120; + + constexpr bool kSubsequent = true; + constraints = expectModeSet(modeRequest, timeline, kSubsequent); + + EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline)); + EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getPendingMode(mDisplayId)); + + mDmc.clearDesiredMode(mDisplayId); + EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId)); +} + +} // namespace +} // namespace android::display diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index bd0accd28e..dedb292b34 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -213,6 +213,17 @@ TEST_F(LayerSnapshotTest, childBehindParentCanBeHiddenByParent) { UPDATE_AND_VERIFY(mSnapshotBuilder, {1, 12, 121, 122, 1221, 13, 2}); } +TEST_F(LayerSnapshotTest, offscreenLayerSnapshotIsInvisible) { + EXPECT_EQ(getSnapshot(111)->isVisible, true); + + reparentLayer(11, UNASSIGNED_LAYER_ID); + destroyLayerHandle(11); + UPDATE_AND_VERIFY(mSnapshotBuilder, {1, 12, 121, 122, 1221, 13, 2}); + + EXPECT_EQ(getSnapshot(111)->isVisible, false); + EXPECT_TRUE(getSnapshot(111)->changes.test(RequestedLayerState::Changes::Visibility)); +} + // relative tests TEST_F(LayerSnapshotTest, RelativeParentCanHideChild) { reparentRelativeLayer(13, 11); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 078b4fe15e..0c3e875432 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -76,6 +76,7 @@ public: mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) .setRefreshRateSelector(std::move(selectorPtr)) .inject(std::move(vsyncController), std::move(vsyncTracker)); + mDisplayId = mDisplay->getPhysicalId(); // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy // will call setActiveConfig instead of setActiveConfigWithConstraints. @@ -112,7 +113,11 @@ public: protected: void setupScheduler(std::shared_ptr<scheduler::RefreshRateSelector>); + auto& dmc() { return mFlinger.mutableDisplayModeController(); } + sp<DisplayDevice> mDisplay, mOuterDisplay; + PhysicalDisplayId mDisplayId; + mock::EventThread* mAppEventThread; static constexpr DisplayModeId kModeId60{0}; @@ -167,17 +172,17 @@ void DisplayModeSwitchingTest::setupScheduler( TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90, false, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; @@ -187,8 +192,8 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequ Mock::VerifyAndClearExpectations(mComposer); - EXPECT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that the next commit will complete the mode change and send // a onModeChanged event to the framework. @@ -198,23 +203,23 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequ mFlinger.commit(); Mock::VerifyAndClearExpectations(mAppEventThread); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90, true, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. @@ -226,8 +231,8 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshR mFlinger.commit(); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90); } TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { @@ -236,8 +241,8 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { // Test that if we call setDesiredDisplayModeSpecs while a previous mode change // is still being processed the later call will be respected. - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); @@ -252,46 +257,45 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId120, false, 0, 180)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120); EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId120); mFlinger.commit(); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120); mFlinger.commit(); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId120); } TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRequired) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); mFlinger.onActiveDisplayChanged(nullptr, *mDisplay); mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), mock::createDisplayModeSpecs(kModeId90_4K, false, 0, 120)); - ASSERT_TRUE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90_4K); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60); + ASSERT_TRUE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90_4K); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60); // Verify that next commit will call setActiveConfigWithConstraints in HWC // and complete the mode change. const VsyncPeriodChangeTimeline timeline{.refreshRequired = false}; EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90_4K); - EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), true)); + EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplayId, true)); - // Misc expecations. We don't need to enforce these method calls, but since the helper methods - // already set expectations we should add new ones here, otherwise the test will fail. + // Override expectations set up by PrimaryDisplayVariant. EXPECT_CALL(*mConsumer, setDefaultBufferSize(static_cast<uint32_t>(kResolution4K.getWidth()), static_cast<uint32_t>(kResolution4K.getHeight()))) @@ -304,31 +308,28 @@ TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRe injectFakeNativeWindowSurfaceFactory(); PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this); - const auto displayToken = mDisplay->getDisplayToken().promote(); - mFlinger.commit(); - // The DisplayDevice will be destroyed and recreated, - // so we need to update with the new instance. - mDisplay = mFlinger.getDisplay(displayToken); - - EXPECT_FALSE(mDisplay->getDesiredMode()); - EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K); + EXPECT_FALSE(dmc().getDesiredMode(mDisplayId)); + EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90_4K); } MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { - if (!arg->getDesiredMode()) { + const auto displayId = arg->getPhysicalId(); + auto& dmc = flinger->mutableDisplayModeController(); + + if (!dmc.getDesiredMode(displayId)) { *result_listener << "No desired mode"; return false; } - if (arg->getDesiredMode()->mode.modePtr->getId() != modeId) { + if (dmc.getDesiredMode(displayId)->mode.modePtr->getId() != modeId) { *result_listener << "Unexpected desired mode " << ftl::to_underlying(modeId); return false; } // VsyncModulator should react to mode switches on the pacesetter display. - if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() && + if (displayId == flinger->scheduler()->pacesetterDisplayId() && !flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { *result_listener << "VsyncModulator did not shift to early phase"; return false; @@ -337,8 +338,10 @@ MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { return true; } -MATCHER_P(ModeSettledTo, modeId, "") { - if (const auto desiredOpt = arg->getDesiredMode()) { +MATCHER_P2(ModeSettledTo, dmc, modeId, "") { + const auto displayId = arg->getPhysicalId(); + + if (const auto desiredOpt = dmc->getDesiredMode(displayId)) { *result_listener << "Unsettled desired mode " << ftl::to_underlying(desiredOpt->mode.modePtr->getId()); return false; @@ -346,7 +349,7 @@ MATCHER_P(ModeSettledTo, modeId, "") { ftl::FakeGuard guard(kMainThreadContext); - if (arg->getActiveMode().modePtr->getId() != modeId) { + if (dmc->getActiveMode(displayId).modePtr->getId() != modeId) { *result_listener << "Settled to unexpected active mode " << ftl::to_underlying(modeId); return false; } @@ -367,14 +370,14 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -400,14 +403,14 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -420,12 +423,12 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); } TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { @@ -440,14 +443,14 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -473,13 +476,13 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); } TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { EXPECT_TRUE(mDisplay->isPoweredOn()); - EXPECT_THAT(mDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId60)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(), @@ -502,7 +505,7 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { mFlinger.commit(); - EXPECT_THAT(mDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId90)); } TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { @@ -518,14 +521,14 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_TRUE(innerDisplay->isPoweredOn()); EXPECT_FALSE(outerDisplay->isPoweredOn()); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); EXPECT_EQ(NO_ERROR, mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), @@ -555,8 +558,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60)); mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); @@ -570,13 +573,13 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90)); + EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120)); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp index 1bae5ffd30..933d03dac1 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp @@ -232,7 +232,9 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { // Invocation DisplayDeviceState state; - if constexpr (constexpr auto connectionType = Case::Display::CONNECTION_TYPE::value) { + + constexpr auto kConnectionTypeOpt = Case::Display::CONNECTION_TYPE::value; + if constexpr (kConnectionTypeOpt) { const auto displayId = PhysicalDisplayId::tryCast(Case::Display::DISPLAY_ID::get()); ASSERT_TRUE(displayId); const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value; @@ -257,7 +259,7 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { const auto it = mFlinger.mutablePhysicalDisplays() .emplace_or_replace(*displayId, displayToken, *displayId, - *connectionType, makeModes(activeMode), + *kConnectionTypeOpt, makeModes(activeMode), std::move(colorModes), std::nullopt) .first; @@ -291,9 +293,12 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() { EXPECT_EQ(Case::Display::DISPLAY_FLAGS & DisplayDevice::eReceivesInput, device->receivesInput()); - if constexpr (Case::Display::CONNECTION_TYPE::value) { + if constexpr (kConnectionTypeOpt) { ftl::FakeGuard guard(kMainThreadContext); - EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode().modePtr->getHwcId()); + EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, + mFlinger.mutableDisplayModeController() + .getActiveMode(device->getPhysicalId()) + .modePtr->getHwcId()); } } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 8c72a7dba5..007383b3d9 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -191,6 +191,8 @@ public: void setupComposer(std::unique_ptr<Hwc2::Composer> composer) { mFlinger->mCompositionEngine->setHwComposer( std::make_unique<impl::HWComposer>(std::move(composer))); + mFlinger->mDisplayModeController.setHwComposer( + &mFlinger->mCompositionEngine->getHwComposer()); } void setupPowerAdvisor(std::unique_ptr<Hwc2::PowerAdvisor> powerAdvisor) { @@ -1055,7 +1057,6 @@ public: auto& modes = mDisplayModes; auto& activeModeId = mActiveModeId; - std::optional<Fps> refreshRateOpt; DisplayDeviceState state; state.isSecure = mCreationArgs.isSecure; @@ -1093,7 +1094,9 @@ public: const auto activeModeOpt = modes.get(activeModeId); LOG_ALWAYS_FATAL_IF(!activeModeOpt); - refreshRateOpt = activeModeOpt->get()->getPeakFps(); + + // Save a copy for use after `modes` is consumed. + const Fps refreshRate = activeModeOpt->get()->getPeakFps(); state.physical = {.id = *physicalId, .hwcDisplayId = *mHwcDisplayId, @@ -1109,6 +1112,9 @@ public: .registerDisplay(*physicalId, it->second.snapshot(), mCreationArgs.refreshRateSelector); + mFlinger.mutableDisplayModeController().setActiveMode(*physicalId, activeModeId, + refreshRate, refreshRate); + if (mFlinger.scheduler() && mSchedulerRegistration) { mFlinger.scheduler()->registerDisplay(*physicalId, mCreationArgs.refreshRateSelector, @@ -1120,10 +1126,6 @@ public: sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs); mFlinger.mutableDisplays().emplace_or_replace(mDisplayToken, display); - if (refreshRateOpt) { - display->setActiveMode(activeModeId, *refreshRateOpt, *refreshRateOpt); - } - mFlinger.mutableCurrentState().displays.add(mDisplayToken, state); mFlinger.mutableDrawingState().displays.add(mDisplayToken, state); diff --git a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp index d4d5b32341..85b61f8fb9 100644 --- a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp @@ -94,7 +94,7 @@ public: HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); layer->setBuffer(externalTexture, bufferData, postTime, /*desiredPresentTime*/ 30, false, - dequeueTime, FrameTimelineInfo{}); + FrameTimelineInfo{}); commitTransaction(layer.get()); nsecs_t latchTime = 25; diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp index 5046a86304..46733b9a83 100644 --- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp @@ -99,7 +99,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, ftInfo); acquireFence->signalForTest(12); commitTransaction(layer.get()); @@ -134,7 +134,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX; @@ -151,7 +151,7 @@ public: 2ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, ftInfo); nsecs_t end = systemTime(); acquireFence2->signalForTest(12); @@ -197,7 +197,7 @@ public: 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, ftInfo); acquireFence->signalForTest(12); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); @@ -232,7 +232,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -275,7 +275,7 @@ public: FrameTimelineInfo ftInfo3; ftInfo3.vsyncId = 3; ftInfo3.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo3); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, ftInfo3); EXPECT_EQ(2u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto bufferSurfaceFrameTX = layer->mDrawingState.bufferSurfaceFrameTX; @@ -320,7 +320,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, ftInfo); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX; @@ -335,7 +335,7 @@ public: 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, ftInfo); acquireFence2->signalForTest(12); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -372,7 +372,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame1 = layer->mDrawingState.bufferSurfaceFrameTX; @@ -392,7 +392,7 @@ public: FrameTimelineInfo ftInfoInv; ftInfoInv.vsyncId = FrameTimelineInfo::INVALID_VSYNC_ID; ftInfoInv.inputEventId = 0; - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfoInv); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, ftInfoInv); auto dropEndTime1 = systemTime(); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -413,7 +413,7 @@ public: FrameTimelineInfo ftInfo2; ftInfo2.vsyncId = 2; ftInfo2.inputEventId = 0; - layer->setBuffer(externalTexture3, bufferData, 10, 20, false, std::nullopt, ftInfo2); + layer->setBuffer(externalTexture3, bufferData, 10, 20, false, ftInfo2); auto dropEndTime2 = systemTime(); acquireFence3->signalForTest(12); @@ -462,7 +462,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, ftInfo); FrameTimelineInfo ftInfo2; ftInfo2.vsyncId = 2; ftInfo2.inputEventId = 0; |