SF: Isolate modesetting in DisplayModeController
Move the per-display state machine for modesetting from DisplayDevice to
DMC. In lieu of mStateLock, protect display lookup from multiple threads
using a mutex internal to DMC, which fixes the following deadlock:
OneShotTimer::loop
SF::requestDisplayModes
mStateLock
SF::commit
mStateLock
SF::processDisplayChangesLocked (hotplug or resolution change)
Scheduler::demotePacesetterDisplay
OneShotTimer::stop
A notable change is that {initiate,finalize}DisplayModeChange(s) are no
longer called under mStateLock, thanks to DMC's granular, internal lock.
finalizeDisplayModeChange still locks mStateLock for resolution changes.
Add an ActiveModeListener to DMC and register a callback in SF to update
the refresh rate overlay, which still lives in DisplayDevice for now.
Fixes: 329450361
Bug: 241285876
Test: DisplayModeControllerTest
Test: libsurfaceflinger_unittest
Change-Id: I30ec756f134d2d67a70ac8797008dc792eac035e
diff --git a/services/surfaceflinger/Display/DisplayModeController.h b/services/surfaceflinger/Display/DisplayModeController.h
index b6a6bee..258b04b 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 @@
// 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)>;
- // TODO(b/241285876): Remove once ownership is no longer shared with DisplayDevice.
+ DisplayModeController() = default;
+
+ 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>;
- // Returns `nullptr` if the display is no longer registered (or never was).
- RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) REQUIRES(kMainThreadContext);
-
// 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);
- }
+ // 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) const EXCLUDES(mDisplayLock);
+
+ 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