summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Dominik Laskowski <domlaskowski@google.com> 2023-06-29 12:01:13 -0400
committer Dominik Laskowski <domlaskowski@google.com> 2023-08-01 11:50:29 -0400
commitfdac5651f516b543f7c2073d058405bfa425d39b (patch)
tree759321062c6bc6f6464b7d2c9c6ee45f918420ee
parent9bd0c2badd3190edc106930933f395d89e0a4e33 (diff)
SF: Fix mode setting for secondary displays
Desired modes were only propagated to HWC for the "active" display, i.e. generally the primary display and specially the rear display when in the folded state. In other words, mode setting did not happen for external displays, and the rear display when driving both displays concurrently. Store per-display state for whether a mode set is pending. Propagate the desired mode for both internal and external displays as long as they are powered on. Fixes: 277776378 Fixes: 289182528 Bug: 255635711 Bug: 255635821 Test: The 60 Hz constraint applies to both displays in concurrent mode. Test: ADB `set-user-preferred-display-mode` applies to external display. Test: DisplayModeSwitchingTest#inner{Xor,And}OuterDisplay Test: DisplayModeSwitchingTest#powerOffDuringModeSet Change-Id: I9da3a0be07f9fbb08f11485aa6ab9400259a4e09
-rw-r--r--services/surfaceflinger/DisplayDevice.cpp17
-rw-r--r--services/surfaceflinger/DisplayDevice.h7
-rw-r--r--services/surfaceflinger/Scheduler/Scheduler.cpp12
-rw-r--r--services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h3
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp183
-rw-r--r--services/surfaceflinger/SurfaceFlinger.h19
-rw-r--r--services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h2
-rw-r--r--services/surfaceflinger/tests/unittests/MessageQueueTest.cpp2
-rw-r--r--services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp160
-rw-r--r--services/surfaceflinger/tests/unittests/TestableScheduler.h2
-rw-r--r--services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h13
11 files changed, 292 insertions, 128 deletions
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 32bd890aee..3c7cbe693f 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -38,7 +38,6 @@
#include <log/log.h>
#include <system/window.h>
-#include "Display/DisplaySnapshot.h"
#include "DisplayDevice.h"
#include "FrontEnd/DisplayInfo.h"
#include "HdrSdrRatioOverlay.h"
@@ -231,10 +230,18 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info,
return BAD_VALUE;
}
mUpcomingActiveMode = info;
- ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), info.modeOpt->modePtr->getFps().getIntValue());
- return mHwComposer.setActiveModeWithConstraints(getPhysicalId(),
- info.modeOpt->modePtr->getHwcId(), constraints,
- outTimeline);
+ mIsModeSetPending = true;
+
+ const auto& pendingMode = *info.modeOpt->modePtr;
+ ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), pendingMode.getFps().getIntValue());
+
+ return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), pendingMode.getHwcId(),
+ constraints, outTimeline);
+}
+
+void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps displayFps, Fps renderFps) {
+ setActiveMode(modeId, displayFps, renderFps);
+ mIsModeSetPending = false;
}
nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index e92125a45d..a3fa701be2 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -218,6 +218,8 @@ public:
return mUpcomingActiveMode;
}
+ bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; }
+
scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) {
return mRefreshRateSelector->getActiveMode();
}
@@ -229,6 +231,9 @@ public:
hal::VsyncPeriodChangeTimeline* outTimeline)
REQUIRES(kMainThreadContext);
+ void finalizeModeChange(DisplayModeId, Fps displayFps, Fps renderFps)
+ REQUIRES(kMainThreadContext);
+
scheduler::RefreshRateSelector& refreshRateSelector() const { return *mRefreshRateSelector; }
// Extends the lifetime of the RefreshRateSelector, so it can outlive this DisplayDevice.
@@ -313,7 +318,9 @@ private:
ActiveModeInfo mDesiredActiveMode GUARDED_BY(mActiveModeLock);
TracedOrdinal<bool> mDesiredActiveModeChanged GUARDED_BY(mActiveModeLock) =
{ftl::Concat("DesiredActiveModeChanged-", getId().value).c_str(), false};
+
ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext);
+ bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false;
};
struct DisplayDeviceState {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index d6d7725f6c..5a19ec5095 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -186,7 +186,17 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId,
FrameTargeter& pacesetterTargeter = *pacesetterOpt->get().targeterPtr;
pacesetterTargeter.beginFrame(beginFrameArgs, *pacesetterOpt->get().schedulePtr);
- if (!compositor.commit(pacesetterTargeter.target())) return;
+ FrameTargets targets;
+ targets.try_emplace(pacesetterId, &pacesetterTargeter.target());
+
+ for (const auto& [id, display] : mDisplays) {
+ if (id == pacesetterId) continue;
+
+ const FrameTargeter& targeter = *display.targeterPtr;
+ targets.try_emplace(id, &targeter.target());
+ }
+
+ if (!compositor.commit(pacesetterId, targets)) return;
// TODO(b/256196556): Choose the frontrunner display.
FrameTargeters targeters;
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h b/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h
index 6fe813a181..12ee36e084 100644
--- a/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h
+++ b/services/surfaceflinger/Scheduler/include/scheduler/interface/ICompositor.h
@@ -29,6 +29,7 @@ namespace scheduler {
class FrameTarget;
class FrameTargeter;
+using FrameTargets = ui::PhysicalDisplayMap<PhysicalDisplayId, const scheduler::FrameTarget*>;
using FrameTargeters = ui::PhysicalDisplayMap<PhysicalDisplayId, scheduler::FrameTargeter*>;
} // namespace scheduler
@@ -39,7 +40,7 @@ struct ICompositor {
// Commits transactions for layers and displays. Returns whether any state has been invalidated,
// i.e. whether a frame should be composited for each display.
- virtual bool commit(const scheduler::FrameTarget&) = 0;
+ virtual bool commit(PhysicalDisplayId pacesetterId, const scheduler::FrameTargets&) = 0;
// Composites a frame for each display. CompositionEngine performs GPU and/or HAL composition
// via RenderEngine and the Composer HAL, respectively.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9f24dd6341..6caf5df0e2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1195,9 +1195,9 @@ status_t SurfaceFlinger::getDisplayStats(const sp<IBinder>& displayToken,
}
void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request, bool force) {
- ATRACE_CALL();
-
const auto displayId = request.mode.modePtr->getPhysicalDisplayId();
+ ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
+
const auto display = getDisplayDeviceLocked(displayId);
if (!display) {
ALOGW("%s: display is no longer valid", __func__);
@@ -1225,17 +1225,24 @@ void SurfaceFlinger::setDesiredActiveMode(display::DisplayModeRequest&& request,
// As we called to set period, we will call to onRefreshRateChangeCompleted once
// VsyncController model is locked.
mScheduler->modulateVsync(displayId, &VsyncModulator::onRefreshRateChangeInitiated);
- updatePhaseConfiguration(mode.fps);
+
+ if (displayId == mActiveDisplayId) {
+ updatePhaseConfiguration(mode.fps);
+ }
+
mScheduler->setModeChangePending(true);
break;
case DisplayDevice::DesiredActiveModeAction::InitiateRenderRateSwitch:
mScheduler->setRenderRate(displayId, mode.fps);
- updatePhaseConfiguration(mode.fps);
- mRefreshRateStats->setRefreshRate(mode.fps);
- if (display->getPhysicalId() == mActiveDisplayId && emitEvent) {
- mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, mode);
+
+ if (displayId == mActiveDisplayId) {
+ updatePhaseConfiguration(mode.fps);
+ mRefreshRateStats->setRefreshRate(mode.fps);
}
+ if (emitEvent) {
+ dispatchDisplayModeChangeEvent(displayId, mode);
+ }
break;
case DisplayDevice::DesiredActiveModeAction::None:
break;
@@ -1291,24 +1298,20 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const sp<display::DisplayToke
return future.get();
}
-void SurfaceFlinger::updateInternalStateWithChangedMode() {
- ATRACE_CALL();
-
- const auto display = getDefaultDisplayDeviceLocked();
- if (!display) {
- return;
- }
+void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) {
+ const auto displayId = display.getPhysicalId();
+ ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
- const auto upcomingModeInfo = display->getUpcomingActiveMode();
+ const auto upcomingModeInfo = display.getUpcomingActiveMode();
if (!upcomingModeInfo.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() !=
+ if (display.getActiveMode().modePtr->getResolution() !=
upcomingModeInfo.modeOpt->modePtr->getResolution()) {
- auto& state = mCurrentState.displays.editValueFor(display->getDisplayToken());
+ 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;
@@ -1319,27 +1322,24 @@ void SurfaceFlinger::updateInternalStateWithChangedMode() {
return;
}
- mPhysicalDisplays.get(display->getPhysicalId())
- .transform(&PhysicalDisplay::snapshotRef)
- .transform(ftl::unit_fn([&](const display::DisplaySnapshot& snapshot) {
- FTL_FAKE_GUARD(kMainThreadContext,
- display->setActiveMode(upcomingModeInfo.modeOpt->modePtr->getId(),
- upcomingModeInfo.modeOpt->modePtr->getFps(),
- upcomingModeInfo.modeOpt->fps));
- }));
-
- const Fps refreshRate = upcomingModeInfo.modeOpt->fps;
- mRefreshRateStats->setRefreshRate(refreshRate);
- updatePhaseConfiguration(refreshRate);
+ const auto& activeMode = *upcomingModeInfo.modeOpt;
+ display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getFps(),
+ activeMode.fps);
+
+ if (displayId == mActiveDisplayId) {
+ mRefreshRateStats->setRefreshRate(activeMode.fps);
+ updatePhaseConfiguration(activeMode.fps);
+ }
if (upcomingModeInfo.event != scheduler::DisplayModeEvent::None) {
- mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, *upcomingModeInfo.modeOpt);
+ dispatchDisplayModeChangeEvent(displayId, activeMode);
}
}
void SurfaceFlinger::clearDesiredActiveModeState(const sp<DisplayDevice>& display) {
display->clearDesiredActiveModeState();
if (display->getPhysicalId() == mActiveDisplayId) {
+ // TODO(b/255635711): Check for pending mode changes on other displays.
mScheduler->setModeChangePending(false);
}
}
@@ -1353,21 +1353,18 @@ void SurfaceFlinger::desiredActiveModeChangeDone(const sp<DisplayDevice>& displa
clearDesiredActiveModeState(display);
mScheduler->resyncToHardwareVsync(displayId, true /* allowToEnable */, displayFps);
mScheduler->setRenderRate(displayId, renderFps);
- updatePhaseConfiguration(renderFps);
+
+ if (displayId == mActiveDisplayId) {
+ updatePhaseConfiguration(renderFps);
+ }
}
-void SurfaceFlinger::setActiveModeInHwcIfNeeded() {
+void SurfaceFlinger::initiateDisplayModeChanges() {
ATRACE_CALL();
std::optional<PhysicalDisplayId> displayToUpdateImmediately;
for (const auto& [id, physical] : mPhysicalDisplays) {
- const auto& snapshot = physical.snapshot();
-
- if (snapshot.connectionType() != ui::DisplayConnectionType::Internal) {
- continue;
- }
-
const auto display = getDisplayDeviceLocked(id);
if (!display) continue;
@@ -1378,14 +1375,14 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() {
continue;
}
- if (id != mActiveDisplayId) {
- // Display is no longer the active display, so abort the mode change.
+ if (!display->isPoweredOn()) {
+ // Display is no longer powered on, so abort the mode change.
clearDesiredActiveModeState(display);
continue;
}
const auto desiredModeId = desiredActiveMode->modeOpt->modePtr->getId();
- const auto displayModePtrOpt = snapshot.displayModes().get(desiredModeId);
+ const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId);
if (!displayModePtrOpt) {
ALOGW("Desired display mode is no longer supported. Mode ID = %d",
@@ -1435,19 +1432,18 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() {
if (outTimeline.refreshRequired) {
scheduleComposite(FrameHint::kNone);
- mSetActiveModePending = true;
} else {
- // Updating the internal state should be done outside the loop,
- // because it can recreate a DisplayDevice and modify mDisplays
- // which will invalidate the iterator.
+ // 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();
}
}
if (displayToUpdateImmediately) {
- updateInternalStateWithChangedMode();
-
const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);
+ finalizeDisplayModeChange(*display);
+
const auto desiredActiveMode = display->getDesiredActiveMode();
if (desiredActiveMode && display->getActiveMode() == desiredActiveMode->modeOpt) {
desiredActiveModeChangeDone(display);
@@ -2398,7 +2394,10 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
return mustComposite;
}
-bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget) {
+bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId,
+ const scheduler::FrameTargets& frameTargets) {
+ const scheduler::FrameTarget& pacesetterFrameTarget = *frameTargets.get(pacesetterId)->get();
+
const VsyncId vsyncId = pacesetterFrameTarget.vsyncId();
ATRACE_NAME(ftl::Concat(__func__, ' ', ftl::to_underlying(vsyncId)).c_str());
@@ -2411,20 +2410,35 @@ bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget)
mTracingEnabledChanged = false;
}
- // If we are in the middle of a mode change and the fence hasn't
- // fired yet just wait for the next commit.
- if (mSetActiveModePending) {
- if (pacesetterFrameTarget.isFramePending()) {
- mScheduler->scheduleFrame();
- return false;
- }
+ // 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;
- // We received the present fence from the HWC, so we assume it successfully updated
- // the mode, hence we update SF.
- mSetActiveModePending = false;
- {
- Mutex::Autolock lock(mStateLock);
- updateInternalStateWithChangedMode();
+ if (const auto display = getDisplayDeviceLocked(pair.first)) {
+ return display->isModeSetPending();
+ }
+
+ return false;
+ })) {
+ 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);
+ }
}
}
@@ -2515,7 +2529,7 @@ bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget)
? &mLayerHierarchyBuilder.getHierarchy()
: nullptr,
updateAttachedChoreographer);
- setActiveModeInHwcIfNeeded();
+ initiateDisplayModeChanges();
}
updateCursorAsync();
@@ -3322,6 +3336,16 @@ void SurfaceFlinger::dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bo
mScheduler->onHotplugReceived(mSfConnectionHandle, displayId, connected);
}
+void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId,
+ const scheduler::FrameRateMode& mode) {
+ // TODO(b/255635821): Merge code paths and move to Scheduler.
+ const auto onDisplayModeChanged = displayId == mActiveDisplayId
+ ? &scheduler::Scheduler::onPrimaryDisplayModeChanged
+ : &scheduler::Scheduler::onNonPrimaryDisplayModeChanged;
+
+ ((*mScheduler).*onDisplayModeChanged)(mAppConnectionHandle, mode);
+}
+
sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay,
@@ -3420,14 +3444,8 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
RenderIntent::COLORIMETRIC});
if (const auto& physical = state.physical) {
- mPhysicalDisplays.get(physical->id)
- .transform(&PhysicalDisplay::snapshotRef)
- .transform(ftl::unit_fn([&](const display::DisplaySnapshot& snapshot) {
- FTL_FAKE_GUARD(kMainThreadContext,
- display->setActiveMode(physical->activeMode->getId(),
- physical->activeMode->getFps(),
- physical->activeMode->getFps()));
- }));
+ const auto& mode = *physical->activeMode;
+ display->setActiveMode(mode.getId(), mode.getFps(), mode.getFps());
}
display->setLayerFilter(makeLayerFilterForDisplay(display->getId(), state.layerStack));
@@ -3946,12 +3964,8 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest
if (!display) continue;
- const bool isInternalDisplay = mPhysicalDisplays.get(displayId)
- .transform(&PhysicalDisplay::isInternal)
- .value_or(false);
-
- if (isInternalDisplay && displayId != mActiveDisplayId) {
- ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str());
+ if (!display->isPoweredOn()) {
+ ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str());
continue;
}
@@ -3959,7 +3973,7 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest
setDesiredActiveMode(std::move(request));
} else {
ALOGV("%s: Mode %d is disallowed for display %s", __func__, modePtr->getId().value(),
- to_string(display->getId()).c_str());
+ to_string(displayId).c_str());
}
}
}
@@ -7922,6 +7936,7 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal(
const sp<DisplayDevice>& display,
const scheduler::RefreshRateSelector::PolicyVariant& policy) {
const auto displayId = display->getPhysicalId();
+ ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
Mutex::Autolock lock(mStateLock);
@@ -7942,13 +7957,11 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal(
break;
}
- const bool isInternalDisplay = mPhysicalDisplays.get(displayId)
- .transform(&PhysicalDisplay::isInternal)
- .value_or(false);
-
- if (isInternalDisplay && displayId != mActiveDisplayId) {
- // The policy will be be applied when the display becomes active.
- ALOGV("%s(%s): Inactive display", __func__, to_string(displayId).c_str());
+ // TODO(b/255635711): Apply the policy once the display is powered on, which is currently only
+ // done for the internal display that becomes active on fold/unfold. For now, assume that DM
+ // always powers on the secondary (internal or external) display before setting its policy.
+ if (!display->isPoweredOn()) {
+ ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str());
return NO_ERROR;
}
@@ -8311,7 +8324,9 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD
resetPhaseConfiguration(activeDisplay.getActiveMode().fps);
+ // TODO(b/255635711): Check for pending mode changes on other displays.
mScheduler->setModeChangePending(false);
+
mScheduler->setPacesetterDisplay(mActiveDisplayId);
onActiveDisplaySizeChanged(activeDisplay);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index b07910d360..f61f9a2345 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -638,7 +638,8 @@ private:
// ICompositor overrides:
void configure() override REQUIRES(kMainThreadContext);
- bool commit(const scheduler::FrameTarget&) override REQUIRES(kMainThreadContext);
+ bool commit(PhysicalDisplayId pacesetterId, const scheduler::FrameTargets&) override
+ REQUIRES(kMainThreadContext);
CompositeResultsPerDisplay composite(PhysicalDisplayId pacesetterId,
const scheduler::FrameTargeters&) override
REQUIRES(kMainThreadContext);
@@ -684,11 +685,10 @@ private:
REQUIRES(mStateLock);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId);
- // Sets the active mode and a new refresh rate in SF.
- void updateInternalStateWithChangedMode() REQUIRES(mStateLock, kMainThreadContext);
- // Calls to setActiveMode on the main thread if there is a pending mode change
- // that needs to be applied.
- void setActiveModeInHwcIfNeeded() REQUIRES(mStateLock, kMainThreadContext);
+
+ 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);
@@ -1011,7 +1011,9 @@ private:
const DisplayDeviceState& drawingState)
REQUIRES(mStateLock, kMainThreadContext);
- void dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected);
+ void dispatchDisplayHotplugEvent(PhysicalDisplayId, bool connected);
+ void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&)
+ REQUIRES(mStateLock);
/*
* VSYNC
@@ -1330,9 +1332,6 @@ private:
std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats;
scheduler::PresentLatencyTracker mPresentLatencyTracker GUARDED_BY(kMainThreadContext);
- // below flags are set by main thread only
- bool mSetActiveModePending = false;
-
bool mLumaSampling = true;
sp<RegionSamplingThread> mRegionSamplingThread;
sp<FpsReporter> mFpsReporter;
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index 28ac664ba3..ca1af6ed84 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -286,7 +286,7 @@ public:
private:
// ICompositor overrides:
void configure() override {}
- bool commit(const scheduler::FrameTarget&) override { return false; }
+ bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; }
CompositeResultsPerDisplay composite(PhysicalDisplayId,
const scheduler::FrameTargeters&) override {
return {};
diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp
index 1dcf222834..9aa089f900 100644
--- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp
@@ -35,7 +35,7 @@ using CallbackToken = scheduler::VSyncDispatch::CallbackToken;
struct NoOpCompositor final : ICompositor {
void configure() override {}
- bool commit(const scheduler::FrameTarget&) override { return false; }
+ bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; }
CompositeResultsPerDisplay composite(PhysicalDisplayId,
const scheduler::FrameTargeters&) override {
return {};
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 703bdda694..24eb31821a 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -67,10 +67,36 @@ public:
.WillByDefault(Return(true));
}
+ static constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID;
+ static constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1;
+
+ auto injectOuterDisplay() {
+ constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u);
+
+ constexpr bool kIsPrimary = false;
+ TestableSurfaceFlinger::FakeHwcDisplayInjector(kOuterDisplayId, hal::DisplayType::PHYSICAL,
+ kIsPrimary)
+ .setHwcDisplayId(kOuterDisplayHwcId)
+ .setPowerMode(hal::PowerMode::OFF)
+ .inject(&mFlinger, mComposer);
+
+ mOuterDisplay = mFakeDisplayInjector.injectInternalDisplay(
+ [&](FakeDisplayDeviceInjector& injector) {
+ injector.setPowerMode(hal::PowerMode::OFF);
+ injector.setDisplayModes(mock::cloneForDisplay(kOuterDisplayId, kModes),
+ kModeId120);
+ },
+ {.displayId = kOuterDisplayId,
+ .hwcDisplayId = kOuterDisplayHwcId,
+ .isPrimary = kIsPrimary});
+
+ return std::forward_as_tuple(mDisplay, mOuterDisplay);
+ }
+
protected:
void setupScheduler(std::shared_ptr<scheduler::RefreshRateSelector>);
- sp<DisplayDevice> mDisplay;
+ sp<DisplayDevice> mDisplay, mOuterDisplay;
mock::EventThread* mAppEventThread;
static constexpr DisplayModeId kModeId60{0};
@@ -328,32 +354,16 @@ MATCHER_P(ModeSettledTo, modeId, "") {
return true;
}
-TEST_F(DisplayModeSwitchingTest, multiDisplay) {
- constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID;
- constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1;
-
- constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u);
+TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
+ const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
- constexpr bool kIsPrimary = false;
- TestableSurfaceFlinger::FakeHwcDisplayInjector(kOuterDisplayId, hal::DisplayType::PHYSICAL,
- kIsPrimary)
- .setHwcDisplayId(kOuterDisplayHwcId)
- .inject(&mFlinger, mComposer);
-
- const auto outerDisplay = mFakeDisplayInjector.injectInternalDisplay(
- [&](FakeDisplayDeviceInjector& injector) {
- injector.setDisplayModes(mock::cloneForDisplay(kOuterDisplayId, kModes),
- kModeId120);
- },
- {.displayId = kOuterDisplayId,
- .hwcDisplayId = kOuterDisplayHwcId,
- .isPrimary = kIsPrimary});
-
- const auto& innerDisplay = mDisplay;
+ EXPECT_TRUE(innerDisplay->isPoweredOn());
+ EXPECT_FALSE(outerDisplay->isPoweredOn());
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ // Only the inner display is powered on.
mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
@@ -388,6 +398,10 @@ TEST_F(DisplayModeSwitchingTest, multiDisplay) {
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ innerDisplay->setPowerMode(hal::PowerMode::OFF);
+ outerDisplay->setPowerMode(hal::PowerMode::ON);
+
+ // Only the outer display is powered on.
mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
@@ -409,5 +423,107 @@ TEST_F(DisplayModeSwitchingTest, multiDisplay) {
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
}
+TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
+ const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
+
+ EXPECT_TRUE(innerDisplay->isPoweredOn());
+ EXPECT_FALSE(outerDisplay->isPoweredOn());
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ outerDisplay->setPowerMode(hal::PowerMode::ON);
+
+ // Both displays are powered on.
+ mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId90.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId60.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
+
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kInnerDisplayHwcId,
+ hal::HWConfigId(kModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kOuterDisplayHwcId,
+ hal::HWConfigId(kModeId60.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+}
+
+TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
+ const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
+
+ EXPECT_TRUE(innerDisplay->isPoweredOn());
+ EXPECT_FALSE(outerDisplay->isPoweredOn());
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ outerDisplay->setPowerMode(hal::PowerMode::ON);
+
+ // Both displays are powered on.
+ mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId90.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId60.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
+
+ // Power off the outer display before the mode has been set.
+ outerDisplay->setPowerMode(hal::PowerMode::OFF);
+
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kInnerDisplayHwcId,
+ hal::HWConfigId(kModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index f3c9d0dd44..151b178c01 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -180,7 +180,7 @@ public:
private:
// ICompositor overrides:
void configure() override {}
- bool commit(const scheduler::FrameTarget&) override { return false; }
+ bool commit(PhysicalDisplayId, const scheduler::FrameTargets&) override { return false; }
CompositeResultsPerDisplay composite(PhysicalDisplayId,
const scheduler::FrameTargeters&) override {
return {};
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 9b3a893409..e59d44d745 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -386,10 +386,19 @@ public:
.sfWorkDuration = 10ms},
*mScheduler->getVsyncSchedule());
- mFlinger->commit(frameTargeter.target());
+ scheduler::FrameTargets targets;
+ scheduler::FrameTargeters targeters;
+
+ for (const auto& [id, display] :
+ FTL_FAKE_GUARD(mFlinger->mStateLock, mFlinger->mPhysicalDisplays)) {
+ targets.try_emplace(id, &frameTargeter.target());
+ targeters.try_emplace(id, &frameTargeter);
+ }
+
+ mFlinger->commit(displayId, targets);
if (composite) {
- mFlinger->composite(displayId, ftl::init::map(displayId, &frameTargeter));
+ mFlinger->composite(displayId, targeters);
}
}