SF: Store display modes in DisplayDevice

Make DisplayDevice the source of truth for display modes
and call HWComposer only on hotplug in order to populate
the state of DisplayDevice. Additionally rename s/config/mode
in DisplayDevice.

Bug: 159590486
Bug: 175678215
Test: atest libsurfaceflinger_unittest
Change-Id: I3d8bcbf8f74d910213e0cbe3bcdd839323076e56
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index fe9db5a..e3f15eb 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -62,6 +62,7 @@
         mConnectionType(args.connectionType),
         mCompositionDisplay{args.compositionDisplay},
         mPhysicalOrientation(args.physicalOrientation),
+        mSupportedModes(std::move(args.supportedModes)),
         mIsPrimary(args.isPrimary) {
     mCompositionDisplay->editState().isSecure = args.isSecure;
     mCompositionDisplay->createRenderSurface(
@@ -139,12 +140,26 @@
     return mPowerMode != hal::PowerMode::OFF;
 }
 
-void DisplayDevice::setActiveMode(DisplayModeId mode) {
-    mActiveMode = mode;
+void DisplayDevice::setActiveMode(DisplayModeId id) {
+    LOG_FATAL_IF(id.value() >= mSupportedModes.size(),
+                 "Cannot set active mode which is not supported.");
+    mActiveModeId = id;
 }
 
-DisplayModeId DisplayDevice::getActiveMode() const {
-    return mActiveMode;
+const DisplayModePtr& DisplayDevice::getActiveMode() const {
+    return mSupportedModes[mActiveModeId.value()];
+}
+
+const DisplayModes& DisplayDevice::getSupportedModes() const {
+    return mSupportedModes;
+}
+
+DisplayModePtr DisplayDevice::getMode(DisplayModeId modeId) const {
+    const auto id = modeId.value();
+    if (id < mSupportedModes.size()) {
+        return mSupportedModes[id];
+    }
+    return nullptr;
 }
 
 ui::Dataspace DisplayDevice::getCompositionDataSpace() const {
@@ -206,17 +221,24 @@
 
 void DisplayDevice::dump(std::string& result) const {
     StringAppendF(&result, "+ %s\n", getDebugName().c_str());
-
-    result.append("   ");
-    StringAppendF(&result, "powerMode=%s (%d), ", to_string(mPowerMode).c_str(),
+    StringAppendF(&result, "   powerMode=%s (%d)\n", to_string(mPowerMode).c_str(),
                   static_cast<int32_t>(mPowerMode));
-    StringAppendF(&result, "activeConfig=%zu, ", mActiveMode.value());
-    StringAppendF(&result, "deviceProductInfo=");
+    StringAppendF(&result, "   activeMode=%s\n", to_string(*getActiveMode()).c_str());
+
+    result.append("   supportedModes=\n");
+
+    for (const auto& mode : mSupportedModes) {
+        result.append("     ");
+        result.append(to_string(*mode));
+        result.append("\n");
+    }
+    StringAppendF(&result, "   deviceProductInfo=");
     if (mDeviceProductInfo) {
         mDeviceProductInfo->dump(result);
     } else {
         result.append("{}");
     }
+    result.append("\n");
     getCompositionDisplay()->dump(result);
 }
 
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index d29f97d..3acdb16 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -157,10 +157,20 @@
     ui::Dataspace getCompositionDataSpace() const;
 
     /* ------------------------------------------------------------------------
-     * Display active config management.
+     * Display mode management.
      */
-    DisplayModeId getActiveMode() const;
-    void setActiveMode(DisplayModeId mode);
+    const DisplayModePtr& getActiveMode() const;
+    void setActiveMode(DisplayModeId);
+
+    // Return the immutable list of supported display modes. The HWC may report different modes
+    // after a hotplug reconnect event, in which case the DisplayDevice object will be recreated.
+    // Hotplug reconnects are common for external displays.
+    const DisplayModes& getSupportedModes() const;
+
+    // Returns nullptr if the given mode ID is not supported. A previously
+    // supported mode may be no longer supported for some devices like TVs and
+    // set-top boxes after a hotplug reconnect.
+    DisplayModePtr getMode(DisplayModeId) const;
 
     // release HWC resources (if any) for removable displays
     void disconnect();
@@ -189,7 +199,8 @@
 
     hardware::graphics::composer::hal::PowerMode mPowerMode =
             hardware::graphics::composer::hal::PowerMode::OFF;
-    DisplayModeId mActiveMode;
+    DisplayModeId mActiveModeId;
+    const DisplayModes mSupportedModes;
 
     // TODO(b/74619554): Remove special cases for primary display.
     const bool mIsPrimary;
@@ -248,6 +259,7 @@
     hardware::graphics::composer::hal::PowerMode initialPowerMode{
             hardware::graphics::composer::hal::PowerMode::ON};
     bool isPrimary{false};
+    DisplayModes supportedModes;
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/DisplayHardware/DisplayMode.h b/services/surfaceflinger/DisplayHardware/DisplayMode.h
index 7ae54f1..61c1b61 100644
--- a/services/surfaceflinger/DisplayHardware/DisplayMode.h
+++ b/services/surfaceflinger/DisplayHardware/DisplayMode.h
@@ -17,8 +17,10 @@
 #pragma once
 
 #include "DisplayHardware/Hal.h"
+#include "Fps.h"
 #include "Scheduler/StrongTyping.h"
 
+#include <android-base/stringprintf.h>
 #include <android/configuration.h>
 #include <utils/Timers.h>
 
@@ -61,7 +63,7 @@
         }
 
         Builder& setVsyncPeriod(int32_t vsyncPeriod) {
-            mDisplayMode->mVsyncPeriod = vsyncPeriod;
+            mDisplayMode->mFps = Fps::fromPeriodNsecs(vsyncPeriod);
             return *this;
         }
 
@@ -111,7 +113,8 @@
 
     int32_t getWidth() const { return mWidth; }
     int32_t getHeight() const { return mHeight; }
-    nsecs_t getVsyncPeriod() const { return mVsyncPeriod; }
+    Fps getFps() const { return mFps; }
+    nsecs_t getVsyncPeriod() const { return mFps.getPeriodNsecs(); }
     float getDpiX() const { return mDpiX; }
     float getDpiY() const { return mDpiY; }
     int32_t getConfigGroup() const { return mConfigGroup; }
@@ -124,10 +127,18 @@
 
     int32_t mWidth = -1;
     int32_t mHeight = -1;
-    nsecs_t mVsyncPeriod = -1;
+    Fps mFps;
     float mDpiX = -1;
     float mDpiY = -1;
     int32_t mConfigGroup = -1;
 };
 
+inline std::string to_string(const DisplayMode& mode) {
+    return base::StringPrintf("{id=%zu, hwcId=%d, width=%d, height=%d, refreshRate=%s, "
+                              "dpiX=%.2f, dpiY=%.2f, configGroup=%d}",
+                              mode.getId().value(), mode.getHwcId(), mode.getWidth(),
+                              mode.getHeight(), to_string(mode.getFps()).c_str(), mode.getDpiX(),
+                              mode.getDpiY(), mode.getConfigGroup());
+}
+
 } // namespace android
\ No newline at end of file
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 0a51659..894f352 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -900,16 +900,14 @@
 
     Mutex::Autolock lock(mStateLock);
 
-    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
-    if (!displayId) {
+    const auto display = getDisplayDeviceLocked(displayToken);
+    if (!display) {
         return NAME_NOT_FOUND;
     }
 
-    const bool isInternal = (displayId == getInternalDisplayIdLocked());
-
     configs->clear();
 
-    for (const auto& mode : getHwComposer().getModes(*displayId)) {
+    for (const auto& mode : display->getSupportedModes()) {
         DisplayConfig config;
 
         auto width = mode->getWidth();
@@ -918,7 +916,7 @@
         auto xDpi = mode->getDpiX();
         auto yDpi = mode->getDpiY();
 
-        if (isInternal &&
+        if (display->isPrimary() &&
             (internalDisplayOrientation == ui::ROTATION_90 ||
              internalDisplayOrientation == ui::ROTATION_270)) {
             std::swap(width, height);
@@ -981,7 +979,7 @@
         Mutex::Autolock lock(mStateLock);
 
         if (const auto display = getDisplayDeviceLocked(displayToken)) {
-            activeConfig = display->getActiveMode().value();
+            activeConfig = display->getActiveMode()->getId().value();
             isPrimary = display->isPrimary();
         } else {
             ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
@@ -1011,9 +1009,9 @@
         mDesiredActiveConfig = info;
         mDesiredActiveConfig.event = mDesiredActiveConfig.event | prevConfig;
     } else {
-        // Check is we are already at the desired config
+        // Check if we are already at the desired config
         const auto display = getDefaultDisplayDeviceLocked();
-        if (!display || display->getActiveMode() == refreshRate.getConfigId()) {
+        if (!display || display->getActiveMode()->getId() == refreshRate.getConfigId()) {
             return;
         }
 
@@ -1030,7 +1028,7 @@
         // VsyncController model is locked.
         modulateVsync(&VsyncModulator::onRefreshRateChangeInitiated);
 
-        updatePhaseConfiguration(refreshRate);
+        updatePhaseConfiguration(refreshRate.getFps());
         mScheduler->setConfigChangePending(true);
     }
 
@@ -1039,7 +1037,7 @@
     }
 }
 
-status_t SurfaceFlinger::setActiveConfig(const sp<IBinder>& displayToken, int mode) {
+status_t SurfaceFlinger::setActiveConfig(const sp<IBinder>& displayToken, int modeId) {
     ATRACE_CALL();
 
     if (!displayToken) {
@@ -1052,22 +1050,29 @@
             ALOGE("Attempt to set allowed display configs for invalid display token %p",
                   displayToken.get());
             return NAME_NOT_FOUND;
-        } else if (display->isVirtual()) {
+        }
+
+        if (display->isVirtual()) {
             ALOGW("Attempt to set allowed display configs for virtual display");
             return INVALID_OPERATION;
-        } else {
-            const DisplayModeId config(mode);
-            const auto fps = mRefreshRateConfigs->getRefreshRateFromConfigId(config).getFps();
-            // Keep the old switching type.
-            const auto allowGroupSwitching =
-                    mRefreshRateConfigs->getCurrentPolicy().allowGroupSwitching;
-            const scheduler::RefreshRateConfigs::Policy policy{config,
-                                                               allowGroupSwitching,
-                                                               {fps, fps}};
-            constexpr bool kOverridePolicy = false;
-
-            return setDesiredDisplayConfigSpecsInternal(display, policy, kOverridePolicy);
         }
+
+        const auto mode = display->getMode(DisplayModeId{modeId});
+        if (!mode) {
+            ALOGW("Attempt to switch to an unsupported mode %d.", modeId);
+            return BAD_VALUE;
+        }
+
+        const auto fps = mode->getFps();
+        // Keep the old switching type.
+        const auto allowGroupSwitching =
+                mRefreshRateConfigs->getCurrentPolicy().allowGroupSwitching;
+        const scheduler::RefreshRateConfigs::Policy policy{mode->getId(),
+                                                           allowGroupSwitching,
+                                                           {fps, fps}};
+        constexpr bool kOverridePolicy = false;
+
+        return setDesiredDisplayConfigSpecsInternal(display, policy, kOverridePolicy);
     });
 
     return future.get();
@@ -1081,48 +1086,58 @@
         return;
     }
 
-    auto oldRefreshRate = mRefreshRateConfigs->getRefreshRateFromConfigId(display->getActiveMode());
+    const auto upcomingConfig = display->getMode(mUpcomingActiveConfig.configId);
+    if (!upcomingConfig) {
+        ALOGW("Upcoming active config is no longer supported. ConfigId = %zu",
+              mUpcomingActiveConfig.configId.value());
+        // TODO(b/159590486) Handle the error better. Some parts of SurfaceFlinger may
+        // have been already updated with the upcoming active config.
+        return;
+    }
+    const Fps oldRefreshRate = display->getActiveMode()->getFps();
 
     std::lock_guard<std::mutex> lock(mActiveConfigLock);
     mRefreshRateConfigs->setCurrentConfigId(mUpcomingActiveConfig.configId);
     display->setActiveMode(mUpcomingActiveConfig.configId);
 
-    auto refreshRate =
-            mRefreshRateConfigs->getRefreshRateFromConfigId(mUpcomingActiveConfig.configId);
-    mRefreshRateStats->setRefreshRate(refreshRate.getFps());
+    const Fps refreshRate = upcomingConfig->getFps();
 
-    if (refreshRate.getVsyncPeriod() != oldRefreshRate.getVsyncPeriod()) {
+    mRefreshRateStats->setRefreshRate(refreshRate);
+
+    if (!refreshRate.equalsWithMargin(oldRefreshRate)) {
         mTimeStats->incrementRefreshRateSwitches();
     }
     updatePhaseConfiguration(refreshRate);
-    ATRACE_INT("ActiveConfigFPS", refreshRate.getFps().getValue());
+    ATRACE_INT("ActiveConfigFPS", refreshRate.getValue());
 
     if (mUpcomingActiveConfig.event != Scheduler::ConfigEvent::None) {
-        const nsecs_t vsyncPeriod =
-                mRefreshRateConfigs->getRefreshRateFromConfigId(mUpcomingActiveConfig.configId)
-                        .getVsyncPeriod();
+        const nsecs_t vsyncPeriod = refreshRate.getPeriodNsecs();
         const auto physicalId = display->getPhysicalId();
         mScheduler->onPrimaryDisplayConfigChanged(mAppConnectionHandle, physicalId,
                                                   mUpcomingActiveConfig.configId, vsyncPeriod);
     }
 }
 
-void SurfaceFlinger::desiredActiveConfigChangeDone() {
+void SurfaceFlinger::clearDesiredActiveConfigState() {
     std::lock_guard<std::mutex> lock(mActiveConfigLock);
     mDesiredActiveConfig.event = Scheduler::ConfigEvent::None;
     mDesiredActiveConfigChanged = false;
-
-    const auto& refreshRate =
-            mRefreshRateConfigs->getRefreshRateFromConfigId(mDesiredActiveConfig.configId);
-
-    mScheduler->resyncToHardwareVsync(true, refreshRate.getVsyncPeriod());
-    updatePhaseConfiguration(refreshRate);
     mScheduler->setConfigChangePending(false);
 }
 
+void SurfaceFlinger::desiredActiveConfigChangeDone() {
+    const auto modeId = getDesiredActiveConfig()->configId;
+
+    clearDesiredActiveConfigState();
+
+    const auto& refreshRate = getDefaultDisplayDeviceLocked()->getMode(modeId)->getFps();
+    mScheduler->resyncToHardwareVsync(true, refreshRate.getPeriodNsecs());
+    updatePhaseConfiguration(refreshRate);
+}
+
 void SurfaceFlinger::performSetActiveConfig() {
     ATRACE_CALL();
-    ALOGV("performSetActiveConfig");
+    ALOGV("%s", __FUNCTION__);
     // Store the local variable to release the lock.
     const auto desiredActiveConfig = getDesiredActiveConfig();
     if (!desiredActiveConfig) {
@@ -1130,12 +1145,19 @@
         return;
     }
 
-    auto refreshRate =
-            mRefreshRateConfigs->getRefreshRateFromConfigId(desiredActiveConfig->configId);
-    ALOGV("performSetActiveConfig changing active config to %zu(%s)",
-          refreshRate.getConfigId().value(), refreshRate.getName().c_str());
     const auto display = getDefaultDisplayDeviceLocked();
-    if (!display || display->getActiveMode() == desiredActiveConfig->configId) {
+    const auto desiredConfig = display->getMode(desiredActiveConfig->configId);
+    if (!desiredConfig) {
+        ALOGW("Desired display config is no longer supported. Config ID = %zu",
+              desiredActiveConfig->configId.value());
+        clearDesiredActiveConfigState();
+        return;
+    }
+    const auto refreshRate = desiredConfig->getFps();
+    ALOGV("performSetActiveConfig changing active config to %zu(%s)",
+          desiredConfig->getId().value(), to_string(refreshRate).c_str());
+
+    if (!display || display->getActiveMode()->getId() == desiredActiveConfig->configId) {
         // display is not valid or we are already in the requested mode
         // on both cases there is nothing left to do
         desiredActiveConfigChangeDone();
@@ -1143,7 +1165,7 @@
     }
 
     // Desired active config was set, it is different than the config currently in use, however
-    // allowed configs might have change by the time we process the refresh.
+    // allowed configs might have changed by the time we process the refresh.
     // Make sure the desired config is still allowed
     if (!isDisplayConfigAllowed(desiredActiveConfig->configId)) {
         desiredActiveConfigChangeDone();
@@ -1153,7 +1175,7 @@
     mUpcomingActiveConfig = *desiredActiveConfig;
     const auto displayId = display->getPhysicalId();
 
-    ATRACE_INT("ActiveConfigFPS_HWC", refreshRate.getFps().getValue());
+    ATRACE_INT("ActiveConfigFPS_HWC", refreshRate.getValue());
 
     // TODO(b/142753666) use constrains
     hal::VsyncPeriodChangeConstraints constraints;
@@ -1161,7 +1183,7 @@
     constraints.seamlessRequired = false;
 
     hal::VsyncPeriodChangeTimeline outTimeline;
-    auto status =
+    const auto status =
             getHwComposer().setActiveModeWithConstraints(displayId, mUpcomingActiveConfig.configId,
                                                          constraints, &outTimeline);
     if (status != NO_ERROR) {
@@ -2422,6 +2444,7 @@
 
     if (const auto id = PhysicalDisplayId::tryCast(compositionDisplay->getId())) {
         creationArgs.isPrimary = id == getInternalDisplayIdLocked();
+        creationArgs.supportedModes = getHwComposer().getModes(*id);
 
         if (useColorManagement) {
             std::vector<ColorMode> modes = getHwComposer().getColorModes(*id);
@@ -2629,7 +2652,7 @@
                 // TODO(b/175678215) Handle the case when currentConfig is not in configs
                 mRefreshRateConfigs->updateDisplayConfigs(configs, currentConfig);
                 mVsyncConfiguration->reset();
-                updatePhaseConfiguration(mRefreshRateConfigs->getCurrentRefreshRate());
+                updatePhaseConfiguration(mRefreshRateConfigs->getCurrentRefreshRate().getFps());
                 if (mRefreshRateOverlay) {
                     mRefreshRateOverlay->reset();
                 }
@@ -2968,10 +2991,10 @@
             getHwComposer().hasCapability(hal::Capability::PRESENT_FENCE_IS_NOT_RELIABLE));
 }
 
-void SurfaceFlinger::updatePhaseConfiguration(const RefreshRate& refreshRate) {
-    mVsyncConfiguration->setRefreshRateFps(refreshRate.getFps());
+void SurfaceFlinger::updatePhaseConfiguration(const Fps& refreshRate) {
+    mVsyncConfiguration->setRefreshRateFps(refreshRate);
     setVsyncConfig(mVsyncModulator->setVsyncConfigSet(mVsyncConfiguration->getCurrentConfigs()),
-                   refreshRate.getVsyncPeriod());
+                   refreshRate.getPeriodNsecs());
 }
 
 void SurfaceFlinger::setVsyncConfig(const VsyncModulator::VsyncConfig& config,
@@ -5324,22 +5347,22 @@
                 return NO_ERROR;
             }
             case 1035: {
-                const int newConfigId = data.readInt32();
+                const int modeId = data.readInt32();
                 mDebugDisplayConfigSetByBackdoor = false;
+
                 const auto displayId = getInternalDisplayId();
                 if (!displayId) {
                     ALOGE("No internal display found.");
                     return NO_ERROR;
                 }
-                const auto numConfigs = getHwComposer().getModes(*displayId).size();
-                if (newConfigId >= 0 && newConfigId < numConfigs) {
-                    const auto displayToken = getInternalDisplayToken();
-                    status_t result = setActiveConfig(displayToken, newConfigId);
-                    if (result != NO_ERROR) {
-                        return result;
-                    }
-                    mDebugDisplayConfigSetByBackdoor = true;
+
+                status_t result = setActiveConfig(getPhysicalDisplayToken(*displayId), modeId);
+                if (result != NO_ERROR) {
+                    return result;
                 }
+
+                mDebugDisplayConfigSetByBackdoor = true;
+
                 return NO_ERROR;
             }
             case 1036: {
@@ -6040,9 +6063,7 @@
         }
 
         display->setActiveMode(policy->defaultConfig);
-        const nsecs_t vsyncPeriod = getHwComposer()
-                                            .getModes(displayId)[policy->defaultConfig.value()]
-                                            ->getVsyncPeriod();
+        const nsecs_t vsyncPeriod = display->getMode(policy->defaultConfig)->getVsyncPeriod();
         mScheduler->onNonPrimaryDisplayConfigChanged(mAppConnectionHandle, displayId,
                                                      policy->defaultConfig, vsyncPeriod);
         return NO_ERROR;
@@ -6068,12 +6089,11 @@
 
     // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might
     // be depending in this callback.
-    const nsecs_t vsyncPeriod =
-            mRefreshRateConfigs->getRefreshRateFromConfigId(display->getActiveMode())
-                    .getVsyncPeriod();
+    const auto activeConfig = display->getActiveMode();
+    const nsecs_t vsyncPeriod = activeConfig->getVsyncPeriod();
     const auto physicalId = display->getPhysicalId();
     mScheduler->onPrimaryDisplayConfigChanged(mAppConnectionHandle, physicalId,
-                                              display->getActiveMode(), vsyncPeriod);
+                                              activeConfig->getId(), vsyncPeriod);
     toggleKernelIdleTimer();
 
     auto configId = mScheduler->getPreferredConfigId();
@@ -6161,8 +6181,7 @@
     } else if (display->isVirtual()) {
         return INVALID_OPERATION;
     } else {
-        const auto displayId = display->getPhysicalId();
-        const auto activeMode = getHwComposer().getActiveMode(displayId);
+        const auto activeMode = display->getActiveMode();
         *outDefaultConfig = activeMode->getId().value();
         *outAllowGroupSwitching = false;
         auto vsyncPeriod = activeMode->getVsyncPeriod();
@@ -6382,10 +6401,8 @@
 
             if (const auto display = getDefaultDisplayDeviceLocked()) {
                 mRefreshRateOverlay->setViewport(display->getSize());
+                mRefreshRateOverlay->changeRefreshRate(display->getActiveMode()->getFps());
             }
-
-            mRefreshRateOverlay->changeRefreshRate(
-                    mRefreshRateConfigs->getCurrentRefreshRate().getFps());
         }
     }));
 }
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c90fb4a..6c91fbd 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -54,6 +54,7 @@
 #include "DisplayHardware/PowerAdvisor.h"
 #include "DisplayIdGenerator.h"
 #include "Effects/Daltonizer.h"
+#include "Fps.h"
 #include "FrameTracker.h"
 #include "LayerVector.h"
 #include "Scheduler/RefreshRateConfigs.h"
@@ -667,7 +668,7 @@
     void signalLayerUpdate();
     void signalRefresh();
 
-    // called on the main thread in response to initializeDisplays()
+    // Called on the main thread in response to initializeDisplays()
     void onInitializeDisplays() REQUIRES(mStateLock);
     // Sets the desired active config bit. It obtains the lock, and sets mDesiredActiveConfig.
     void setDesiredActiveConfig(const ActiveConfigInfo& info) REQUIRES(mStateLock);
@@ -678,9 +679,10 @@
     // Calls to setActiveConfig on the main thread if there is a pending config
     // that needs to be applied.
     void performSetActiveConfig() REQUIRES(mStateLock);
+    void clearDesiredActiveConfigState() REQUIRES(mStateLock) EXCLUDES(mActiveConfigLock);
     // Called when active config is no longer is progress
     void desiredActiveConfigChangeDone() REQUIRES(mStateLock);
-    // called on the main thread in response to setPowerMode()
+    // Called on the main thread in response to setPowerMode()
     void setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode)
             REQUIRES(mStateLock);
 
@@ -712,8 +714,8 @@
     void commitInputWindowCommands() REQUIRES(mStateLock);
     void updateCursorAsync();
 
-    void initScheduler(PhysicalDisplayId primaryDisplayId);
-    void updatePhaseConfiguration(const RefreshRate&);
+    void initScheduler(PhysicalDisplayId primaryDisplayId) REQUIRES(mStateLock);
+    void updatePhaseConfiguration(const Fps&) REQUIRES(mStateLock);
     void setVsyncConfig(const VsyncModulator::VsyncConfig&, nsecs_t vsyncPeriod);
 
     /* handlePageFlip - latch a new buffer if available and compute the dirty
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index ca7c755..9a95ab4 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -262,12 +262,14 @@
     EXPECT_EQ(Case::HdrSupport::HDR10_SUPPORTED, device->hasHDR10Support());
     EXPECT_EQ(Case::HdrSupport::HDR_HLG_SUPPORTED, device->hasHLGSupport());
     EXPECT_EQ(Case::HdrSupport::HDR_DOLBY_VISION_SUPPORTED, device->hasDolbyVisionSupport());
-    // Note: This is not Case::Display::HWC_ACTIVE_CONFIG_ID as the ids are
-    // remapped, and the test only ever sets up one config. If there were an error
-    // looking up the remapped index, device->getActiveMode() would be -1 instead.
-    EXPECT_EQ(0, device->getActiveMode().value());
     EXPECT_EQ(Case::PerFrameMetadataSupport::PER_FRAME_METADATA_KEYS,
               device->getSupportedPerFrameMetadata());
+
+    if constexpr (Case::Display::CONNECTION_TYPE::value) {
+        EXPECT_EQ(1, device->getSupportedModes().size());
+        EXPECT_NE(nullptr, device->getActiveMode());
+        EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode()->getHwcId());
+    }
 }
 
 TEST_F(SetupNewDisplayDeviceInternalTest, createSimplePrimaryDisplay) {