SF: Tidy naming of mode set FSM
Rename {Desired,Upcoming}ActiveMode to {Desired,Pending}Mode for clarity
that the FSM is in one of desired/pending/active states.
Rename clearDesiredActiveModeState to dropModeRequest, as its purpose is
to undo the effect of a DisplayModeRequest, regardless of FSM state.
Rename desiredActiveModeChangeDone to applyActiveMode, as it pertains to
applying the DisplayModeRequest that has become active to the Scheduler.
This function will later be moved from SF to Scheduler.
Bug: 255635711
Test: presubmit
Change-Id: Ie7fd9a353e38325d7b8dfd1d1143c7d52e5ad58e
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 2ffe92b..50e94bf 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -63,14 +63,14 @@
mDisplayToken(args.displayToken),
mSequenceId(args.sequenceId),
mCompositionDisplay{args.compositionDisplay},
- mActiveModeFPSTrace(concatId("ActiveModeFPS")),
- mActiveModeFPSHwcTrace(concatId("ActiveModeFPS_HWC")),
- mRenderFrameRateFPSTrace(concatId("RenderRateFPS")),
+ mPendingModeFpsTrace(concatId("PendingModeFps")),
+ mActiveModeFpsTrace(concatId("ActiveModeFps")),
+ mRenderRateFpsTrace(concatId("RenderRateFps")),
mPhysicalOrientation(args.physicalOrientation),
mIsPrimary(args.isPrimary),
mRequestedRefreshRate(args.requestedRefreshRate),
mRefreshRateSelector(std::move(args.refreshRateSelector)),
- mDesiredActiveModeChanged(concatId("DesiredActiveModeChanged"), false) {
+ mDesiredModeChanged(concatId("DesiredModeChanged"), false) {
mCompositionDisplay->editState().isSecure = args.isSecure;
mCompositionDisplay->createRenderSurface(
compositionengine::RenderSurfaceCreationArgsBuilder()
@@ -210,8 +210,8 @@
}
void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) {
- ATRACE_INT(mActiveModeFPSTrace.c_str(), vsyncRate.getIntValue());
- ATRACE_INT(mRenderFrameRateFPSTrace.c_str(), renderFps.getIntValue());
+ ATRACE_INT(mActiveModeFpsTrace.c_str(), vsyncRate.getIntValue());
+ ATRACE_INT(mRenderRateFpsTrace.c_str(), renderFps.getIntValue());
mRefreshRateSelector->setActiveMode(modeId, renderFps);
updateRefreshRateOverlayRate(vsyncRate, renderFps);
@@ -227,11 +227,11 @@
to_string(getId()).c_str());
return BAD_VALUE;
}
- mUpcomingActiveMode = info;
+ mPendingMode = info;
mIsModeSetPending = true;
const auto& pendingMode = *info.modeOpt->modePtr;
- ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), pendingMode.getVsyncRate().getIntValue());
+ ATRACE_INT(mPendingModeFpsTrace.c_str(), pendingMode.getVsyncRate().getIntValue());
return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), pendingMode.getHwcId(),
constraints, outTimeline);
@@ -524,8 +524,7 @@
}
}
-auto DisplayDevice::setDesiredActiveMode(const ActiveModeInfo& info, bool force)
- -> DesiredActiveModeAction {
+auto DisplayDevice::setDesiredMode(const ActiveModeInfo& info, bool force) -> DesiredModeAction {
ATRACE_CALL();
LOG_ALWAYS_FATAL_IF(!info.modeOpt, "desired mode not provided");
@@ -534,49 +533,50 @@
ALOGV("%s(%s)", __func__, to_string(*info.modeOpt->modePtr).c_str());
- std::scoped_lock lock(mActiveModeLock);
- if (mDesiredActiveModeChanged) {
- // If a mode change is pending, just cache the latest request in mDesiredActiveMode
- const auto prevConfig = mDesiredActiveMode.event;
- mDesiredActiveMode = info;
- mDesiredActiveMode.event = mDesiredActiveMode.event | prevConfig;
- return DesiredActiveModeAction::None;
+ std::scoped_lock lock(mDesiredModeLock);
+ if (mDesiredModeChanged) {
+ // A mode transition was already scheduled, so just override the desired mode.
+ const auto event = mDesiredMode.event;
+ mDesiredMode = info;
+ mDesiredMode.event = mDesiredMode.event | event;
+ return DesiredModeAction::None;
}
const auto& desiredMode = *info.modeOpt->modePtr;
- // Check if we are already at the desired mode
- const auto currentMode = refreshRateSelector().getActiveMode();
- if (!force && currentMode.modePtr->getId() == desiredMode.getId()) {
- if (currentMode == info.modeOpt) {
- return DesiredActiveModeAction::None;
+ // If the desired mode is already active...
+ const auto activeMode = refreshRateSelector().getActiveMode();
+ if (!force && activeMode.modePtr->getId() == desiredMode.getId()) {
+ if (activeMode == info.modeOpt) {
+ return DesiredModeAction::None;
}
+ // ...but the render rate changed:
setActiveMode(desiredMode.getId(), desiredMode.getVsyncRate(), info.modeOpt->fps);
- return DesiredActiveModeAction::InitiateRenderRateSwitch;
+ return DesiredModeAction::InitiateRenderRateSwitch;
}
- // Set the render frame rate to the current physical refresh rate to schedule the next
+ // Set the render frame rate to the active physical refresh rate to schedule the next
// frame as soon as possible.
- setActiveMode(currentMode.modePtr->getId(), currentMode.modePtr->getVsyncRate(),
- currentMode.modePtr->getVsyncRate());
+ setActiveMode(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(),
+ activeMode.modePtr->getVsyncRate());
// Initiate a mode change.
- mDesiredActiveModeChanged = true;
- mDesiredActiveMode = info;
- return DesiredActiveModeAction::InitiateDisplayModeSwitch;
+ mDesiredModeChanged = true;
+ mDesiredMode = info;
+ return DesiredModeAction::InitiateDisplayModeSwitch;
}
-std::optional<DisplayDevice::ActiveModeInfo> DisplayDevice::getDesiredActiveMode() const {
- std::scoped_lock lock(mActiveModeLock);
- if (mDesiredActiveModeChanged) return mDesiredActiveMode;
+auto DisplayDevice::getDesiredMode() const -> ftl::Optional<ActiveModeInfo> {
+ std::scoped_lock lock(mDesiredModeLock);
+ if (mDesiredModeChanged) return mDesiredMode;
return std::nullopt;
}
-void DisplayDevice::clearDesiredActiveModeState() {
- std::scoped_lock lock(mActiveModeLock);
- mDesiredActiveMode.event = scheduler::DisplayModeEvent::None;
- mDesiredActiveModeChanged = false;
+void DisplayDevice::clearDesiredMode() {
+ std::scoped_lock lock(mDesiredModeLock);
+ mDesiredMode.event = scheduler::DisplayModeEvent::None;
+ mDesiredModeChanged = false;
}
void DisplayDevice::adjustRefreshRate(Fps pacesetterDisplayRefreshRate) {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index a40f310..c70c1df 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -17,7 +17,6 @@
#pragma once
#include <memory>
-#include <optional>
#include <string>
#include <unordered_map>
@@ -25,6 +24,7 @@
#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>
@@ -51,6 +51,7 @@
#include "ThreadContext.h"
#include "TracedOrdinal.h"
#include "Utils/Dumper.h"
+
namespace android {
class Fence;
@@ -205,19 +206,15 @@
}
};
- enum class DesiredActiveModeAction {
- None,
- InitiateDisplayModeSwitch,
- InitiateRenderRateSwitch
- };
- DesiredActiveModeAction setDesiredActiveMode(const ActiveModeInfo&, bool force = false)
- EXCLUDES(mActiveModeLock);
- std::optional<ActiveModeInfo> getDesiredActiveMode() const EXCLUDES(mActiveModeLock);
- void clearDesiredActiveModeState() EXCLUDES(mActiveModeLock);
- ActiveModeInfo getUpcomingActiveMode() const REQUIRES(kMainThreadContext) {
- return mUpcomingActiveMode;
- }
+ enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch };
+ DesiredModeAction setDesiredMode(const ActiveModeInfo&, bool force = false)
+ EXCLUDES(mDesiredModeLock);
+
+ ftl::Optional<ActiveModeInfo> getDesiredMode() const EXCLUDES(mDesiredModeLock);
+ void clearDesiredMode() EXCLUDES(mDesiredModeLock);
+
+ ActiveModeInfo getPendingMode() const REQUIRES(kMainThreadContext) { return mPendingMode; }
bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; }
scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) {
@@ -282,9 +279,9 @@
const std::shared_ptr<compositionengine::Display> mCompositionDisplay;
std::string mDisplayName;
- std::string mActiveModeFPSTrace;
- std::string mActiveModeFPSHwcTrace;
- std::string mRenderFrameRateFPSTrace;
+ std::string mPendingModeFpsTrace;
+ std::string mActiveModeFpsTrace;
+ std::string mRenderRateFpsTrace;
const ui::Rotation mPhysicalOrientation;
ui::Rotation mOrientation = ui::ROTATION_0;
@@ -319,11 +316,11 @@
// This parameter is only used for hdr/sdr ratio overlay
float mHdrSdrRatio = 1.0f;
- mutable std::mutex mActiveModeLock;
- ActiveModeInfo mDesiredActiveMode GUARDED_BY(mActiveModeLock);
- TracedOrdinal<bool> mDesiredActiveModeChanged GUARDED_BY(mActiveModeLock);
+ mutable std::mutex mDesiredModeLock;
+ ActiveModeInfo mDesiredMode GUARDED_BY(mDesiredModeLock);
+ TracedOrdinal<bool> mDesiredModeChanged GUARDED_BY(mDesiredModeLock);
- ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext);
+ ActiveModeInfo mPendingMode GUARDED_BY(kMainThreadContext);
bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false;
};
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
index 5892b2b..bc0d448 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
@@ -36,7 +36,6 @@
#include <scheduler/FrameRateMode.h>
#include <utils/Trace.h>
-#include "../SurfaceFlingerProperties.h"
#include "RefreshRateSelector.h"
#include <com_android_graphics_surfaceflinger_flags.h>
@@ -971,12 +970,12 @@
}
ftl::Optional<FrameRateMode> RefreshRateSelector::onKernelTimerChanged(
- std::optional<DisplayModeId> desiredActiveModeId, bool timerExpired) const {
+ ftl::Optional<DisplayModeId> desiredModeIdOpt, bool timerExpired) const {
std::lock_guard lock(mLock);
const auto current = [&]() REQUIRES(mLock) -> FrameRateMode {
- if (desiredActiveModeId) {
- const auto& modePtr = mDisplayModes.get(*desiredActiveModeId)->get();
+ if (desiredModeIdOpt) {
+ const auto& modePtr = mDisplayModes.get(*desiredModeIdOpt)->get();
return FrameRateMode{modePtr->getPeakFps(), ftl::as_non_null(modePtr)};
}
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
index 545b939..40e9a83 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
@@ -16,9 +16,6 @@
#pragma once
-#include <algorithm>
-#include <numeric>
-#include <set>
#include <type_traits>
#include <utility>
#include <variant>
@@ -258,9 +255,8 @@
mMaxRefreshRateModeIt->second->getPeakFps()};
}
- ftl::Optional<FrameRateMode> onKernelTimerChanged(
- std::optional<DisplayModeId> desiredActiveModeId, bool timerExpired) const
- EXCLUDES(mLock);
+ ftl::Optional<FrameRateMode> onKernelTimerChanged(ftl::Optional<DisplayModeId> desiredModeIdOpt,
+ bool timerExpired) const EXCLUDES(mLock);
void setActiveMode(DisplayModeId, Fps renderFrameRate) EXCLUDES(mLock);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 437e27e..04088ec 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1177,7 +1177,7 @@
return NO_ERROR;
}
-void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request, bool force) {
+void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool force) {
const auto displayId = request.mode.modePtr->getPhysicalDisplayId();
ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
@@ -1190,10 +1190,9 @@
const auto mode = request.mode;
const bool emitEvent = request.emitEvent;
- switch (display->setDesiredActiveMode(DisplayDevice::ActiveModeInfo(std::move(request)),
- force)) {
- case DisplayDevice::DesiredActiveModeAction::InitiateDisplayModeSwitch:
- // Set the render rate as setDesiredActiveMode updated it.
+ switch (display->setDesiredMode(DisplayDevice::ActiveModeInfo(std::move(request)), force)) {
+ case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch:
+ // DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler.
mScheduler->setRenderRate(displayId,
display->refreshRateSelector().getActiveMode().fps);
@@ -1215,7 +1214,7 @@
mScheduler->setModeChangePending(true);
break;
- case DisplayDevice::DesiredActiveModeAction::InitiateRenderRateSwitch:
+ case DisplayDevice::DesiredModeAction::InitiateRenderRateSwitch:
mScheduler->setRenderRate(displayId, mode.fps);
if (displayId == mActiveDisplayId) {
@@ -1227,7 +1226,7 @@
dispatchDisplayModeChangeEvent(displayId, mode);
}
break;
- case DisplayDevice::DesiredActiveModeAction::None:
+ case DisplayDevice::DesiredModeAction::None:
break;
}
}
@@ -1287,27 +1286,27 @@
const auto displayId = display.getPhysicalId();
ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
- const auto upcomingModeInfo = display.getUpcomingActiveMode();
- if (!upcomingModeInfo.modeOpt) {
+ const auto pendingMode = display.getPendingMode();
+ if (!pendingMode.modeOpt) {
// There is no pending mode change. This can happen if the active
// display changed and the mode change happened on a different display.
return;
}
if (display.getActiveMode().modePtr->getResolution() !=
- upcomingModeInfo.modeOpt->modePtr->getResolution()) {
+ pendingMode.modeOpt->modePtr->getResolution()) {
auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken());
// We need to generate new sequenceId in order to recreate the display (and this
// way the framebuffer).
state.sequenceId = DisplayDeviceState{}.sequenceId;
- state.physical->activeMode = upcomingModeInfo.modeOpt->modePtr.get();
+ state.physical->activeMode = pendingMode.modeOpt->modePtr.get();
processDisplayChangesLocked();
// processDisplayChangesLocked will update all necessary components so we're done here.
return;
}
- const auto& activeMode = *upcomingModeInfo.modeOpt;
+ const auto& activeMode = *pendingMode.modeOpt;
display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(),
activeMode.fps);
@@ -1316,26 +1315,26 @@
updatePhaseConfiguration(activeMode.fps);
}
- if (upcomingModeInfo.event != scheduler::DisplayModeEvent::None) {
+ if (pendingMode.event != scheduler::DisplayModeEvent::None) {
dispatchDisplayModeChangeEvent(displayId, activeMode);
}
}
-void SurfaceFlinger::clearDesiredActiveModeState(const sp<DisplayDevice>& display) {
- display->clearDesiredActiveModeState();
+void SurfaceFlinger::dropModeRequest(const sp<DisplayDevice>& display) {
+ display->clearDesiredMode();
if (display->getPhysicalId() == mActiveDisplayId) {
// TODO(b/255635711): Check for pending mode changes on other displays.
mScheduler->setModeChangePending(false);
}
}
-void SurfaceFlinger::desiredActiveModeChangeDone(const sp<DisplayDevice>& display) {
- const auto desiredActiveMode = display->getDesiredActiveMode();
- const auto& modeOpt = desiredActiveMode->modeOpt;
+void SurfaceFlinger::applyActiveMode(const sp<DisplayDevice>& display) {
+ const auto desiredModeOpt = display->getDesiredMode();
+ const auto& modeOpt = desiredModeOpt->modeOpt;
const auto displayId = modeOpt->modePtr->getPhysicalDisplayId();
const auto vsyncRate = modeOpt->modePtr->getVsyncRate();
const auto renderFps = modeOpt->fps;
- clearDesiredActiveModeState(display);
+ dropModeRequest(display);
mScheduler->resyncToHardwareVsync(displayId, true /* allowToEnable */, vsyncRate);
mScheduler->setRenderRate(displayId, renderFps);
@@ -1353,25 +1352,23 @@
const auto display = getDisplayDeviceLocked(id);
if (!display) continue;
- // Store the local variable to release the lock.
- const auto desiredActiveMode = display->getDesiredActiveMode();
- if (!desiredActiveMode) {
- // No desired active mode pending to be applied.
+ const auto desiredModeOpt = display->getDesiredMode();
+ if (!desiredModeOpt) {
continue;
}
if (!shouldApplyRefreshRateSelectorPolicy(*display)) {
- clearDesiredActiveModeState(display);
+ dropModeRequest(display);
continue;
}
- const auto desiredModeId = desiredActiveMode->modeOpt->modePtr->getId();
+ const auto desiredModeId = desiredModeOpt->modeOpt->modePtr->getId();
const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId);
if (!displayModePtrOpt) {
ALOGW("Desired display mode is no longer supported. Mode ID = %d",
desiredModeId.value());
- clearDesiredActiveModeState(display);
+ dropModeRequest(display);
continue;
}
@@ -1379,9 +1376,8 @@
to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
to_string(display->getId()).c_str());
- if (display->getActiveMode() == desiredActiveMode->modeOpt) {
- // we are already in the requested mode, there is nothing left to do
- desiredActiveModeChangeDone(display);
+ if (display->getActiveMode() == desiredModeOpt->modeOpt) {
+ applyActiveMode(display);
continue;
}
@@ -1389,9 +1385,9 @@
// allowed modes might have changed by the time we process the refresh.
// Make sure the desired mode is still allowed
const auto displayModeAllowed =
- display->refreshRateSelector().isModeAllowed(*desiredActiveMode->modeOpt);
+ display->refreshRateSelector().isModeAllowed(*desiredModeOpt->modeOpt);
if (!displayModeAllowed) {
- clearDesiredActiveModeState(display);
+ dropModeRequest(display);
continue;
}
@@ -1401,9 +1397,7 @@
constraints.seamlessRequired = false;
hal::VsyncPeriodChangeTimeline outTimeline;
- const auto status =
- display->initiateModeChange(*desiredActiveMode, constraints, &outTimeline);
-
+ const auto status = display->initiateModeChange(*desiredModeOpt, constraints, &outTimeline);
if (status != NO_ERROR) {
// initiateModeChange may fail if a hotplug event is just about
// to be sent. We just log the error in this case.
@@ -1428,9 +1422,9 @@
const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);
finalizeDisplayModeChange(*display);
- const auto desiredActiveMode = display->getDesiredActiveMode();
- if (desiredActiveMode && display->getActiveMode() == desiredActiveMode->modeOpt) {
- desiredActiveModeChangeDone(display);
+ const auto desiredModeOpt = display->getDesiredMode();
+ if (desiredModeOpt && display->getActiveMode() == desiredModeOpt->modeOpt) {
+ applyActiveMode(display);
}
}
}
@@ -4016,7 +4010,7 @@
}
if (display->refreshRateSelector().isModeAllowed(request.mode)) {
- setDesiredActiveMode(std::move(request));
+ setDesiredMode(std::move(request));
} else {
ALOGV("%s: Mode %d is disallowed for display %s", __func__, modePtr->getId().value(),
to_string(displayId).c_str());
@@ -7284,15 +7278,14 @@
}
if (!display->isRefreshRateOverlayEnabled()) return;
- const auto desiredActiveMode = display->getDesiredActiveMode();
- const std::optional<DisplayModeId> desiredModeId = desiredActiveMode
- ? std::make_optional(desiredActiveMode->modeOpt->modePtr->getId())
-
- : std::nullopt;
+ const auto desiredModeIdOpt =
+ display->getDesiredMode().transform([](const DisplayDevice::ActiveModeInfo& info) {
+ return info.modeOpt->modePtr->getId();
+ });
const bool timerExpired = mKernelIdleTimerEnabled && expired;
- if (display->onKernelTimerChanged(desiredModeId, timerExpired)) {
+ if (display->onKernelTimerChanged(desiredModeIdOpt, timerExpired)) {
mScheduler->scheduleFrame();
}
}));
@@ -8227,18 +8220,19 @@
auto preferredMode = std::move(*preferredModeOpt);
const auto preferredModeId = preferredMode.modePtr->getId();
+ const Fps preferredFps = preferredMode.fps;
ALOGV("Switching to Scheduler preferred mode %d (%s)", preferredModeId.value(),
- to_string(preferredMode.fps).c_str());
+ to_string(preferredFps).c_str());
if (!selector.isModeAllowed(preferredMode)) {
ALOGE("%s: Preferred mode %d is disallowed", __func__, preferredModeId.value());
return INVALID_OPERATION;
}
- setDesiredActiveMode({preferredMode, .emitEvent = true}, force);
+ setDesiredMode({std::move(preferredMode), .emitEvent = true}, force);
// Update the frameRateOverride list as the display render rate might have changed
- if (mScheduler->updateFrameRateOverrides(/*consideredSignals*/ {}, preferredMode.fps)) {
+ if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) {
triggerOnFrameRateOverridesChanged();
}
@@ -8575,7 +8569,7 @@
const DisplayDevice& activeDisplay) {
ATRACE_CALL();
- // For the first display activated during boot, there is no need to force setDesiredActiveMode,
+ // For the first display activated during boot, there is no need to force setDesiredMode,
// because DM is about to send its policy via setDesiredDisplayModeSpecs.
bool forceApplyPolicy = false;
@@ -8599,9 +8593,9 @@
sActiveDisplayRotationFlags = ui::Transform::toRotationFlags(activeDisplay.getOrientation());
// The policy of the new active/pacesetter display may have changed while it was inactive. In
- // that case, its preferred mode has not been propagated to HWC (via setDesiredActiveMode). In
- // either case, the Scheduler's cachedModeChangedParams must be initialized to the newly active
- // mode, and the kernel idle timer of the newly active display must be toggled.
+ // that case, its preferred mode has not been propagated to HWC (via setDesiredMode). In either
+ // case, the Scheduler's cachedModeChangedParams must be initialized to the newly active mode,
+ // and the kernel idle timer of the newly active display must be toggled.
applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector(),
forceApplyPolicy);
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1e90340..b614a36 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -686,8 +686,7 @@
// Show hdr sdr ratio overlay
bool mHdrSdrRatioOverlay = false;
- void setDesiredActiveMode(display::DisplayModeRequest&&, bool force = false)
- REQUIRES(mStateLock);
+ void setDesiredMode(display::DisplayModeRequest&&, bool force = false) REQUIRES(mStateLock);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
Fps maxFps);
@@ -695,9 +694,10 @@
void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext);
void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext);
- void clearDesiredActiveModeState(const sp<DisplayDevice>&) REQUIRES(mStateLock);
- // Called when active mode is no longer is progress
- void desiredActiveModeChangeDone(const sp<DisplayDevice>&) REQUIRES(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);
+
// Called on the main thread in response to setPowerMode()
void setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode)
REQUIRES(mStateLock, kMainThreadContext);
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
index 2d87ddd..8da641c 100644
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -30,7 +30,7 @@
class InitiateModeChangeTest : public DisplayTransactionTest {
public:
- using Action = DisplayDevice::DesiredActiveModeAction;
+ using Action = DisplayDevice::DesiredModeAction;
using Event = scheduler::DisplayModeEvent;
void SetUp() override {
@@ -66,47 +66,42 @@
ftl::as_non_null(createDisplayMode(kModeId120, 120_Hz));
};
-TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setCurrentMode) {
+TEST_F(InitiateModeChangeTest, setDesiredModeToActiveMode) {
EXPECT_EQ(Action::None,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{60_Hz, kMode60}, Event::None}));
- EXPECT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{60_Hz, kMode60}, Event::None}));
+ EXPECT_FALSE(mDisplay->getDesiredMode());
}
-TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) {
+TEST_F(InitiateModeChangeTest, setDesiredMode) {
EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
- // Setting another mode should be cached but return None
+ // Setting another mode should be cached but return None.
EXPECT_EQ(Action::None,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredMode()->modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
}
-TEST_F(InitiateModeChangeTest, clearDesiredActiveModeState) {
+TEST_F(InitiateModeChangeTest, clearDesiredMode) {
EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
+ EXPECT_TRUE(mDisplay->getDesiredMode());
- mDisplay->clearDesiredActiveModeState();
- ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->clearDesiredMode();
+ EXPECT_FALSE(mDisplay->getDesiredMode());
}
-TEST_F(InitiateModeChangeTest, initiateModeChange) NO_THREAD_SAFETY_ANALYSIS {
+TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) {
EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
hal::VsyncPeriodChangeConstraints constraints{
.desiredTimeNanos = systemTime(),
@@ -114,30 +109,26 @@
};
hal::VsyncPeriodChangeTimeline timeline;
EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
- &timeline));
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getUpcomingActiveMode().modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+ mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
- mDisplay->clearDesiredActiveModeState();
- ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->clearDesiredMode();
+ EXPECT_FALSE(mDisplay->getDesiredMode());
}
-TEST_F(InitiateModeChangeTest, initiateRenderRateChange) {
+TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) {
EXPECT_EQ(Action::InitiateRenderRateSwitch,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{30_Hz, kMode60}, Event::None}));
- EXPECT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{30_Hz, kMode60}, Event::None}));
+ EXPECT_FALSE(mDisplay->getDesiredMode());
}
-TEST_F(InitiateModeChangeTest, getUpcomingActiveMode_desiredActiveModeChanged)
-NO_THREAD_SAFETY_ANALYSIS {
+TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) {
EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
hal::VsyncPeriodChangeConstraints constraints{
.desiredTimeNanos = systemTime(),
@@ -145,29 +136,26 @@
};
hal::VsyncPeriodChangeTimeline timeline;
EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
- &timeline));
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getUpcomingActiveMode().modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+ mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
EXPECT_EQ(Action::None,
- mDisplay->setDesiredActiveMode(
- {scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
- ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+ mDisplay->setDesiredMode({scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredMode()->modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getUpcomingActiveMode().modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
- &timeline));
- EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getUpcomingActiveMode().modeOpt);
- EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+ mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getPendingMode().modeOpt);
+ EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
- mDisplay->clearDesiredActiveModeState();
- ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+ mDisplay->clearDesiredMode();
+ EXPECT_FALSE(mDisplay->getDesiredMode());
}
} // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index aeac80d..075f974 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -145,11 +145,11 @@
TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp);
}
-TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) {
+TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
@@ -157,9 +157,9 @@
mock::createDisplayModeSpecs(kModeId90.value(), false, 0,
120));
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId90);
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getDesiredMode()->modeOpt->modePtr->getId(), kModeId90);
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
@@ -171,8 +171,9 @@
mFlinger.commit();
Mock::VerifyAndClearExpectations(mComposer);
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+
+ EXPECT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
// Verify that the next commit will complete the mode change and send
// a onModeChanged event to the framework.
@@ -182,14 +183,14 @@
mFlinger.commit();
Mock::VerifyAndClearExpectations(mAppEventThread);
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
}
-TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefreshRequired) {
+TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ EXPECT_FALSE(mDisplay->getDesiredMode());
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
@@ -197,9 +198,9 @@
mock::createDisplayModeSpecs(kModeId90.value(), true, 0,
120));
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId90);
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getDesiredMode()->modeOpt->modePtr->getId(), kModeId90);
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
@@ -214,8 +215,8 @@
mFlinger.commit();
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
}
TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {
@@ -224,8 +225,8 @@
// Test that if we call setDesiredDisplayModeSpecs while a previous mode change
// is still being processed the later call will be respected.
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
@@ -245,8 +246,8 @@
mock::createDisplayModeSpecs(kModeId120.value(), false, 0,
180));
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId120);
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getDesiredMode()->modeOpt->modePtr->getId(), kModeId120);
EXPECT_CALL(*mComposer,
setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
@@ -255,20 +256,20 @@
mFlinger.commit();
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId120);
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getDesiredMode()->modeOpt->modePtr->getId(), kModeId120);
mFlinger.commit();
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120);
}
-TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefreshRequired) {
+TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
@@ -276,9 +277,9 @@
mock::createDisplayModeSpecs(kModeId90_4K.value(), false, 0,
120));
- ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getDesiredActiveMode()->modeOpt->modePtr->getId(), kModeId90_4K);
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getDesiredMode()->modeOpt->modePtr->getId(), kModeId90_4K);
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
@@ -312,18 +313,18 @@
// so we need to update with the new instance.
mDisplay = mFlinger.getDisplay(displayToken);
- ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
- ASSERT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K);
+ EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K);
}
MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") {
- if (!arg->getDesiredActiveMode()) {
- *result_listener << "No desired active mode";
+ if (!arg->getDesiredMode()) {
+ *result_listener << "No desired mode";
return false;
}
- if (arg->getDesiredActiveMode()->modeOpt->modePtr->getId() != modeId) {
- *result_listener << "Unexpected desired active mode " << modeId;
+ if (arg->getDesiredMode()->modeOpt->modePtr->getId() != modeId) {
+ *result_listener << "Unexpected desired mode " << modeId;
return false;
}
@@ -336,9 +337,8 @@
}
MATCHER_P(ModeSettledTo, modeId, "") {
- if (const auto desiredOpt = arg->getDesiredActiveMode()) {
- *result_listener << "Unsettled desired active mode "
- << desiredOpt->modeOpt->modePtr->getId();
+ if (const auto desiredOpt = arg->getDesiredMode()) {
+ *result_listener << "Unsettled desired mode " << desiredOpt->modeOpt->modePtr->getId();
return false;
}