Merge changes from topic "sf-kernel-idle-timeout" into qt-dev

* changes:
  When building dispsync model, only callback on observed vsync + offset.
  Support repurposing idle timer to work with kernel timer.

11 files changed, 135 insertions, 14 deletions

diff --git a/services/surfaceflinger/Scheduler/DispSync.cpp b/services/surfaceflinger/Scheduler/DispSync.cpp
index abf7b71006..cd6fa41940 100644
--- a/services/surfaceflinger/Scheduler/DispSync.cpp
+++ b/services/surfaceflinger/Scheduler/DispSync.cpp
@@ -64,6 +64,7 @@ public:
     DispSyncThread(const char* name, bool showTraceDetailedInfo)
           : mName(name),
             mStop(false),
+            mModelLocked(false),
             mPeriod(0),
             mPhase(0),
             mReferenceTime(0),
@@ -78,6 +79,11 @@ public:
         Mutex::Autolock lock(mMutex);
 
         mPhase = phase;
+        if (mReferenceTime != referenceTime) {
+            for (auto& eventListener : mEventListeners) {
+                eventListener.mHasFired = false;
+            }
+        }
         mReferenceTime = referenceTime;
         if (mPeriod != 0 && mPeriod != period && mReferenceTime != 0) {
             // Inflate the reference time to be the most recent predicted
@@ -106,6 +112,16 @@ public:
         mCond.signal();
     }
 
+    void lockModel() {
+        Mutex::Autolock lock(mMutex);
+        mModelLocked = true;
+    }
+
+    void unlockModel() {
+        Mutex::Autolock lock(mMutex);
+        mModelLocked = false;
+    }
+
     virtual bool threadLoop() {
         status_t err;
         nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC);
@@ -288,6 +304,7 @@ private:
         nsecs_t mLastEventTime;
         nsecs_t mLastCallbackTime;
         DispSync::Callback* mCallback;
+        bool mHasFired = false;
     };
 
     struct CallbackInvocation {
@@ -335,6 +352,12 @@ private:
                           eventListener.mName);
                     continue;
                 }
+                if (eventListener.mHasFired && !mModelLocked) {
+                    eventListener.mLastEventTime = t;
+                    ALOGV("[%s] [%s] Skipping event due to already firing", mName,
+                          eventListener.mName);
+                    continue;
+                }
                 CallbackInvocation ci;
                 ci.mCallback = eventListener.mCallback;
                 ci.mEventTime = t;
@@ -343,6 +366,7 @@ private:
                 callbackInvocations.push_back(ci);
                 eventListener.mLastEventTime = t;
                 eventListener.mLastCallbackTime = now;
+                eventListener.mHasFired = true;
             }
         }
 
@@ -410,6 +434,7 @@ private:
     const char* const mName;
 
     bool mStop;
+    bool mModelLocked;
 
     nsecs_t mPeriod;
     nsecs_t mPhase;
@@ -497,6 +522,7 @@ void DispSync::resetLocked() {
     mNumResyncSamples = 0;
     mFirstResyncSample = 0;
     mNumResyncSamplesSincePresent = 0;
+    mThread->unlockModel();
     resetErrorLocked();
 }
 
@@ -519,6 +545,7 @@ bool DispSync::addPresentFence(const std::shared_ptr<FenceTime>& fenceTime) {
 void DispSync::beginResync() {
     Mutex::Autolock lock(mMutex);
     ALOGV("[%s] beginResync", mName);
+    mThread->unlockModel();
     mModelUpdated = false;
     mNumResyncSamples = 0;
 }
@@ -581,10 +608,15 @@ bool DispSync::addResyncSample(nsecs_t timestamp, bool* periodChanged) {
     // resync again
     bool modelLocked = mModelUpdated && mError < (kErrorThreshold / 2) && mPendingPeriod == 0;
     ALOGV("[%s] addResyncSample returning %s", mName, modelLocked ? "locked" : "unlocked");
+    if (modelLocked) {
+        mThread->lockModel();
+    }
     return !modelLocked;
 }
 
-void DispSync::endResync() {}
+void DispSync::endResync() {
+    mThread->lockModel();
+}
 
 status_t DispSync::addEventListener(const char* name, nsecs_t phase, Callback* callback,
                                     nsecs_t lastCallbackTime) {
diff --git a/services/surfaceflinger/Scheduler/IdleTimer.cpp b/services/surfaceflinger/Scheduler/IdleTimer.cpp
index b28b1aa051..37fdfc7c62 100644
--- a/services/surfaceflinger/Scheduler/IdleTimer.cpp
+++ b/services/surfaceflinger/Scheduler/IdleTimer.cpp
@@ -84,8 +84,11 @@ void IdleTimer::loop() {
                 constexpr auto zero = std::chrono::steady_clock::duration::zero();
                 auto waitTime = triggerTime - std::chrono::steady_clock::now();
                 if (waitTime > zero) mCondition.wait_for(mMutex, waitTime);
-                if (mState == TimerState::WAITING &&
-                    (triggerTime - std::chrono::steady_clock::now()) <= zero) {
+                if (mState == TimerState::RESET) {
+                    triggerTime = std::chrono::steady_clock::now() + mInterval;
+                    mState = TimerState::WAITING;
+                } else if (mState == TimerState::WAITING &&
+                           (triggerTime - std::chrono::steady_clock::now()) <= zero) {
                     triggerTimeout = true;
                     mState = TimerState::IDLE;
                 }
diff --git a/services/surfaceflinger/Scheduler/IdleTimer.h b/services/surfaceflinger/Scheduler/IdleTimer.h
index 19f1267c11..2646688860 100644
--- a/services/surfaceflinger/Scheduler/IdleTimer.h
+++ b/services/surfaceflinger/Scheduler/IdleTimer.h
@@ -39,13 +39,31 @@ public:
               const TimeoutCallback& timeoutCallback);
     ~IdleTimer();
 
+    // Initializes and turns on the idle timer.
     void start();
+    // Stops the idle timer and any held resources.
     void stop();
+    // Resets the wakeup time and fires the reset callback.
     void reset();
 
 private:
     // Enum to track in what state is the timer.
-    enum class TimerState { STOPPED = 0, RESET = 1, WAITING = 2, IDLE = 3 };
+    enum class TimerState {
+        // The internal timer thread has been destroyed, and no state is
+        // tracked.
+        // Possible state transitions: RESET
+        STOPPED = 0,
+        // An external thread has just reset this timer.
+        // If there is a reset callback, then that callback is fired.
+        // Possible state transitions: STOPPED, WAITING
+        RESET = 1,
+        // This timer is waiting for the timeout interval to expire.
+        // Possible state transaitions: STOPPED, RESET, IDLE
+        WAITING = 2,
+        // The timeout interval has expired, so we are sleeping now.
+        // Possible state transaitions: STOPPED, RESET
+        IDLE = 3
+    };
 
     // Function that loops until the condition for stopping is met.
     void loop();
@@ -59,6 +77,7 @@ private:
     // Lock used for synchronizing the waiting thread with the application thread.
     std::mutex mMutex;
 
+    // Current timer state
     TimerState mState GUARDED_BY(mMutex) = TimerState::RESET;
 
     // Interval after which timer expires.
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 5b87a3da69..3f9a88df75 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -73,6 +73,7 @@ Scheduler::Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function,
     mEventControlThread = std::make_unique<impl::EventControlThread>(function);
 
     mSetIdleTimerMs = set_idle_timer_ms(0);
+    mSupportKernelTimer = support_kernel_idle_timer(false);
 
     char value[PROPERTY_VALUE_MAX];
     property_get("debug.sf.set_idle_timer_ms", value, "0");
@@ -82,10 +83,20 @@ Scheduler::Scheduler(impl::EventControlThread::SetVSyncEnabledFunction function,
     }
 
     if (mSetIdleTimerMs > 0) {
-        mIdleTimer =
-                std::make_unique<scheduler::IdleTimer>(std::chrono::milliseconds(mSetIdleTimerMs),
-                                                       [this] { resetTimerCallback(); },
-                                                       [this] { expiredTimerCallback(); });
+        if (mSupportKernelTimer) {
+            mIdleTimer =
+                    std::make_unique<scheduler::IdleTimer>(std::chrono::milliseconds(
+                                                                   mSetIdleTimerMs),
+                                                           [this] { resetKernelTimerCallback(); },
+                                                           [this] {
+                                                               expiredKernelTimerCallback();
+                                                           });
+        } else {
+            mIdleTimer = std::make_unique<scheduler::IdleTimer>(std::chrono::milliseconds(
+                                                                        mSetIdleTimerMs),
+                                                                [this] { resetTimerCallback(); },
+                                                                [this] { expiredTimerCallback(); });
+        }
         mIdleTimer->start();
     }
 }
@@ -354,6 +365,11 @@ void Scheduler::setChangeRefreshRateCallback(
     mChangeRefreshRateCallback = changeRefreshRateCallback;
 }
 
+void Scheduler::setGetVsyncPeriodCallback(const GetVsyncPeriod&& getVsyncPeriod) {
+    std::lock_guard<std::mutex> lock(mCallbackLock);
+    mGetVsyncPeriod = getVsyncPeriod;
+}
+
 void Scheduler::updateFrameSkipping(const int64_t skipCount) {
     ATRACE_INT("FrameSkipCount", skipCount);
     if (mSkipCount != skipCount) {
@@ -370,17 +386,30 @@ void Scheduler::resetIdleTimer() {
 }
 
 void Scheduler::resetTimerCallback() {
-    // We do not notify the applications about config changes when idle timer is reset.
     timerChangeRefreshRate(IdleTimerState::RESET);
     ATRACE_INT("ExpiredIdleTimer", 0);
 }
 
+void Scheduler::resetKernelTimerCallback() {
+    ATRACE_INT("ExpiredKernelIdleTimer", 0);
+    std::lock_guard<std::mutex> lock(mCallbackLock);
+    if (mGetVsyncPeriod) {
+        resyncToHardwareVsync(false, mGetVsyncPeriod());
+    }
+}
+
 void Scheduler::expiredTimerCallback() {
-    // We do not notify the applications about config changes when idle timer expires.
     timerChangeRefreshRate(IdleTimerState::EXPIRED);
     ATRACE_INT("ExpiredIdleTimer", 1);
 }
 
+void Scheduler::expiredKernelTimerCallback() {
+    ATRACE_INT("ExpiredKernelIdleTimer", 1);
+    // Disable HW Vsync if the timer expired, as we don't need it
+    // enabled if we're not pushing frames.
+    disableHardwareVsync(false);
+}
+
 std::string Scheduler::doDump() {
     std::ostringstream stream;
     stream << "+  Idle timer interval: " << mSetIdleTimerMs << " ms" << std::endl;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 76b1ee314d..7c8adf0036 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -160,6 +160,7 @@ public:
     void updateFpsBasedOnContent();
     // Callback that gets invoked when Scheduler wants to change the refresh rate.
     void setChangeRefreshRateCallback(const ChangeRefreshRateCallback& changeRefreshRateCallback);
+    void setGetVsyncPeriodCallback(const GetVsyncPeriod&& getVsyncPeriod);
 
     // Returns whether idle timer is enabled or not
     bool isIdleTimerEnabled() { return mSetIdleTimerMs > 0; }
@@ -194,6 +195,14 @@ private:
     void resetTimerCallback();
     // Function that is called when the timer expires.
     void expiredTimerCallback();
+    // Function that is called when the timer resets when paired with a display
+    // driver timeout in the kernel. This enables hardware vsync when we move
+    // out from idle.
+    void resetKernelTimerCallback();
+    // Function that is called when the timer expires when paired with a display
+    // driver timeout in the kernel. This disables hardware vsync when we move
+    // into idle.
+    void expiredKernelTimerCallback();
     // Sets vsync period.
     void setVsyncPeriod(const nsecs_t period);
     // Idle timer feature's function to change the refresh rate.
@@ -245,9 +254,13 @@ private:
     // interval, a callback is fired. Set this variable to >0 to use this feature.
     int64_t mSetIdleTimerMs = 0;
     std::unique_ptr<scheduler::IdleTimer> mIdleTimer;
+    // Enables whether to use idle timer callbacks that support the kernel
+    // timer.
+    bool mSupportKernelTimer;
 
     std::mutex mCallbackLock;
     ChangeRefreshRateCallback mChangeRefreshRateCallback GUARDED_BY(mCallbackLock);
+    GetVsyncPeriod mGetVsyncPeriod GUARDED_BY(mCallbackLock);
 
     // In order to make sure that the features don't override themselves, we need a state machine
     // to keep track which feature requested the config change.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3d71267bb3..3f2240ee57 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -704,6 +704,10 @@ void SurfaceFlinger::init() {
                 Mutex::Autolock lock(mStateLock);
                 setRefreshRateTo(type, event);
             });
+    mScheduler->setGetVsyncPeriodCallback([this] {
+        Mutex::Autolock lock(mStateLock);
+        return getVsyncPeriod();
+    });
 
     mRefreshRateConfigs.populate(getHwComposer().getConfigs(*display->getId()));
     mRefreshRateStats.setConfigMode(getHwComposer().getActiveConfigIndex(*display->getId()));
diff --git a/services/surfaceflinger/SurfaceFlingerProperties.cpp b/services/surfaceflinger/SurfaceFlingerProperties.cpp
index 3522429a82..c43d095341 100644
--- a/services/surfaceflinger/SurfaceFlingerProperties.cpp
+++ b/services/surfaceflinger/SurfaceFlingerProperties.cpp
@@ -246,6 +246,14 @@ bool enable_protected_contents(bool defaultValue) {
     return defaultValue;
 }
 
+bool support_kernel_idle_timer(bool defaultValue) {
+    auto temp = SurfaceFlingerProperties::support_kernel_idle_timer();
+    if (temp.has_value()) {
+        return *temp;
+    }
+    return defaultValue;
+}
+
 #define DISPLAY_PRIMARY_SIZE 3
 
 constexpr float kSrgbRedX = 0.4123f;
diff --git a/services/surfaceflinger/SurfaceFlingerProperties.h b/services/surfaceflinger/SurfaceFlingerProperties.h
index 18642901d1..d772fa84a3 100644
--- a/services/surfaceflinger/SurfaceFlingerProperties.h
+++ b/services/surfaceflinger/SurfaceFlingerProperties.h
@@ -76,6 +76,8 @@ bool use_smart_90_for_video(bool defaultValue);
 
 bool enable_protected_contents(bool defaultValue);
 
+bool support_kernel_idle_timer(bool defaultValue);
+
 android::ui::DisplayPrimaries getDisplayNativePrimaries();
 } // namespace sysprop
 } // namespace android
diff --git a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
index d3690967dc..bd8aacdff6 100644
--- a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
+++ b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
@@ -315,3 +315,13 @@ prop {
     access: Readonly
     prop_name: "ro.surface_flinger.protected_contents"
 }
+
+# Indicates whether Scheduler's idle timer should support a display driver timeout in the kernel.
+# The value of set_idle_timer_ms should be shorter in time than the timeout duration in the kernel.
+prop {
+    api_name: "support_kernel_idle_timer"
+    type: Boolean
+    scope: System
+    access: Readonly
+    prop_name: "ro.surface_flinger.support_kernel_idle_timer"
+}
diff --git a/services/surfaceflinger/sysprop/api/system-current.txt b/services/surfaceflinger/sysprop/api/system-current.txt
index 3c39b51f9b..dd5dbe04b3 100644
--- a/services/surfaceflinger/sysprop/api/system-current.txt
+++ b/services/surfaceflinger/sysprop/api/system-current.txt
@@ -19,6 +19,7 @@ package android.sysprop {
     method public static java.util.Optional<java.lang.Boolean> running_without_sync_framework();
     method public static java.util.Optional<java.lang.Integer> set_idle_timer_ms();
     method public static java.util.Optional<java.lang.Boolean> start_graphics_allocator_service();
+    method public static java.util.Optional<java.lang.Boolean> support_kernel_idle_timer();
     method public static java.util.Optional<java.lang.Boolean> use_color_management();
     method public static java.util.Optional<java.lang.Boolean> use_context_priority();
     method public static java.util.Optional<java.lang.Boolean> use_smart_90_for_video();
diff --git a/services/surfaceflinger/tests/unittests/IdleTimerTest.cpp b/services/surfaceflinger/tests/unittests/IdleTimerTest.cpp
index ea39bf561b..eff22b6640 100644
--- a/services/surfaceflinger/tests/unittests/IdleTimerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/IdleTimerTest.cpp
@@ -111,19 +111,19 @@ TEST_F(IdleTimerTest, resetBackToBackTest) {
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
 
     mIdleTimer->reset();
-    EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
+    EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value());
     EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
 
     mIdleTimer->reset();
-    EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
+    EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value());
     EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
 
     mIdleTimer->reset();
-    EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
+    EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value());
     EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
 
     mIdleTimer->reset();
-    EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
+    EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value());
     EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
 
     // A single callback should be generated after 30ms