Fix FenceTracker releaseFence
This patch:
* Fixes the release fence when GPU compositing.
* Stores the final release fence in ConsumerBase just
before releasing the Buffer, which helps ensure
sync points aren't added unknowningly.
* Makes HWC2 release pending buffers as the first step
of postCompostion, rather than the last, which should
allow dequeue to unblock a little earlier and helps
make sure the previous buffer's release fence has
been finalized before FenceTracker::addFrame is
called.
* Fence tracker only sets the release fence once it
has been finalized so it does not report a release
fence for a buffer that is still latched.
Test: adb shell /data/nativetest/libgui_test/libgui_test
--gtest_filter=*GetFrameTimestamps*
Change-Id: I27d484bfd48f730bdcea2628f96795c6f4b4df7b
diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 9f8b638..ce85fc3 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -180,7 +180,7 @@
// Derived classes should override this method to perform any cleanup that
// must take place when a buffer is released back to the BufferQueue. If
// it is overridden the derived class's implementation must call
- // ConsumerBase::releaseBufferLocked.e
+ // ConsumerBase::releaseBufferLocked.
virtual status_t releaseBufferLocked(int slot,
const sp<GraphicBuffer> graphicBuffer,
EGLDisplay display, EGLSyncKHR eglFence);
@@ -244,6 +244,10 @@
// if none is supplied
sp<IGraphicBufferConsumer> mConsumer;
+ // The final release fence of the most recent buffer released by
+ // releaseBufferLocked.
+ sp<Fence> mPrevFinalReleaseFence;
+
// mMutex is the mutex used to prevent concurrent access to the member
// variables of ConsumerBase objects. It must be locked whenever the
// member variables are accessed or when any of the *Locked methods are
diff --git a/include/gui/GLConsumer.h b/include/gui/GLConsumer.h
index 6267625..6ff1303 100644
--- a/include/gui/GLConsumer.h
+++ b/include/gui/GLConsumer.h
@@ -250,7 +250,7 @@
// mEglSlots array in addition to the ConsumerBase.
virtual status_t releaseBufferLocked(int slot,
const sp<GraphicBuffer> graphicBuffer,
- EGLDisplay display, EGLSyncKHR eglFence);
+ EGLDisplay display, EGLSyncKHR eglFence) override;
status_t releaseBufferLocked(int slot,
const sp<GraphicBuffer> graphicBuffer, EGLSyncKHR eglFence) {
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index 3cf3078..be2b1af 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -56,7 +56,8 @@
ConsumerBase::ConsumerBase(const sp<IGraphicBufferConsumer>& bufferQueue, bool controlledByApp) :
mAbandoned(false),
- mConsumer(bufferQueue) {
+ mConsumer(bufferQueue),
+ mPrevFinalReleaseFence(Fence::NO_FENCE) {
// Choose a name using the PID and a process-unique ID.
mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId());
@@ -366,6 +367,7 @@
freeBufferLocked(slot);
}
+ mPrevFinalReleaseFence = mSlots[slot].mFence;
mSlots[slot].mFence = Fence::NO_FENCE;
return err;
diff --git a/services/surfaceflinger/FenceTracker.cpp b/services/surfaceflinger/FenceTracker.cpp
index 742c00d..4dac0db 100644
--- a/services/surfaceflinger/FenceTracker.cpp
+++ b/services/surfaceflinger/FenceTracker.cpp
@@ -158,38 +158,17 @@
layers[i]->getFenceData(&name, &frameNumber, &glesComposition,
&requestedPresentTime, &acquireFence, &prevReleaseFence);
-#ifdef USE_HWC2
- if (glesComposition) {
- frame.layers.emplace(std::piecewise_construct,
- std::forward_as_tuple(layerId),
- std::forward_as_tuple(name, frameNumber, glesComposition,
- requestedPresentTime, 0, 0, acquireFence,
- prevReleaseFence));
- wasGlesCompositionDone = true;
- } else {
- frame.layers.emplace(std::piecewise_construct,
- std::forward_as_tuple(layerId),
- std::forward_as_tuple(name, frameNumber, glesComposition,
- requestedPresentTime, 0, 0, acquireFence, Fence::NO_FENCE));
- auto prevLayer = prevFrame.layers.find(layerId);
- if (prevLayer != prevFrame.layers.end()) {
- prevLayer->second.releaseFence = prevReleaseFence;
+
+ frame.layers.emplace(std::piecewise_construct,
+ std::forward_as_tuple(layerId),
+ std::forward_as_tuple(name, frameNumber, glesComposition,
+ requestedPresentTime, 0, 0, acquireFence, Fence::NO_FENCE));
+ auto prevLayer = prevFrame.layers.find(layerId);
+ if (prevLayer != prevFrame.layers.end()) {
+ if (frameNumber != prevLayer->second.frameNumber) {
+ prevLayer->second.releaseFence = prevReleaseFence;
}
}
-#else
- frame.layers.emplace(std::piecewise_construct,
- std::forward_as_tuple(layerId),
- std::forward_as_tuple(name, frameNumber, glesComposition,
- requestedPresentTime, 0, 0, acquireFence,
- glesComposition ? Fence::NO_FENCE : prevReleaseFence));
- if (glesComposition) {
- wasGlesCompositionDone = true;
- }
-#endif
- frame.layers.emplace(std::piecewise_construct,
- std::forward_as_tuple(layerId),
- std::forward_as_tuple(name, frameNumber, glesComposition,
- requestedPresentTime, 0, 0, acquireFence, prevReleaseFence));
}
frame.frameId = mFrameCounter;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 20c0261..4772fc1 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2173,7 +2173,7 @@
#endif
*outRequestedPresentTime = mSurfaceFlingerConsumer->getTimestamp();
*outAcquireFence = mSurfaceFlingerConsumer->getCurrentFence();
- *outPrevReleaseFence = mSurfaceFlingerConsumer->getPrevReleaseFence();
+ *outPrevReleaseFence = mSurfaceFlingerConsumer->getPrevFinalReleaseFence();
}
std::vector<OccupancyTracker::Segment> Layer::getOccupancyHistory(
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1697540..2f273ae 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1148,10 +1148,6 @@
mHwc->hasClientComposition(displayDevice->getHwcDisplayId());
}
- // Release any buffers which were replaced this frame
- for (auto& layer : mLayersWithQueuedFrames) {
- layer->releasePendingBuffer();
- }
mLayersWithQueuedFrames.clear();
}
@@ -1222,6 +1218,11 @@
ATRACE_CALL();
ALOGV("postComposition");
+ // Release any buffers which were replaced this frame
+ for (auto& layer : mLayersWithQueuedFrames) {
+ layer->releasePendingBuffer();
+ }
+
const LayerVector& layers(mDrawingState.layersSortedByZ);
const size_t count = layers.size();
for (size_t i=0 ; i<count ; i++) {
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.cpp b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
index 6f2520b..5317cc7 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.cpp
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
@@ -189,10 +189,14 @@
return nextRefresh + extraPadding;
}
+sp<Fence> SurfaceFlingerConsumer::getPrevFinalReleaseFence() const {
+ Mutex::Autolock lock(mMutex);
+ return ConsumerBase::mPrevFinalReleaseFence;
+}
+
#ifdef USE_HWC2
void SurfaceFlingerConsumer::setReleaseFence(const sp<Fence>& fence)
{
- mPrevReleaseFence = fence;
if (!mPendingRelease.isPending) {
GLConsumer::setReleaseFence(fence);
return;
@@ -222,17 +226,8 @@
strerror(-result), result);
mPendingRelease = PendingRelease();
}
-#else
-void SurfaceFlingerConsumer::setReleaseFence(const sp<Fence>& fence) {
- mPrevReleaseFence = fence;
- GLConsumer::setReleaseFence(fence);
-}
#endif
-sp<Fence> SurfaceFlingerConsumer::getPrevReleaseFence() const {
- return mPrevReleaseFence;
-}
-
void SurfaceFlingerConsumer::setContentsChangedListener(
const wp<ContentsChangedListener>& listener) {
setFrameAvailableListener(listener);
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.h b/services/surfaceflinger/SurfaceFlingerConsumer.h
index 4271039..cb4c334 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.h
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.h
@@ -39,8 +39,7 @@
SurfaceFlingerConsumer(const sp<IGraphicBufferConsumer>& consumer,
uint32_t tex, const Layer* layer)
: GLConsumer(consumer, tex, GLConsumer::TEXTURE_EXTERNAL, false, false),
- mTransformToDisplayInverse(false), mSurfaceDamage(),
- mPrevReleaseFence(Fence::NO_FENCE), mLayer(layer)
+ mTransformToDisplayInverse(false), mSurfaceDamage(), mLayer(layer)
{}
class BufferRejecter {
@@ -61,7 +60,7 @@
// texture.
status_t updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync,
bool* autoRefresh, bool* queuedBuffer,
- uint64_t maxFrameNumber = 0);
+ uint64_t maxFrameNumber);
// See GLConsumer::bindTextureImageLocked().
status_t bindTextureImage();
@@ -79,9 +78,9 @@
nsecs_t computeExpectedPresent(const DispSync& dispSync);
- virtual void setReleaseFence(const sp<Fence>& fence) override;
- sp<Fence> getPrevReleaseFence() const;
+ sp<Fence> getPrevFinalReleaseFence() const;
#ifdef USE_HWC2
+ virtual void setReleaseFence(const sp<Fence>& fence) override;
void releasePendingBuffer();
#endif
@@ -107,9 +106,6 @@
PendingRelease mPendingRelease;
#endif
- // The release fence of the already displayed buffer (previous frame).
- sp<Fence> mPrevReleaseFence;
-
// The layer for this SurfaceFlingerConsumer
wp<const Layer> mLayer;
};