Merge "Assign previous release fence to previous frame ID" into android14-gsi

10 files changed, 116 insertions, 15 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 5c324b29cd..5b6740f072 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -99,12 +99,11 @@ void BLASTBufferItemConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry
     }
 }
 
-void BLASTBufferItemConsumer::updateFrameTimestamps(uint64_t frameNumber, nsecs_t refreshStartTime,
-                                                    const sp<Fence>& glDoneFence,
-                                                    const sp<Fence>& presentFence,
-                                                    const sp<Fence>& prevReleaseFence,
-                                                    CompositorTiming compositorTiming,
-                                                    nsecs_t latchTime, nsecs_t dequeueReadyTime) {
+void BLASTBufferItemConsumer::updateFrameTimestamps(
+        uint64_t frameNumber, uint64_t previousFrameNumber, nsecs_t refreshStartTime,
+        const sp<Fence>& glDoneFence, const sp<Fence>& presentFence,
+        const sp<Fence>& prevReleaseFence, CompositorTiming compositorTiming, nsecs_t latchTime,
+        nsecs_t dequeueReadyTime) {
     Mutex::Autolock lock(mMutex);
 
     // if the producer is not connected, don't bother updating,
@@ -115,10 +114,13 @@ void BLASTBufferItemConsumer::updateFrameTimestamps(uint64_t frameNumber, nsecs_
     std::shared_ptr<FenceTime> releaseFenceTime = std::make_shared<FenceTime>(prevReleaseFence);
 
     mFrameEventHistory.addLatch(frameNumber, latchTime);
-    mFrameEventHistory.addRelease(frameNumber, dequeueReadyTime, std::move(releaseFenceTime));
     mFrameEventHistory.addPreComposition(frameNumber, refreshStartTime);
     mFrameEventHistory.addPostComposition(frameNumber, glDoneFenceTime, presentFenceTime,
                                           compositorTiming);
+    if (previousFrameNumber > 0) {
+        mFrameEventHistory.addRelease(previousFrameNumber, dequeueReadyTime,
+                                      std::move(releaseFenceTime));
+    }
 }
 
 void BLASTBufferItemConsumer::getConnectionEvents(uint64_t frameNumber, bool* needsDisconnect) {
@@ -356,6 +358,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
                 if (stat.latchTime > 0) {
                     mBufferItemConsumer
                             ->updateFrameTimestamps(stat.frameEventStats.frameNumber,
+                                                    stat.frameEventStats.previousFrameNumber,
                                                     stat.frameEventStats.refreshStartTime,
                                                     stat.frameEventStats.gpuCompositionDoneFence,
                                                     stat.presentFence, stat.previousReleaseFence,
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index ffe79a3a03..46088b034e 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -49,6 +49,9 @@ status_t FrameEventHistoryStats::writeToParcel(Parcel* output) const {
     status_t err = output->writeUint64(frameNumber);
     if (err != NO_ERROR) return err;
 
+    err = output->writeUint64(previousFrameNumber);
+    if (err != NO_ERROR) return err;
+
     if (gpuCompositionDoneFence) {
         err = output->writeBool(true);
         if (err != NO_ERROR) return err;
@@ -79,6 +82,9 @@ status_t FrameEventHistoryStats::readFromParcel(const Parcel* input) {
     status_t err = input->readUint64(&frameNumber);
     if (err != NO_ERROR) return err;
 
+    err = input->readUint64(&previousFrameNumber);
+    if (err != NO_ERROR) return err;
+
     bool hasFence = false;
     err = input->readBool(&hasFence);
     if (err != NO_ERROR) return err;
diff --git a/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp b/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp
index 17f4c630ce..2e270b721f 100644
--- a/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp
+++ b/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp
@@ -141,8 +141,8 @@ void BufferQueueFuzzer::invokeBlastBufferQueue() {
     CompositorTiming compTiming;
     sp<Fence> previousFence = new Fence(memfd_create("pfd", MFD_ALLOW_SEALING));
     sp<Fence> gpuFence = new Fence(memfd_create("gfd", MFD_ALLOW_SEALING));
-    FrameEventHistoryStats frameStats(frameNumber, gpuFence, compTiming,
-                                      mFdp.ConsumeIntegral<int64_t>(),
+    FrameEventHistoryStats frameStats(frameNumber, mFdp.ConsumeIntegral<uint64_t>(), gpuFence,
+                                      compTiming, mFdp.ConsumeIntegral<int64_t>(),
                                       mFdp.ConsumeIntegral<int64_t>());
     std::vector<SurfaceControlStats> stats;
     sp<Fence> presentFence = new Fence(memfd_create("fd", MFD_ALLOW_SEALING));
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index a49a85984f..8cd916f95f 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -47,8 +47,8 @@ public:
     void onDisconnect() override EXCLUDES(mMutex);
     void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
                                   FrameEventHistoryDelta* outDelta) override EXCLUDES(mMutex);
-    void updateFrameTimestamps(uint64_t frameNumber, nsecs_t refreshStartTime,
-                               const sp<Fence>& gpuCompositionDoneFence,
+    void updateFrameTimestamps(uint64_t frameNumber, uint64_t previousFrameNumber,
+                               nsecs_t refreshStartTime, const sp<Fence>& gpuCompositionDoneFence,
                                const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence,
                                CompositorTiming compositorTiming, nsecs_t latchTime,
                                nsecs_t dequeueReadyTime) EXCLUDES(mMutex);
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index 39bcb4a56c..508c143782 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -95,15 +95,18 @@ public:
     status_t readFromParcel(const Parcel* input) override;
 
     FrameEventHistoryStats() = default;
-    FrameEventHistoryStats(uint64_t fn, const sp<Fence>& gpuCompFence, CompositorTiming compTiming,
+    FrameEventHistoryStats(uint64_t frameNumber, uint64_t previousFrameNumber,
+                           const sp<Fence>& gpuCompFence, CompositorTiming compTiming,
                            nsecs_t refreshTime, nsecs_t dequeueReadyTime)
-          : frameNumber(fn),
+          : frameNumber(frameNumber),
+            previousFrameNumber(previousFrameNumber),
             gpuCompositionDoneFence(gpuCompFence),
             compositorTiming(compTiming),
             refreshStartTime(refreshTime),
             dequeueReadyTime(dequeueReadyTime) {}
 
     uint64_t frameNumber;
+    uint64_t previousFrameNumber;
     sp<Fence> gpuCompositionDoneFence;
     CompositorTiming compositorTiming;
     nsecs_t refreshStartTime;
diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp
index a3ad6807c5..90674ca09f 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -1656,6 +1656,9 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) {
     nsecs_t postedTimeB = 0;
     setUpAndQueueBuffer(igbProducer, &requestedPresentTimeB, &postedTimeB, &qbOutput, true);
     history.applyDelta(qbOutput.frameTimestamps);
+
+    adapter.waitForCallback(2);
+
     events = history.getFrame(1);
     ASSERT_NE(nullptr, events);
 
@@ -1665,7 +1668,7 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) {
     ASSERT_GE(events->postedTime, postedTimeA);
 
     ASSERT_GE(events->latchTime, postedTimeA);
-    ASSERT_GE(events->dequeueReadyTime, events->latchTime);
+    ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING);
     ASSERT_NE(nullptr, events->gpuCompositionDoneFence);
     ASSERT_NE(nullptr, events->displayPresentFence);
     ASSERT_NE(nullptr, events->releaseFence);
@@ -1677,6 +1680,48 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) {
     ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime);
     ASSERT_GE(events->postedTime, postedTimeB);
 
+    // Now do the same as above with a third buffer, so that timings related to
+    // buffer releases make it back to the first frame.
+    nsecs_t requestedPresentTimeC = 0;
+    nsecs_t postedTimeC = 0;
+    setUpAndQueueBuffer(igbProducer, &requestedPresentTimeC, &postedTimeC, &qbOutput, true);
+    history.applyDelta(qbOutput.frameTimestamps);
+
+    adapter.waitForCallback(3);
+
+    // Check the first frame...
+    events = history.getFrame(1);
+    ASSERT_NE(nullptr, events);
+    ASSERT_EQ(1, events->frameNumber);
+    ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime);
+    ASSERT_GE(events->postedTime, postedTimeA);
+    ASSERT_GE(events->latchTime, postedTimeA);
+    // Now dequeueReadyTime is valid, because the release timings finally
+    // propaged to queueBuffer()
+    ASSERT_GE(events->dequeueReadyTime, events->latchTime);
+    ASSERT_NE(nullptr, events->gpuCompositionDoneFence);
+    ASSERT_NE(nullptr, events->displayPresentFence);
+    ASSERT_NE(nullptr, events->releaseFence);
+
+    // ...and the second
+    events = history.getFrame(2);
+    ASSERT_NE(nullptr, events);
+    ASSERT_EQ(2, events->frameNumber);
+    ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime);
+    ASSERT_GE(events->postedTime, postedTimeB);
+    ASSERT_GE(events->latchTime, postedTimeB);
+    ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING);
+    ASSERT_NE(nullptr, events->gpuCompositionDoneFence);
+    ASSERT_NE(nullptr, events->displayPresentFence);
+    ASSERT_NE(nullptr, events->releaseFence);
+
+    // ...and finally the third!
+    events = history.getFrame(3);
+    ASSERT_NE(nullptr, events);
+    ASSERT_EQ(3, events->frameNumber);
+    ASSERT_EQ(requestedPresentTimeC, events->requestedPresentTime);
+    ASSERT_GE(events->postedTime, postedTimeC);
+
     // wait for any callbacks that have not been received
     adapter.waitForCallbacks();
 }
@@ -1735,7 +1780,42 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_DroppedFrame) {
     setUpAndQueueBuffer(igbProducer, &requestedPresentTimeC, &postedTimeC, &qbOutput, true);
     history.applyDelta(qbOutput.frameTimestamps);
 
+    adapter.waitForCallback(3);
+
+    // frame number, requestedPresentTime, and postTime should not have changed
+    ASSERT_EQ(1, events->frameNumber);
+    ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime);
+    ASSERT_GE(events->postedTime, postedTimeA);
+
+    // a valid latchtime and pre and post composition info should not be set for the dropped frame
+    ASSERT_FALSE(events->hasLatchInfo());
+    ASSERT_FALSE(events->hasDequeueReadyInfo());
+    ASSERT_FALSE(events->hasGpuCompositionDoneInfo());
+    ASSERT_FALSE(events->hasDisplayPresentInfo());
+    ASSERT_FALSE(events->hasReleaseInfo());
+
+    // we should also have gotten values for the presented frame
+    events = history.getFrame(2);
+    ASSERT_NE(nullptr, events);
+    ASSERT_EQ(2, events->frameNumber);
+    ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime);
+    ASSERT_GE(events->postedTime, postedTimeB);
+    ASSERT_GE(events->latchTime, postedTimeB);
+    ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING);
+    ASSERT_NE(nullptr, events->gpuCompositionDoneFence);
+    ASSERT_NE(nullptr, events->displayPresentFence);
+    ASSERT_NE(nullptr, events->releaseFence);
+
+    // Queue another buffer to check for timestamps that came late
+    nsecs_t requestedPresentTimeD = 0;
+    nsecs_t postedTimeD = 0;
+    setUpAndQueueBuffer(igbProducer, &requestedPresentTimeD, &postedTimeD, &qbOutput, true);
+    history.applyDelta(qbOutput.frameTimestamps);
+
+    adapter.waitForCallback(4);
+
     // frame number, requestedPresentTime, and postTime should not have changed
+    events = history.getFrame(1);
     ASSERT_EQ(1, events->frameNumber);
     ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime);
     ASSERT_GE(events->postedTime, postedTimeA);
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index f12aab766e..d28714b4db 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -167,6 +167,7 @@ Layer::Layer(const LayerCreationArgs& args)
     mDrawingState.sequence = 0;
     mDrawingState.transform.set(0, 0);
     mDrawingState.frameNumber = 0;
+    mDrawingState.previousFrameNumber = 0;
     mDrawingState.barrierFrameNumber = 0;
     mDrawingState.producerId = 0;
     mDrawingState.barrierProducerId = 0;
@@ -2854,6 +2855,10 @@ void Layer::onLayerDisplayed(ftl::SharedFuture<FenceResult> futureFenceResult,
         ch->name = mName;
     }
     mPreviouslyPresentedLayerStacks.push_back(layerStack);
+
+    if (mDrawingState.frameNumber > 0) {
+        mDrawingState.previousFrameNumber = mDrawingState.frameNumber;
+    }
 }
 
 void Layer::onSurfaceFrameCreated(
@@ -3032,6 +3037,7 @@ bool Layer::setPosition(float x, float y) {
 void Layer::resetDrawingStateBufferInfo() {
     mDrawingState.producerId = 0;
     mDrawingState.frameNumber = 0;
+    mDrawingState.previousFrameNumber = 0;
     mDrawingState.releaseBufferListener = nullptr;
     mDrawingState.buffer = nullptr;
     mDrawingState.acquireFence = sp<Fence>::make(-1);
@@ -3272,6 +3278,7 @@ bool Layer::setTransactionCompletedListeners(const std::vector<sp<CallbackHandle
             // If this transaction set an acquire fence on this layer, set its acquire time
             handle->acquireTimeOrFence = mCallbackHandleAcquireTimeOrFence;
             handle->frameNumber = mDrawingState.frameNumber;
+            handle->previousFrameNumber = mDrawingState.previousFrameNumber;
 
             // Store so latched time and release fence can be set
             mDrawingState.callbackHandles.push_back(handle);
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index f7596e20e5..48c757f2a0 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -139,6 +139,7 @@ public:
         ui::Dataspace dataspace;
 
         uint64_t frameNumber;
+        uint64_t previousFrameNumber;
         // high watermark framenumber to use to check for barriers to protect ourselves
         // from out of order transactions
         uint64_t barrierFrameNumber;
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index 3587a726cd..6a155c17df 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -158,7 +158,7 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>&
         handle->previousReleaseFence = prevFence;
         handle->previousReleaseFences.clear();
 
-        FrameEventHistoryStats eventStats(handle->frameNumber,
+        FrameEventHistoryStats eventStats(handle->frameNumber, handle->previousFrameNumber,
                                           handle->gpuCompositionDoneFence->getSnapshot().fence,
                                           handle->compositorTiming, handle->refreshStartTime,
                                           handle->dequeueReadyTime);
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index 3074795f62..245398f2f4 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -56,6 +56,7 @@ public:
     nsecs_t refreshStartTime = 0;
     nsecs_t dequeueReadyTime = 0;
     uint64_t frameNumber = 0;
+    uint64_t previousFrameNumber = 0;
     ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
 };