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

8 files changed, 31 insertions, 54 deletions

diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 9f8b638a3a..ce85fc3e8d 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -180,7 +180,7 @@ protected:
     // 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 @@ protected:
     // 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 6267625e96..6ff130364b 100644
--- a/include/gui/GLConsumer.h
+++ b/include/gui/GLConsumer.h
@@ -250,7 +250,7 @@ protected:
     // 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 3cf3078345..be2b1afd36 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -56,7 +56,8 @@ static int32_t createProcessUniqueId() {
 
 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 @@ status_t ConsumerBase::releaseBufferLocked(
         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 742c00d951..4dac0dbc4c 100644
--- a/services/surfaceflinger/FenceTracker.cpp
+++ b/services/surfaceflinger/FenceTracker.cpp
@@ -158,38 +158,17 @@ void FenceTracker::addFrame(nsecs_t refreshStartTime, sp<Fence> presentFence,
 
         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;
-            }
-        }
-#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;
+                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;
+            }
         }
-#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 20c0261017..4772fc11ab 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2173,7 +2173,7 @@ void Layer::getFenceData(String8* outName, uint64_t* outFrameNumber,
 #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 16975409d1..2f273ae909 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1148,10 +1148,6 @@ void SurfaceFlinger::handleMessageRefresh() {
                 mHwc->hasClientComposition(displayDevice->getHwcDisplayId());
     }
 
-    // Release any buffers which were replaced this frame
-    for (auto& layer : mLayersWithQueuedFrames) {
-        layer->releasePendingBuffer();
-    }
     mLayersWithQueuedFrames.clear();
 }
 
@@ -1222,6 +1218,11 @@ void SurfaceFlinger::postComposition(nsecs_t refreshStartTime)
     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 6f2520be23..5317cc7fd8 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.cpp
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
@@ -189,10 +189,14 @@ nsecs_t SurfaceFlingerConsumer::computeExpectedPresent(const DispSync& dispSync)
     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 @@ void SurfaceFlingerConsumer::releasePendingBuffer()
             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 4271039d25..cb4c334d32 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.h
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.h
@@ -39,8 +39,7 @@ public:
     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 @@ public:
     // 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 @@ public:
 
     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 @@ private:
     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;
 };