From 50c44062d3d59f82afec0e817ef045fff92c20e4 Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Mon, 17 Jan 2022 20:48:10 +0000
Subject: Change SF power hints to use early frame predictions

Currently we issue hints at the end of the frame reporting what happened
that frame. However, this is often too late anyway and lacks critical
information not known until after the frame completes, like gpu timing.
By waiting until the next frame start and using those timings along with
information about gpu duration, the new expected present time, and the
new wakeup time, we can instead use those timings to predict the
duration of the upcoming frame and signal in advance of a frame drop.
This patch also changes how and where frame timing is done and measures
gpu duration so we can predict if a cpu speedup can help gpu composition
happen earlier, preventing a drop that way.

Bug: b/195990840
Test: manual

Change-Id: I5f0ae796198631fced503dce8e6c495885fb256a
---
 services/surfaceflinger/SurfaceFlinger.cpp | 64 +++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 23 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e72e21ceb3..0cb8e0e5b4 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -444,6 +444,11 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI
     }
 
     mIgnoreHdrCameraLayers = ignore_hdr_camera_layers(false);
+
+    // Power hint session mode, representing which hint(s) to send: early, late, or both)
+    mPowerHintSessionMode =
+            {.late = base::GetBoolProperty("debug.sf.send_late_power_session_hint"s, true),
+             .early = base::GetBoolProperty("debug.sf.send_early_power_session_hint"s, true)};
 }
 
 LatchUnsignaledConfig SurfaceFlinger::getLatchUnsignaledConfig() {
@@ -1991,12 +1996,6 @@ nsecs_t SurfaceFlinger::calculateExpectedPresentTime(DisplayStatInfo stats) cons
 
 bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expectedVsyncTime)
         FTL_FAKE_GUARD(kMainThreadContext) {
-    // we set this once at the beginning of commit to ensure consistency throughout the whole frame
-    mPowerHintSessionData.sessionEnabled = mPowerAdvisor->usePowerHintSession();
-    if (mPowerHintSessionData.sessionEnabled) {
-        mPowerHintSessionData.commitStart = systemTime();
-    }
-
     // calculate the expected present time once and use the cached
     // value throughout this frame to make sure all layers are
     // seeing this same value.
@@ -2010,10 +2009,6 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected
     const nsecs_t lastScheduledPresentTime = mScheduledPresentTime;
     mScheduledPresentTime = expectedVsyncTime;
 
-    if (mPowerHintSessionData.sessionEnabled) {
-        mPowerAdvisor->setTargetWorkDuration(mExpectedPresentTime -
-                                             mPowerHintSessionData.commitStart);
-    }
     const auto vsyncIn = [&] {
         if (!ATRACE_ENABLED()) return 0.f;
         return (mExpectedPresentTime - systemTime()) / 1e6f;
@@ -2089,6 +2084,30 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected
         }
     }
 
+    // Save this once per commit + composite to ensure consistency
+    mPowerHintSessionEnabled = mPowerAdvisor->usePowerHintSession();
+    if (mPowerHintSessionEnabled) {
+        nsecs_t vsyncPeriod;
+        {
+            Mutex::Autolock lock(mStateLock);
+            vsyncPeriod = getVsyncPeriodFromHWC();
+        }
+        mPowerAdvisor->setCommitStart(frameTime);
+        mPowerAdvisor->setExpectedPresentTime(mExpectedPresentTime);
+        const nsecs_t idealSfWorkDuration =
+                mVsyncModulator->getVsyncConfig().sfWorkDuration.count();
+        // Frame delay is how long we should have minus how long we actually have
+        mPowerAdvisor->setFrameDelay(idealSfWorkDuration - (mExpectedPresentTime - frameTime));
+        mPowerAdvisor->setTotalFrameTargetWorkDuration(idealSfWorkDuration);
+        mPowerAdvisor->setTargetWorkDuration(vsyncPeriod);
+
+        // Send early hint here to make sure there's not another frame pending
+        if (mPowerHintSessionMode.early) {
+            // Send a rough prediction for this frame based on last frame's timing info
+            mPowerAdvisor->sendPredictedWorkDuration();
+        }
+    }
+
     if (mTracingEnabledChanged) {
         mLayerTracingEnabled = mLayerTracing.isEnabled();
         mTracingEnabledChanged = false;
@@ -2165,16 +2184,15 @@ void SurfaceFlinger::composite(nsecs_t frameTime, int64_t vsyncId)
         FTL_FAKE_GUARD(kMainThreadContext) {
     ATRACE_FORMAT("%s %" PRId64, __func__, vsyncId);
 
-    if (mPowerHintSessionData.sessionEnabled) {
-        mPowerHintSessionData.compositeStart = systemTime();
-    }
-
     compositionengine::CompositionRefreshArgs refreshArgs;
     const auto& displays = FTL_FAKE_GUARD(mStateLock, mDisplays);
     refreshArgs.outputs.reserve(displays.size());
+    std::vector<DisplayId> displayIds;
     for (const auto& [_, display] : displays) {
         refreshArgs.outputs.push_back(display->getCompositionDisplay());
+        displayIds.push_back(display->getId());
     }
+    mPowerAdvisor->setDisplays(displayIds);
     mDrawingState.traverseInZOrder([&refreshArgs](Layer* layer) {
         if (auto layerFE = layer->getCompositionEngineLayerFE())
             refreshArgs.layers.push_back(layerFE);
@@ -2222,12 +2240,15 @@ void SurfaceFlinger::composite(nsecs_t frameTime, int64_t vsyncId)
 
     mCompositionEngine->present(refreshArgs);
 
-    if (mPowerHintSessionData.sessionEnabled) {
-        mPowerHintSessionData.presentEnd = systemTime();
-    }
-
     mTimeStats->recordFrameDuration(frameTime, systemTime());
 
+    // Send a power hint hint after presentation is finished
+    if (mPowerHintSessionEnabled) {
+        if (mPowerHintSessionMode.late) {
+            mPowerAdvisor->sendActualWorkDuration();
+        }
+    }
+
     if (mScheduler->onPostComposition(presentTime)) {
         scheduleComposite(FrameHint::kNone);
     }
@@ -2272,11 +2293,8 @@ void SurfaceFlinger::composite(nsecs_t frameTime, int64_t vsyncId)
         scheduleCommit(FrameHint::kNone);
     }
 
-    // calculate total render time for performance hinting if adpf cpu hint is enabled,
-    if (mPowerHintSessionData.sessionEnabled) {
-        const nsecs_t flingerDuration =
-                (mPowerHintSessionData.presentEnd - mPowerHintSessionData.commitStart);
-        mPowerAdvisor->sendActualWorkDuration(flingerDuration, mPowerHintSessionData.presentEnd);
+    if (mPowerHintSessionEnabled) {
+        mPowerAdvisor->setCompositeEnd(systemTime());
     }
 }
 
-- 
cgit v1.2.3-59-g8ed1b


From 6a9731d3ed5c5bd1cce64db28e2bc6ada25ebc85 Mon Sep 17 00:00:00 2001
From: Rachel Lee <rnlee@google.com>
Date: Mon, 6 Jun 2022 17:08:14 -0700
Subject: HW Vsync turns off correctly in Doze/AOD.

Otherwise when entering AOD mode, SF keeps HW Vsync on indefinitely,
leading to power inefficiency.

Bug: 219109873
Test: atest libsurfaceflinger_test
Test: perfetto trace
Change-Id: Idf80d4c64abe78593809be9d1ba6b7dd42d29054
---
 services/surfaceflinger/Scheduler/Scheduler.cpp         |  8 +++++---
 services/surfaceflinger/Scheduler/Scheduler.h           |  4 ++--
 services/surfaceflinger/Scheduler/VSyncReactor.cpp      | 10 ++++++++++
 services/surfaceflinger/Scheduler/VSyncReactor.h        |  4 ++++
 services/surfaceflinger/Scheduler/VsyncController.h     | 10 ++++++++++
 services/surfaceflinger/SurfaceFlinger.cpp              |  2 +-
 .../surfaceflinger/tests/unittests/SchedulerTest.cpp    |  8 ++++----
 .../surfaceflinger/tests/unittests/VSyncReactorTest.cpp | 17 +++++++++++++++++
 .../tests/unittests/mock/MockVsyncController.h          |  1 +
 9 files changed, 54 insertions(+), 10 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 37f0fec425..08a1edea60 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -554,11 +554,12 @@ void Scheduler::onTouchHint() {
     }
 }
 
-void Scheduler::setDisplayPowerState(bool normal) {
+void Scheduler::setDisplayPowerMode(hal::PowerMode powerMode) {
     {
         std::lock_guard<std::mutex> lock(mPolicyLock);
-        mPolicy.isDisplayPowerStateNormal = normal;
+        mPolicy.displayPowerMode = powerMode;
     }
+    mVsyncSchedule->getController().setDisplayPowerMode(powerMode);
 
     if (mDisplayPowerTimer) {
         mDisplayPowerTimer->reset();
@@ -706,7 +707,8 @@ auto Scheduler::chooseDisplayMode() -> std::pair<DisplayModePtr, GlobalSignals>
     // If Display Power is not in normal operation we want to be in performance mode. When coming
     // back to normal mode, a grace period is given with DisplayPowerTimer.
     if (mDisplayPowerTimer &&
-        (!mPolicy.isDisplayPowerStateNormal || mPolicy.displayPowerTimer == TimerState::Reset)) {
+        (mPolicy.displayPowerMode != hal::PowerMode::ON ||
+         mPolicy.displayPowerTimer == TimerState::Reset)) {
         constexpr GlobalSignals kNoSignals;
         return {configs->getMaxRefreshRateByPolicy(), kNoSignals};
     }
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 0c72124119..a8043bf94c 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -186,7 +186,7 @@ public:
     // Indicates that touch interaction is taking place.
     void onTouchHint();
 
-    void setDisplayPowerState(bool normal);
+    void setDisplayPowerMode(hal::PowerMode powerMode);
 
     VSyncDispatch& getVsyncDispatch() { return mVsyncSchedule->getDispatch(); }
 
@@ -325,7 +325,7 @@ private:
         TimerState idleTimer = TimerState::Reset;
         TouchState touch = TouchState::Inactive;
         TimerState displayPowerTimer = TimerState::Expired;
-        bool isDisplayPowerStateNormal = true;
+        hal::PowerMode displayPowerMode = hal::PowerMode::ON;
 
         // Chosen display mode.
         DisplayModePtr mode;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index bdcab515f2..13cd304a5f 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -146,6 +146,11 @@ bool VSyncReactor::periodConfirmed(nsecs_t vsync_timestamp, std::optional<nsecs_
         return false;
     }
 
+    if (mDisplayPowerMode == hal::PowerMode::DOZE ||
+        mDisplayPowerMode == hal::PowerMode::DOZE_SUSPEND) {
+        return true;
+    }
+
     if (!mLastHwVsync && !HwcVsyncPeriod) {
         return false;
     }
@@ -206,6 +211,11 @@ bool VSyncReactor::addHwVsyncTimestamp(nsecs_t timestamp, std::optional<nsecs_t>
     return mMoreSamplesNeeded;
 }
 
+void VSyncReactor::setDisplayPowerMode(hal::PowerMode powerMode) {
+    std::scoped_lock lock(mMutex);
+    mDisplayPowerMode = powerMode;
+}
+
 void VSyncReactor::dump(std::string& result) const {
     std::lock_guard lock(mMutex);
     StringAppendF(&result, "VsyncReactor in use\n");
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.h b/services/surfaceflinger/Scheduler/VSyncReactor.h
index 6a1950ac77..4501487392 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.h
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.h
@@ -49,6 +49,8 @@ public:
     bool addHwVsyncTimestamp(nsecs_t timestamp, std::optional<nsecs_t> hwcVsyncPeriod,
                              bool* periodFlushed) final;
 
+    void setDisplayPowerMode(hal::PowerMode powerMode) final;
+
     void dump(std::string& result) const final;
 
 private:
@@ -73,6 +75,8 @@ private:
     std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);
     std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);
 
+    hal::PowerMode mDisplayPowerMode GUARDED_BY(mMutex) = hal::PowerMode::ON;
+
     const bool mSupportKernelIdleTimer = false;
 };
 
diff --git a/services/surfaceflinger/Scheduler/VsyncController.h b/services/surfaceflinger/Scheduler/VsyncController.h
index 59f65372a9..726a420649 100644
--- a/services/surfaceflinger/Scheduler/VsyncController.h
+++ b/services/surfaceflinger/Scheduler/VsyncController.h
@@ -18,7 +18,10 @@
 
 #include <cstddef>
 #include <memory>
+#include <mutex>
 
+#include <DisplayHardware/HWComposer.h>
+#include <DisplayHardware/Hal.h>
 #include <ui/FenceTime.h>
 #include <utils/Mutex.h>
 #include <utils/RefBase.h>
@@ -70,6 +73,13 @@ public:
      */
     virtual void setIgnorePresentFences(bool ignore) = 0;
 
+    /*
+     * Sets the primary display power mode to the controller.
+     *
+     * \param [in] powerMode
+     */
+    virtual void setDisplayPowerMode(hal::PowerMode powerMode) = 0;
+
     virtual void dump(std::string& result) const = 0;
 
 protected:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5cf9558e7e..906cf73ade 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4947,7 +4947,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
     if (isDisplayActiveLocked(display)) {
         mTimeStats->setPowerMode(mode);
         mRefreshRateStats->setPowerMode(mode);
-        mScheduler->setDisplayPowerState(mode == hal::PowerMode::ON);
+        mScheduler->setDisplayPowerMode(mode);
     }
 
     ALOGD("Finished setting power mode %d on display %s", mode, to_string(displayId).c_str());
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index aab27957c3..93c809e2fd 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -159,8 +159,8 @@ TEST_F(SchedulerTest, chooseRefreshRateForContentIsNoopWhenModeSwitchingIsNotSup
     mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer);
     ASSERT_EQ(0u, mScheduler->getNumActiveLayers());
 
-    constexpr bool kPowerStateNormal = true;
-    mScheduler->setDisplayPowerState(kPowerStateNormal);
+    constexpr hal::PowerMode kPowerModeOn = hal::PowerMode::ON;
+    mScheduler->setDisplayPowerMode(kPowerModeOn);
 
     constexpr uint32_t kDisplayArea = 999'999;
     mScheduler->onActiveDisplayAreaChanged(kDisplayArea);
@@ -226,8 +226,8 @@ TEST_F(SchedulerTest, chooseRefreshRateForContentSelectsMaxRefreshRate) {
 
     mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer);
 
-    constexpr bool kPowerStateNormal = true;
-    mScheduler->setDisplayPowerState(kPowerStateNormal);
+    constexpr hal::PowerMode kPowerModeOn = hal::PowerMode::ON;
+    mScheduler->setDisplayPowerMode(kPowerModeOn);
 
     constexpr uint32_t kDisplayArea = 999'999;
     mScheduler->onActiveDisplayAreaChanged(kDisplayArea);
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 4eb90558ab..30a3f9a4a7 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -349,6 +349,23 @@ TEST_F(VSyncReactorTest, addResyncSamplePeriodChanges) {
     }
 }
 
+TEST_F(VSyncReactorTest, addHwVsyncTimestampDozePreempt) {
+    bool periodFlushed = false;
+    nsecs_t const newPeriod = 4000;
+
+    mReactor.startPeriodTransition(newPeriod);
+
+    auto time = 0;
+    // If the power mode is not DOZE or DOZE_SUSPEND, it is still collecting timestamps.
+    EXPECT_TRUE(mReactor.addHwVsyncTimestamp(time, std::nullopt, &periodFlushed));
+    EXPECT_FALSE(periodFlushed);
+
+    // Set power mode to DOZE to trigger period flushing.
+    mReactor.setDisplayPowerMode(hal::PowerMode::DOZE);
+    EXPECT_FALSE(mReactor.addHwVsyncTimestamp(time, std::nullopt, &periodFlushed));
+    EXPECT_TRUE(periodFlushed);
+}
+
 TEST_F(VSyncReactorTest, addPresentFenceWhileAwaitingPeriodConfirmationRequestsHwVsync) {
     auto time = 0;
     bool periodFlushed = false;
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVsyncController.h b/services/surfaceflinger/tests/unittests/mock/MockVsyncController.h
index 314f681545..4ef91dacb2 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVsyncController.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVsyncController.h
@@ -31,6 +31,7 @@ public:
     MOCK_METHOD3(addHwVsyncTimestamp, bool(nsecs_t, std::optional<nsecs_t>, bool*));
     MOCK_METHOD1(startPeriodTransition, void(nsecs_t));
     MOCK_METHOD1(setIgnorePresentFences, void(bool));
+    MOCK_METHOD(void, setDisplayPowerMode, (hal::PowerMode), (override));
 
     MOCK_CONST_METHOD1(dump, void(std::string&));
 };
-- 
cgit v1.2.3-59-g8ed1b


From a92a56868ea8c5cd1866ca1f6c057f31c5b0b9d9 Mon Sep 17 00:00:00 2001
From: Ady Abraham <adyabr@google.com>
Date: Tue, 7 Jun 2022 23:35:38 +0000
Subject: SurfaceFlinger: add logging when changing refresh rate policy

Add a log message when refresh rate policy is changed to help
debug issues when SF changes refresh rate unexpectedly.

Bug: 230590870
Test: change policy over adb and observe logs
Change-Id: I31cdc3ad47ea25c0f700ebad897e7efbc986bd24
---
 services/surfaceflinger/DisplayDevice.cpp  | 22 ++++++++++++++++++++++
 services/surfaceflinger/DisplayDevice.h    |  6 ++++++
 services/surfaceflinger/SurfaceFlinger.cpp |  4 +---
 3 files changed, 29 insertions(+), 3 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index a915b615d9..86809007b4 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -211,6 +211,7 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info,
               to_string(getId()).c_str());
         return BAD_VALUE;
     }
+    mNumModeSwitchesInPolicy++;
     mUpcomingActiveMode = info;
     ATRACE_INT(mActiveModeFPSHwcTrace.c_str(), info.mode->getFps().getIntValue());
     return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), info.mode->getHwcId(),
@@ -537,6 +538,27 @@ void DisplayDevice::clearDesiredActiveModeState() {
     mDesiredActiveModeChanged = false;
 }
 
+status_t DisplayDevice::setRefreshRatePolicy(
+        const std::optional<scheduler::RefreshRateConfigs::Policy>& policy, bool overridePolicy) {
+    const auto oldPolicy = mRefreshRateConfigs->getCurrentPolicy();
+    const status_t setPolicyResult = overridePolicy
+            ? mRefreshRateConfigs->setOverridePolicy(policy)
+            : mRefreshRateConfigs->setDisplayManagerPolicy(*policy);
+
+    if (setPolicyResult == OK) {
+        const int numModeChanges = mNumModeSwitchesInPolicy.exchange(0);
+
+        ALOGI("Display %s policy changed\n"
+              "Previous: {%s}\n"
+              "Current:  {%s}\n"
+              "%d mode changes were performed under the previous policy",
+              to_string(getId()).c_str(), oldPolicy.toString().c_str(),
+              policy ? policy->toString().c_str() : "null", numModeChanges);
+    }
+
+    return setPolicyResult;
+}
+
 std::atomic<int32_t> DisplayDeviceState::sNextSequenceId(1);
 
 }  // namespace android
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index d5d87b40de..2161436d44 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -249,6 +249,10 @@ public:
     nsecs_t getVsyncPeriodFromHWC() const;
     nsecs_t getRefreshTimestamp() const;
 
+    status_t setRefreshRatePolicy(
+            const std::optional<scheduler::RefreshRateConfigs::Policy>& policy,
+            bool overridePolicy);
+
     // release HWC resources (if any) for removable displays
     void disconnect();
 
@@ -303,6 +307,8 @@ private:
     TracedOrdinal<bool> mDesiredActiveModeChanged
             GUARDED_BY(mActiveModeLock) = {"DesiredActiveModeChanged", false};
     ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext);
+
+    std::atomic_int mNumModeSwitchesInPolicy = 0;
 };
 
 struct DisplayDeviceState {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5cf9558e7e..2cd9e1e485 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -6913,9 +6913,7 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal(
         return NO_ERROR;
     }
 
-    status_t setPolicyResult = overridePolicy
-            ? display->refreshRateConfigs().setOverridePolicy(policy)
-            : display->refreshRateConfigs().setDisplayManagerPolicy(*policy);
+    const status_t setPolicyResult = display->setRefreshRatePolicy(policy, overridePolicy);
     if (setPolicyResult < 0) {
         return BAD_VALUE;
     }
-- 
cgit v1.2.3-59-g8ed1b


From c6b9d38c2702ee3b11422a3ebeac1dbbde33e400 Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Fri, 17 Jun 2022 15:28:07 -0700
Subject: Use true present fence times in PowerHintSession duration calc

The current implementation of power hint session timing relies on
estimated present times rather than actual present fence times,
causing mis-timings when a vsync is skipped. This patch fixes that by
providing the known present fence times to the PowerAdvisor.

Present fence times are also renamed as such to keep naming consistent.

Bug: b/236423436
Bug: b/195990840
Test: manual testing
Change-Id: I3cb25269c6231470bd23cc8b145a21559aaa1c70
---
 .../CompositionEngine/tests/MockPowerAdvisor.h     |  1 +
 .../DisplayHardware/PowerAdvisor.cpp               | 55 +++++++++++++---------
 .../surfaceflinger/DisplayHardware/PowerAdvisor.h  | 22 +++++----
 services/surfaceflinger/SurfaceFlinger.cpp         |  1 +
 .../mock/DisplayHardware/MockPowerAdvisor.h        |  1 +
 5 files changed, 48 insertions(+), 32 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
index 579636f4ee..8c164edded 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
@@ -55,6 +55,7 @@ public:
     MOCK_METHOD(void, setRequiresClientComposition,
                 (DisplayId displayId, bool requiresClientComposition), (override));
     MOCK_METHOD(void, setExpectedPresentTime, (nsecs_t expectedPresentTime), (override));
+    MOCK_METHOD(void, setPresentFenceTime, (nsecs_t presentFenceTime), (override));
     MOCK_METHOD(void, setHwcPresentDelayedTime,
                 (DisplayId displayId,
                  std::chrono::steady_clock::time_point earliestFrameStartTime));
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index 40b11324db..0ed55b2864 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -315,6 +315,10 @@ void PowerAdvisor::setExpectedPresentTime(nsecs_t expectedPresentTime) {
     mExpectedPresentTimes.append(expectedPresentTime);
 }
 
+void PowerAdvisor::setPresentFenceTime(nsecs_t presentFenceTime) {
+    mLastPresentFenceTime = presentFenceTime;
+}
+
 void PowerAdvisor::setFrameDelay(nsecs_t frameDelayDuration) {
     mFrameDelayDuration = frameDelayDuration;
 }
@@ -384,11 +388,11 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
     // If we're predicting at the end of the frame, we use the current frame as a reference point
     nsecs_t referenceFrameStartTime = (earlyHint ? mCommitStartTimes[-1] : mCommitStartTimes[0]);
 
-    // We need an idea of when the last present fence fired and how long it made us wait
-    // If we're predicting at the start of the frame, we want frame n-2's present fence time
-    // If we're predicting at the end of the frame we want frame n-1's present time
-    nsecs_t referenceFenceTime =
-            (earlyHint ? mExpectedPresentTimes[-2] : mExpectedPresentTimes[-1]);
+    // When the prior frame should be presenting to the display
+    // If we're predicting at the start of the frame, we use last frame's expected present time
+    // If we're predicting at the end of the frame, the present fence time is already known
+    nsecs_t lastFramePresentTime = (earlyHint ? mExpectedPresentTimes[-1] : mLastPresentFenceTime);
+
     // The timing info for the previously calculated display, if there was one
     std::optional<DisplayTimeline> previousDisplayReferenceTiming;
     std::vector<DisplayId>&& displayIds =
@@ -402,7 +406,11 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
         }
 
         auto& displayData = mDisplayTimingData.at(displayId);
-        referenceTiming = displayData.calculateDisplayTimeline(referenceFenceTime);
+
+        // mLastPresentFenceTime should always be the time of the reference frame, since it will be
+        // the previous frame's present fence if called at the start, and current frame's if called
+        // at the end
+        referenceTiming = displayData.calculateDisplayTimeline(mLastPresentFenceTime);
 
         // If this is the first display, include the duration before hwc present starts
         if (!previousDisplayReferenceTiming.has_value()) {
@@ -412,14 +420,15 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
                     previousDisplayReferenceTiming->hwcPresentEndTime;
         }
 
-        estimatedTiming = referenceTiming.estimateTimelineFromReference(mExpectedPresentTimes[-1],
+        estimatedTiming = referenceTiming.estimateTimelineFromReference(lastFramePresentTime,
                                                                         estimatedEndTime);
+
         // Update predicted present finish time with this display's present time
         estimatedEndTime = estimatedTiming.hwcPresentEndTime;
 
         // Track how long we spent waiting for the fence, can be excluded from the timing estimate
-        idleDuration += estimatedTiming.probablyWaitsForReleaseFence
-                ? mExpectedPresentTimes[-1] - estimatedTiming.releaseFenceWaitStartTime
+        idleDuration += estimatedTiming.probablyWaitsForPresentFence
+                ? lastFramePresentTime - estimatedTiming.presentFenceWaitStartTime
                 : 0;
 
         // Track how long we spent waiting to present, can be excluded from the timing estimate
@@ -476,15 +485,15 @@ PowerAdvisor::DisplayTimeline PowerAdvisor::DisplayTimeline::estimateTimelineFro
     // We don't predict waiting for vsync alignment yet
     estimated.hwcPresentDelayDuration = 0;
 
-    // For now just re-use last frame's post-present duration and assume it will not change much
     // How long we expect to run before we start waiting for the fence
-    // If it's the early hint we exclude time we spent waiting for a vsync, otherwise don't
-    estimated.releaseFenceWaitStartTime = estimated.hwcPresentStartTime +
-            (releaseFenceWaitStartTime - (hwcPresentStartTime + hwcPresentDelayDuration));
-    estimated.probablyWaitsForReleaseFence = fenceTime > estimated.releaseFenceWaitStartTime;
-    estimated.hwcPresentEndTime = postReleaseFenceHwcPresentDuration +
-            (estimated.probablyWaitsForReleaseFence ? fenceTime
-                                                    : estimated.releaseFenceWaitStartTime);
+    // For now just re-use last frame's post-present duration and assume it will not change much
+    // Excludes time spent waiting for vsync since that's not going to be consistent
+    estimated.presentFenceWaitStartTime = estimated.hwcPresentStartTime +
+            (presentFenceWaitStartTime - (hwcPresentStartTime + hwcPresentDelayDuration));
+    estimated.probablyWaitsForPresentFence = fenceTime > estimated.presentFenceWaitStartTime;
+    estimated.hwcPresentEndTime = postPresentFenceHwcPresentDuration +
+            (estimated.probablyWaitsForPresentFence ? fenceTime
+                                                    : estimated.presentFenceWaitStartTime);
     return estimated;
 }
 
@@ -510,16 +519,16 @@ PowerAdvisor::DisplayTimeline PowerAdvisor::DisplayTimingData::calculateDisplayT
     // How long hwc present was delayed waiting for the next appropriate vsync
     timeline.hwcPresentDelayDuration =
             (waitedOnHwcPresentTime ? *hwcPresentDelayedTime - *hwcPresentStartTime : 0);
-    // When we started waiting for the release fence after calling into hwc present
-    timeline.releaseFenceWaitStartTime =
+    // When we started waiting for the present fence after calling into hwc present
+    timeline.presentFenceWaitStartTime =
             timeline.hwcPresentStartTime + timeline.hwcPresentDelayDuration + fenceWaitStartDelay;
-    timeline.probablyWaitsForReleaseFence = fenceTime > timeline.releaseFenceWaitStartTime &&
+    timeline.probablyWaitsForPresentFence = fenceTime > timeline.presentFenceWaitStartTime &&
             fenceTime < timeline.hwcPresentEndTime;
 
     // How long we ran after we finished waiting for the fence but before hwc present finished
-    timeline.postReleaseFenceHwcPresentDuration = timeline.hwcPresentEndTime -
-            (timeline.probablyWaitsForReleaseFence ? fenceTime
-                                                   : timeline.releaseFenceWaitStartTime);
+    timeline.postPresentFenceHwcPresentDuration = timeline.hwcPresentEndTime -
+            (timeline.probablyWaitsForPresentFence ? fenceTime
+                                                   : timeline.presentFenceWaitStartTime);
     return timeline;
 }
 
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
index bdc7927742..98921b0861 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
@@ -70,7 +70,10 @@ public:
     // Reports the start and end times of a hwc present call this frame for a given display
     virtual void setHwcPresentTiming(DisplayId displayId, nsecs_t presentStartTime,
                                      nsecs_t presentEndTime) = 0;
+    // Reports the expected time that the current frame will present to the display
     virtual void setExpectedPresentTime(nsecs_t expectedPresentTime) = 0;
+    // Reports the most recent present fence time once it's known at the end of the frame
+    virtual void setPresentFenceTime(nsecs_t presentFenceTime) = 0;
     // Reports whether a display used client composition this frame
     virtual void setRequiresClientComposition(DisplayId displayId,
                                               bool requiresClientComposition) = 0;
@@ -139,6 +142,7 @@ public:
     void setSkippedValidate(DisplayId displayId, bool skipped) override;
     void setRequiresClientComposition(DisplayId displayId, bool requiresClientComposition) override;
     void setExpectedPresentTime(nsecs_t expectedPresentTime) override;
+    void setPresentFenceTime(nsecs_t presentFenceTime) override;
     void setHwcPresentDelayedTime(
             DisplayId displayId,
             std::chrono::steady_clock::time_point earliestFrameStartTime) override;
@@ -172,13 +176,13 @@ private:
         nsecs_t hwcPresentEndTime = -1;
         // How long the actual hwc present was delayed after hwcPresentStartTime
         nsecs_t hwcPresentDelayDuration = 0;
-        // When we think we started waiting for the release fence after calling into hwc present and
+        // When we think we started waiting for the present fence after calling into hwc present and
         // after potentially waiting for the earliest present time
-        nsecs_t releaseFenceWaitStartTime = -1;
+        nsecs_t presentFenceWaitStartTime = -1;
         // How long we ran after we finished waiting for the fence but before hwc present finished
-        nsecs_t postReleaseFenceHwcPresentDuration = 0;
+        nsecs_t postPresentFenceHwcPresentDuration = 0;
         // Are we likely to have waited for the present fence during composition
-        bool probablyWaitsForReleaseFence = false;
+        bool probablyWaitsForPresentFence = false;
         // Estimate one frame's timeline from that of a previous frame
         DisplayTimeline estimateTimelineFromReference(nsecs_t fenceTime, nsecs_t displayStartTime);
     };
@@ -248,7 +252,9 @@ private:
     // Buffer of recent commit start times
     RingBuffer<nsecs_t, 2> mCommitStartTimes;
     // Buffer of recent expected present times
-    RingBuffer<nsecs_t, 3> mExpectedPresentTimes;
+    RingBuffer<nsecs_t, 2> mExpectedPresentTimes;
+    // Most recent present fence time, set at the end of the frame once known
+    nsecs_t mLastPresentFenceTime = -1;
     // Target for the entire pipeline including gpu
     std::optional<nsecs_t> mTotalFrameTargetDuration;
     // Updated list of display IDs
@@ -258,10 +264,8 @@ private:
     std::optional<bool> mSupportsPowerHint;
     bool mPowerHintSessionRunning = false;
 
-    // An adjustable safety margin which moves the "target" earlier to allow flinger to
-    // go a bit over without dropping a frame, especially since we can't measure
-    // the exact time hwc finishes composition so "actual" durations are measured
-    // from the end of present() instead, which is a bit later.
+    // An adjustable safety margin which pads the "actual" value sent to PowerHAL,
+    // encouraging more aggressive boosting to give SurfaceFlinger a larger margin for error
     static constexpr const std::chrono::nanoseconds kTargetSafetyMargin = 1ms;
 
     // How long we expect hwc to run after the present call until it waits for the fence
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index b7d696889d..6e74eef068 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2244,6 +2244,7 @@ void SurfaceFlinger::composite(nsecs_t frameTime, int64_t vsyncId)
 
     // Send a power hint hint after presentation is finished
     if (mPowerHintSessionEnabled) {
+        mPowerAdvisor->setPresentFenceTime(mPreviousPresentFences[0].fenceTime->getSignalTime());
         if (mPowerHintSessionMode.late) {
             mPowerAdvisor->sendActualWorkDuration();
         }
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
index e347883782..d6dca45188 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
@@ -53,6 +53,7 @@ public:
     MOCK_METHOD(void, setRequiresClientComposition,
                 (DisplayId displayId, bool requiresClientComposition), (override));
     MOCK_METHOD(void, setExpectedPresentTime, (nsecs_t expectedPresentTime), (override));
+    MOCK_METHOD(void, setPresentFenceTime, (nsecs_t presentFenceTime), (override));
     MOCK_METHOD(void, setHwcPresentDelayedTime,
                 (DisplayId displayId,
                  std::chrono::steady_clock::time_point earliestFrameStartTime));
-- 
cgit v1.2.3-59-g8ed1b


From d8f17c54eb2de73c14057614cf9e1df5dbe4d832 Mon Sep 17 00:00:00 2001
From: Alec Mouri <alecmouri@google.com>
Date: Thu, 21 Jul 2022 18:43:10 +0000
Subject: Fail gracefully when allocating screenshot buffers

Some devices have limited protected memory, and over-allocate buffers in
the decoder during DRM playback of high resolution content. The decoder
is able to fail gracefully, but SurfaceFlinger is stricter, causing the
device to crash.

More generally, SurfaceFlinger should not be so strict, because a
malicious app could intentionally allocate many buffers and cause the
system to crash. So, fail gracefully instead to prevent the entire
system from falling over.

Bug: 236200340
Test: 4K DRM playback
Change-Id: Ia0018974fffc753342f78917ede0b67faa94916b
---
 services/surfaceflinger/SurfaceFlinger.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6a17cd8881..d6f665aa5d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -6620,8 +6620,13 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenCommon(
                                              1 /* layerCount */, usage, "screenshot");
 
     const status_t bufferStatus = buffer->initCheck();
-    LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "captureScreenCommon: Buffer failed to allocate: %d",
-                        bufferStatus);
+    if (bufferStatus != OK) {
+        // Animations may end up being really janky, but don't crash here.
+        // Otherwise an irreponsible process may cause an SF crash by allocating
+        // too much.
+        ALOGE("%s: Buffer failed to allocate: %d", __func__, bufferStatus);
+        return ftl::yield<FenceResult>(base::unexpected(bufferStatus)).share();
+    }
     const std::shared_ptr<renderengine::ExternalTexture> texture = std::make_shared<
             renderengine::impl::ExternalTexture>(buffer, getRenderEngine(),
                                                  renderengine::impl::ExternalTexture::Usage::
-- 
cgit v1.2.3-59-g8ed1b


From 009487079366e7099891082a9baa8ee80815bda6 Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Wed, 20 Jul 2022 20:53:32 +0000
Subject: Disable early hint in SurfaceFlinger adpf cpu hints by default

Early hint for SurfaceFlinger currently does not provide enough value
and needs more tweaking to be viable, so this patch disables it
by default for the feature rollout.

Bug: b/239730433
Test: manual
Change-Id: I48387f5a14b242e8b9fb5c7dfbc3d041c303eb71
(cherry picked from commit 58a3645426efbefc900d93e3d510b612cd54e532)
---
 services/surfaceflinger/SurfaceFlinger.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6e74eef068..44645b5066 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -448,7 +448,7 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI
     // Power hint session mode, representing which hint(s) to send: early, late, or both)
     mPowerHintSessionMode =
             {.late = base::GetBoolProperty("debug.sf.send_late_power_session_hint"s, true),
-             .early = base::GetBoolProperty("debug.sf.send_early_power_session_hint"s, true)};
+             .early = base::GetBoolProperty("debug.sf.send_early_power_session_hint"s, false)};
 }
 
 LatchUnsignaledConfig SurfaceFlinger::getLatchUnsignaledConfig() {
-- 
cgit v1.2.3-59-g8ed1b


From 83aca38885df5030532c6dfe74a1fcef3ddfb432 Mon Sep 17 00:00:00 2001
From: Prabir Pradhan <prabirmsp@google.com>
Date: Fri, 22 Jul 2022 19:53:04 +0000
Subject: Reland: Send WindowInfo even if the window isn't associated with a
 DisplayDevice

Previously reverted for b/240320932. Fixed broken test.

The previous behavior was the following: If a window was in a layerStack
for which there were no DisplayDevices that mapped to it, there was no
WindowInfo sent for it.

When a display is turned OFF, it is mapped to an invalid layer stack.
So when the primary display is turned OFF, there is no WindowInfo sent
for any windows on layerStack 0 as there are no DisplayDevices that now
map to it. In this model, no window can receive input when the display
is OFF.

In this CL, we change the behavior so that WindowInfos are sent even if
there are no DisplayDevices that map to its layerStack -- except that
in this case, we do not let the window receive touches, as we do not
have a valid transform associated with it.

Bug: 239788987
Test: See bug for repro steps
Test: Observe logs at runtime
Test: atest libgui_tests
Change-Id: Ib7e343685a0e3a87769883bdd5e24ac0e6d5e83f
(cherry picked from commit da0f62ce866950a67decdfed0748458fdeff9257)
---
 libs/gui/tests/EndToEndNativeInputTest.cpp | 11 ++++++++---
 services/surfaceflinger/Layer.cpp          | 17 ++++++++++++++---
 services/surfaceflinger/Layer.h            |  6 +++++-
 services/surfaceflinger/SurfaceFlinger.cpp | 10 +++++-----
 4 files changed, 32 insertions(+), 12 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp
index c6cdeb7706..2637f59b5e 100644
--- a/libs/gui/tests/EndToEndNativeInputTest.cpp
+++ b/libs/gui/tests/EndToEndNativeInputTest.cpp
@@ -1184,18 +1184,23 @@ public:
     std::vector<sp<IGraphicBufferProducer>> mProducers;
 };
 
-TEST_F(MultiDisplayTests, drop_input_if_layer_on_invalid_display) {
+TEST_F(MultiDisplayTests, drop_touch_if_layer_on_invalid_display) {
     ui::LayerStack layerStack = ui::LayerStack::fromValue(42);
     // Do not create a display associated with the LayerStack.
     std::unique_ptr<InputSurface> surface = makeSurface(100, 100);
     surface->doTransaction([&](auto &t, auto &sc) { t.setLayerStack(sc, layerStack); });
     surface->showAt(100, 100);
 
+    // Touches should be dropped if the layer is on an invalid display.
     injectTapOnDisplay(101, 101, layerStack.id);
+    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+
+    // However, we still let the window be focused and receive keys.
     surface->requestFocus(layerStack.id);
-    injectKeyOnDisplay(AKEYCODE_V, layerStack.id);
+    surface->assertFocusChange(true);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    injectKeyOnDisplay(AKEYCODE_V, layerStack.id);
+    surface->expectKey(AKEYCODE_V);
 }
 
 TEST_F(MultiDisplayTests, virtual_display_receives_input) {
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index be16942d40..bcc94d3146 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -81,6 +81,7 @@
 namespace android {
 namespace {
 constexpr int kDumpTableRowLength = 159;
+const ui::Transform kIdentityTransform;
 } // namespace
 
 using namespace ftl::flag_operators;
@@ -2162,7 +2163,8 @@ void Layer::writeToProtoCommonState(LayerProto* layerInfo, LayerVector::StateSet
     if ((traceFlags & LayerTracing::TRACE_INPUT) && needsInputInfo()) {
         WindowInfo info;
         if (useDrawing) {
-            info = fillInputInfo(ui::Transform(), /* displayIsSecure */ true);
+            info = fillInputInfo(
+                    InputDisplayArgs{.transform = &kIdentityTransform, .isSecure = true});
         } else {
             info = state.inputInfo;
         }
@@ -2369,7 +2371,7 @@ void Layer::handleDropInputMode(gui::WindowInfo& info) const {
     }
 }
 
-WindowInfo Layer::fillInputInfo(const ui::Transform& displayTransform, bool displayIsSecure) {
+WindowInfo Layer::fillInputInfo(const InputDisplayArgs& displayArgs) {
     if (!hasInputInfo()) {
         mDrawingState.inputInfo.name = getName();
         mDrawingState.inputInfo.ownerUid = mOwnerUid;
@@ -2378,12 +2380,21 @@ WindowInfo Layer::fillInputInfo(const ui::Transform& displayTransform, bool disp
         mDrawingState.inputInfo.displayId = getLayerStack().id;
     }
 
+    const ui::Transform& displayTransform =
+            displayArgs.transform != nullptr ? *displayArgs.transform : kIdentityTransform;
+
     WindowInfo info = mDrawingState.inputInfo;
     info.id = sequence;
     info.displayId = getLayerStack().id;
 
     fillInputFrameInfo(info, displayTransform);
 
+    if (displayArgs.transform == nullptr) {
+        // Do not let the window receive touches if it is not associated with a valid display
+        // transform. We still allow the window to receive keys and prevent ANRs.
+        info.inputConfig |= WindowInfo::InputConfig::NOT_TOUCHABLE;
+    }
+
     // For compatibility reasons we let layers which can receive input
     // receive input before they have actually submitted a buffer. Because
     // of this we use canReceiveInput instead of isVisible to check the
@@ -2401,7 +2412,7 @@ WindowInfo Layer::fillInputInfo(const ui::Transform& displayTransform, bool disp
 
     // If the window will be blacked out on a display because the display does not have the secure
     // flag and the layer has the secure flag set, then drop input.
-    if (!displayIsSecure && isSecure()) {
+    if (!displayArgs.isSecure && isSecure()) {
         info.inputConfig |= WindowInfo::InputConfig::DROP_INPUT;
     }
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 24abad9b45..c547da06cb 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -844,7 +844,11 @@ public:
     bool getPremultipledAlpha() const;
     void setInputInfo(const gui::WindowInfo& info);
 
-    gui::WindowInfo fillInputInfo(const ui::Transform& displayTransform, bool displayIsSecure);
+    struct InputDisplayArgs {
+        const ui::Transform* transform = nullptr;
+        bool isSecure = false;
+    };
+    gui::WindowInfo fillInputInfo(const InputDisplayArgs& displayArgs);
 
     /**
      * Returns whether this layer has an explicitly set input-info.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 14e5c692c7..cbc068f2bb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3329,11 +3329,11 @@ void SurfaceFlinger::buildWindowInfos(std::vector<WindowInfo>& outWindowInfos,
     mDrawingState.traverseInReverseZOrder([&](Layer* layer) {
         if (!layer->needsInputInfo()) return;
 
-        // Do not create WindowInfos for windows on displays that cannot receive input.
-        if (const auto opt = displayInputInfos.get(layer->getLayerStack())) {
-            const auto& info = opt->get();
-            outWindowInfos.push_back(layer->fillInputInfo(info.transform, info.isSecure));
-        }
+        const auto opt = displayInputInfos.get(layer->getLayerStack(),
+                                               [](const auto& info) -> Layer::InputDisplayArgs {
+                                                   return {&info.transform, info.isSecure};
+                                               });
+        outWindowInfos.push_back(layer->fillInputInfo(opt.value_or(Layer::InputDisplayArgs{})));
     });
 
     sNumWindowInfos = outWindowInfos.size();
-- 
cgit v1.2.3-59-g8ed1b


From 672e526d76b2a7835455030e9efc80e78da10a22 Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Fri, 29 Jul 2022 00:13:27 +0000
Subject: Get vsync period for ADPF CPU hints in SF using display mode

Replace the existing implementation using getVsyncPeriodFromHWC with
vsync data from the current display mode

Bug: b/240621816
Test: manual
Change-Id: Ie70d17400481eed31595a5f1ae4296c4df9d4071
(cherry picked from commit 1151f4677dc80ab2acf09af7586798e974244ca1)
---
 services/surfaceflinger/SurfaceFlinger.cpp | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index cbc068f2bb..8621d31d91 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2087,11 +2087,9 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected
     // Save this once per commit + composite to ensure consistency
     mPowerHintSessionEnabled = mPowerAdvisor->usePowerHintSession();
     if (mPowerHintSessionEnabled) {
-        nsecs_t vsyncPeriod;
-        {
-            Mutex::Autolock lock(mStateLock);
-            vsyncPeriod = getVsyncPeriodFromHWC();
-        }
+        const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get();
+        // get stable vsync period from display mode
+        const nsecs_t vsyncPeriod = display->getActiveMode()->getVsyncPeriod();
         mPowerAdvisor->setCommitStart(frameTime);
         mPowerAdvisor->setExpectedPresentTime(mExpectedPresentTime);
         const nsecs_t idealSfWorkDuration =
-- 
cgit v1.2.3-59-g8ed1b


From 3e68cce46450bc156ed9ded9e3b0db0d58fdb87c Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Fri, 5 Aug 2022 06:51:43 +0000
Subject: Fix SF hint sessions for virtual multi-display case

Currently timing for hint sessions in SurfaceFlinger do not work for
multi-display case with virtual displays (eg: Android Auto). This
patch fixes that by tracking true sf present completion time to
supplement hwc time tracking in the sf main thread deadline case.

Bug: b/241465879

Test: manual

Change-Id: I169dd4f4afd2223ed0a648440d5f59dc19dc07f6
(cherry picked from commit 1809d90ed310216a4d0d8da38b63ecd17d8d6ed2)
---
 .../CompositionEngine/tests/MockPowerAdvisor.h      |  3 ++-
 .../surfaceflinger/DisplayHardware/PowerAdvisor.cpp | 21 +++++++++++----------
 .../surfaceflinger/DisplayHardware/PowerAdvisor.h   | 10 +++++-----
 services/surfaceflinger/SurfaceFlinger.cpp          |  3 ++-
 .../mock/DisplayHardware/MockPowerAdvisor.h         |  3 ++-
 5 files changed, 22 insertions(+), 18 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
index 8c164edded..c8bd5e436c 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
@@ -55,7 +55,8 @@ public:
     MOCK_METHOD(void, setRequiresClientComposition,
                 (DisplayId displayId, bool requiresClientComposition), (override));
     MOCK_METHOD(void, setExpectedPresentTime, (nsecs_t expectedPresentTime), (override));
-    MOCK_METHOD(void, setPresentFenceTime, (nsecs_t presentFenceTime), (override));
+    MOCK_METHOD(void, setSfPresentTiming, (nsecs_t presentFenceTime, nsecs_t presentEndTime),
+                (override));
     MOCK_METHOD(void, setHwcPresentDelayedTime,
                 (DisplayId displayId,
                  std::chrono::steady_clock::time_point earliestFrameStartTime));
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index b9d4753f45..ac46280331 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -315,7 +315,8 @@ void PowerAdvisor::setExpectedPresentTime(nsecs_t expectedPresentTime) {
     mExpectedPresentTimes.append(expectedPresentTime);
 }
 
-void PowerAdvisor::setPresentFenceTime(nsecs_t presentFenceTime) {
+void PowerAdvisor::setSfPresentTiming(nsecs_t presentFenceTime, nsecs_t presentEndTime) {
+    mLastSfPresentEndTime = presentEndTime;
     mLastPresentFenceTime = presentFenceTime;
 }
 
@@ -334,13 +335,7 @@ void PowerAdvisor::setCommitStart(nsecs_t commitStartTime) {
 }
 
 void PowerAdvisor::setCompositeEnd(nsecs_t compositeEnd) {
-    mLastCompositeEndTime = compositeEnd;
-    // calculate the postcomp time here as well
-    std::vector<DisplayId>&& displays = getOrderedDisplayIds(&DisplayTimingData::hwcPresentEndTime);
-    DisplayTimingData& timingData = mDisplayTimingData[displays.back()];
-    mLastPostcompDuration = compositeEnd -
-            (timingData.skippedValidate ? *timingData.hwcValidateEndTime
-                                        : *timingData.hwcPresentEndTime);
+    mLastPostcompDuration = compositeEnd - mLastSfPresentEndTime;
 }
 
 void PowerAdvisor::setDisplays(std::vector<DisplayId>& displayIds) {
@@ -399,7 +394,7 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
             getOrderedDisplayIds(&DisplayTimingData::hwcPresentStartTime);
     DisplayTimeline referenceTiming, estimatedTiming;
 
-    // Iterate over the displays in the same order they are presented
+    // Iterate over the displays that use hwc in the same order they are presented
     for (DisplayId displayId : displayIds) {
         if (mDisplayTimingData.count(displayId) == 0) {
             continue;
@@ -451,8 +446,11 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
     }
     ATRACE_INT64("Idle duration", idleDuration);
 
+    nsecs_t estimatedFlingerEndTime = earlyHint ? estimatedEndTime : mLastSfPresentEndTime;
+
     // Don't count time spent idly waiting in the estimate as we could do more work in that time
     estimatedEndTime -= idleDuration;
+    estimatedFlingerEndTime -= idleDuration;
 
     // We finish the frame when both present and the gpu are done, so wait for the later of the two
     // Also add the frame delay duration since the target did not move while we were delayed
@@ -460,7 +458,10 @@ std::optional<nsecs_t> PowerAdvisor::estimateWorkDuration(bool earlyHint) {
             std::max(estimatedEndTime, estimatedGpuEndTime.value_or(0)) - mCommitStartTimes[0];
 
     // We finish SurfaceFlinger when post-composition finishes, so add that in here
-    nsecs_t flingerDuration = estimatedEndTime + mLastPostcompDuration - mCommitStartTimes[0];
+    nsecs_t flingerDuration =
+            estimatedFlingerEndTime + mLastPostcompDuration - mCommitStartTimes[0];
+
+    // Combine the two timings into a single normalized one
     nsecs_t combinedDuration = combineTimingEstimates(totalDuration, flingerDuration);
 
     return std::make_optional(combinedDuration);
diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
index 98921b0861..71a1f8c03b 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
@@ -72,8 +72,8 @@ public:
                                      nsecs_t presentEndTime) = 0;
     // Reports the expected time that the current frame will present to the display
     virtual void setExpectedPresentTime(nsecs_t expectedPresentTime) = 0;
-    // Reports the most recent present fence time once it's known at the end of the frame
-    virtual void setPresentFenceTime(nsecs_t presentFenceTime) = 0;
+    // Reports the most recent present fence time and end time once known
+    virtual void setSfPresentTiming(nsecs_t presentFenceTime, nsecs_t presentEndTime) = 0;
     // Reports whether a display used client composition this frame
     virtual void setRequiresClientComposition(DisplayId displayId,
                                               bool requiresClientComposition) = 0;
@@ -142,7 +142,7 @@ public:
     void setSkippedValidate(DisplayId displayId, bool skipped) override;
     void setRequiresClientComposition(DisplayId displayId, bool requiresClientComposition) override;
     void setExpectedPresentTime(nsecs_t expectedPresentTime) override;
-    void setPresentFenceTime(nsecs_t presentFenceTime) override;
+    void setSfPresentTiming(nsecs_t presentFenceTime, nsecs_t presentEndTime) override;
     void setHwcPresentDelayedTime(
             DisplayId displayId,
             std::chrono::steady_clock::time_point earliestFrameStartTime) override;
@@ -245,8 +245,6 @@ private:
 
     // Current frame's delay
     nsecs_t mFrameDelayDuration = 0;
-    // Last frame's composite end time
-    nsecs_t mLastCompositeEndTime = -1;
     // Last frame's post-composition duration
     nsecs_t mLastPostcompDuration = 0;
     // Buffer of recent commit start times
@@ -255,6 +253,8 @@ private:
     RingBuffer<nsecs_t, 2> mExpectedPresentTimes;
     // Most recent present fence time, set at the end of the frame once known
     nsecs_t mLastPresentFenceTime = -1;
+    // Most recent present fence time, set at the end of the frame once known
+    nsecs_t mLastSfPresentEndTime = -1;
     // Target for the entire pipeline including gpu
     std::optional<nsecs_t> mTotalFrameTargetDuration;
     // Updated list of display IDs
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8621d31d91..240991ccc6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2242,7 +2242,8 @@ void SurfaceFlinger::composite(nsecs_t frameTime, int64_t vsyncId)
 
     // Send a power hint hint after presentation is finished
     if (mPowerHintSessionEnabled) {
-        mPowerAdvisor->setPresentFenceTime(mPreviousPresentFences[0].fenceTime->getSignalTime());
+        mPowerAdvisor->setSfPresentTiming(mPreviousPresentFences[0].fenceTime->getSignalTime(),
+                                          systemTime());
         if (mPowerHintSessionMode.late) {
             mPowerAdvisor->sendActualWorkDuration();
         }
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
index d6dca45188..aede250db5 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
@@ -53,7 +53,8 @@ public:
     MOCK_METHOD(void, setRequiresClientComposition,
                 (DisplayId displayId, bool requiresClientComposition), (override));
     MOCK_METHOD(void, setExpectedPresentTime, (nsecs_t expectedPresentTime), (override));
-    MOCK_METHOD(void, setPresentFenceTime, (nsecs_t presentFenceTime), (override));
+    MOCK_METHOD(void, setSfPresentTiming, (nsecs_t presentFenceTime, nsecs_t presentEndTime),
+                (override));
     MOCK_METHOD(void, setHwcPresentDelayedTime,
                 (DisplayId displayId,
                  std::chrono::steady_clock::time_point earliestFrameStartTime));
-- 
cgit v1.2.3-59-g8ed1b


From 5b59d94a5a6739416ecd00251616c437e6c84368 Mon Sep 17 00:00:00 2001
From: Matt Buckley <mattbuckley@google.com>
Date: Tue, 2 Aug 2022 20:10:15 +0000
Subject: Disable ADPF CPU hints for SF unless active display is on

Currently AOD is bugged and does not always match refresh rate between
SF and HWC, which disrupts ADPF hint sessions in SF which are sensitive
to refresh rate (b/218066882). This patch disables ADPF CPU hints for
the AOD case until this issue is resolved.

Bug: b/240619471
Test: atest libsurfaceflinger_unittest:libsurfaceflinger_unittest.SurfaceFlingerPowerHintTest
Change-Id: I4fa617a108f14da9df278c4a449795363ec29cf3
Merged-In: I4fa617a108f14da9df278c4a449795363ec29cf3
(cherry picked from commit ba8d15a176ba57dfd692b8c98a7ad884b0d9260c)
---
 services/surfaceflinger/SurfaceFlinger.cpp         |  6 +++++-
 .../unittests/SurfaceFlinger_PowerHintTest.cpp     | 24 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8621d31d91..83ec4ec213 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2085,7 +2085,11 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected
     }
 
     // Save this once per commit + composite to ensure consistency
-    mPowerHintSessionEnabled = mPowerAdvisor->usePowerHintSession();
+    // TODO (b/240619471): consider removing active display check once AOD is fixed
+    const auto activeDisplay =
+            FTL_FAKE_GUARD(mStateLock, getDisplayDeviceLocked(mActiveDisplayToken));
+    mPowerHintSessionEnabled = mPowerAdvisor->usePowerHintSession() && activeDisplay &&
+            activeDisplay->getPowerMode() == hal::PowerMode::ON;
     if (mPowerHintSessionEnabled) {
         const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get();
         // get stable vsync period from display mode
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
index c2d87f2484..2c9888dd8e 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
@@ -97,6 +97,7 @@ void SurfaceFlingerPowerHintTest::SetUp() {
                     .setNativeWindow(mNativeWindow)
                     .setPowerMode(hal::PowerMode::ON)
                     .inject();
+    mFlinger.mutableActiveDisplayToken() = mDisplay->getDisplayToken();
 }
 
 void SurfaceFlingerPowerHintTest::setupScheduler() {
@@ -148,5 +149,28 @@ TEST_F(SurfaceFlingerPowerHintTest, sendDurationsIncludingHwcWaitTime) {
     mFlinger.commitAndComposite(now, kVsyncId, now + mockVsyncPeriod.count());
 }
 
+TEST_F(SurfaceFlingerPowerHintTest, inactiveOnDisplayDoze) {
+    ON_CALL(*mPowerAdvisor, usePowerHintSession()).WillByDefault(Return(true));
+
+    mDisplay->setPowerMode(hal::PowerMode::DOZE);
+
+    const std::chrono::nanoseconds mockVsyncPeriod = 15ms;
+    EXPECT_CALL(*mPowerAdvisor, setTargetWorkDuration(_)).Times(0);
+
+    const nsecs_t now = systemTime();
+    const std::chrono::nanoseconds mockHwcRunTime = 20ms;
+    EXPECT_CALL(*mDisplaySurface,
+                prepareFrame(compositionengine::DisplaySurface::CompositionType::Hwc))
+            .Times(1);
+    EXPECT_CALL(*mComposer, presentOrValidateDisplay(HWC_DISPLAY, _, _, _, _, _))
+            .WillOnce([mockHwcRunTime] {
+                std::this_thread::sleep_for(mockHwcRunTime);
+                return hardware::graphics::composer::V2_1::Error::NONE;
+            });
+    EXPECT_CALL(*mPowerAdvisor, sendActualWorkDuration()).Times(0);
+    static constexpr bool kVsyncId = 123; // arbitrary
+    mFlinger.commitAndComposite(now, kVsyncId, now + mockVsyncPeriod.count());
+}
+
 } // namespace
 } // namespace android
-- 
cgit v1.2.3-59-g8ed1b


From e80593238c3af1dea4885ca0e32d44392f3d42ee Mon Sep 17 00:00:00 2001
From: linpeter <linpeter@google.com>
Date: Wed, 13 Jul 2022 19:20:48 +0800
Subject: Allow first power mode as off state

Bug: 237745199
test: atest libsurfaceflinger_unittest:OnInitializeDisplaysTest
      atest libsurfaceflinger_unittest:SetPowerModeInternalTest
Merged-In: I3a2eae4efc4a5c6113700a9ca9e9b261e364a878
Change-Id: I3a2eae4efc4a5c6113700a9ca9e9b261e364a878
---
 services/surfaceflinger/DisplayDevice.cpp                   | 12 +++++++-----
 services/surfaceflinger/DisplayDevice.h                     |  9 ++++-----
 services/surfaceflinger/SurfaceFlinger.cpp                  | 13 +++++++------
 .../surfaceflinger/tests/unittests/TestableSurfaceFlinger.h |  1 +
 4 files changed, 19 insertions(+), 16 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 86809007b4..f3b6254b52 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -104,7 +104,7 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args)
 
     mCompositionDisplay->getRenderSurface()->initialize();
 
-    setPowerMode(args.initialPowerMode);
+    if (args.initialPowerMode.has_value()) setPowerMode(args.initialPowerMode.value());
 
     // initialize the display orientation transform.
     setProjection(ui::ROTATION_0, Rect::INVALID_RECT, Rect::INVALID_RECT);
@@ -174,19 +174,21 @@ auto DisplayDevice::getInputInfo() const -> InputInfo {
 
 void DisplayDevice::setPowerMode(hal::PowerMode mode) {
     mPowerMode = mode;
-    getCompositionDisplay()->setCompositionEnabled(mPowerMode != hal::PowerMode::OFF);
+
+    getCompositionDisplay()->setCompositionEnabled(mPowerMode.has_value() &&
+                                                   *mPowerMode != hal::PowerMode::OFF);
 }
 
 void DisplayDevice::enableLayerCaching(bool enable) {
     getCompositionDisplay()->setLayerCachingEnabled(enable);
 }
 
-hal::PowerMode DisplayDevice::getPowerMode() const {
+std::optional<hal::PowerMode> DisplayDevice::getPowerMode() const {
     return mPowerMode;
 }
 
 bool DisplayDevice::isPoweredOn() const {
-    return mPowerMode != hal::PowerMode::OFF;
+    return mPowerMode && *mPowerMode != hal::PowerMode::OFF;
 }
 
 void DisplayDevice::setActiveMode(DisplayModeId id) {
@@ -373,7 +375,7 @@ void DisplayDevice::dump(std::string& result) const {
     }
 
     result += "\n   powerMode="s;
-    result += to_string(mPowerMode);
+    result += mPowerMode.has_value() ? to_string(mPowerMode.value()) : "OFF(reset)";
     result += '\n';
 
     if (mRefreshRateConfigs) {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 2161436d44..fc24a9ce49 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -181,7 +181,7 @@ public:
     /* ------------------------------------------------------------------------
      * Display power mode management.
      */
-    hardware::graphics::composer::hal::PowerMode getPowerMode() const;
+    std::optional<hardware::graphics::composer::hal::PowerMode> getPowerMode() const;
     void setPowerMode(hardware::graphics::composer::hal::PowerMode mode);
     bool isPoweredOn() const;
 
@@ -281,8 +281,8 @@ private:
 
     static ui::Transform::RotationFlags sPrimaryDisplayRotationFlags;
 
-    hardware::graphics::composer::hal::PowerMode mPowerMode =
-            hardware::graphics::composer::hal::PowerMode::OFF;
+     // allow initial power mode as null.
+    std::optional<hardware::graphics::composer::hal::PowerMode> mPowerMode;
     DisplayModePtr mActiveMode;
     std::optional<float> mStagedBrightness = std::nullopt;
     float mBrightness = -1.f;
@@ -366,8 +366,7 @@ struct DisplayDeviceCreationArgs {
     HdrCapabilities hdrCapabilities;
     int32_t supportedPerFrameMetadata{0};
     std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>> hwcColorModes;
-    hardware::graphics::composer::hal::PowerMode initialPowerMode{
-            hardware::graphics::composer::hal::PowerMode::ON};
+    std::optional<hardware::graphics::composer::hal::PowerMode> initialPowerMode;
     bool isPrimary{false};
     DisplayModes supportedModes;
     DisplayModeId activeModeId;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fd824dc502..db2447c40b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2889,7 +2889,8 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
     ALOGV("Display Orientation: %s", toCString(creationArgs.physicalOrientation));
 
     // virtual displays are always considered enabled
-    creationArgs.initialPowerMode = state.isVirtual() ? hal::PowerMode::ON : hal::PowerMode::OFF;
+    creationArgs.initialPowerMode =
+            state.isVirtual() ? std::make_optional(hal::PowerMode::ON) : std::nullopt;
 
     sp<DisplayDevice> display = getFactory().createDisplayDevice(creationArgs);
 
@@ -4870,8 +4871,8 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
     const auto displayId = display->getPhysicalId();
     ALOGD("Setting power mode %d on display %s", mode, to_string(displayId).c_str());
 
-    const hal::PowerMode currentMode = display->getPowerMode();
-    if (mode == currentMode) {
+    std::optional<hal::PowerMode> currentMode = display->getPowerMode();
+    if (currentMode.has_value() && mode == *currentMode) {
         return;
     }
 
@@ -4887,7 +4888,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
         mInterceptor->savePowerModeUpdate(display->getSequenceId(), static_cast<int32_t>(mode));
     }
     const auto refreshRate = display->refreshRateConfigs().getActiveMode()->getFps();
-    if (currentMode == hal::PowerMode::OFF) {
+    if (*currentMode == hal::PowerMode::OFF) {
         // Turn on the display
         if (display->isInternal() && (!activeDisplay || !activeDisplay->isPoweredOn())) {
             onActiveDisplayChangedLocked(display);
@@ -4918,7 +4919,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
         if (SurfaceFlinger::setSchedAttr(false) != NO_ERROR) {
             ALOGW("Couldn't set uclamp.min on display off: %s\n", strerror(errno));
         }
-        if (isDisplayActiveLocked(display) && currentMode != hal::PowerMode::DOZE_SUSPEND) {
+        if (isDisplayActiveLocked(display) && *currentMode != hal::PowerMode::DOZE_SUSPEND) {
             mScheduler->disableHardwareVsync(true);
             mScheduler->onScreenReleased(mAppConnectionHandle);
         }
@@ -4932,7 +4933,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal:
     } else if (mode == hal::PowerMode::DOZE || mode == hal::PowerMode::ON) {
         // Update display while dozing
         getHwComposer().setPowerMode(displayId, mode);
-        if (isDisplayActiveLocked(display) && currentMode == hal::PowerMode::DOZE_SUSPEND) {
+        if (isDisplayActiveLocked(display) && *currentMode == hal::PowerMode::DOZE_SUSPEND) {
             mScheduler->onScreenAcquired(mAppConnectionHandle);
             mScheduler->resyncToHardwareVsync(true, refreshRate);
         }
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index b70fdcd93e..283f9ca77b 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -745,6 +745,7 @@ public:
                 mHwcDisplayId(hwcDisplayId) {
             mCreationArgs.connectionType = connectionType;
             mCreationArgs.isPrimary = isPrimary;
+            mCreationArgs.initialPowerMode = hal::PowerMode::ON;
         }
 
         sp<IBinder> token() const { return mDisplayToken; }
-- 
cgit v1.2.3-59-g8ed1b


From 3b3e59185dc1e9a319d8ce20ac19c30a966a5a9c Mon Sep 17 00:00:00 2001
From: Alec Mouri <alecmouri@google.com>
Date: Mon, 26 Sep 2022 21:37:01 +0000
Subject: Fix use-after-free in SurfaceFlinger::doDump

SurfaceFlinger::doDump previously contained two layer traversals, one on
the main thread and one off the main thread. During concurrent dumpsys
commands, the layer traversals may race with each other, which causes
shared ownership of the underlying storage of a SortedVector containing
the z-ordered list of layers. Because the implementation of
SortedVector's STL iterators assumes that the underlying storage may be
edited, this can cause the storage to be copied whenever SortedVector::begin
and SortedVector::end are called, which means that SortedVector::begin()
+ SortedVector::size() == SortedVector::end() is not always true, which
causes invalid iteration.

In general, this use-after-free can happen as long as the off-main
thread traversal exists in doDump(), because the traversal can run in
parallel with any workload on the main thread that executes a layer
traversal. So, this patch moves the traversal for dumping out the list
of composition layers into the main thread.

A future patch could explore either fixing SortedVector to fix judicious
iterator invalidation, or building LayerVector on top of std::set, but
either option is an invasive data structure change.

Bug: 237291506
Test: Test script that calls dumpsys SurfaceFlinger on many threads
Change-Id: I0748396519c924dc1b84113d44259f22d0d7ebd6
(cherry picked from commit 6761733ad1dd775f011588c59d5a6d210175c546)
Merged-In: I0748396519c924dc1b84113d44259f22d0d7ebd6
---
 services/surfaceflinger/SurfaceFlinger.cpp | 37 +++++++++++++++++++-----------
 services/surfaceflinger/SurfaceFlinger.h   |  3 ++-
 2 files changed, 25 insertions(+), 15 deletions(-)

(limited to 'services/surfaceflinger/SurfaceFlinger.cpp')

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index db2447c40b..0e1acb4154 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5005,6 +5005,25 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {
 
         const auto flag = args.empty() ? ""s : std::string(String8(args[0]));
 
+        // Traversal of drawing state must happen on the main thread.
+        // Otherwise, SortedVector may have shared ownership during concurrent
+        // traversals, which can result in use-after-frees.
+        std::string compositionLayers;
+        mScheduler
+                ->schedule([&] {
+                    StringAppendF(&compositionLayers, "Composition layers\n");
+                    mDrawingState.traverseInZOrder([&](Layer* layer) {
+                        auto* compositionState = layer->getCompositionState();
+                        if (!compositionState || !compositionState->isVisible) return;
+
+                        android::base::StringAppendF(&compositionLayers, "* Layer %p (%s)\n", layer,
+                                                     layer->getDebugName() ? layer->getDebugName()
+                                                                           : "<unknown>");
+                        compositionState->dump(compositionLayers);
+                    });
+                })
+                .get();
+
         bool dumpLayers = true;
         {
             TimedLock lock(mStateLock, s2ns(1), __func__);
@@ -5017,7 +5036,7 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {
                 (it->second)(args, asProto, result);
                 dumpLayers = false;
             } else if (!asProto) {
-                dumpAllLocked(args, result);
+                dumpAllLocked(args, compositionLayers, result);
             }
         }
 
@@ -5316,7 +5335,8 @@ void SurfaceFlinger::dumpOffscreenLayers(std::string& result) {
     result.append(future.get());
 }
 
-void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) const {
+void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,
+                                   std::string& result) const {
     const bool colorize = !args.empty() && args[0] == String16("--color");
     Colorizer colorizer(colorize);
 
@@ -5367,18 +5387,7 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co
     StringAppendF(&result, "Visible layers (count = %zu)\n", mNumLayers.load());
     colorizer.reset(result);
 
-    {
-        StringAppendF(&result, "Composition layers\n");
-        mDrawingState.traverseInZOrder([&](Layer* layer) {
-            auto* compositionState = layer->getCompositionState();
-            if (!compositionState || !compositionState->isVisible) return;
-
-            android::base::StringAppendF(&result, "* Layer %p (%s)\n", layer,
-                                         layer->getDebugName() ? layer->getDebugName()
-                                                               : "<unknown>");
-            compositionState->dump(result);
-        });
-    }
+    result.append(compositionLayers);
 
     colorizer.bold(result);
     StringAppendF(&result, "Displays (%zu entries)\n", mDisplays.size());
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 83134a2ebc..9e0cee8fd4 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1085,7 +1085,8 @@ private:
     /*
      * Debugging & dumpsys
      */
-    void dumpAllLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);
+    void dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,
+                       std::string& result) const REQUIRES(mStateLock);
 
     void appendSfConfigString(std::string& result) const;
     void listLayersLocked(std::string& result) const;
-- 
cgit v1.2.3-59-g8ed1b