Merge "Release virtual display buffer immediately after HWC set" into jb-mr2-dev
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index ecd12d0..5493e7d 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -236,7 +236,7 @@
void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const {
if (hwc.initCheck() == NO_ERROR) {
int fd = hwc.getAndResetReleaseFenceFd(mType);
- mDisplaySurface->setReleaseFenceFd(fd);
+ mDisplaySurface->onFrameCommitted(fd);
}
}
diff --git a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp
index d8ad224..91f9aea 100644
--- a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp
@@ -16,6 +16,7 @@
#undef LOG_TAG
#define LOG_TAG "BQInterposer"
+//#define LOG_NDEBUG 0
#include "BufferQueueInterposer.h"
@@ -42,19 +43,6 @@
mAcquired(false)
{
BQI_LOGV("BufferQueueInterposer sink=%p", sink.get());
-
- // We need one additional dequeued buffer beyond what the source needs.
- // To have more than one (the default), we must call setBufferCount. But
- // we have no way of knowing what the sink has set as the minimum buffer
- // count, so if we just call setBufferCount(3) it may fail (and does, on
- // one device using a video encoder sink). So far on the devices we care
- // about, this is the smallest value that works.
- //
- // TODO: Change IGraphicBufferProducer and implementations to support this.
- // Maybe change it so both the consumer and producer declare how many
- // buffers they need, and the IGBP adds them? Then BQInterposer would just
- // add 1 to the source's buffer count.
- mSink->setBufferCount(6);
}
BufferQueueInterposer::~BufferQueueInterposer() {
diff --git a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h
index 7208630..7e84e97 100644
--- a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h
+++ b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h
@@ -59,20 +59,20 @@
// existing interfaces have some problems when the implementation isn't the
// final consumer.
//
-// - The interposer needs at least one buffer in addition to those used by the
-// source and sink. setBufferCount and QueueBufferOutput both need to
-// account for this. It's not possible currently to do this generically,
-// since we can't find out how many buffers the source and sink need. (See
-// the horrible hack in the BufferQueueInterposer constructor).
+// - The client of the interposer may need one or more buffers in addition to
+// those used by the source and sink. IGraphicBufferProducer will probably
+// need to change to allow the producer to specify how many buffers it needs
+// to dequeue at a time, and then the interposer can add its requirements to
+// those of the source.
//
// - Abandoning, disconnecting, and connecting need to pass through somehow.
// There needs to be a way to tell the interposer client to release its
// buffer immediately so it can be queued/released, e.g. when the source
// calls disconnect().
//
-// - Right now the source->BQI queue is synchronous even if the BQI->sink
-// queue is asynchronous. Need to figure out how asynchronous should behave
-// and implement that.
+// - Right now the source->BQI queue is synchronous even if the BQI->sink queue
+// is asynchronous. Need to figure out how asynchronous should behave and
+// implement that.
class BufferQueueInterposer : public BnGraphicBufferProducer {
public:
diff --git a/services/surfaceflinger/DisplayHardware/DisplaySurface.h b/services/surfaceflinger/DisplayHardware/DisplaySurface.h
index 2de6b4c..6445848 100644
--- a/services/surfaceflinger/DisplayHardware/DisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/DisplaySurface.h
@@ -43,15 +43,16 @@
// this frame. Some implementations may only push a new buffer to
// HWComposer if GLES composition took place, others need to push a new
// buffer on every frame.
+ //
+ // advanceFrame must be followed by a call to onFrameCommitted before
+ // advanceFrame may be called again.
virtual status_t advanceFrame() = 0;
- // setReleaseFenceFd stores a fence file descriptor that will signal when
- // the current buffer is no longer being read. This fence will be returned
- // to the producer when the current buffer is released by updateTexImage().
- // Multiple fences can be set for a given buffer; they will be merged into
- // a single union fence. The GLConsumer will close the file descriptor
- // when finished with it.
- virtual status_t setReleaseFenceFd(int fenceFd) = 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(int fenceFd) = 0;
virtual void dump(String8& result) const = 0;
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index b5abaac..83ab38e 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -140,17 +140,15 @@
}
}
-status_t FramebufferSurface::setReleaseFenceFd(int fenceFd) {
- status_t err = NO_ERROR;
+void FramebufferSurface::onFrameCommitted(int fenceFd) {
if (fenceFd >= 0) {
sp<Fence> fence(new Fence(fenceFd));
if (mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT) {
- err = addReleaseFence(mCurrentBufferSlot, fence);
+ status_t err = addReleaseFence(mCurrentBufferSlot, fence);
ALOGE_IF(err, "setReleaseFenceFd: failed to add the fence: %s (%d)",
strerror(-err), err);
}
}
- return err;
}
status_t FramebufferSurface::compositionComplete()
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
index 0aab742..1402740 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
@@ -43,7 +43,7 @@
virtual status_t compositionComplete();
virtual status_t advanceFrame();
- virtual status_t setReleaseFenceFd(int fenceFd);
+ virtual void onFrameCommitted(int fenceFd);
// Implementation of DisplaySurface::dump(). Note that ConsumerBase also
// has a non-virtual dump() with the same signature.
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
index 433e1eb..fac6c3e 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -26,16 +26,14 @@
: mHwc(hwc),
mDisplayId(disp),
mSource(new BufferQueueInterposer(sink, name)),
- mName(name),
- mReleaseFence(Fence::NO_FENCE)
+ mName(name)
{}
VirtualDisplaySurface::~VirtualDisplaySurface() {
if (mAcquiredBuffer != NULL) {
- status_t result = mSource->releaseBuffer(mReleaseFence);
+ status_t result = mSource->releaseBuffer(Fence::NO_FENCE);
ALOGE_IF(result != NO_ERROR, "VirtualDisplaySurface \"%s\": "
- "failed to release previous buffer: %d",
- mName.string(), result);
+ "failed to release buffer: %d", mName.string(), result);
}
}
@@ -52,12 +50,10 @@
status_t result = NO_ERROR;
if (mAcquiredBuffer != NULL) {
- result = mSource->releaseBuffer(mReleaseFence);
- ALOGE_IF(result != NO_ERROR, "VirtualDisplaySurface \"%s\": "
- "failed to release previous buffer: %d",
- mName.string(), result);
- mAcquiredBuffer.clear();
- mReleaseFence = Fence::NO_FENCE;
+ ALOGE("VirtualDisplaySurface \"%s\": "
+ "advanceFrame called twice without onFrameCommitted",
+ mName.string());
+ return INVALID_OPERATION;
}
sp<Fence> fence;
@@ -74,25 +70,15 @@
return mHwc.fbPost(mDisplayId, fence, mAcquiredBuffer);
}
-status_t VirtualDisplaySurface::setReleaseFenceFd(int fenceFd) {
- if (fenceFd >= 0) {
- sp<Fence> fence(new Fence(fenceFd));
- Mutex::Autolock lock(mMutex);
- sp<Fence> mergedFence = Fence::merge(
- String8::format("VirtualDisplaySurface \"%s\"",
- mName.string()),
- mReleaseFence, fence);
- if (!mergedFence->isValid()) {
- ALOGE("VirtualDisplaySurface \"%s\": failed to merge release fence",
- mName.string());
- // synchronization is broken, the best we can do is hope fences
- // signal in order so the new fence will act like a union
- mReleaseFence = fence;
- return BAD_VALUE;
- }
- mReleaseFence = mergedFence;
+void VirtualDisplaySurface::onFrameCommitted(int fenceFd) {
+ Mutex::Autolock lock(mMutex);
+ sp<Fence> fence(new Fence(fenceFd));
+ if (mAcquiredBuffer != NULL) {
+ status_t result = mSource->releaseBuffer(fence);
+ ALOGE_IF(result != NO_ERROR, "VirtualDisplaySurface \"%s\": "
+ "failed to release buffer: %d", mName.string(), result);
+ mAcquiredBuffer.clear();
}
- return NO_ERROR;
}
void VirtualDisplaySurface::dump(String8& result) const {
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
index 66f8580..85a7a87 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
@@ -58,7 +58,7 @@
virtual status_t compositionComplete();
virtual status_t advanceFrame();
- virtual status_t setReleaseFenceFd(int fenceFd);
+ virtual void onFrameCommitted(int fenceFd);
virtual void dump(String8& result) const;
private:
@@ -73,7 +73,6 @@
// mutable, must be synchronized with mMutex
Mutex mMutex;
sp<GraphicBuffer> mAcquiredBuffer;
- sp<Fence> mReleaseFence;
};
// ---------------------------------------------------------------------------