From 7adb0f8a9fdb961692ffd2f0c65cacb155143f64 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Wed, 6 Mar 2013 16:13:49 -0800 Subject: [PATCH 1/4] Minor cleanups/fixes before virtual display refactoring None of these should change behavior, except for removing some incorrect log messages when using a virtual display. - HWComposer::getAndResetReleaseFenceFd() checks the HWC version, so no need to do that in the DisplayDevice::onSwapBuffersCompleted(). However, it should check that mFramebufferSurface is not NULL like it is for virtual displays. - Comment that FramebufferSurface::dump() overrides the non-virtual ConsumerBase::dump(), and fix it so the right thing happens regardless of the static type of the pointer/reference the callee has. FramebufferSurface::dump() could be removed right now, but I'd need to bring it back in a later change. - Use the right enum for validating display type ids. - Don't try to send hotplug events for virtual displays. - Mark virtual displays as connected so HWComposer::prepare() doesn't think something is wrong when it gets a non-NULL layer list. - Remove unused FramebufferSurface methods. Bug: 8384764 Change-Id: Id28a2f9be86b45f4bb7915fdf7752157035f4294 --- include/gui/ConsumerBase.h | 7 +++---- services/surfaceflinger/DisplayDevice.cpp | 2 +- .../DisplayHardware/FramebufferSurface.cpp | 15 ++++++++------- .../DisplayHardware/FramebufferSurface.h | 9 ++++++--- .../DisplayHardware/HWComposer.cpp | 2 ++ services/surfaceflinger/EventThread.cpp | 16 ++++++++-------- services/surfaceflinger/SurfaceFlinger.cpp | 6 ++++-- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 49a764f95..78a36089e 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -69,10 +69,9 @@ public: // ConsumerBase is connected. sp getBufferQueue() const; - // dump writes the current state to a string. These methods should NOT be - // overridden by child classes. Instead they should override the - // dumpLocked method, which is called by these methods after locking the - // mutex. + // dump writes the current state to a string. Child classes should add + // their state to the dump by overriding the dumpLocked method, which is + // called by these methods after locking the mutex. void dump(String8& result) const; void dump(String8& result, const char* prefix, char* buffer, size_t SIZE) const; diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 2bbe49cde..37a14c8fd 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -244,7 +244,7 @@ void DisplayDevice::swapBuffers(HWComposer& hwc) const { void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const { if (hwc.initCheck() == NO_ERROR) { - if (hwc.supportsFramebufferTarget()) { + if (mFramebufferSurface != NULL) { int fd = hwc.getAndResetReleaseFenceFd(mType); mFramebufferSurface->setReleaseFenceFd(fd); } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 7557e3fdc..960f9a953 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -134,7 +134,7 @@ status_t FramebufferSurface::setReleaseFenceFd(int fenceFd) { if (fenceFd >= 0) { sp fence(new Fence(fenceFd)); if (mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT) { - status_t err = addReleaseFence(mCurrentBufferSlot, fence); + err = addReleaseFence(mCurrentBufferSlot, fence); ALOGE_IF(err, "setReleaseFenceFd: failed to add the fence: %s (%d)", strerror(-err), err); } @@ -142,21 +142,22 @@ status_t FramebufferSurface::setReleaseFenceFd(int fenceFd) { return err; } -status_t FramebufferSurface::setUpdateRectangle(const Rect& r) -{ - return INVALID_OPERATION; -} - status_t FramebufferSurface::compositionComplete() { return mHwc.fbCompositionComplete(); } void FramebufferSurface::dump(String8& result) { - mHwc.fbDump(result); ConsumerBase::dump(result); } +void FramebufferSurface::dumpLocked(String8& result, const char* prefix, + char* buffer, size_t SIZE) const +{ + mHwc.fbDump(result); + ConsumerBase::dumpLocked(result, prefix, buffer, SIZE); +} + // ---------------------------------------------------------------------------- }; // namespace android // ---------------------------------------------------------------------------- diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index b61b7f509..639e72061 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -36,11 +36,11 @@ class FramebufferSurface : public ConsumerBase { public: FramebufferSurface(HWComposer& hwc, int disp); - bool isUpdateOnDemand() const { return false; } - status_t setUpdateRectangle(const Rect& updateRect); status_t compositionComplete(); - virtual void dump(String8& result); + // TODO(jessehall): This overrides the non-virtual ConsumerBase version. + // Will rework slightly in a following change. + void dump(String8& result); // setReleaseFenceFd stores a fence file descriptor that will signal when the // current buffer is no longer being read. This fence will be returned to @@ -56,6 +56,9 @@ private: virtual void onFrameAvailable(); virtual void freeBufferLocked(int slotIndex); + virtual void dumpLocked(String8& result, const char* prefix, + char* buffer, size_t SIZE) const; + // nextBuffer waits for and then latches the next buffer from the // BufferQueue and releases the previously latched buffer to the // BufferQueue. The new buffer is returned in the 'buffer' argument. diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index bb567e270..7df3671ab 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -380,6 +380,7 @@ int32_t HWComposer::allocateDisplayId() { } int32_t id = mAllocatedDisplayIDs.firstUnmarkedBit(); mAllocatedDisplayIDs.markBit(id); + mDisplayData[id].connected = true; return id; } @@ -392,6 +393,7 @@ status_t HWComposer::freeDisplayId(int32_t id) { return BAD_INDEX; } mAllocatedDisplayIDs.clearBit(id); + mDisplayData[id].connected = false; return NO_ERROR; } diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index edb9fa580..4d0fc79a1 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -41,7 +41,7 @@ EventThread::EventThread(const sp& flinger) mUseSoftwareVSync(false), mDebugVsyncEnabled(false) { - for (int32_t i=0 ; i= HWC_DISPLAY_TYPES_SUPPORTED, - "received event for an invalid display (id=%d)", type); + ALOGE_IF(type >= HWC_NUM_DISPLAY_TYPES, + "received vsync event for an invalid display (id=%d)", type); Mutex::Autolock _l(mLock); - if (type < HWC_DISPLAY_TYPES_SUPPORTED) { + if (type < HWC_NUM_DISPLAY_TYPES) { mVSyncEvent[type].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; mVSyncEvent[type].header.id = type; mVSyncEvent[type].header.timestamp = timestamp; @@ -126,11 +126,11 @@ void EventThread::onVSyncReceived(int type, nsecs_t timestamp) { } void EventThread::onHotplugReceived(int type, bool connected) { - ALOGE_IF(type >= HWC_DISPLAY_TYPES_SUPPORTED, - "received event for an invalid display (id=%d)", type); + ALOGE_IF(type >= HWC_NUM_DISPLAY_TYPES, + "received hotplug event for an invalid display (id=%d)", type); Mutex::Autolock _l(mLock); - if (type < HWC_DISPLAY_TYPES_SUPPORTED) { + if (type < HWC_NUM_DISPLAY_TYPES) { DisplayEventReceiver::Event event; event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG; event.header.id = type; @@ -184,7 +184,7 @@ Vector< sp > EventThread::waitForEvent( size_t vsyncCount = 0; nsecs_t timestamp = 0; - for (int32_t i=0 ; ionHotplugReceived(draw[i].type, false); + if (draw[i].type < DisplayDevice::NUM_DISPLAY_TYPES) + mEventThread->onHotplugReceived(draw[i].type, false); } else { ALOGW("trying to remove the main display"); } @@ -1204,7 +1205,8 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) state.viewport, state.frame); hw->setDisplayName(state.displayName); mDisplays.add(display, hw); - mEventThread->onHotplugReceived(state.type, true); + if (state.type < DisplayDevice::NUM_DISPLAY_TYPES) + mEventThread->onHotplugReceived(state.type, true); } } } From 4c00cc11141da7d159eb2323b186ed344115c0f1 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Fri, 15 Mar 2013 21:34:30 -0700 Subject: [PATCH 2/4] Fix argument types in IGraphicBufferProducer methods Bug: 8384764 Change-Id: I7a3f1e1a0584a70af04f9eafef900505389d2202 --- include/gui/BufferQueue.h | 4 ++-- include/gui/IGraphicBufferProducer.h | 4 ++-- libs/gui/BufferQueue.cpp | 6 +++--- libs/gui/IGraphicBufferProducer.cpp | 10 +++++----- libs/gui/Surface.cpp | 4 ++-- libs/gui/tests/BufferQueue_test.cpp | 4 ++-- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index c59e00e0d..6a86db6ab 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -128,7 +128,7 @@ public: // GL_MAX_VIEWPORT_DIMS and GL_MAX_TEXTURE_SIZE (see: glGetIntegerv). // An error due to invalid dimensions might not be reported until // updateTexImage() is called. - virtual status_t dequeueBuffer(int *buf, sp& fence, + virtual status_t dequeueBuffer(int *buf, sp* fence, uint32_t width, uint32_t height, uint32_t format, uint32_t usage); // queueBuffer returns a filled buffer to the BufferQueue. In addition, a @@ -139,7 +139,7 @@ public: virtual status_t queueBuffer(int buf, const QueueBufferInput& input, QueueBufferOutput* output); - virtual void cancelBuffer(int buf, sp fence); + virtual void cancelBuffer(int buf, const sp& fence); // setSynchronousMode set whether dequeueBuffer is synchronous or // asynchronous. In synchronous mode, dequeueBuffer blocks until diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index a3e258df3..29c7ff371 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -84,7 +84,7 @@ public: // the buffer. The contents of the buffer must not be overwritten until the // fence signals. If the fence is NULL, the buffer may be written // immediately. - virtual status_t dequeueBuffer(int *slot, sp& fence, + virtual status_t dequeueBuffer(int *slot, sp* fence, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) = 0; // queueBuffer indicates that the client has finished filling in the @@ -165,7 +165,7 @@ public: // cancelBuffer indicates that the client does not wish to fill in the // buffer associated with slot and transfers ownership of the slot back to // the server. - virtual void cancelBuffer(int slot, sp fence) = 0; + virtual void cancelBuffer(int slot, const sp& fence) = 0; // query retrieves some information for this surface // 'what' tokens allowed are that of android_natives.h diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 41ee1be92..75a02963e 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -254,7 +254,7 @@ status_t BufferQueue::requestBuffer(int slot, sp* buf) { return NO_ERROR; } -status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, +status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { ATRACE_CALL(); ST_LOGV("dequeueBuffer: w=%d h=%d fmt=%#x usage=%#x", w, h, format, usage); @@ -393,7 +393,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, dpy = mSlots[buf].mEglDisplay; eglFence = mSlots[buf].mEglFence; - outFence = mSlots[buf].mFence; + *outFence = mSlots[buf].mFence; mSlots[buf].mEglFence = EGL_NO_SYNC_KHR; mSlots[buf].mFence = Fence::NO_FENCE; } // end lock scope @@ -590,7 +590,7 @@ status_t BufferQueue::queueBuffer(int buf, return OK; } -void BufferQueue::cancelBuffer(int buf, sp fence) { +void BufferQueue::cancelBuffer(int buf, const sp& fence) { ATRACE_CALL(); ST_LOGV("cancelBuffer: slot=%d", buf); Mutex::Autolock lock(mMutex); diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index 54860d794..63d7628a5 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -81,7 +81,7 @@ public: return result; } - virtual status_t dequeueBuffer(int *buf, sp& fence, + virtual status_t dequeueBuffer(int *buf, sp* fence, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); @@ -99,8 +99,8 @@ public: // If the fence was written by the callee, then overwrite the // caller's fence here. If it wasn't written then don't touch the // caller's fence. - fence = new Fence(); - reply.read(*fence.get()); + *fence = new Fence(); + reply.read(*(fence->get())); } result = reply.readInt32(); return result; @@ -121,7 +121,7 @@ public: return result; } - virtual void cancelBuffer(int buf, sp fence) { + virtual void cancelBuffer(int buf, const sp& fence) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); data.writeInt32(buf); @@ -215,7 +215,7 @@ status_t BnGraphicBufferProducer::onTransact( uint32_t usage = data.readInt32(); int buf; sp fence; - int result = dequeueBuffer(&buf, fence, w, h, format, usage); + int result = dequeueBuffer(&buf, &fence, w, h, format, usage); reply->writeInt32(buf); reply->writeInt32(fence != NULL); if (fence != NULL) { diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index ec55b5799..4a5802354 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -182,8 +182,8 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int reqW = mReqWidth ? mReqWidth : mUserWidth; int reqH = mReqHeight ? mReqHeight : mUserHeight; sp fence; - status_t result = mGraphicBufferProducer->dequeueBuffer(&buf, fence, reqW, reqH, - mReqFormat, mReqUsage); + status_t result = mGraphicBufferProducer->dequeueBuffer(&buf, &fence, + reqW, reqH, mReqFormat, mReqUsage); if (result < 0) { ALOGV("dequeueBuffer: IGraphicBufferProducer::dequeueBuffer(%d, %d, %d, %d)" "failed: %d", mReqWidth, mReqHeight, mReqFormat, mReqUsage, diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 93f8fafc2..62d215ba8 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -76,7 +76,7 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { for (int i = 0; i < 2; i++) { ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, - mBQ->dequeueBuffer(&slot, fence, 1, 1, 0, + mBQ->dequeueBuffer(&slot, &fence, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN)); ASSERT_EQ(OK, mBQ->requestBuffer(slot, &buf)); ASSERT_EQ(OK, mBQ->queueBuffer(slot, qbi, &qbo)); @@ -84,7 +84,7 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { } ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, - mBQ->dequeueBuffer(&slot, fence, 1, 1, 0, + mBQ->dequeueBuffer(&slot, &fence, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN)); ASSERT_EQ(OK, mBQ->requestBuffer(slot, &buf)); ASSERT_EQ(OK, mBQ->queueBuffer(slot, qbi, &qbo)); From 99c7dbb24994df2f3e175f7b25dd2c9dd92a72f0 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Thu, 14 Mar 2013 14:29:29 -0700 Subject: [PATCH 3/4] Add DisplaySurface abstraction DisplayDevice now has a DisplaySurface instead of using FramebufferSurface directly. FramebufferSurface implements DisplaySurface, and so does the new VirtualDisplaySurface class. DisplayDevice now always has a surface, not just for virtual displays. In this change VirtualDisplaySurface is just a stub; buffers still go directly from GLES to the final consumer. Bug: 8384764 Change-Id: I57cb668edbc6c37bfebda90b9222d435bf589f37 --- services/surfaceflinger/Android.mk | 32 ++++---- services/surfaceflinger/DisplayDevice.cpp | 75 ++++++++----------- services/surfaceflinger/DisplayDevice.h | 9 +-- .../DisplayHardware/DisplaySurface.h | 68 +++++++++++++++++ .../DisplayHardware/FramebufferSurface.cpp | 29 ++++++- .../DisplayHardware/FramebufferSurface.h | 23 +++--- .../DisplayHardware/HWComposer.cpp | 6 +- .../DisplayHardware/VirtualDisplaySurface.cpp | 60 +++++++++++++++ .../DisplayHardware/VirtualDisplaySurface.h | 57 ++++++++++++++ services/surfaceflinger/SurfaceFlinger.cpp | 34 ++++----- 10 files changed, 292 insertions(+), 101 deletions(-) create mode 100644 services/surfaceflinger/DisplayHardware/DisplaySurface.h create mode 100644 services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp create mode 100644 services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h diff --git a/services/surfaceflinger/Android.mk b/services/surfaceflinger/Android.mk index 5ff8154e7..6f89ffadf 100644 --- a/services/surfaceflinger/Android.mk +++ b/services/surfaceflinger/Android.mk @@ -2,22 +2,22 @@ LOCAL_PATH:= $(call my-dir) include $(CLEAR_VARS) LOCAL_SRC_FILES:= \ - Client.cpp \ - DisplayDevice.cpp \ - EventThread.cpp \ - FrameTracker.cpp \ - Layer.cpp \ - LayerDim.cpp \ - DisplayHardware/FramebufferSurface.cpp \ - DisplayHardware/HWComposer.cpp \ - DisplayHardware/PowerHAL.cpp \ - GLExtensions.cpp \ - MessageQueue.cpp \ - SurfaceFlinger.cpp \ - SurfaceFlingerConsumer.cpp \ - SurfaceTextureLayer.cpp \ - Transform.cpp \ - + Client.cpp \ + DisplayDevice.cpp \ + EventThread.cpp \ + FrameTracker.cpp \ + GLExtensions.cpp \ + Layer.cpp \ + LayerDim.cpp \ + MessageQueue.cpp \ + SurfaceFlinger.cpp \ + SurfaceFlingerConsumer.cpp \ + SurfaceTextureLayer.cpp \ + Transform.cpp \ + DisplayHardware/FramebufferSurface.cpp \ + DisplayHardware/HWComposer.cpp \ + DisplayHardware/PowerHAL.cpp \ + DisplayHardware/VirtualDisplaySurface.cpp \ LOCAL_CFLAGS:= -DLOG_TAG=\"SurfaceFlinger\" LOCAL_CFLAGS += -DGL_GLEXT_PROTOTYPES -DEGL_EGLEXT_PROTOTYPES diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 37a14c8fd..ecd12d084 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -35,7 +35,7 @@ #include -#include "DisplayHardware/FramebufferSurface.h" +#include "DisplayHardware/DisplaySurface.h" #include "DisplayHardware/HWComposer.h" #include "clz.h" @@ -72,14 +72,12 @@ DisplayDevice::DisplayDevice( DisplayType type, bool isSecure, const wp& displayToken, - const sp& nativeWindow, - const sp& framebufferSurface, + const sp& displaySurface, EGLConfig config) : mFlinger(flinger), mType(type), mHwcDisplayId(-1), mDisplayToken(displayToken), - mNativeWindow(nativeWindow), - mFramebufferSurface(framebufferSurface), + mDisplaySurface(displaySurface), mDisplay(EGL_NO_DISPLAY), mSurface(EGL_NO_SURFACE), mContext(EGL_NO_CONTEXT), @@ -92,6 +90,7 @@ DisplayDevice::DisplayDevice( mLayerStack(NO_LAYER_STACK), mOrientation() { + mNativeWindow = new Surface(mDisplaySurface->getIGraphicBufferProducer()); init(config); } @@ -183,10 +182,7 @@ uint32_t DisplayDevice::getPageFlipCount() const { } status_t DisplayDevice::compositionComplete() const { - if (mFramebufferSurface == NULL) { - return NO_ERROR; - } - return mFramebufferSurface->compositionComplete(); + return mDisplaySurface->compositionComplete(); } void DisplayDevice::flip(const Region& dirty) const @@ -209,45 +205,38 @@ void DisplayDevice::flip(const Region& dirty) const } void DisplayDevice::swapBuffers(HWComposer& hwc) const { - EGLBoolean success = EGL_TRUE; - if (hwc.initCheck() != NO_ERROR) { - // no HWC, we call eglSwapBuffers() - success = eglSwapBuffers(mDisplay, mSurface); - } else { - // We have a valid HWC, but not all displays can use it, in particular - // the virtual displays are on their own. - // TODO: HWC 1.2 will allow virtual displays - if (mType >= DisplayDevice::DISPLAY_VIRTUAL) { - // always call eglSwapBuffers() for virtual displays - success = eglSwapBuffers(mDisplay, mSurface); - } else if (hwc.supportsFramebufferTarget()) { - // as of hwc 1.1 we always call eglSwapBuffers if we have some - // GLES layers - if (hwc.hasGlesComposition(mType)) { - success = eglSwapBuffers(mDisplay, mSurface); + // We need to call eglSwapBuffers() unless: + // (a) there was no GLES composition this frame, or + // (b) we're using a legacy HWC with no framebuffer target support (in + // which case HWComposer::commit() handles things). + if (hwc.initCheck() != NO_ERROR || + (hwc.hasGlesComposition(mHwcDisplayId) && + hwc.supportsFramebufferTarget())) { + EGLBoolean success = eglSwapBuffers(mDisplay, mSurface); + if (!success) { + EGLint error = eglGetError(); + if (error == EGL_CONTEXT_LOST || + mType == DisplayDevice::DISPLAY_PRIMARY) { + LOG_ALWAYS_FATAL("eglSwapBuffers(%p, %p) failed with 0x%08x", + mDisplay, mSurface, error); + } else { + ALOGE("eglSwapBuffers(%p, %p) failed with 0x%08x", + mDisplay, mSurface, error); } - } else { - // HWC doesn't have the framebuffer target, we don't call - // eglSwapBuffers(), since this is handled by HWComposer::commit(). } } - if (!success) { - EGLint error = eglGetError(); - if (error == EGL_CONTEXT_LOST || - mType == DisplayDevice::DISPLAY_PRIMARY) { - LOG_ALWAYS_FATAL("eglSwapBuffers(%p, %p) failed with 0x%08x", - mDisplay, mSurface, error); - } + status_t result = mDisplaySurface->advanceFrame(); + if (result != NO_ERROR) { + ALOGE("[%s] failed pushing new frame to HWC: %d", + mDisplayName.string(), result); } } void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const { if (hwc.initCheck() == NO_ERROR) { - if (mFramebufferSurface != NULL) { - int fd = hwc.getAndResetReleaseFenceFd(mType); - mFramebufferSurface->setReleaseFenceFd(fd); - } + int fd = hwc.getAndResetReleaseFenceFd(mType); + mDisplaySurface->setReleaseFenceFd(fd); } } @@ -455,9 +444,7 @@ void DisplayDevice::dump(String8& result, char* buffer, size_t SIZE) const { result.append(buffer); - String8 fbtargetDump; - if (mFramebufferSurface != NULL) { - mFramebufferSurface->dump(fbtargetDump); - result.append(fbtargetDump); - } + String8 surfaceDump; + mDisplaySurface->dump(surfaceDump); + result.append(surfaceDump); } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index f67101705..d8f55b457 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -37,7 +37,7 @@ struct ANativeWindow; namespace android { class DisplayInfo; -class FramebufferSurface; +class DisplaySurface; class Layer; class SurfaceFlinger; class HWComposer; @@ -74,8 +74,7 @@ public: DisplayType type, bool isSecure, const wp& displayToken, - const sp& nativeWindow, - const sp& framebufferSurface, + const sp& displaySurface, EGLConfig config); ~DisplayDevice(); @@ -165,9 +164,7 @@ private: // ANativeWindow this display is rendering into sp mNativeWindow; - - // set if mNativeWindow is a FramebufferSurface - sp mFramebufferSurface; + sp mDisplaySurface; EGLDisplay mDisplay; EGLSurface mSurface; diff --git a/services/surfaceflinger/DisplayHardware/DisplaySurface.h b/services/surfaceflinger/DisplayHardware/DisplaySurface.h new file mode 100644 index 000000000..2de6b4cc9 --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/DisplaySurface.h @@ -0,0 +1,68 @@ +/* + * Copyright 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_SF_DISPLAY_SURFACE_H +#define ANDROID_SF_DISPLAY_SURFACE_H + +#include +#include +#include + +// --------------------------------------------------------------------------- +namespace android { +// --------------------------------------------------------------------------- + +class IGraphicBufferProducer; +class String8; + +class DisplaySurface : public virtual RefBase { +public: + virtual sp getIGraphicBufferProducer() const = 0; + + // Should be called when composition rendering is complete for a frame (but + // eglSwapBuffers hasn't necessarily been called). Required by certain + // older drivers for synchronization. + // TODO: Remove this when we drop support for HWC 1.0. + virtual status_t compositionComplete() = 0; + + // Inform the surface that GLES composition is complete for this frame, and + // the surface should make sure that HWComposer has the correct buffer for + // 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. + 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; + + virtual void dump(String8& result) const = 0; + +protected: + DisplaySurface() {} + virtual ~DisplaySurface() {} +}; + +// --------------------------------------------------------------------------- +} // namespace android +// --------------------------------------------------------------------------- + +#endif // ANDROID_SF_DISPLAY_SURFACE_H + diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 960f9a953..b5abaacfd 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -68,6 +68,17 @@ FramebufferSurface::FramebufferSurface(HWComposer& hwc, int disp) : mBufferQueue->setDefaultMaxBufferCount(NUM_FRAMEBUFFER_SURFACE_BUFFERS); } +sp FramebufferSurface::getIGraphicBufferProducer() const { + return getBufferQueue(); +} + +status_t FramebufferSurface::advanceFrame() { + // Once we remove FB HAL support, we can call nextBuffer() from here + // instead of using onFrameAvailable(). No real benefit, except it'll be + // more like VirtualDisplaySurface. + return NO_ERROR; +} + status_t FramebufferSurface::nextBuffer(sp& outBuffer, sp& outFence) { Mutex::Autolock lock(mMutex); @@ -147,7 +158,23 @@ status_t FramebufferSurface::compositionComplete() return mHwc.fbCompositionComplete(); } -void FramebufferSurface::dump(String8& result) { +// Since DisplaySurface and ConsumerBase both have a method with this +// signature, results will vary based on the static pointer type the caller is +// using: +// void dump(FrameBufferSurface* fbs, String8& s) { +// // calls FramebufferSurface::dump() +// fbs->dump(s); +// +// // calls ConsumerBase::dump() since it is non-virtual +// static_cast(fbs)->dump(s); +// +// // calls FramebufferSurface::dump() since it is virtual +// static_cast(fbs)->dump(s); +// } +// To make sure that all of these end up doing the same thing, we just redirect +// to ConsumerBase::dump() here. It will take the internal lock, and then call +// virtual dumpLocked(), which is where the real work happens. +void FramebufferSurface::dump(String8& result) const { ConsumerBase::dump(result); } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index 639e72061..0aab7424c 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -22,6 +22,8 @@ #include +#include "DisplaySurface.h" + // --------------------------------------------------------------------------- namespace android { // --------------------------------------------------------------------------- @@ -32,23 +34,20 @@ class HWComposer; // --------------------------------------------------------------------------- -class FramebufferSurface : public ConsumerBase { +class FramebufferSurface : public ConsumerBase, + public DisplaySurface { public: FramebufferSurface(HWComposer& hwc, int disp); - status_t compositionComplete(); + virtual sp getIGraphicBufferProducer() const; - // TODO(jessehall): This overrides the non-virtual ConsumerBase version. - // Will rework slightly in a following change. - void dump(String8& result); + virtual status_t compositionComplete(); + virtual status_t advanceFrame(); + virtual status_t setReleaseFenceFd(int fenceFd); - // 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. - status_t setReleaseFenceFd(int fenceFd); + // Implementation of DisplaySurface::dump(). Note that ConsumerBase also + // has a non-virtual dump() with the same signature. + virtual void dump(String8& result) const; private: virtual ~FramebufferSurface() { }; // this class cannot be overloaded diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 7df3671ab..4b6c4130b 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -617,14 +617,14 @@ status_t HWComposer::prepare() { } bool HWComposer::hasHwcComposition(int32_t id) const { - if (uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id)) + if (!mHwc || uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id)) return false; return mDisplayData[id].hasOvComp; } bool HWComposer::hasGlesComposition(int32_t id) const { - if (uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id)) - return false; + if (!mHwc || uint32_t(id)>31 || !mAllocatedDisplayIDs.hasBit(id)) + return true; return mDisplayData[id].hasFbComp; } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp new file mode 100644 index 000000000..d30e9bf15 --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -0,0 +1,60 @@ +/* + * Copyright 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "VirtualDisplaySurface.h" + +// --------------------------------------------------------------------------- +namespace android { +// --------------------------------------------------------------------------- + +VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int disp, + const sp& sink, const String8& name) +: mHwc(hwc), + mDisplayId(disp), + mSink(sink), + mName(name) +{} + +VirtualDisplaySurface::~VirtualDisplaySurface() { +} + +sp VirtualDisplaySurface::getIGraphicBufferProducer() const { + return mSink; +} + +status_t VirtualDisplaySurface::compositionComplete() { + return NO_ERROR; +} + +status_t VirtualDisplaySurface::advanceFrame() { + return NO_ERROR; +} + +status_t VirtualDisplaySurface::setReleaseFenceFd(int fenceFd) { + if (fenceFd >= 0) { + ALOGW("[%s] unexpected release fence", mName.string()); + close(fenceFd); + } + return NO_ERROR; +} + +void VirtualDisplaySurface::dump(String8& result) const { +} + +// --------------------------------------------------------------------------- +} // namespace android +// --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h new file mode 100644 index 000000000..db886ed0e --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -0,0 +1,57 @@ +/* + * Copyright 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_SF_VIRTUAL_DISPLAY_SURFACE_H +#define ANDROID_SF_VIRTUAL_DISPLAY_SURFACE_H + +#include +#include "DisplaySurface.h" + +// --------------------------------------------------------------------------- +namespace android { +// --------------------------------------------------------------------------- + +class HWComposer; + +class VirtualDisplaySurface : public DisplaySurface { +public: + VirtualDisplaySurface(HWComposer& hwc, int disp, + const sp& sink, + const String8& name); + + virtual sp getIGraphicBufferProducer() const; + + virtual status_t compositionComplete(); + virtual status_t advanceFrame(); + virtual status_t setReleaseFenceFd(int fenceFd); + virtual void dump(String8& result) const; + +private: + virtual ~VirtualDisplaySurface(); + + // immutable after construction + HWComposer& mHwc; + int mDisplayId; + sp mSink; + String8 mName; +}; + +// --------------------------------------------------------------------------- +} // namespace android +// --------------------------------------------------------------------------- + +#endif // ANDROID_SF_VIRTUAL_DISPLAY_SURFACE_H + diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 551f9023e..88c7fa0db 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -66,6 +66,7 @@ #include "DisplayHardware/FramebufferSurface.h" #include "DisplayHardware/HWComposer.h" +#include "DisplayHardware/VirtualDisplaySurface.h" #define EGL_VERSION_HW_ANDROID 0x3143 @@ -501,11 +502,9 @@ status_t SurfaceFlinger::readyToRun() createBuiltinDisplayLocked(type); wp token = mBuiltinDisplays[i]; - sp fbs = new FramebufferSurface(*mHwc, i); - sp stc = new Surface( - static_cast< sp >(fbs->getBufferQueue())); sp hw = new DisplayDevice(this, - type, isSecure, token, stc, fbs, mEGLConfig); + type, isSecure, token, new FramebufferSurface(*mHwc, i), + mEGLConfig); if (i > DisplayDevice::DISPLAY_PRIMARY) { // FIXME: currently we don't get blank/unblank requests // for displays other than the main display, so we always @@ -1174,10 +1173,14 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) if (draw.indexOfKey(curr.keyAt(i)) < 0) { const DisplayDeviceState& state(curr[i]); - sp fbs; - sp stc; - if (!state.isVirtualDisplay()) { - + sp dispSurface; + if (state.isVirtualDisplay()) { + if (state.surface != NULL) { + dispSurface = new VirtualDisplaySurface( + *mHwc, state.type, state.surface, + state.displayName); + } + } else { ALOGE_IF(state.surface!=NULL, "adding a supported display, but rendering " "surface is provided (%p), ignoring it", @@ -1185,21 +1188,14 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) // for supported (by hwc) displays we provide our // own rendering surface - fbs = new FramebufferSurface(*mHwc, state.type); - stc = new Surface( - static_cast< sp >( - fbs->getBufferQueue())); - } else { - if (state.surface != NULL) { - stc = new Surface(state.surface); - } + dispSurface = new FramebufferSurface(*mHwc, state.type); } const wp& display(curr.keyAt(i)); - if (stc != NULL) { + if (dispSurface != NULL) { sp hw = new DisplayDevice(this, - state.type, state.isSecure, display, stc, fbs, - mEGLConfig); + state.type, state.isSecure, display, + dispSurface, mEGLConfig); hw->setLayerStack(state.layerStack); hw->setProjection(state.orientation, state.viewport, state.frame); From 80e0a397a4712666661ecc629a64ec26e7f6aac3 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Fri, 15 Mar 2013 12:32:10 -0700 Subject: [PATCH 4/4] Add BufferQueueInterposer and use it for virtual displays BufferQueueInterposer allows a client to tap into a IGraphicBufferProducer-based buffer queue, and modify buffers as they pass from producer to consumer. VirtualDisplaySurface uses this to layer HWC composition on top of GLES composition before passing the buffer to the virtual display consumer. Bug: 8384764 Change-Id: I61ae54f3d90de6a35f4f02bb5e64e7cc88e1cb83 --- services/surfaceflinger/Android.mk | 1 + .../DisplayHardware/BufferQueueInterposer.cpp | 238 ++++++++++++++++++ .../DisplayHardware/BufferQueueInterposer.h | 152 +++++++++++ .../DisplayHardware/HWComposer.cpp | 12 + .../DisplayHardware/HWComposer.h | 2 +- .../DisplayHardware/VirtualDisplaySurface.cpp | 57 ++++- .../DisplayHardware/VirtualDisplaySurface.h | 31 ++- 7 files changed, 483 insertions(+), 10 deletions(-) create mode 100644 services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp create mode 100644 services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h diff --git a/services/surfaceflinger/Android.mk b/services/surfaceflinger/Android.mk index 6f89ffadf..30a01be02 100644 --- a/services/surfaceflinger/Android.mk +++ b/services/surfaceflinger/Android.mk @@ -14,6 +14,7 @@ LOCAL_SRC_FILES:= \ SurfaceFlingerConsumer.cpp \ SurfaceTextureLayer.cpp \ Transform.cpp \ + DisplayHardware/BufferQueueInterposer.cpp \ DisplayHardware/FramebufferSurface.cpp \ DisplayHardware/HWComposer.cpp \ DisplayHardware/PowerHAL.cpp \ diff --git a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp new file mode 100644 index 000000000..d8ad22444 --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.cpp @@ -0,0 +1,238 @@ +/* + * Copyright 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#undef LOG_TAG +#define LOG_TAG "BQInterposer" + +#include "BufferQueueInterposer.h" + +// --------------------------------------------------------------------------- +namespace android { +// --------------------------------------------------------------------------- + +#define BQI_LOGV(x, ...) ALOGV("[%s] "x, mName.string(), ##__VA_ARGS__) +#define BQI_LOGD(x, ...) ALOGD("[%s] "x, mName.string(), ##__VA_ARGS__) +#define BQI_LOGI(x, ...) ALOGI("[%s] "x, mName.string(), ##__VA_ARGS__) +#define BQI_LOGW(x, ...) ALOGW("[%s] "x, mName.string(), ##__VA_ARGS__) +#define BQI_LOGE(x, ...) ALOGE("[%s] "x, mName.string(), ##__VA_ARGS__) + +// Get an ID that's unique within this process. +static int32_t createProcessUniqueId() { + static volatile int32_t globalCounter = 0; + return android_atomic_inc(&globalCounter); +} + +BufferQueueInterposer::BufferQueueInterposer( + const sp& sink, const String8& name) +: mSink(sink), + mName(name), + 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() { + Mutex::Autolock lock(mMutex); + flushQueuedBuffersLocked(); + BQI_LOGV("~BufferQueueInterposer"); +} + +status_t BufferQueueInterposer::requestBuffer(int slot, + sp* outBuf) { + BQI_LOGV("requestBuffer slot=%d", slot); + Mutex::Autolock lock(mMutex); + + if (size_t(slot) >= mBuffers.size()) { + size_t size = mBuffers.size(); + mBuffers.insertAt(size, size - slot + 1); + } + sp& buf = mBuffers.editItemAt(slot); + + status_t result = mSink->requestBuffer(slot, &buf); + *outBuf = buf; + return result; +} + +status_t BufferQueueInterposer::setBufferCount(int bufferCount) { + BQI_LOGV("setBufferCount count=%d", bufferCount); + Mutex::Autolock lock(mMutex); + + bufferCount += 1; + + status_t result = flushQueuedBuffersLocked(); + if (result != NO_ERROR) + return result; + + result = mSink->setBufferCount(bufferCount); + if (result != NO_ERROR) + return result; + + for (size_t i = 0; i < mBuffers.size(); i++) + mBuffers.editItemAt(i).clear(); + ssize_t n = mBuffers.resize(bufferCount); + result = (n < 0) ? n : result; + + return result; +} + +status_t BufferQueueInterposer::dequeueBuffer(int* slot, sp* fence, + uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { + BQI_LOGV("dequeueBuffer %ux%u fmt=%u usage=%#x", w, h, format, usage); + return mSink->dequeueBuffer(slot, fence, w, h, format, usage); +} + +status_t BufferQueueInterposer::queueBuffer(int slot, + const QueueBufferInput& input, QueueBufferOutput* output) { + BQI_LOGV("queueBuffer slot=%d", slot); + Mutex::Autolock lock(mMutex); + mQueue.push(QueuedBuffer(slot, input)); + *output = mQueueBufferOutput; + return NO_ERROR; +} + +void BufferQueueInterposer::cancelBuffer(int slot, const sp& fence) { + BQI_LOGV("cancelBuffer slot=%d", slot); + mSink->cancelBuffer(slot, fence); +} + +int BufferQueueInterposer::query(int what, int* value) { + BQI_LOGV("query what=%d", what); + return mSink->query(what, value); +} + +status_t BufferQueueInterposer::setSynchronousMode(bool enabled) { + BQI_LOGV("setSynchronousMode %s", enabled ? "true" : "false"); + return mSink->setSynchronousMode(enabled); +} + +status_t BufferQueueInterposer::connect(int api, QueueBufferOutput* output) { + BQI_LOGV("connect api=%d", api); + Mutex::Autolock lock(mMutex); + status_t result = mSink->connect(api, &mQueueBufferOutput); + if (result == NO_ERROR) { + *output = mQueueBufferOutput; + } + return result; +} + +status_t BufferQueueInterposer::disconnect(int api) { + BQI_LOGV("disconnect: api=%d", api); + Mutex::Autolock lock(mMutex); + flushQueuedBuffersLocked(); + return mSink->disconnect(api); +} + +status_t BufferQueueInterposer::pullEmptyBuffer() { + status_t result; + + int slot; + sp fence; + result = dequeueBuffer(&slot, &fence, 0, 0, 0, 0); + if (result == IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) { + sp buffer; + result = requestBuffer(slot, &buffer); + } else if (result != NO_ERROR) { + return result; + } + + uint32_t w, h, transformHint, numPendingBuffers; + mQueueBufferOutput.deflate(&w, &h, &transformHint, &numPendingBuffers); + + IGraphicBufferProducer::QueueBufferInput qbi(0, Rect(w, h), + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, fence); + IGraphicBufferProducer::QueueBufferOutput qbo; + result = queueBuffer(slot, qbi, &qbo); + if (result != NO_ERROR) + return result; + + return NO_ERROR; +} + +status_t BufferQueueInterposer::acquireBuffer(sp* buf, + sp* fence) { + Mutex::Autolock lock(mMutex); + if (mQueue.empty()) { + BQI_LOGV("acquireBuffer: no buffers available"); + return NO_BUFFER_AVAILABLE; + } + if (mAcquired) { + BQI_LOGE("acquireBuffer: buffer already acquired"); + return BUFFER_ALREADY_ACQUIRED; + } + BQI_LOGV("acquireBuffer: acquiring slot %d", mQueue[0].slot); + + *buf = mBuffers[mQueue[0].slot]; + *fence = mQueue[0].fence; + mAcquired = true; + return NO_ERROR; +} + +status_t BufferQueueInterposer::releaseBuffer(const sp& fence) { + Mutex::Autolock lock(mMutex); + if (!mAcquired) { + BQI_LOGE("releaseBuffer: releasing a non-acquired buffer"); + return BUFFER_NOT_ACQUIRED; + } + BQI_LOGV("releaseBuffer: releasing slot %d to sink", mQueue[0].slot); + + const QueuedBuffer& b = mQueue[0]; + status_t result = mSink->queueBuffer(b.slot, + QueueBufferInput(b.timestamp, b.crop, b.scalingMode, + b.transform, b.fence), + &mQueueBufferOutput); + mQueue.removeAt(0); + mAcquired = false; + + return result; +} + +status_t BufferQueueInterposer::flushQueuedBuffersLocked() { + if (mAcquired) { + BQI_LOGE("flushQueuedBuffersLocked: buffer acquired, can't flush"); + return INVALID_OPERATION; + } + + status_t result = NO_ERROR; + for (size_t i = 0; i < mQueue.size(); i++) { + const QueuedBuffer& b = mQueue[i]; + BQI_LOGV("flushing queued slot %d to sink", b.slot); + status_t err = mSink->queueBuffer(b.slot, + QueueBufferInput(b.timestamp, b.crop, b.scalingMode, + b.transform, b.fence), + &mQueueBufferOutput); + if (err != NO_ERROR && result == NO_ERROR) // latch first error + result = err; + } + mQueue.clear(); + return result; +} + +// --------------------------------------------------------------------------- +} // namespace android +// --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h new file mode 100644 index 000000000..720863011 --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/BufferQueueInterposer.h @@ -0,0 +1,152 @@ +/* + * Copyright 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_SF_BUFFERQUEUEINTERPOSER_H +#define ANDROID_SF_BUFFERQUEUEINTERPOSER_H + +#include +#include +#include + +// --------------------------------------------------------------------------- +namespace android { +// --------------------------------------------------------------------------- + +// BufferQueueInterposers introduce an extra stage between a buffer producer +// (the source) and a buffer consumer (the sink), which communicate via the +// IGraphicBufferProducer interface. It is designed to be as transparent as +// possible to both endpoints, so that they can work the same whether an +// interposer is present or not. +// +// When the interpose is present, the source queues buffers to the +// IGraphicBufferProducer implemented by BufferQueueInterposer. A client of +// the BufferQueueInterposer can acquire each buffer in turn and read or +// modify it, releasing the buffer when finished. When the buffer is released, +// the BufferQueueInterposer queues it to the original IGraphicBufferProducer +// interface representing the sink. +// +// A BufferQueueInterposer can be used to do additional rendering to a buffer +// before it is consumed -- essentially pipelining two producers. As an +// example, SurfaceFlinger uses this to implement mixed GLES and HWC +// compositing to the same buffer for virtual displays. If it used two separate +// buffer queues, then in GLES-only or mixed GLES+HWC compositing, the HWC +// would have to copy the GLES output buffer to the HWC output buffer, using +// more bandwidth than having HWC do additional composition "in place" on the +// GLES output buffer. +// +// The goal for this class is to be usable in a variety of situations and be +// part of libgui. But both the interface and implementation need some +// iteration before then, so for now it should only be used by +// VirtualDisplaySurface, which is why it's currently in SurfaceFlinger. +// +// Some of the problems that still need to be solved are: +// +// - Refactor the interposer interface along with BufferQueue and ConsumerBase, +// so that there is a common interface for the consumer end of a queue. The +// 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). +// +// - 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. + +class BufferQueueInterposer : public BnGraphicBufferProducer { +public: + BufferQueueInterposer(const sp& sink, + const String8& name); + + // + // IGraphicBufferProducer interface + // + virtual status_t requestBuffer(int slot, sp* outBuf); + virtual status_t setBufferCount(int bufferCount); + virtual status_t dequeueBuffer(int* slot, sp* fence, + uint32_t w, uint32_t h, uint32_t format, uint32_t usage); + virtual status_t queueBuffer(int slot, + const QueueBufferInput& input, QueueBufferOutput* output); + virtual void cancelBuffer(int slot, const sp& fence); + virtual int query(int what, int* value); + virtual status_t setSynchronousMode(bool enabled); + virtual status_t connect(int api, QueueBufferOutput* output); + virtual status_t disconnect(int api); + + // + // Interposer interface + // + + enum { + NO_BUFFER_AVAILABLE = 2, // matches BufferQueue + BUFFER_NOT_ACQUIRED, + BUFFER_ALREADY_ACQUIRED, + }; + + // Acquire the oldest queued buffer. If no buffers are pending, returns + // NO_BUFFER_AVAILABLE. If a buffer is currently acquired, returns + // BUFFER_ALREADY_ACQUIRED. + status_t acquireBuffer(sp* buf, sp* fence); + + // Release the currently acquired buffer, queueing it to the sink. If the + // current buffer hasn't been acquired, returns BUFFER_NOT_ACQUIRED. + status_t releaseBuffer(const sp& fence); + + // pullEmptyBuffer dequeues a buffer from the sink, then immediately + // queues it to the interposer. This makes a buffer available for the + // client to acquire even if the source hasn't queued one. + status_t pullEmptyBuffer(); + +private: + struct QueuedBuffer { + QueuedBuffer(): slot(-1) {} + QueuedBuffer(int slot, const QueueBufferInput& qbi): slot(slot) { + qbi.deflate(×tamp, &crop, &scalingMode, &transform, &fence); + } + int slot; + int64_t timestamp; + Rect crop; + int scalingMode; + uint32_t transform; + sp fence; + }; + + virtual ~BufferQueueInterposer(); + status_t flushQueuedBuffersLocked(); + + const sp mSink; + String8 mName; + + Mutex mMutex; + Vector > mBuffers; + Vector mQueue; + bool mAcquired; + QueueBufferOutput mQueueBufferOutput; +}; + +// --------------------------------------------------------------------------- +} // namespace android +// --------------------------------------------------------------------------- + +#endif // ANDROID_SF_BUFFERQUEUEINTERPOSER_H diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 4b6c4130b..eeb1fef4c 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -655,6 +655,18 @@ 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=HWC_NUM_DISPLAY_TYPES; ioutbuf = disp.framebufferTarget->handle; + mLists[i]->outbufAcquireFenceFd = + dup(disp.framebufferTarget->acquireFenceFd); + } + } + err = mHwc->set(mHwc, mNumDisplays, mLists); for (size_t i=0 ; i #include "VirtualDisplaySurface.h" +#include "HWComposer.h" // --------------------------------------------------------------------------- namespace android { @@ -25,15 +25,22 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int disp, const sp& sink, const String8& name) : mHwc(hwc), mDisplayId(disp), - mSink(sink), - mName(name) + mSource(new BufferQueueInterposer(sink, name)), + mName(name), + mReleaseFence(Fence::NO_FENCE) {} VirtualDisplaySurface::~VirtualDisplaySurface() { + if (mAcquiredBuffer != NULL) { + status_t result = mSource->releaseBuffer(mReleaseFence); + ALOGE_IF(result != NO_ERROR, "VirtualDisplaySurface \"%s\": " + "failed to release previous buffer: %d", + mName.string(), result); + } } sp VirtualDisplaySurface::getIGraphicBufferProducer() const { - return mSink; + return mSource; } status_t VirtualDisplaySurface::compositionComplete() { @@ -41,13 +48,49 @@ status_t VirtualDisplaySurface::compositionComplete() { } status_t VirtualDisplaySurface::advanceFrame() { - return NO_ERROR; + Mutex::Autolock lock(mMutex); + 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; + } + + sp fence; + result = mSource->acquireBuffer(&mAcquiredBuffer, &fence); + if (result == BufferQueueInterposer::NO_BUFFER_AVAILABLE) { + result = mSource->pullEmptyBuffer(); + if (result != NO_ERROR) + return result; + result = mSource->acquireBuffer(&mAcquiredBuffer, &fence); + } + if (result != NO_ERROR) + return result; + + return mHwc.fbPost(mDisplayId, fence, mAcquiredBuffer); } status_t VirtualDisplaySurface::setReleaseFenceFd(int fenceFd) { if (fenceFd >= 0) { - ALOGW("[%s] unexpected release fence", mName.string()); - close(fenceFd); + sp fence(new Fence(fenceFd)); + Mutex::Autolock lock(mMutex); + sp 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; } return NO_ERROR; } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index db886ed0e..66f858081 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -17,7 +17,7 @@ #ifndef ANDROID_SF_VIRTUAL_DISPLAY_SURFACE_H #define ANDROID_SF_VIRTUAL_DISPLAY_SURFACE_H -#include +#include "BufferQueueInterposer.h" #include "DisplaySurface.h" // --------------------------------------------------------------------------- @@ -26,6 +26,28 @@ namespace android { class HWComposer; +/* This DisplaySurface implementation uses a BufferQueueInterposer to pass + * partially- or fully-composited buffers from the OpenGL ES driver to + * HWComposer to use as the output buffer for virtual displays. Allowing HWC + * to compose into the same buffer that contains GLES results saves bandwidth + * compared to having two separate BufferQueues for frames with at least some + * GLES composition. + * + * The alternative would be to have two complete BufferQueues, one from GLES + * to HWC and one from HWC to the virtual display sink (e.g. video encoder). + * For GLES-only frames, the same bandwidth saving could be achieved if buffers + * could be acquired from the GLES->HWC queue and inserted into the HWC->sink + * queue. That would be complicated and doesn't help the mixed GLES+HWC case. + * + * On frames with no GLES composition, the VirtualDisplaySurface dequeues a + * buffer directly from the sink IGraphicBufferProducer and passes it to HWC, + * bypassing the GLES driver. This is only guaranteed to work if + * eglSwapBuffers doesn't immediately dequeue a buffer for the next frame, + * since we can't rely on being able to dequeue more than one buffer at a time. + * + * TODO(jessehall): Add a libgui test that ensures that EGL/GLES do lazy + * dequeBuffers; we've wanted to require that for other reasons anyway. + */ class VirtualDisplaySurface : public DisplaySurface { public: VirtualDisplaySurface(HWComposer& hwc, int disp, @@ -45,8 +67,13 @@ private: // immutable after construction HWComposer& mHwc; int mDisplayId; - sp mSink; + sp mSource; String8 mName; + + // mutable, must be synchronized with mMutex + Mutex mMutex; + sp mAcquiredBuffer; + sp mReleaseFence; }; // ---------------------------------------------------------------------------