Revert "SF: Set an initial mode in response to hotplug for external displays"
This reverts commit fb078ab329818c116104f61e7c67c3b2ae9a8152.
Reason for revert: b/320901698
Bug: 320901698
Change-Id: Ia714380dade4f2eeb5d369713405f33d548b6e40
diff --git a/services/surfaceflinger/Display/DisplayModeRequest.h b/services/surfaceflinger/Display/DisplayModeRequest.h
index c0e77bb..d07cdf5 100644
--- a/services/surfaceflinger/Display/DisplayModeRequest.h
+++ b/services/surfaceflinger/Display/DisplayModeRequest.h
@@ -27,9 +27,6 @@
// Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE.
bool emitEvent = false;
-
- // Whether to force the request to be applied, even if the mode is unchanged.
- bool force = false;
};
inline bool operator==(const DisplayModeRequest& lhs, const DisplayModeRequest& rhs) {
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index b96264b..799d62c 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -24,7 +24,6 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-#include <common/FlagManager.h>
#include <compositionengine/CompositionEngine.h>
#include <compositionengine/Display.h>
#include <compositionengine/DisplayColorProfile.h>
@@ -222,17 +221,6 @@
bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode,
const hal::VsyncPeriodChangeConstraints& constraints,
hal::VsyncPeriodChangeTimeline& outTimeline) {
- // TODO(b/255635711): Flow the DisplayModeRequest through the desired/pending/active states. For
- // now, `desiredMode` and `mDesiredModeOpt` are one and the same, but the latter is not cleared
- // until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been consumed
- // at this point, so clear the `force` flag to prevent an endless loop of `initiateModeChange`.
- if (FlagManager::getInstance().connected_display()) {
- std::scoped_lock lock(mDesiredModeLock);
- if (mDesiredModeOpt) {
- mDesiredModeOpt->force = false;
- }
- }
-
mPendingModeOpt = std::move(desiredMode);
mIsModeSetPending = true;
@@ -538,7 +526,8 @@
}
}
-auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode) -> DesiredModeAction {
+auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode, bool force)
+ -> DesiredModeAction {
ATRACE_CALL();
const auto& desiredModePtr = desiredMode.mode.modePtr;
@@ -546,26 +535,20 @@
LOG_ALWAYS_FATAL_IF(getPhysicalId() != desiredModePtr->getPhysicalDisplayId(),
"DisplayId mismatch");
- // TODO (b/318533819): Stringize DisplayModeRequest.
- ALOGD("%s(%s, force=%s)", __func__, to_string(*desiredModePtr).c_str(),
- desiredMode.force ? "true" : "false");
+ ALOGV("%s(%s)", __func__, to_string(*desiredModePtr).c_str());
std::scoped_lock lock(mDesiredModeLock);
if (mDesiredModeOpt) {
// A mode transition was already scheduled, so just override the desired mode.
const bool emitEvent = mDesiredModeOpt->emitEvent;
- const bool force = mDesiredModeOpt->force;
mDesiredModeOpt = std::move(desiredMode);
mDesiredModeOpt->emitEvent |= emitEvent;
- if (FlagManager::getInstance().connected_display()) {
- mDesiredModeOpt->force |= force;
- }
return DesiredModeAction::None;
}
// If the desired mode is already active...
const auto activeMode = refreshRateSelector().getActiveMode();
- if (!desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) {
+ if (!force && activeMode.modePtr->getId() == desiredModePtr->getId()) {
if (activeMode == desiredMode.mode) {
return DesiredModeAction::None;
}
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 46534de..97b56a2 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -189,7 +189,8 @@
enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch };
- DesiredModeAction setDesiredMode(display::DisplayModeRequest&&) EXCLUDES(mDesiredModeLock);
+ DesiredModeAction setDesiredMode(display::DisplayModeRequest&&, bool force = false)
+ EXCLUDES(mDesiredModeLock);
using DisplayModeRequestOpt = ftl::Optional<display::DisplayModeRequest>;
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index db66f5b..704ece5 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -27,7 +27,6 @@
#include "HWC2.h"
#include <android/configuration.h>
-#include <common/FlagManager.h>
#include <ui/Fence.h>
#include <ui/FloatRect.h>
#include <ui/GraphicBuffer.h>
@@ -417,19 +416,7 @@
VsyncPeriodChangeTimeline* outTimeline) {
ALOGV("[%" PRIu64 "] setActiveConfigWithConstraints", mId);
- // FIXME (b/319505580): At least the first config set on an external display must be
- // `setActiveConfig`, so skip over the block that calls `setActiveConfigWithConstraints`
- // for simplicity.
- ui::DisplayConnectionType type = ui::DisplayConnectionType::Internal;
- const bool connected_display = FlagManager::getInstance().connected_display();
- if (connected_display) {
- if (auto err = getConnectionType(&type); err != Error::NONE) {
- return err;
- }
- }
-
- if (isVsyncPeriodSwitchSupported() &&
- (!connected_display || type != ui::DisplayConnectionType::External)) {
+ if (isVsyncPeriodSwitchSupported()) {
Hwc2::IComposerClient::VsyncPeriodChangeConstraints hwc2Constraints;
hwc2Constraints.desiredTimeNanos = constraints.desiredTimeNanos;
hwc2Constraints.seamlessRequired = constraints.seamlessRequired;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fe5b159..6910a57 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1227,10 +1227,8 @@
return NO_ERROR;
}
-void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
- const auto mode = desiredMode.mode;
- const auto displayId = mode.modePtr->getPhysicalDisplayId();
-
+void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool force) {
+ const auto displayId = request.mode.modePtr->getPhysicalDisplayId();
ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
const auto display = getDisplayDeviceLocked(displayId);
@@ -1239,9 +1237,10 @@
return;
}
- const bool emitEvent = desiredMode.emitEvent;
+ const auto mode = request.mode;
+ const bool emitEvent = request.emitEvent;
- switch (display->setDesiredMode(std::move(desiredMode))) {
+ switch (display->setDesiredMode(std::move(request), force)) {
case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch:
// DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler.
mScheduler->setRenderRate(displayId,
@@ -1427,8 +1426,7 @@
to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
to_string(display->getId()).c_str());
- if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) &&
- display->getActiveMode() == desiredModeOpt->mode) {
+ if (display->getActiveMode() == desiredModeOpt->mode) {
applyActiveMode(display);
continue;
}
@@ -3294,88 +3292,14 @@
std::vector<HWComposer::HWCDisplayMode> hwcModes;
std::optional<hal::HWDisplayId> activeModeHwcId;
- const bool isExternalDisplay = FlagManager::getInstance().connected_display() &&
- getHwComposer().getDisplayConnectionType(displayId) ==
- ui::DisplayConnectionType::External;
-
int attempt = 0;
constexpr int kMaxAttempts = 3;
do {
hwcModes = getHwComposer().getModes(displayId,
scheduler::RefreshRateSelector::kMinSupportedFrameRate
.getPeriodNsecs());
-
activeModeHwcId = getHwComposer().getActiveMode(displayId);
- if (isExternalDisplay) {
- constexpr nsecs_t k59HzVsyncPeriod = 16949153;
- constexpr nsecs_t k60HzVsyncPeriod = 16666667;
-
- // DM sets the initial mode for an external display to 1080p@60, but
- // this comes after SF creates its own state (including the
- // DisplayDevice). For now, pick the same mode in order to avoid
- // inconsistent state and unnecessary mode switching.
- // TODO (b/318534874): Let DM decide the initial mode.
- //
- // Try to find 1920x1080 @ 60 Hz
- if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(),
- [](const auto& mode) {
- return mode.width == 1920 &&
- mode.height == 1080 &&
- mode.vsyncPeriod == k60HzVsyncPeriod;
- });
- iter != hwcModes.end()) {
- activeModeHwcId = iter->hwcId;
- break;
- }
-
- // Try to find 1920x1080 @ 59-60 Hz
- if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(),
- [](const auto& mode) {
- return mode.width == 1920 &&
- mode.height == 1080 &&
- mode.vsyncPeriod >= k60HzVsyncPeriod &&
- mode.vsyncPeriod <= k59HzVsyncPeriod;
- });
- iter != hwcModes.end()) {
- activeModeHwcId = iter->hwcId;
- break;
- }
-
- // The display does not support 1080p@60, and this is the last attempt to pick a display
- // mode. Prefer 60 Hz if available, with the closest resolution to 1080p.
- if (attempt + 1 == kMaxAttempts) {
- std::vector<HWComposer::HWCDisplayMode> hwcModeOpts;
-
- for (const auto& mode : hwcModes) {
- if (mode.width <= 1920 && mode.height <= 1080 &&
- mode.vsyncPeriod >= k60HzVsyncPeriod &&
- mode.vsyncPeriod <= k59HzVsyncPeriod) {
- hwcModeOpts.push_back(mode);
- }
- }
-
- if (const auto iter = std::max_element(hwcModeOpts.begin(), hwcModeOpts.end(),
- [](const auto& a, const auto& b) {
- const auto aSize = a.width * a.height;
- const auto bSize = b.width * b.height;
- if (aSize < bSize)
- return true;
- else if (aSize == bSize)
- return a.vsyncPeriod > b.vsyncPeriod;
- else
- return false;
- });
- iter != hwcModeOpts.end()) {
- activeModeHwcId = iter->hwcId;
- break;
- }
-
- // hwcModeOpts was empty, use hwcModes[0] as the last resort
- activeModeHwcId = hwcModes[0].hwcId;
- }
- }
-
const auto isActiveMode = [activeModeHwcId](const HWComposer::HWCDisplayMode& mode) {
return mode.hwcId == activeModeHwcId;
};
@@ -3435,10 +3359,6 @@
return pair.second->getHwcId() == activeModeHwcId;
})->second;
- if (isExternalDisplay) {
- ALOGI("External display %s initial mode: {%s}", to_string(displayId).c_str(),
- to_string(*activeMode).c_str());
- }
return {modes, activeMode};
}
@@ -3745,27 +3665,6 @@
}
mDisplays.try_emplace(displayToken, std::move(display));
-
- // For an external display, loadDisplayModes already selected the same mode
- // as DM, but SF still needs to be updated to match.
- // TODO (b/318534874): Let DM decide the initial mode.
- if (const auto& physical = state.physical;
- mScheduler && physical && FlagManager::getInstance().connected_display()) {
- const bool isInternalDisplay = mPhysicalDisplays.get(physical->id)
- .transform(&PhysicalDisplay::isInternal)
- .value_or(false);
-
- if (!isInternalDisplay) {
- auto activeModePtr = physical->activeMode;
- const auto fps = activeModePtr->getPeakFps();
-
- setDesiredMode(
- {.mode = scheduler::FrameRateMode{fps,
- ftl::as_non_null(std::move(activeModePtr))},
- .emitEvent = false,
- .force = true});
- }
- }
}
void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) {
@@ -8434,7 +8333,7 @@
return INVALID_OPERATION;
}
- setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force});
+ setDesiredMode({std::move(preferredMode), .emitEvent = true}, force);
// Update the frameRateOverride list as the display render rate might have changed
if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6ae05d6..6e12172 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -718,7 +718,7 @@
// Show hdr sdr ratio overlay
bool mHdrSdrRatioOverlay = false;
- void setDesiredMode(display::DisplayModeRequest&&) REQUIRES(mStateLock);
+ void setDesiredMode(display::DisplayModeRequest&&, bool force = false) REQUIRES(mStateLock);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
Fps maxFps);
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 99fef7e..6671414 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -447,11 +447,9 @@
? IComposerClient::DisplayConnectionType::INTERNAL
: IComposerClient::DisplayConnectionType::EXTERNAL;
- using ::testing::AtLeast;
EXPECT_CALL(*test->mComposer, getDisplayConnectionType(HWC_DISPLAY_ID, _))
- .Times(AtLeast(1))
- .WillRepeatedly(DoAll(SetArgPointee<1>(CONNECTION_TYPE),
- Return(hal::V2_4::Error::NONE)));
+ .WillOnce(DoAll(SetArgPointee<1>(CONNECTION_TYPE),
+ Return(hal::V2_4::Error::NONE)));
}
EXPECT_CALL(*test->mComposer, setClientTargetSlotCount(_))
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 82b4ad0..8b16a8a 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -21,13 +21,9 @@
#include "mock/DisplayHardware/MockDisplayMode.h"
#include "mock/MockDisplayModeSpecs.h"
-#include <com_android_graphics_surfaceflinger_flags.h>
-#include <common/test/FlagUtils.h>
#include <ftl/fake_guard.h>
#include <scheduler/Fps.h>
-using namespace com::android::graphics::surfaceflinger;
-
namespace android {
namespace {
@@ -364,13 +360,6 @@
}
TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
-
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -440,12 +429,6 @@
}
TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -529,13 +512,6 @@
}
TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
-
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());