diff options
8 files changed, 37 insertions, 65 deletions
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::swapBuffers(HWComposer& hwc) const { 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 @@ BufferQueueInterposer::BufferQueueInterposer( 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 @@ namespace android { // 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 @@ public: // 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 @@ void FramebufferSurface::freeBufferLocked(int slotIndex) { } } -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 @@ public: 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 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int disp, : 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 VirtualDisplaySurface::advanceFrame() { 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 @@ status_t VirtualDisplaySurface::advanceFrame() { 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 @@ public: 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 @@ private: // mutable, must be synchronized with mMutex Mutex mMutex; sp<GraphicBuffer> mAcquiredBuffer; - sp<Fence> mReleaseFence; }; // --------------------------------------------------------------------------- |