summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDominik Laskowski <domlaskowski@google.com>2022-11-23 15:45:17 -0500
committerDominik Laskowski <domlaskowski@google.com>2022-12-09 18:36:48 -0500
commitaf148a2c2a0273c601bb1184e7af94a2e558540c (patch)
tree9982f09f1beebbb4525e249ce15aa64d63861d1e
parente63a592135f918583489ab1dbb6861f8f6c93956 (diff)
downloadnative-af148a2c2a0273c601bb1184e7af94a2e558540c.tar.gz
SF: Obey active display's RefreshRateConfigs
When a display becomes active, apply its RefreshRateConfigs' policy, as it may have changed while the display was inactive. When booting while folded, DisplayManager first sends DisplayModeSpecs for each display, and then powers on the outer display. Before this CL, the outer display would become the new active display, but its DisplayManagerPolicy would never be applied. This backport also includes I63d21c2216a3ab864b040bdb2c9e8fba0768b66a for expanded unit tests. Bug: 250421145 Test: Force 120 Hz, and reboot while folded. Test: SetPowerModeInternalTest.activeDisplay{Boot,Single,Dual} Change-Id: I15e0f5a280e62baf6d4e6ea2748d95342e79ac44 Merged-In: I15e0f5a280e62baf6d4e6ea2748d95342e79ac44
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp15
-rw-r--r--services/surfaceflinger/SurfaceFlinger.h2
-rw-r--r--services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp45
-rw-r--r--services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h7
-rw-r--r--services/surfaceflinger/tests/unittests/FakeDisplayInjector.h96
-rw-r--r--services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp85
-rw-r--r--services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h6
7 files changed, 204 insertions, 52 deletions
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a701fe3475..18807346fa 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -6942,9 +6942,12 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal(
return NO_ERROR;
}
- scheduler::RefreshRateConfigs::Policy currentPolicy =
- display->refreshRateConfigs().getCurrentPolicy();
+ return applyRefreshRateConfigsPolicy(display);
+}
+status_t SurfaceFlinger::applyRefreshRateConfigsPolicy(const sp<DisplayDevice>& display) {
+ const scheduler::RefreshRateConfigs::Policy currentPolicy =
+ display->refreshRateConfigs().getCurrentPolicy();
ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str());
// TODO(b/140204874): Leave the event in until we do proper testing with all apps that might
@@ -7317,9 +7320,11 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const sp<DisplayDevice>& activ
onActiveDisplaySizeChanged(activeDisplay);
mActiveDisplayTransformHint = activeDisplay->getTransformHint();
- // Update the kernel timer for the current active display, since the policy
- // for this display might have changed when it was not the active display.
- toggleKernelIdleTimer();
+ // The policy of the new active/leader 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.
+ applyRefreshRateConfigsPolicy(activeDisplay);
}
status_t SurfaceFlinger::addWindowInfosListener(
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 9e0cee8fd4..64c0ae9d0c 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -735,6 +735,8 @@ private:
const std::optional<scheduler::RefreshRateConfigs::Policy>& policy, bool overridePolicy)
EXCLUDES(mStateLock);
+ status_t applyRefreshRateConfigsPolicy(const sp<DisplayDevice>&) REQUIRES(mStateLock);
+
void commitTransactions() EXCLUDES(mStateLock);
void commitTransactionsLocked(uint32_t transactionFlags) REQUIRES(mStateLock);
void doCommitTransactions() REQUIRES(mStateLock);
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index f04221cc21..5ddd1c86d5 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -120,51 +120,6 @@ void DisplayTransactionTest::injectFakeNativeWindowSurfaceFactory() {
});
}
-sp<DisplayDevice> DisplayTransactionTest::injectDefaultInternalDisplay(
- std::function<void(FakeDisplayDeviceInjector&)> injectExtra) {
- constexpr PhysicalDisplayId DEFAULT_DISPLAY_ID = PhysicalDisplayId::fromPort(255u);
- constexpr int DEFAULT_DISPLAY_WIDTH = 1080;
- constexpr int DEFAULT_DISPLAY_HEIGHT = 1920;
- constexpr HWDisplayId DEFAULT_DISPLAY_HWC_DISPLAY_ID = 0;
-
- // The DisplayDevice is required to have a framebuffer (behind the
- // ANativeWindow interface) which uses the actual hardware display
- // size.
- EXPECT_CALL(*mNativeWindow, query(NATIVE_WINDOW_WIDTH, _))
- .WillRepeatedly(DoAll(SetArgPointee<1>(DEFAULT_DISPLAY_WIDTH), Return(0)));
- EXPECT_CALL(*mNativeWindow, query(NATIVE_WINDOW_HEIGHT, _))
- .WillRepeatedly(DoAll(SetArgPointee<1>(DEFAULT_DISPLAY_HEIGHT), Return(0)));
- EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_SET_BUFFERS_FORMAT));
- EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_API_CONNECT));
- EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_SET_USAGE64));
- EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_API_DISCONNECT)).Times(AnyNumber());
-
- auto compositionDisplay =
- compositionengine::impl::createDisplay(mFlinger.getCompositionEngine(),
- compositionengine::DisplayCreationArgsBuilder()
- .setId(DEFAULT_DISPLAY_ID)
- .setPixels({DEFAULT_DISPLAY_WIDTH,
- DEFAULT_DISPLAY_HEIGHT})
- .setPowerAdvisor(&mPowerAdvisor)
- .build());
-
- constexpr bool kIsPrimary = true;
- auto injector = FakeDisplayDeviceInjector(mFlinger, compositionDisplay,
- ui::DisplayConnectionType::Internal,
- DEFAULT_DISPLAY_HWC_DISPLAY_ID, kIsPrimary);
-
- injector.setNativeWindow(mNativeWindow);
- if (injectExtra) {
- injectExtra(injector);
- }
-
- auto displayDevice = injector.inject();
-
- Mock::VerifyAndClear(mNativeWindow.get());
-
- return displayDevice;
-}
-
bool DisplayTransactionTest::hasPhysicalHwcDisplay(HWDisplayId hwcDisplayId) const {
return mFlinger.hwcPhysicalDisplayIdMap().count(hwcDisplayId) == 1;
}
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index f5235ce953..60f773fcb0 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -42,6 +42,7 @@
#include <renderengine/mock/RenderEngine.h>
#include <ui/DebugUtils.h>
+#include "FakeDisplayInjector.h"
#include "TestableScheduler.h"
#include "TestableSurfaceFlinger.h"
#include "mock/DisplayHardware/MockComposer.h"
@@ -90,7 +91,9 @@ public:
void injectFakeBufferQueueFactory();
void injectFakeNativeWindowSurfaceFactory();
sp<DisplayDevice> injectDefaultInternalDisplay(
- std::function<void(TestableSurfaceFlinger::FakeDisplayDeviceInjector&)>);
+ std::function<void(TestableSurfaceFlinger::FakeDisplayDeviceInjector&)> injectExtra) {
+ return mFakeDisplayInjector.injectInternalDisplay(injectExtra);
+ }
// --------------------------------------------------------------------
// Postcondition helpers
@@ -115,6 +118,8 @@ public:
sp<GraphicBuffer> mBuffer = new GraphicBuffer();
Hwc2::mock::PowerAdvisor mPowerAdvisor;
+ FakeDisplayInjector mFakeDisplayInjector{mFlinger, mPowerAdvisor, mNativeWindow};
+
// These mocks are created by the test, but are destroyed by SurfaceFlinger
// by virtue of being stored into a std::unique_ptr. However we still need
// to keep a reference to them for use in setting up call expectations.
diff --git a/services/surfaceflinger/tests/unittests/FakeDisplayInjector.h b/services/surfaceflinger/tests/unittests/FakeDisplayInjector.h
new file mode 100644
index 0000000000..6e4bf2b06e
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/FakeDisplayInjector.h
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <gmock/gmock.h>
+
+#include "TestableSurfaceFlinger.h"
+#include "mock/DisplayHardware/MockPowerAdvisor.h"
+#include "mock/system/window/MockNativeWindow.h"
+
+namespace android {
+
+using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector;
+using android::hardware::graphics::composer::hal::HWDisplayId;
+using android::Hwc2::mock::PowerAdvisor;
+
+struct FakeDisplayInjectorArgs {
+ PhysicalDisplayId displayId = PhysicalDisplayId::fromPort(255u);
+ HWDisplayId hwcDisplayId = 0;
+ bool isPrimary = true;
+};
+
+class FakeDisplayInjector {
+public:
+ FakeDisplayInjector(TestableSurfaceFlinger& flinger, Hwc2::mock::PowerAdvisor& powerAdvisor,
+ sp<mock::NativeWindow> nativeWindow)
+ : mFlinger(flinger), mPowerAdvisor(powerAdvisor), mNativeWindow(nativeWindow) {}
+
+ sp<DisplayDevice> injectInternalDisplay(
+ const std::function<void(FakeDisplayDeviceInjector&)>& injectExtra,
+ FakeDisplayInjectorArgs args = {}) {
+ using testing::_;
+ using testing::AnyNumber;
+ using testing::DoAll;
+ using testing::Mock;
+ using testing::Return;
+ using testing::SetArgPointee;
+
+ constexpr ui::Size kResolution = {1080, 1920};
+
+ // The DisplayDevice is required to have a framebuffer (behind the
+ // ANativeWindow interface) which uses the actual hardware display
+ // size.
+ EXPECT_CALL(*mNativeWindow, query(NATIVE_WINDOW_WIDTH, _))
+ .WillRepeatedly(DoAll(SetArgPointee<1>(kResolution.getWidth()), Return(0)));
+ EXPECT_CALL(*mNativeWindow, query(NATIVE_WINDOW_HEIGHT, _))
+ .WillRepeatedly(DoAll(SetArgPointee<1>(kResolution.getHeight()), Return(0)));
+ EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_SET_BUFFERS_FORMAT));
+ EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_API_CONNECT));
+ EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_SET_USAGE64));
+ EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_API_DISCONNECT)).Times(AnyNumber());
+
+ auto compositionDisplay = compositionengine::impl::
+ createDisplay(mFlinger.getCompositionEngine(),
+ compositionengine::DisplayCreationArgsBuilder()
+ .setId(args.displayId)
+ .setPixels(kResolution)
+ .setPowerAdvisor(&mPowerAdvisor)
+ .build());
+
+ auto injector = FakeDisplayDeviceInjector(mFlinger, compositionDisplay,
+ ui::DisplayConnectionType::Internal,
+ args.hwcDisplayId, args.isPrimary);
+
+ injector.setNativeWindow(mNativeWindow);
+ if (injectExtra) {
+ injectExtra(injector);
+ }
+
+ auto displayDevice = injector.inject();
+
+ Mock::VerifyAndClear(mNativeWindow.get());
+
+ return displayDevice;
+ }
+
+ TestableSurfaceFlinger& mFlinger;
+ Hwc2::mock::PowerAdvisor& mPowerAdvisor;
+ sp<mock::NativeWindow> mNativeWindow;
+};
+
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index 583cf5f836..b560025929 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -323,6 +323,9 @@ class SetPowerModeInternalTest : public DisplayTransactionTest {
public:
template <typename Case>
void transitionDisplayCommon();
+
+ template <bool kBoot>
+ sp<DisplayDevice> activeDisplayTest();
};
template <PowerMode PowerMode>
@@ -499,5 +502,87 @@ TEST_F(SetPowerModeInternalTest, transitionsDisplayFromOnToUnknownExternalDispla
transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOnToUnknownVariant>>();
}
+template <bool kBoot>
+sp<DisplayDevice> SetPowerModeInternalTest::activeDisplayTest() {
+ using Case = SimplePrimaryDisplayCase;
+
+ // --------------------------------------------------------------------
+ // Preconditions
+
+ // Inject a primary display.
+ Case::Display::injectHwcDisplay(this);
+ auto injector = Case::Display::makeFakeExistingDisplayInjector(this);
+ injector.setPowerMode(kBoot ? std::nullopt : std::make_optional(PowerMode::OFF));
+
+ const auto display = injector.inject();
+ EXPECT_EQ(display->getDisplayToken(), mFlinger.mutableActiveDisplayToken());
+
+ using PowerCase = PrimaryDisplayPowerCase<TransitionOffToOnVariant>;
+ TransitionOffToOnVariant::template setupCallExpectations<PowerCase>(this);
+
+ constexpr size_t kTimes = kBoot ? 1 : 0;
+ EXPECT_CALL(*mRenderEngine, onActiveDisplaySizeChanged(display->getSize())).Times(kTimes);
+ EXPECT_CALL(*mEventThread, onModeChanged(display->getActiveMode())).Times(kTimes);
+
+ if constexpr (kBoot) {
+ mFlinger.mutableActiveDisplayToken() = nullptr;
+ }
+
+ // --------------------------------------------------------------------
+ // Invocation
+
+ mFlinger.setPowerModeInternal(display, PowerMode::ON);
+
+ // --------------------------------------------------------------------
+ // Postconditions
+
+ // The primary display should be the active display.
+ EXPECT_EQ(display->getDisplayToken(), mFlinger.mutableActiveDisplayToken());
+
+ Mock::VerifyAndClearExpectations(mComposer);
+ Mock::VerifyAndClearExpectations(mRenderEngine);
+ Mock::VerifyAndClearExpectations(mEventThread);
+ Mock::VerifyAndClearExpectations(mVsyncController);
+ Mock::VerifyAndClearExpectations(mVSyncTracker);
+ Mock::VerifyAndClearExpectations(mFlinger.scheduler());
+ Mock::VerifyAndClearExpectations(&mFlinger.mockSchedulerCallback());
+
+ return display;
+}
+
+TEST_F(SetPowerModeInternalTest, activeDisplayBoot) {
+ constexpr bool kBoot = true;
+ activeDisplayTest<kBoot>();
+}
+
+TEST_F(SetPowerModeInternalTest, activeDisplaySingle) {
+ constexpr bool kBoot = false;
+ activeDisplayTest<kBoot>();
+}
+
+TEST_F(SetPowerModeInternalTest, activeDisplayDual) {
+ constexpr bool kBoot = false;
+ const auto innerDisplay = activeDisplayTest<kBoot>();
+
+ // Inject a powered-off outer display.
+ const auto outerDisplay = mFakeDisplayInjector.injectInternalDisplay(
+ [&](FakeDisplayDeviceInjector& injector) { injector.setPowerMode(PowerMode::OFF); },
+ {.displayId = PhysicalDisplayId::fromPort(254u),
+ .hwcDisplayId = 1,
+ .isPrimary = false});
+
+ EXPECT_EQ(innerDisplay->getDisplayToken(), mFlinger.mutableActiveDisplayToken());
+
+ mFlinger.setPowerModeInternal(innerDisplay, PowerMode::OFF);
+ mFlinger.setPowerModeInternal(outerDisplay, PowerMode::ON);
+
+ EXPECT_EQ(outerDisplay->getDisplayToken(), mFlinger.mutableActiveDisplayToken());
+
+ mFlinger.setPowerModeInternal(outerDisplay, PowerMode::OFF);
+ mFlinger.setPowerModeInternal(innerDisplay, PowerMode::ON);
+
+ EXPECT_EQ(innerDisplay->getDisplayToken(), mFlinger.mutableActiveDisplayToken());
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 283f9ca77b..3cffec1ccb 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -798,7 +798,7 @@ public:
return *this;
}
- auto& setPowerMode(hal::PowerMode mode) {
+ auto& setPowerMode(std::optional<hal::PowerMode> mode) {
mCreationArgs.initialPowerMode = mode;
return *this;
}
@@ -867,6 +867,10 @@ public:
.deviceProductInfo = {},
.supportedModes = modes,
.activeMode = activeMode->get()};
+
+ if (mCreationArgs.isPrimary) {
+ mFlinger.mutableActiveDisplayToken() = mDisplayToken;
+ }
}
state.isSecure = mCreationArgs.isSecure;