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
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
index 579636f..8c164ed 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockPowerAdvisor.h
@@ -55,6 +55,7 @@
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 40b1132..0ed55b2 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -315,6 +315,10 @@
mExpectedPresentTimes.append(expectedPresentTime);
}
+void PowerAdvisor::setPresentFenceTime(nsecs_t presentFenceTime) {
+ mLastPresentFenceTime = presentFenceTime;
+}
+
void PowerAdvisor::setFrameDelay(nsecs_t frameDelayDuration) {
mFrameDelayDuration = frameDelayDuration;
}
@@ -384,11 +388,11 @@
// 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 @@
}
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 @@
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 @@
// 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 @@
// 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 bdc7927..98921b0 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h
@@ -70,7 +70,10 @@
// 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 @@
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 @@
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 @@
// 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 @@
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 b7d6968..6e74eef 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2244,6 +2244,7 @@
// 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 e347883..d6dca45 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockPowerAdvisor.h
@@ -53,6 +53,7 @@
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));