Merge "[RESTRICT AUTOMERGE]gpuservice: always dump all in BugReport" into rvc-dev
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index cd15617..abeacfe 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -87,10 +87,26 @@
return ScheduleResult::Scheduled;
}
+void VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(nsecs_t workDuration,
+ nsecs_t earliestVsync) {
+ mWorkloadUpdateInfo = {.earliestVsync = earliestVsync, .duration = workDuration};
+}
+
+bool VSyncDispatchTimerQueueEntry::hasPendingWorkloadUpdate() const {
+ return mWorkloadUpdateInfo.has_value();
+}
+
void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {
- if (!mArmedInfo) {
+ if (!mArmedInfo && !mWorkloadUpdateInfo) {
return;
}
+
+ if (mWorkloadUpdateInfo) {
+ mEarliestVsync = mWorkloadUpdateInfo->earliestVsync;
+ mWorkDuration = mWorkloadUpdateInfo->duration;
+ mWorkloadUpdateInfo.reset();
+ }
+
auto const nextVsyncTime =
tracker.nextAnticipatedVSyncTimeFrom(std::max(mEarliestVsync, now + mWorkDuration));
mArmedInfo = {nextVsyncTime - mWorkDuration, nextVsyncTime};
@@ -192,7 +208,7 @@
std::optional<std::string_view> nextWakeupName;
for (auto it = mCallbacks.begin(); it != mCallbacks.end(); it++) {
auto& callback = it->second;
- if (!callback->wakeupTime()) {
+ if (!callback->wakeupTime() && !callback->hasPendingWorkloadUpdate()) {
continue;
}
@@ -291,6 +307,15 @@
}
auto& callback = it->second;
auto const now = mTimeKeeper->now();
+
+ /* If the timer thread will run soon, we'll apply this work update via the callback
+ * timer recalculation to avoid cancelling a callback that is about to fire. */
+ auto const rearmImminent = now > mIntendedWakeupTime;
+ if (CC_UNLIKELY(rearmImminent)) {
+ callback->addPendingWorkloadUpdate(workDuration, earliestVsync);
+ return ScheduleResult::Scheduled;
+ }
+
result = callback->schedule(workDuration, earliestVsync, mTracker, now);
if (result == ScheduleResult::CannotSchedule) {
return result;
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
index 26a8ec0..957c0d1 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
@@ -64,6 +64,13 @@
// This moves the state from armed->running.
// Store the timestamp that this was intended for as the last called timestamp.
nsecs_t executing();
+
+ // Adds a pending upload of the earliestVSync and workDuration that will be applied on the next
+ // call to update()
+ void addPendingWorkloadUpdate(nsecs_t workDuration, nsecs_t earliestVsync);
+
+ // Checks if there is a pending update to the workload, returning true if so.
+ bool hasPendingWorkloadUpdate() const;
// End: functions that are not threadsafe.
// Invoke the callback with the two given timestamps, moving the state from running->disarmed.
@@ -88,6 +95,12 @@
std::optional<ArmingInfo> mArmedInfo;
std::optional<nsecs_t> mLastDispatchTime;
+ struct WorkloadUpdateInfo {
+ nsecs_t duration;
+ nsecs_t earliestVsync;
+ };
+ std::optional<WorkloadUpdateInfo> mWorkloadUpdateInfo;
+
mutable std::mutex mRunningMutex;
std::condition_variable mCv;
bool mRunning GUARDED_BY(mRunningMutex) = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2e22735..68dec66 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2675,7 +2675,8 @@
}
processDisplayAdded(displayToken, currentState);
if (currentState.physical) {
- initializeDisplays();
+ const auto display = getDisplayDeviceLocked(displayToken);
+ setPowerModeInternal(display, hal::PowerMode::ON);
}
return;
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 1899bed..793cb8b 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -89,15 +89,18 @@
void advanceBy(nsecs_t advancement) {
mCurrentTime += advancement;
- if (mCurrentTime >= mNextCallbackTime && mCallback) {
+ if (mCurrentTime >= (mNextCallbackTime + mLag) && mCallback) {
mCallback();
}
};
+ void setLag(nsecs_t lag) { mLag = lag; }
+
private:
std::function<void()> mCallback;
nsecs_t mNextCallbackTime = 0;
nsecs_t mCurrentTime = 0;
+ nsecs_t mLag = 0;
};
class CountingCallback {
@@ -658,6 +661,46 @@
cb1.cancel();
}
+// b/154303580
+TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent) {
+ Sequence seq;
+ EXPECT_CALL(mMockClock, alarmIn(_, 600)).InSequence(seq);
+ EXPECT_CALL(mMockClock, alarmIn(_, 1200)).InSequence(seq);
+ CountingCallback cb1(mDispatch);
+ CountingCallback cb2(mDispatch);
+
+ EXPECT_EQ(mDispatch.schedule(cb1, 400, 1000), ScheduleResult::Scheduled);
+
+ mMockClock.setLag(100);
+ mMockClock.advanceBy(620);
+
+ EXPECT_EQ(mDispatch.schedule(cb2, 100, 2000), ScheduleResult::Scheduled);
+ mMockClock.advanceBy(80);
+
+ EXPECT_THAT(cb1.mCalls.size(), Eq(1));
+ EXPECT_THAT(cb2.mCalls.size(), Eq(0));
+}
+
+// b/154303580.
+// If the same callback tries to reschedule itself after it's too late, timer opts to apply the
+// update later, as opposed to blocking the calling thread.
+TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminentSameCallback) {
+ Sequence seq;
+ EXPECT_CALL(mMockClock, alarmIn(_, 600)).InSequence(seq);
+ EXPECT_CALL(mMockClock, alarmIn(_, 930)).InSequence(seq);
+ CountingCallback cb(mDispatch);
+
+ EXPECT_EQ(mDispatch.schedule(cb, 400, 1000), ScheduleResult::Scheduled);
+
+ mMockClock.setLag(100);
+ mMockClock.advanceBy(620);
+
+ EXPECT_EQ(mDispatch.schedule(cb, 370, 2000), ScheduleResult::Scheduled);
+ mMockClock.advanceBy(80);
+
+ EXPECT_THAT(cb.mCalls.size(), Eq(1));
+}
+
class VSyncDispatchTimerQueueEntryTest : public testing::Test {
protected:
nsecs_t const mPeriod = 1000;
@@ -817,6 +860,19 @@
EXPECT_THAT(entry.schedule(1200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
}
+TEST_F(VSyncDispatchTimerQueueEntryTest, storesPendingUpdatesUntilUpdate) {
+ static constexpr auto effectualOffset = 200;
+ VSyncDispatchTimerQueueEntry entry(
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
+ EXPECT_FALSE(entry.hasPendingWorkloadUpdate());
+ entry.addPendingWorkloadUpdate(100, 400);
+ entry.addPendingWorkloadUpdate(effectualOffset, 700);
+ EXPECT_TRUE(entry.hasPendingWorkloadUpdate());
+ entry.update(mStubTracker, 0);
+ EXPECT_FALSE(entry.hasPendingWorkloadUpdate());
+ EXPECT_THAT(*entry.wakeupTime(), Eq(mPeriod - effectualOffset));
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues