From 028dc8f2d72bc7cd4fbe7808781443125a742f78 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Tue, 20 Aug 2013 16:35:32 -0700 Subject: [PATCH] Provide HWC prepare with a valid output buffer We weren't dequeing and setting the output buffer until just before set(). This didn't allow HWC to make decisions in prepare() based on the output buffer format, dimensions, etc. Now we dequeue the output buffer at the beginning of the composition loop and provide it to HWC in prepare. In GLES-only rendering, we may have to cancel the buffer and acquire a new one if GLES requests a buffer with properties different than the one we already dequeued. Bug: 10365313 Change-Id: I96b4b0a851920e4334ef05080d58097d46467ab8 --- services/surfaceflinger/DisplayDevice.cpp | 4 + services/surfaceflinger/DisplayDevice.h | 1 + .../DisplayHardware/DisplaySurface.h | 5 + .../DisplayHardware/FramebufferSurface.cpp | 4 + .../DisplayHardware/FramebufferSurface.h | 1 + .../DisplayHardware/VirtualDisplaySurface.cpp | 105 ++++++++++++------ .../DisplayHardware/VirtualDisplaySurface.h | 14 ++- services/surfaceflinger/SurfaceFlinger.cpp | 4 + 8 files changed, 100 insertions(+), 38 deletions(-) diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 79b6689c8..e6905b79c 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -198,6 +198,10 @@ void DisplayDevice::flip(const Region& dirty) const mPageFlipCount++; } +status_t DisplayDevice::beginFrame() const { + return mDisplaySurface->beginFrame(); +} + status_t DisplayDevice::prepareFrame(const HWComposer& hwc) const { DisplaySurface::CompositionType compositionType; bool haveGles = hwc.hasGlesComposition(mHwcDisplayId); diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 8e24df757..3131cf255 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -121,6 +121,7 @@ public: int32_t getHwcDisplayId() const { return mHwcDisplayId; } const wp& getDisplayToken() const { return mDisplayToken; } + status_t beginFrame() const; status_t prepareFrame(const HWComposer& hwc) const; void swapBuffers(HWComposer& hwc) const; diff --git a/services/surfaceflinger/DisplayHardware/DisplaySurface.h b/services/surfaceflinger/DisplayHardware/DisplaySurface.h index 9192db5ba..48bf3f286 100644 --- a/services/surfaceflinger/DisplayHardware/DisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/DisplaySurface.h @@ -30,6 +30,11 @@ class String8; class DisplaySurface : public virtual RefBase { public: + // beginFrame is called at the beginning of the composition loop, before + // the configuration is known. The DisplaySurface should do anything it + // needs to do to enable HWComposer to decide how to compose the frame. + virtual status_t beginFrame() = 0; + // prepareFrame is called after the composition configuration is known but // before composition takes place. The DisplaySurface can use the // composition type to decide how to manage the flow of buffers between diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 807d4e17f..8c634ed64 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -68,6 +68,10 @@ FramebufferSurface::FramebufferSurface(HWComposer& hwc, int disp, mConsumer->setDefaultMaxBufferCount(NUM_FRAMEBUFFER_SURFACE_BUFFERS); } +status_t FramebufferSurface::beginFrame() { + return NO_ERROR; +} + status_t FramebufferSurface::prepareFrame(CompositionType compositionType) { return NO_ERROR; } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index fcda28717..1d67446b9 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -39,6 +39,7 @@ class FramebufferSurface : public ConsumerBase, public: FramebufferSurface(HWComposer& hwc, int disp, const sp& consumer); + virtual status_t beginFrame(); virtual status_t prepareFrame(CompositionType compositionType); virtual status_t compositionComplete(); virtual status_t advanceFrame(); diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 89c87c70f..01ca38ae3 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -71,11 +71,26 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int32_t dispId, VirtualDisplaySurface::~VirtualDisplaySurface() { } -status_t VirtualDisplaySurface::prepareFrame(CompositionType compositionType) { +status_t VirtualDisplaySurface::beginFrame() { if (mDisplayId < 0) return NO_ERROR; VDS_LOGW_IF(mDbgState != DBG_STATE_IDLE, + "Unexpected beginFrame() in %s state", dbgStateStr()); + mDbgState = DBG_STATE_BEGUN; + + uint32_t transformHint, numPendingBuffers; + mQueueBufferOutput.deflate(&mSinkBufferWidth, &mSinkBufferHeight, + &transformHint, &numPendingBuffers); + + return refreshOutputBuffer(); +} + +status_t VirtualDisplaySurface::prepareFrame(CompositionType compositionType) { + if (mDisplayId < 0) + return NO_ERROR; + + VDS_LOGW_IF(mDbgState != DBG_STATE_BEGUN, "Unexpected prepareFrame() in %s state", dbgStateStr()); mDbgState = DBG_STATE_PREPARED; @@ -109,31 +124,11 @@ status_t VirtualDisplaySurface::advanceFrame() { } mDbgState = DBG_STATE_HWC; - status_t result; - sp outFence; - if (mCompositionType != COMPOSITION_GLES) { - // Dequeue an output buffer from the sink - uint32_t transformHint, numPendingBuffers; - mQueueBufferOutput.deflate(&mSinkBufferWidth, &mSinkBufferHeight, - &transformHint, &numPendingBuffers); - int sslot; - result = dequeueBuffer(SOURCE_SINK, 0, &sslot, &outFence, false); - if (result < 0) - return result; - mOutputProducerSlot = mapSource2ProducerSlot(SOURCE_SINK, sslot); - } - if (mCompositionType == COMPOSITION_HWC) { - // We just dequeued the output buffer, use it for FB as well + // Use the output buffer for the FB as well, though conceptually the + // FB is unused on this frame. mFbProducerSlot = mOutputProducerSlot; - mFbFence = outFence; - } else if (mCompositionType == COMPOSITION_GLES) { - mOutputProducerSlot = mFbProducerSlot; - outFence = mFbFence; - } else { - // mFbFence and mFbProducerSlot were set in queueBuffer, - // and mOutputProducerSlot and outFence were set above when dequeueing - // the sink buffer. + mFbFence = mOutputFence; } if (mFbProducerSlot < 0 || mOutputProducerSlot < 0) { @@ -152,12 +147,7 @@ status_t VirtualDisplaySurface::advanceFrame() { mFbProducerSlot, fbBuffer.get(), mOutputProducerSlot, outBuffer.get()); - result = mHwc.fbPost(mDisplayId, mFbFence, fbBuffer); - if (result == NO_ERROR) { - result = mHwc.setOutputBuffer(mDisplayId, outFence, outBuffer); - } - - return result; + return mHwc.fbPost(mDisplayId, mFbFence, fbBuffer); } void VirtualDisplaySurface::onFrameCommitted() { @@ -256,17 +246,39 @@ status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp* fence, bool VDS_LOGV("dequeueBuffer %dx%d fmt=%d usage=%#x", w, h, format, usage); + status_t result = NO_ERROR; mProducerUsage = usage | GRALLOC_USAGE_HW_COMPOSER; Source source = fbSourceForCompositionType(mCompositionType); + if (source == SOURCE_SINK) { - mSinkBufferWidth = w; - mSinkBufferHeight = h; + // We already dequeued the output buffer. If the GLES driver wants + // something incompatible, we have to cancel and get a new one. This + // will mean that HWC will see a different output buffer between + // prepare and set, but since we're in GLES-only mode already it + // shouldn't matter. + + const sp& buf = mProducerBuffers[mOutputProducerSlot]; + if ((mProducerUsage & ~buf->getUsage()) != 0 || + (format != 0 && format != (uint32_t)buf->getPixelFormat()) || + (w != 0 && w != mSinkBufferWidth) || + (h != 0 && h != mSinkBufferHeight)) { + VDS_LOGV("dequeueBuffer: output buffer doesn't satisfy GLES " + "request, getting a new buffer"); + result = refreshOutputBuffer(); + if (result < 0) + return result; + } } - int sslot; - status_t result = dequeueBuffer(source, format, &sslot, fence, async); - if (result >= 0) { - *pslot = mapSource2ProducerSlot(source, sslot); + if (source == SOURCE_SINK) { + *pslot = mOutputProducerSlot; + *fence = mOutputFence; + } else { + int sslot; + result = dequeueBuffer(source, format, &sslot, fence, async); + if (result >= 0) { + *pslot = mapSource2ProducerSlot(source, sslot); + } } return result; } @@ -318,6 +330,7 @@ status_t VirtualDisplaySurface::queueBuffer(int pslot, &transform, &async, &mFbFence); mFbProducerSlot = pslot; + mOutputFence = mFbFence; } *output = mQueueBufferOutput; @@ -365,10 +378,30 @@ void VirtualDisplaySurface::resetPerFrameState() { mSinkBufferWidth = 0; mSinkBufferHeight = 0; mFbFence = Fence::NO_FENCE; + mOutputFence = Fence::NO_FENCE; mFbProducerSlot = -1; mOutputProducerSlot = -1; } +status_t VirtualDisplaySurface::refreshOutputBuffer() { + if (mOutputProducerSlot >= 0) { + mSource[SOURCE_SINK]->cancelBuffer( + mapProducer2SourceSlot(SOURCE_SINK, mOutputProducerSlot), + mOutputFence); + } + + int sslot; + status_t result = dequeueBuffer(SOURCE_SINK, 0, &sslot, &mOutputFence, false); + if (result < 0) + return result; + mOutputProducerSlot = mapSource2ProducerSlot(SOURCE_SINK, sslot); + + result = mHwc.setOutputBuffer(mDisplayId, mOutputFence, + mProducerBuffers[mOutputProducerSlot]); + + return result; +} + // This slot mapping function is its own inverse, so two copies are unnecessary. // Both are kept to make the intent clear where the function is called, and for // the (unlikely) chance that we switch to a different mapping function. diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 64a8dce28..4ab80e4f6 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -79,6 +79,7 @@ public: // // DisplaySurface interface // + virtual status_t beginFrame(); virtual status_t prepareFrame(CompositionType compositionType); virtual status_t compositionComplete(); virtual status_t advanceFrame(); @@ -112,6 +113,7 @@ private: int* sslot, sp* fence, bool async); void updateQueueBufferOutput(const QueueBufferOutput& qbo); void resetPerFrameState(); + status_t refreshOutputBuffer(); // Both the sink and scratch buffer pools have their own set of slots // ("source slots", or "sslot"). We have to merge these into the single @@ -169,6 +171,10 @@ private: // target buffer. sp mFbFence; + // mOutputFence is the fence HWC should wait for before writing to the + // output buffer. + sp mOutputFence; + // Producer slot numbers for the buffers to use for HWC framebuffer target // and output. int mFbProducerSlot; @@ -181,7 +187,8 @@ private: // +-----------+-------------------+-------------+ // | State | Event || Next State | // +-----------+-------------------+-------------+ - // | IDLE | prepareFrame || PREPARED | + // | IDLE | beginFrame || BEGUN | + // | BEGUN | prepareFrame || PREPARED | // | PREPARED | dequeueBuffer [1] || GLES | // | PREPARED | advanceFrame [2] || HWC | // | GLES | queueBuffer || GLES_DONE | @@ -194,7 +201,10 @@ private: enum DbgState { // no buffer dequeued, don't know anything about the next frame DBG_STATE_IDLE, - // no buffer dequeued, but we know the buffer source for the frame + // output buffer dequeued, framebuffer source not yet known + DBG_STATE_BEGUN, + // output buffer dequeued, framebuffer source known but not provided + // to GLES yet. DBG_STATE_PREPARED, // GLES driver has a buffer dequeued DBG_STATE_GLES, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index dcdd1d0f9..f73da3591 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -855,6 +855,10 @@ void SurfaceFlinger::rebuildLayerStacks() { } void SurfaceFlinger::setUpHWComposer() { + for (size_t dpy=0 ; dpybeginFrame(); + } + HWComposer& hwc(getHwComposer()); if (hwc.initCheck() == NO_ERROR) { // build the h/w work list