SF: check previous present fence to avoid early presentation
If SurfaceFlinger finishes composition a vsyncPeriod or more before it
planned to, it sleeps before calling composer to present the frame to
avoid early presentation. In this CL we are removing the sleep if the
previous present fence is still pending, as early presentation is not
possible (composer has already a frame queued).
Bug: 190842189
Test: Bouncy ball while forcing client composition
Change-Id: Id913a21b3489973d82498c6154843656972b6e1a
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index 29937fb..554e2f4 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -24,6 +24,7 @@
#include <compositionengine/LayerFE.h>
#include <compositionengine/OutputColorSetting.h>
#include <math/mat4.h>
+#include <ui/FenceTime.h>
#include <ui/Transform.h>
namespace android::compositionengine {
@@ -83,6 +84,10 @@
// The earliest time to send the present command to the HAL
std::chrono::steady_clock::time_point earliestPresentTime;
+ // The previous present fence. Used together with earliestPresentTime
+ // to prevent an early presentation of a frame.
+ std::shared_ptr<FenceTime> previousPresentFence;
+
// The predicted next invalidation time
std::optional<std::chrono::steady_clock::time_point> nextInvalidateTime;
};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index d41c2dd..f34cb94 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -19,6 +19,7 @@
#include <cstdint>
#include <math/mat4.h>
+#include <ui/FenceTime.h>
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic push
@@ -118,6 +119,10 @@
// The earliest time to send the present command to the HAL
std::chrono::steady_clock::time_point earliestPresentTime;
+ // The previous present fence. Used together with earliestPresentTime
+ // to prevent an early presentation of a frame.
+ std::shared_ptr<FenceTime> previousPresentFence;
+
// Current display brightness
float displayBrightnessNits{-1.f};
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index ae1336e..2f2c686 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -229,7 +229,8 @@
auto& hwc = getCompositionEngine().getHwComposer();
if (status_t result =
hwc.getDeviceCompositionChanges(*halDisplayId, anyLayersRequireClientComposition(),
- getState().earliestPresentTime, &changes);
+ getState().earliestPresentTime,
+ getState().previousPresentFence, &changes);
result != NO_ERROR) {
ALOGE("chooseCompositionStrategy failed for %s: %d (%s)", getName().c_str(), result,
strerror(-result));
@@ -330,7 +331,8 @@
}
auto& hwc = getCompositionEngine().getHwComposer();
- hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime);
+ hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime,
+ getState().previousPresentFence);
fences.presentFence = hwc.getPresentFence(*halDisplayIdOpt);
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 67bb149..cafcb40 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -729,6 +729,7 @@
}
editState().earliestPresentTime = refreshArgs.earliestPresentTime;
+ editState().previousPresentFence = refreshArgs.previousPresentFence;
compositionengine::OutputLayer* peekThroughLayer = nullptr;
sp<GraphicBuffer> previousOverride = nullptr;
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index db9437b..c037cc6 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -579,7 +579,7 @@
TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutOnHwcError) {
EXPECT_CALL(*mDisplay, anyLayersRequireClientComposition()).WillOnce(Return(false));
EXPECT_CALL(mHwComposer,
- getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _))
+ getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _, _))
.WillOnce(Return(INVALID_OPERATION));
mDisplay->chooseCompositionStrategy();
@@ -602,7 +602,7 @@
.WillOnce(Return(false));
EXPECT_CALL(mHwComposer,
- getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))
+ getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
.WillOnce(Return(NO_ERROR));
EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false));
@@ -633,8 +633,8 @@
.WillOnce(Return(false));
EXPECT_CALL(mHwComposer,
- getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))
- .WillOnce(DoAll(SetArgPointee<3>(changes), Return(NO_ERROR)));
+ getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
+ .WillOnce(DoAll(SetArgPointee<4>(changes), Return(NO_ERROR)));
EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(changes.changedTypes)).Times(1);
EXPECT_CALL(*mDisplay, applyDisplayRequests(changes.displayRequests)).Times(1);
EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(changes.layerRequests)).Times(1);
@@ -844,7 +844,7 @@
sp<Fence> layer1Fence = new Fence();
sp<Fence> layer2Fence = new Fence();
- EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _))
+ EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _, _))
.Times(1);
EXPECT_CALL(mHwComposer, getPresentFence(HalDisplayId(DEFAULT_DISPLAY_ID)))
.WillOnce(Return(presentFence));
@@ -1020,7 +1020,7 @@
mDisplay->editState().isEnabled = true;
- EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _));
+ EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _, _));
EXPECT_CALL(*mDisplaySurface, onFrameCommitted());
mDisplay->postFramebuffer();
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 64cbea9..a195e58 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -52,14 +52,16 @@
std::optional<PhysicalDisplayId>));
MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
- MOCK_METHOD4(getDeviceCompositionChanges,
+ MOCK_METHOD5(getDeviceCompositionChanges,
status_t(HalDisplayId, bool, std::chrono::steady_clock::time_point,
+ const std::shared_ptr<FenceTime>&,
std::optional<android::HWComposer::DeviceRequestedChanges>*));
MOCK_METHOD5(setClientTarget,
status_t(HalDisplayId, uint32_t, const sp<Fence>&, const sp<GraphicBuffer>&,
ui::Dataspace));
- MOCK_METHOD2(presentAndGetReleaseFences,
- status_t(HalDisplayId, std::chrono::steady_clock::time_point));
+ MOCK_METHOD3(presentAndGetReleaseFences,
+ status_t(HalDisplayId, std::chrono::steady_clock::time_point,
+ const std::shared_ptr<FenceTime>&));
MOCK_METHOD2(setPowerMode, status_t(PhysicalDisplayId, hal::PowerMode));
MOCK_METHOD2(setActiveConfig, status_t(HalDisplayId, size_t));
MOCK_METHOD2(setColorTransform, status_t(HalDisplayId, const mat4&));
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 32f04e5..7e45dab 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -471,6 +471,7 @@
status_t HWComposer::getDeviceCompositionChanges(
HalDisplayId displayId, bool frameUsesClientComposition,
std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence,
std::optional<android::HWComposer::DeviceRequestedChanges>* outChanges) {
ATRACE_CALL();
@@ -487,12 +488,16 @@
hal::Error error = hal::Error::NONE;
- // First try to skip validate altogether when we passed the earliest time
- // to present and there is no client. Otherwise, we may present a frame too
- // early or in case of client composition we first need to render the
+ // First try to skip validate altogether. We can do that when
+ // 1. The previous frame has not been presented yet or already passed the
+ // earliest time to present. Otherwise, we may present a frame too early.
+ // 2. There is no client composition. Otherwise, we first need to render the
// client target buffer.
- const bool canSkipValidate =
- std::chrono::steady_clock::now() >= earliestPresentTime && !frameUsesClientComposition;
+ const bool prevFencePending =
+ previousPresentFence->getSignalTime() == Fence::SIGNAL_TIME_PENDING;
+ const bool canPresentEarly =
+ !prevFencePending && std::chrono::steady_clock::now() < earliestPresentTime;
+ const bool canSkipValidate = !canPresentEarly && !frameUsesClientComposition;
displayData.validateWasSkipped = false;
if (canSkipValidate) {
sp<Fence> outPresentFence;
@@ -559,7 +564,8 @@
}
status_t HWComposer::presentAndGetReleaseFences(
- HalDisplayId displayId, std::chrono::steady_clock::time_point earliestPresentTime) {
+ HalDisplayId displayId, std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence) {
ATRACE_CALL();
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
@@ -575,7 +581,9 @@
return NO_ERROR;
}
- {
+ const bool previousFramePending =
+ previousPresentFence->getSignalTime() == Fence::SIGNAL_TIME_PENDING;
+ if (!previousFramePending) {
ATRACE_NAME("wait for earliest present time");
std::this_thread::sleep_until(earliestPresentTime);
}
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index cd6f9f5..b1849e8 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -27,7 +27,7 @@
#include <vector>
#include <android-base/thread_annotations.h>
-#include <ui/Fence.h>
+#include <ui/FenceTime.h>
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic push
@@ -134,6 +134,7 @@
virtual status_t getDeviceCompositionChanges(
HalDisplayId, bool frameUsesClientComposition,
std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence,
std::optional<DeviceRequestedChanges>* outChanges) = 0;
virtual status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
@@ -141,7 +142,8 @@
// Present layers to the display and read releaseFences.
virtual status_t presentAndGetReleaseFences(
- HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime) = 0;
+ HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence) = 0;
// set power mode
virtual status_t setPowerMode(PhysicalDisplayId, hal::PowerMode) = 0;
@@ -275,6 +277,7 @@
status_t getDeviceCompositionChanges(
HalDisplayId, bool frameUsesClientComposition,
std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence,
std::optional<DeviceRequestedChanges>* outChanges) override;
status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
@@ -282,7 +285,8 @@
// Present layers to the display and read releaseFences.
status_t presentAndGetReleaseFences(
- HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime) override;
+ HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
+ const std::shared_ptr<FenceTime>& previousPresentFence) override;
// set power mode
status_t setPowerMode(PhysicalDisplayId, hal::PowerMode mode) override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e2f3ebb..813eebe 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2070,6 +2070,7 @@
const auto prevVsyncTime = mScheduler->getPreviousVsyncFrom(mExpectedPresentTime);
const auto hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration;
refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration;
+ refreshArgs.previousPresentFence = mPreviousPresentFences[0].fenceTime;
refreshArgs.nextInvalidateTime = mEventQueue->nextExpectedInvalidate();
mGeometryInvalid = false;