Isolate knowledge that fb target == output buffer

HWComposer didn't allow the virtual display output buffer to be set
directly, instead it always used the framebuffer target buffer.
DisplayDevice was only providing the framebuffer release fence to
DisplaySurfaces after a commit.

This change fixes both of these, so both HWComposer and DisplayDevice
should continue to work if VirtualDisplaySurface changes to use
separate framebuffer and output buffers. It's also more correct since
VirtualDisplaySurface uses the correct release fence when queueing the
buffer to the sink.

Bug: 8384764
Change-Id: I95c71e8d4f67705e23f122259ec8dd5dbce70dcf

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index e16abef..cdc16ed 100644 (file)
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -235,8 +235,7 @@ void DisplayDevice::swapBuffers(HWComposer& hwc) const {
 
 void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const {
     if (hwc.initCheck() == NO_ERROR) {
-        sp<Fence> fence = hwc.getAndResetReleaseFence(mType);
-        mDisplaySurface->onFrameCommitted(fence);
+        mDisplaySurface->onFrameCommitted();
     }
 }
 

diff --git a/services/surfaceflinger/DisplayHardware/DisplaySurface.h b/services/surfaceflinger/DisplayHardware/DisplaySurface.h
index bc717a9..2eca3cb 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/DisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/DisplaySurface.h
@@ -49,10 +49,9 @@ public:
     virtual status_t advanceFrame() = 0;
 
     // onFrameCommitted is called after the frame has been committed to the
-    // hardware composer and a release fence is available for the buffer.
-    // Further operations on the buffer can be queued as long as they wait for
-    // the fence to signal.
-    virtual void onFrameCommitted(const sp<Fence>& fence) = 0;
+    // hardware composer. The surface collects the release fence for this
+    // frame's buffer.
+    virtual void onFrameCommitted() = 0;
 
     virtual void dump(String8& result) const = 0;
 

diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index c35ac95..1936893 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -140,7 +140,8 @@ void FramebufferSurface::freeBufferLocked(int slotIndex) {
     }
 }
 
-void FramebufferSurface::onFrameCommitted(const sp<Fence>& fence) {
+void FramebufferSurface::onFrameCommitted() {
+    sp<Fence> fence = mHwc.getAndResetReleaseFence(mDisplayType);
     if (fence->isValid() &&
             mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT) {
         status_t err = addReleaseFence(mCurrentBufferSlot, fence);

diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
index 164f81f..2fde789 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
@@ -43,7 +43,7 @@ public:
 
     virtual status_t compositionComplete();
     virtual status_t advanceFrame();
-    virtual void onFrameCommitted(const sp<Fence>& fence);
+    virtual void onFrameCommitted();
 
     // Implementation of DisplaySurface::dump(). Note that ConsumerBase also
     // has a non-virtual dump() with the same signature.

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 9d32410..497593b 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -657,15 +657,12 @@ status_t HWComposer::commit() {
             mLists[0]->sur = eglGetCurrentSurface(EGL_DRAW);
         }
 
-        // For virtual displays, the framebufferTarget buffer also serves as
-        // the HWC output buffer, so we need to copy the buffer handle and
-        // dup() the acquire fence.
         for (size_t i=VIRTUAL_DISPLAY_ID_BASE; i<mNumDisplays; i++) {
             DisplayData& disp(mDisplayData[i]);
-            if (disp.framebufferTarget) {
-                mLists[i]->outbuf = disp.framebufferTarget->handle;
+            if (disp.outbufHandle) {
+                mLists[i]->outbuf = disp.outbufHandle;
                 mLists[i]->outbufAcquireFenceFd =
-                        dup(disp.framebufferTarget->acquireFenceFd);
+                        disp.outbufAcquireFence->dup();
             }
         }
 
@@ -706,17 +703,15 @@ status_t HWComposer::acquire(int disp) {
 
 void HWComposer::disconnectDisplay(int disp) {
     LOG_ALWAYS_FATAL_IF(disp < 0 || disp == HWC_DISPLAY_PRIMARY);
-    if (disp >= HWC_NUM_DISPLAY_TYPES) {
-        // nothing to do for these yet
-        return;
-    }
     DisplayData& dd(mDisplayData[disp]);
-    if (dd.list != NULL) {
-        free(dd.list);
-        dd.list = NULL;
-        dd.framebufferTarget = NULL;    // points into dd.list
-        dd.fbTargetHandle = NULL;
-    }
+    free(dd.list);
+    dd.list = NULL;
+    dd.framebufferTarget = NULL;    // points into dd.list
+    dd.fbTargetHandle = NULL;
+    dd.outbufHandle = NULL;
+    dd.lastRetireFence = Fence::NO_FENCE;
+    dd.lastDisplayFence = Fence::NO_FENCE;
+    dd.outbufAcquireFence = Fence::NO_FENCE;
 }
 
 int HWComposer::getVisualID() const {
@@ -765,6 +760,25 @@ void HWComposer::fbDump(String8& result) {
     }
 }
 
+status_t HWComposer::setOutputBuffer(int32_t id, const sp<Fence>& acquireFence,
+        const sp<GraphicBuffer>& buf) {
+    if (uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id))
+        return BAD_INDEX;
+    if (id < VIRTUAL_DISPLAY_ID_BASE)
+        return INVALID_OPERATION;
+
+    DisplayData& disp(mDisplayData[id]);
+    disp.outbufHandle = buf->handle;
+    disp.outbufAcquireFence = acquireFence;
+    return NO_ERROR;
+}
+
+sp<Fence> HWComposer::getLastRetireFence(int32_t id) {
+    if (uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id))
+        return Fence::NO_FENCE;
+    return mDisplayData[id].lastRetireFence;
+}
+
 /*
  * Helper template to implement a concrete HWCLayer
  * This holds the pointer to the concrete hwc layer type
@@ -1071,6 +1085,7 @@ HWComposer::DisplayData::DisplayData()
     capacity(0), list(NULL),
     framebufferTarget(NULL), fbTargetHandle(0),
     lastRetireFence(Fence::NO_FENCE), lastDisplayFence(Fence::NO_FENCE),
+    outbufHandle(NULL), outbufAcquireFence(Fence::NO_FENCE),
     events(0)
 {}
 

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 346526a..58f7e8f 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -128,6 +128,17 @@ public:
     int fbCompositionComplete();
     void fbDump(String8& result);
 
+    // Set the output buffer and acquire fence for a virtual display.
+    // Returns INVALID_OPERATION if id is not a virtual display.
+    status_t setOutputBuffer(int32_t id, const sp<Fence>& acquireFence,
+            const sp<GraphicBuffer>& buf);
+
+    // Get the retire fence for the last committed frame. This fence will
+    // signal when the h/w composer is completely finished with the frame.
+    // For physical displays, it is no longer being displayed. For virtual
+    // displays, writes to the output buffer are complete.
+    sp<Fence> getLastRetireFence(int32_t id);
+
     /*
      * Interface to hardware composer's layers functionality.
      * This abstracts the HAL interface to layers which can evolve in
@@ -306,6 +317,8 @@ private:
         sp<Fence> lastRetireFence;  // signals when the last set op retires
         sp<Fence> lastDisplayFence; // signals when the last set op takes
                                     // effect on screen
+        buffer_handle_t outbufHandle;
+        sp<Fence> outbufAcquireFence;
 
         // protected by mEventControlLock
         int32_t events;

diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
index 255b77f..b28500e 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -70,9 +70,21 @@ status_t VirtualDisplaySurface::advanceFrame() {
     return mHwc.fbPost(mDisplayId, fence, mAcquiredBuffer);
 }
 
-void VirtualDisplaySurface::onFrameCommitted(const sp<Fence>& fence) {
+void VirtualDisplaySurface::onFrameCommitted() {
     Mutex::Autolock lock(mMutex);
     if (mAcquiredBuffer != NULL) {
+        // fbFence signals when reads from the framebuffer are finished
+        // outFence signals when writes to the output buffer are finished
+        // It's unlikely that there will be an implementation where fbFence
+        // signals after outFence (in fact they'll typically be the same
+        // sync_pt), but just to be pedantic we merge them so the sink will
+        // be sure to wait until both are complete.
+        sp<Fence> fbFence = mHwc.getAndResetReleaseFence(mDisplayId);
+        sp<Fence> outFence = mHwc.getLastRetireFence(mDisplayId);
+        sp<Fence> fence = Fence::merge(
+                String8::format("HWC done: %.21s", mName.string()),
+                fbFence, outFence);
+
         status_t result = mSource->releaseBuffer(fence);
         ALOGE_IF(result != NO_ERROR, "VirtualDisplaySurface \"%s\": "
                 "failed to release buffer: %d", mName.string(), result);

diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
index 1347680..61bafed 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
@@ -58,7 +58,7 @@ public:
 
     virtual status_t compositionComplete();
     virtual status_t advanceFrame();
-    virtual void onFrameCommitted(const sp<Fence>& fence);
+    virtual void onFrameCommitted();
     virtual void dump(String8& result) const;
 
 private: