From 529efdf60f2015e591dd9cd62e8802c583a8917a Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 15 Oct 2012 18:24:43 -0700 Subject: SurfaceFlinger: add animation transactions This change adds a transaction flag for WindowManager to indicate that a transaction is being used to animate windows around the screen. SurfaceFlinger will not allow more than one of these transactions to be outstanding at a time to prevent the animation "frames" from being dropped. Bug: 7353840 Change-Id: I6488a6e0e1ed13d27356d2203c9dc766dc6b1759 --- include/gui/ISurfaceComposer.h | 1 + include/gui/SurfaceComposerClient.h | 5 ++++- libs/gui/SurfaceComposerClient.cpp | 23 ++++++++++++++++++++- services/surfaceflinger/SurfaceFlinger.cpp | 32 ++++++++++++++++++++++++------ services/surfaceflinger/SurfaceFlinger.h | 3 ++- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/include/gui/ISurfaceComposer.h b/include/gui/ISurfaceComposer.h index 5d2d8d7..002aafc 100644 --- a/include/gui/ISurfaceComposer.h +++ b/include/gui/ISurfaceComposer.h @@ -46,6 +46,7 @@ public: // flags for setTransactionState() enum { eSynchronous = 0x01, + eAnimation = 0x02, }; enum { diff --git a/include/gui/SurfaceComposerClient.h b/include/gui/SurfaceComposerClient.h index 581ec8d..21d16a9 100644 --- a/include/gui/SurfaceComposerClient.h +++ b/include/gui/SurfaceComposerClient.h @@ -101,10 +101,13 @@ public: //! Open a composer transaction on all active SurfaceComposerClients. static void openGlobalTransaction(); - + //! Close a composer transaction on all active SurfaceComposerClients. static void closeGlobalTransaction(bool synchronous = false); + //! Flag the currently open transaction as an animation transaction. + static void setAnimationTransaction(); + status_t hide(SurfaceID id); status_t show(SurfaceID id); status_t setFlags(SurfaceID id, uint32_t flags, uint32_t mask); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 3efd086..8586ed2 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -115,12 +115,15 @@ class Composer : public Singleton SortedVector mComposerStates; SortedVector mDisplayStates; uint32_t mForceSynchronous; + bool mAnimation; Composer() : Singleton(), - mForceSynchronous(0) + mForceSynchronous(0), + mAnimation(false) { } void closeGlobalTransactionImpl(bool synchronous); + void setAnimationTransactionImpl(); layer_state_t* getLayerStateLocked( const sp& client, SurfaceID id); @@ -159,6 +162,10 @@ public: const Rect& layerStackRect, const Rect& displayRect); + static void setAnimationTransaction() { + Composer::getInstance().setAnimationTransactionImpl(); + } + static void closeGlobalTransaction(bool synchronous) { Composer::getInstance().closeGlobalTransactionImpl(synchronous); } @@ -194,12 +201,22 @@ void Composer::closeGlobalTransactionImpl(bool synchronous) { if (synchronous || mForceSynchronous) { flags |= ISurfaceComposer::eSynchronous; } + if (mAnimation) { + flags |= ISurfaceComposer::eAnimation; + } + mForceSynchronous = false; + mAnimation = false; } sm->setTransactionState(transaction, displayTransaction, flags); } +void Composer::setAnimationTransactionImpl() { + Mutex::Autolock _l(mLock); + mAnimation = true; +} + layer_state_t* Composer::getLayerStateLocked( const sp& client, SurfaceID id) { @@ -471,6 +488,10 @@ void SurfaceComposerClient::closeGlobalTransaction(bool synchronous) { Composer::closeGlobalTransaction(synchronous); } +void SurfaceComposerClient::setAnimationTransaction() { + Composer::setAnimationTransaction(); +} + // ---------------------------------------------------------------------------- status_t SurfaceComposerClient::setCrop(SurfaceID id, const Rect& crop) { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 492d1cf..e21e2bf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -86,7 +86,8 @@ const String16 sDump("android.permission.DUMP"); SurfaceFlinger::SurfaceFlinger() : BnSurfaceComposer(), Thread(false), mTransactionFlags(0), - mTransationPending(false), + mTransactionPending(false), + mAnimTransactionPending(false), mLayersRemoved(false), mRepaintEverything(0), mBootTime(systemTime()), @@ -1264,7 +1265,8 @@ void SurfaceFlinger::commitTransaction() } mDrawingState = mCurrentState; - mTransationPending = false; + mTransactionPending = false; + mAnimTransactionPending = false; mTransactionCV.broadcast(); } @@ -1665,6 +1667,21 @@ void SurfaceFlinger::setTransactionState( Mutex::Autolock _l(mStateLock); uint32_t transactionFlags = 0; + if (flags & eAnimation) { + // For window updates that are part of an animation we must wait for + // previous animation "frames" to be handled. + while (mAnimTransactionPending) { + status_t err = mTransactionCV.waitRelative(mStateLock, 500 * 1000); + if (CC_UNLIKELY(err != NO_ERROR)) { + // just in case something goes wrong in SF, return to the + // caller after a few hundred microseconds. + ALOGW_IF(err == TIMED_OUT, "setTransactionState timed out!"); + mAnimTransactionPending = false; + break; + } + } + } + size_t count = displays.size(); for (size_t i=0 ; i > mLayerPurgatory; - bool mTransationPending; + bool mTransactionPending; + bool mAnimTransactionPending; Vector > mLayersPendingRemoval; // protected by mStateLock (but we could use another lock) -- cgit v1.1 From 4ea5d656db0f5fcc4ae6c792ec83e157a2c1bae8 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Mon, 15 Oct 2012 12:38:33 -0700 Subject: Always set vertex alpha when drawing screenshot layers The screenshot is a GL_RGB texture, and the GL_REPLACE texture env mode uses vertex alpha for GL_RGB textures instead of alpha=1.0. Bug: 7340077 Change-Id: I6fbb907023e48f9c422b15a33da79757d6726840 --- services/surfaceflinger/LayerScreenshot.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/LayerScreenshot.cpp b/services/surfaceflinger/LayerScreenshot.cpp index 1aa8c09..0fd744f 100644 --- a/services/surfaceflinger/LayerScreenshot.cpp +++ b/services/surfaceflinger/LayerScreenshot.cpp @@ -122,13 +122,14 @@ void LayerScreenshot::onDraw(const sp& hw, const Region& cl } else { glEnable(GL_BLEND); glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); - glColor4f(alpha, alpha, alpha, alpha); glTexEnvx(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE); } LayerMesh mesh; computeGeometry(hw, &mesh); + glColor4f(alpha, alpha, alpha, alpha); + glDisable(GL_TEXTURE_EXTERNAL_OES); glEnable(GL_TEXTURE_2D); -- cgit v1.1 From c11f56e5615c5d388c072705322df5bcf22c2012 Mon Sep 17 00:00:00 2001 From: Dmitry Shmidt Date: Wed, 7 Nov 2012 10:42:05 -0800 Subject: bugreport: Add wlan FW counters dump for bcmdhd Bug: 7494877 Change-Id: I4d24dbcf7596777ec02ab1312c2ab996ba7688f1 Signed-off-by: Dmitry Shmidt --- cmds/dumpstate/dumpstate.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmds/dumpstate/dumpstate.c b/cmds/dumpstate/dumpstate.c index f3fcca0..bb54e4b 100644 --- a/cmds/dumpstate/dumpstate.c +++ b/cmds/dumpstate/dumpstate.c @@ -188,6 +188,10 @@ static void dumpstate() { run_command("WIFI NETWORKS", 20, SU_PATH, "root", "wpa_cli", "list_networks", NULL); +#ifdef FWDUMP_bcmdhd + run_command("DUMP WIFI INTERNAL COUNTERS", 20, + SU_PATH, "root", "wlutil", "counters", NULL); +#endif property_get("dhcp.wlan0.gateway", network, ""); if (network[0]) run_command("PING GATEWAY", 10, SU_PATH, "root", "ping", "-c", "3", "-i", ".5", network, NULL); @@ -197,7 +201,7 @@ static void dumpstate() { property_get("dhcp.wlan0.dns2", network, ""); if (network[0]) run_command("PING DNS2", 10, SU_PATH, "root", "ping", "-c", "3", "-i", ".5", network, NULL); -#ifdef FWDUMP_bcm4329 +#ifdef FWDUMP_bcmdhd run_command("DUMP WIFI STATUS", 20, SU_PATH, "root", "dhdutil", "-i", "wlan0", "dump", NULL); run_command("DUMP WIFI INTERNAL COUNTERS", 20, -- cgit v1.1 From 0b2c9268265e9a165551eaa66cb461d3fab8b564 Mon Sep 17 00:00:00 2001 From: Dmitry Shmidt Date: Wed, 7 Nov 2012 11:09:46 -0800 Subject: bugreport: Add /proc/interrupts dump Bug: 7301178 Change-Id: Ifad3b981ac904d4637d69dfc223d5bb2c9d80bda Signed-off-by: Dmitry Shmidt --- cmds/dumpstate/dumpstate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmds/dumpstate/dumpstate.c b/cmds/dumpstate/dumpstate.c index bb54e4b..8718bb6 100644 --- a/cmds/dumpstate/dumpstate.c +++ b/cmds/dumpstate/dumpstate.c @@ -192,6 +192,8 @@ static void dumpstate() { run_command("DUMP WIFI INTERNAL COUNTERS", 20, SU_PATH, "root", "wlutil", "counters", NULL); #endif + dump_file("INTERRUPTS (1)", "/proc/interrupts"); + property_get("dhcp.wlan0.gateway", network, ""); if (network[0]) run_command("PING GATEWAY", 10, SU_PATH, "root", "ping", "-c", "3", "-i", ".5", network, NULL); @@ -207,6 +209,7 @@ static void dumpstate() { run_command("DUMP WIFI INTERNAL COUNTERS", 20, SU_PATH, "root", "wlutil", "counters", NULL); #endif + dump_file("INTERRUPTS (2)", "/proc/interrupts"); print_properties(); -- cgit v1.1 From 3fdeb48e3c89be15fe3b539a26ec9a3cf28c898c Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Mon, 29 Oct 2012 16:49:44 -0700 Subject: Reduce emulator logspam The emulator doesn't support systrace, but we should point that out at most once per process. Bug 7436352 Change-Id: I06b2c1ea0df6c02c11cd2496423c337f8d7c62a1 --- include/utils/Trace.h | 2 ++ libs/utils/Trace.cpp | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/utils/Trace.h b/include/utils/Trace.h index 93e2285..41bce00 100644 --- a/include/utils/Trace.h +++ b/include/utils/Trace.h @@ -54,6 +54,8 @@ #define ATRACE_TAG_CAMERA (1<<10) #define ATRACE_TAG_LAST ATRACE_TAG_CAMERA +#define ATRACE_TAG_NOT_READY (1LL<<63) // Reserved for use during init + #define ATRACE_TAG_VALID_MASK ((ATRACE_TAG_LAST - 1) | ATRACE_TAG_LAST) #ifndef ATRACE_TAG diff --git a/libs/utils/Trace.cpp b/libs/utils/Trace.cpp index 5cd5731..f5aaea3 100644 --- a/libs/utils/Trace.cpp +++ b/libs/utils/Trace.cpp @@ -25,7 +25,7 @@ namespace android { volatile int32_t Tracer::sIsReady = 0; int Tracer::sTraceFD = -1; -uint64_t Tracer::sEnabledTags = 0; +uint64_t Tracer::sEnabledTags = ATRACE_TAG_NOT_READY; Mutex Tracer::sMutex; void Tracer::changeCallback() { @@ -46,7 +46,7 @@ void Tracer::init() { sTraceFD = open(traceFileName, O_WRONLY); if (sTraceFD == -1) { ALOGE("error opening trace file: %s (%d)", strerror(errno), errno); - // sEnabledTags remains zero indicating that no tracing can occur + sEnabledTags = 0; // no tracing can occur } else { loadSystemProperty(); } -- cgit v1.1 From ee932d0ad1a16cc93b4bd9eaf9cb3cc756fb2dfc Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 14 Nov 2012 14:41:42 -0800 Subject: Reset compositionType to HWC_FRAMEBUFFER before calling prepare() Honor the documentation. this broke in JB-MR1. Change-Id: I841a93b409fc940374bc748c4e143d82a192669c --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 4 +++- services/surfaceflinger/DisplayHardware/HWComposer.h | 1 + services/surfaceflinger/LayerBase.cpp | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 31d731e..29b778d 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -803,7 +803,9 @@ public: virtual void setAcquireFenceFd(int fenceFd) { getLayer()->acquireFenceFd = fenceFd; } - + virtual void setPerFrameDefaultState() { + getLayer()->compositionType = HWC_FRAMEBUFFER; + } virtual void setDefaultState() { getLayer()->compositionType = HWC_FRAMEBUFFER; getLayer()->hints = 0; diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index a78ffac..7c67407 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -141,6 +141,7 @@ public: virtual int32_t getCompositionType() const = 0; virtual uint32_t getHints() const = 0; virtual int getAndResetReleaseFenceFd() = 0; + virtual void setPerFrameDefaultState() = 0; virtual void setDefaultState() = 0; virtual void setSkip(bool skip) = 0; virtual void setBlending(uint32_t blending) = 0; diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index db4ef87..9b03c74 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -300,6 +300,7 @@ void LayerBase::setGeometry( void LayerBase::setPerFrameData(const sp& hw, HWComposer::HWCLayerInterface& layer) { + layer.setPerFrameDefaultState(); // we have to set the visible region on every frame because // we currently free it during onLayerDisplayed(), which is called // after HWComposer::commit() -- every frame. -- cgit v1.1 From 2a8c49eb5dd51b2e60c9a78bea00870867d91c03 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 15 Nov 2012 17:19:48 -0800 Subject: fix an out-of-bounds memory access in this particular case, this OOB is always harmless (and that's why it didn't get fixed from MR1), however, it interfers with valgrind debugging. Change-Id: Ic977e03287e59c4b124a89146c9023bd0cb540a8 --- libs/gui/BufferQueue.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 590946a..607e0bd 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -314,10 +314,12 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, * the consumer may still have pending reads of the * buffers in flight. */ - bool isOlder = mSlots[i].mFrameNumber < - mSlots[found].mFrameNumber; - if (found < 0 || isOlder) { - found = i; + if (found >= 0) { + bool isOlder = mSlots[i].mFrameNumber < + mSlots[found].mFrameNumber; + if (isOlder) { + found = i; + } } } } -- cgit v1.1 From 3e095b251503d71bea04d6b707e8188cd30034e2 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 19 Nov 2012 15:07:33 -0800 Subject: workaround: don't reset compositionType to HWC_FRAMEBUFFER [DO NOT MERGE] This workaround a HWC HAL issue in Nexus 7, which causes videos and live wallpapers to animate slowly. Bug: 7563862 Change-Id: I16ad85317e3e7f47f005e7397357c14186b0a13d --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 29b778d..2eb74b7 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -804,7 +804,7 @@ public: getLayer()->acquireFenceFd = fenceFd; } virtual void setPerFrameDefaultState() { - getLayer()->compositionType = HWC_FRAMEBUFFER; + //getLayer()->compositionType = HWC_FRAMEBUFFER; } virtual void setDefaultState() { getLayer()->compositionType = HWC_FRAMEBUFFER; -- cgit v1.1 From 93f838b3a6ccf6a6a17b0bc345391b139c34185d Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 19 Nov 2012 15:07:33 -0800 Subject: workaround: don't reset compositionType to HWC_FRAMEBUFFER [DO NOT MERGE] This workaround a HWC HAL issue in Nexus 7, which causes videos and live wallpapers to animate slowly. Bug: 7563862 Change-Id: I16ad85317e3e7f47f005e7397357c14186b0a13d --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 29b778d..2eb74b7 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -804,7 +804,7 @@ public: getLayer()->acquireFenceFd = fenceFd; } virtual void setPerFrameDefaultState() { - getLayer()->compositionType = HWC_FRAMEBUFFER; + //getLayer()->compositionType = HWC_FRAMEBUFFER; } virtual void setDefaultState() { getLayer()->compositionType = HWC_FRAMEBUFFER; -- cgit v1.1 From 764c197c6fc2bf10b038c33b320a4e95594d52d8 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 19 Nov 2012 16:50:24 -0800 Subject: fix typo that broke all the builds Bug: 7584338 Change-Id: Ieb8c27a544ac583af9aa1e0376e33a673d2d9673 --- libs/gui/BufferQueue.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 607e0bd..086e298 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -314,12 +314,9 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, * the consumer may still have pending reads of the * buffers in flight. */ - if (found >= 0) { - bool isOlder = mSlots[i].mFrameNumber < - mSlots[found].mFrameNumber; - if (isOlder) { - found = i; - } + if ((found < 0) || + mSlots[i].mFrameNumber < mSlots[found].mFrameNumber) { + found = i; } } } -- cgit v1.1 From a9a4cd4806ea5b2cf525c8ab4c6604d78c6e3f8f Mon Sep 17 00:00:00 2001 From: Siva Velusamy Date: Tue, 20 Nov 2012 13:39:57 -0800 Subject: gltrace: Make sure device is debuggable. (cherry picked from commit 6482fa4db0a7ac99cd3503d6bf170f80b26fb695) Change-Id: I205aabcab1932025c12e7ba3d1b3cf94684f6758 --- opengl/libs/EGL/egl.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opengl/libs/EGL/egl.cpp b/opengl/libs/EGL/egl.cpp index 96e1cba..0d4bed5 100644 --- a/opengl/libs/EGL/egl.cpp +++ b/opengl/libs/EGL/egl.cpp @@ -124,6 +124,12 @@ void initEglTraceLevel() { void initEglDebugLevel() { int propertyLevel = 0; char value[PROPERTY_VALUE_MAX]; + + // check system property only on userdebug or eng builds + property_get("ro.debuggable", value, "0"); + if (value[0] == '0') + return; + property_get("debug.egl.debug_proc", value, ""); if (strlen(value) > 0) { long pid = getpid(); -- cgit v1.1 From 8430095879d2fa6878e68f8f12da4e704815ac09 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 21 Nov 2012 16:02:13 -0800 Subject: make transform hint multi-display aware if a layer is not mirrored, we now use its display as the source for the transfrom hint calculation instead of always using the default (main) display. this change does two thing: 1) we make updateTransformHint take a DisplayDevice as a parameter instead of hard-coding the main display. 2) each time we do a transaction that could change the hint, we go through all layers and figure out which display should be used for their transform hint. Bug: 7599344 Change-Id: I9b04a95e6c372dd770bacf81d8ef6f8e31b87b83 --- services/surfaceflinger/Layer.cpp | 12 +++--- services/surfaceflinger/Layer.h | 4 +- services/surfaceflinger/LayerBase.h | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 69 ++++++++++++++++++++++++------ 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 064f689..7edbdc5 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -110,7 +110,8 @@ void Layer::onFirstRef() mSurfaceTexture->setDefaultMaxBufferCount(3); #endif - updateTransformHint(); + const sp hw(mFlinger->getDefaultDisplayDevice()); + updateTransformHint(hw); } Layer::~Layer() @@ -767,15 +768,12 @@ uint32_t Layer::getEffectiveUsage(uint32_t usage) const return usage; } -void Layer::updateTransformHint() const { +void Layer::updateTransformHint(const sp& hw) const { uint32_t orientation = 0; if (!mFlinger->mDebugDisableTransformHint) { - // The transform hint is used to improve performance on the main - // display -- we can only have a single transform hint, it cannot + // The transform hint is used to improve performance, but we can + // only have a single transform hint, it cannot // apply to all displays. - // This is why we use the default display here. This is not an - // oversight. - sp hw(mFlinger->getDefaultDisplayDevice()); const Transform& planeTransform(hw->getTransform()); orientation = planeTransform.getOrientation(); if (orientation & Transform::ROT_INVALID) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 6f75d8c..c5eb26b 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -91,8 +91,8 @@ public: inline const sp& getActiveBuffer() const { return mActiveBuffer; } // Updates the transform hint in our SurfaceTexture to match - // the current orientation of the default display device. - virtual void updateTransformHint() const; + // the current orientation of the display device. + virtual void updateTransformHint(const sp& hw) const; protected: virtual void onFirstRef(); diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index 00c4ffe..4d5a5b0 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -246,7 +246,7 @@ public: * Updates the SurfaceTexture's transform hint, for layers that have * a SurfaceTexture. */ - virtual void updateTransformHint() const { } + virtual void updateTransformHint(const sp& hw) const { } /** always call base class first */ virtual void dump(String8& result, char* scratch, size_t size) const; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ce10c78..842471f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1069,7 +1069,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) if (transactionFlags & eTraversalNeeded) { for (size_t i=0 ; i& layer = currentLayers[i]; + const sp& layer(currentLayers[i]); uint32_t trFlags = layer->getTransactionFlags(eTransactionNeeded); if (!trFlags) continue; @@ -1142,18 +1142,6 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) disp->setProjection(state.orientation, state.viewport, state.frame); } - - // Walk through all the layers in currentLayers, - // and update their transform hint. - // - // TODO: we could be much more clever about which - // layers we touch and how often we do these updates - // (e.g. only touch the layers associated with this - // display, and only on a rotation). - for (size_t i = 0; i < count; i++) { - const sp& layerBase = currentLayers[i]; - layerBase->updateTransformHint(); - } } } } @@ -1208,6 +1196,61 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) } } + if (transactionFlags & (eTraversalNeeded|eDisplayTransactionNeeded)) { + // The transform hint might have changed for some layers + // (either because a display has changed, or because a layer + // as changed). + // + // Walk through all the layers in currentLayers, + // and update their transform hint. + // + // If a layer is visible only on a single display, then that + // display is used to calculate the hint, otherwise we use the + // default display. + // + // NOTE: we do this here, rather than in rebuildLayerStacks() so that + // the hint is set before we acquire a buffer from the surface texture. + // + // NOTE: layer transactions have taken place already, so we use their + // drawing state. However, SurfaceFlinger's own transaction has not + // happened yet, so we must use the current state layer list + // (soon to become the drawing state list). + // + sp disp; + uint32_t currentlayerStack = 0; + for (size_t i=0; i& layerBase(currentLayers[i]); + uint32_t layerStack = layerBase->drawingState().layerStack; + if (i==0 || currentlayerStack != layerStack) { + currentlayerStack = layerStack; + // figure out if this layerstack is mirrored + // (more than one display) if so, pick the default display, + // if not, pick the only display it's on. + disp.clear(); + for (size_t dpy=0 ; dpy hw(mDisplays[dpy]); + if (hw->getLayerStack() == currentlayerStack) { + if (disp == NULL) { + disp = hw; + } else { + disp = getDefaultDisplayDevice(); + break; + } + } + } + } + if (disp != NULL) { + // presumably this means this layer is using a layerStack + // that is not visible on any display + layerBase->updateTransformHint(disp); + } + } + } + + /* * Perform our own transaction if needed */ -- cgit v1.1 From 378787cf626276474ea7e4fbbbca0c7e10bb2a10 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 26 Nov 2012 13:54:56 -0800 Subject: DO NOT MERGE - native_fence_sync: fix an enum name This change replaces all references to the "EGL_NO_NATIVE_FENCE_ANDROID" enum with "EGL_NO_NATIVE_FENCE_FD_ANDROID". Bug: http://code.google.com/p/android/issues/detail?id=40295 (cherry-pick from master) Change-Id: Ie25d4ab9721d8b69b8d4afcf18e902ef8e3ad911 --- opengl/specs/EGL_ANDROID_native_fence_sync.txt | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/opengl/specs/EGL_ANDROID_native_fence_sync.txt b/opengl/specs/EGL_ANDROID_native_fence_sync.txt index 8273be4..ee05b40 100644 --- a/opengl/specs/EGL_ANDROID_native_fence_sync.txt +++ b/opengl/specs/EGL_ANDROID_native_fence_sync.txt @@ -100,10 +100,10 @@ Changes to Chapter 3 of the EGL 1.2 Specification (EGL Functions and Errors) EGL_SYNC_TYPE_KHR EGL_SYNC_NATIVE_FENCE_ANDROID EGL_SYNC_STATUS_KHR EGL_UNSIGNALED_KHR EGL_SYNC_CONDITION_KHR EGL_SYNC_PRIOR_COMMANDS_COMPLETE_KHR - EGL_SYNC_NATIVE_FENCE_FD_ANDROID EGL_NO_NATIVE_FENCE_ANDROID + EGL_SYNC_NATIVE_FENCE_FD_ANDROID EGL_NO_NATIVE_FENCE_FD_ANDROID If the EGL_SYNC_NATIVE_FENCE_FD_ANDROID attribute is not - EGL_NO_NATIVE_FENCE_ANDROID then the EGL_SYNC_CONDITION_KHR attribute is + EGL_NO_NATIVE_FENCE_FD_ANDROID then the EGL_SYNC_CONDITION_KHR attribute is set to EGL_SYNC_NATIVE_FENCE_SIGNALED_ANDROID and the EGL_SYNC_STATUS_KHR attribute is set to reflect the signal status of the native fence object. Additionally, the EGL implementation assumes ownership of the file @@ -114,7 +114,7 @@ Changes to Chapter 3 of the EGL 1.2 Specification (EGL Functions and Errors) "When a fence sync object is created or when an EGL native fence sync object is created with the EGL_SYNC_NATIVE_FENCE_FD_ANDROID attribute set - to EGL_NO_NATIVE_FENCE_ANDROID, eglCreateSyncKHR also inserts a fence + to EGL_NO_NATIVE_FENCE_FD_ANDROID, eglCreateSyncKHR also inserts a fence command into the command stream of the bound client API's current context (i.e., the context returned by eglGetCurrentContext), and associates it with the newly created sync object. @@ -157,8 +157,9 @@ Changes to Chapter 3 of the EGL 1.2 Specification (EGL Functions and Errors) empty (containing only EGL_NONE), EGL_NO_SYNC_KHR is returned and an EGL_BAD_ATTRIBUTE error is generated. * If is EGL_SYNC_NATIVE_FENCE_ANDROID and contains - an attribute other than EGL_SYNC_NATIVE_FENCE_FD_ANDROID, EGL_NO_SYNC_KHR is - returned and an EGL_BAD_ATTRIBUTE error is generated. + an attribute other than EGL_SYNC_NATIVE_FENCE_FD_ANDROID, + EGL_NO_SYNC_KHR is returned and an EGL_BAD_ATTRIBUTE error is + generated. * If is not a supported type of sync object, EGL_NO_SYNC_KHR is returned and an EGL_BAD_ATTRIBUTE error is generated. @@ -193,8 +194,8 @@ Changes to Chapter 3 of the EGL 1.2 Specification (EGL Functions and Errors) "If no errors are generated, EGL_TRUE is returned, and will no longer be the handle of a valid sync object. Additionally, if is an EGL native fence sync object and the EGL_SYNC_NATIVE_FENCE_FD_ANDROID - attribute is not EGL_NO_NATIVE_FENCE_ANDROID then that file descriptor is - closed." + attribute is not EGL_NO_NATIVE_FENCE_FD_ANDROID then that file descriptor + is closed." Add the following after the last paragraph of Section 3.8.1 (Sync Objects), added by KHR_fence_sync @@ -213,11 +214,11 @@ Changes to Chapter 3 of the EGL 1.2 Specification (EGL Functions and Errors) ------ * If is not a valid sync object for , - EGL_NO_NATIVE_FENCE_ANDROID is returned and an EGL_BAD_PARAMETER + EGL_NO_NATIVE_FENCE_FD_ANDROID is returned and an EGL_BAD_PARAMETER error is generated. * If the EGL_SYNC_NATIVE_FENCE_FD_ANDROID attribute of is - EGL_NO_NATIVE_FENCE_ANDROID, EGL_NO_NATIVE_FENCE_ANDROID is returned - and an EGL_BAD_PARAMETER error is generated. + EGL_NO_NATIVE_FENCE_FD_ANDROID, EGL_NO_NATIVE_FENCE_FD_ANDROID is + returned and an EGL_BAD_PARAMETER error is generated. * If does not match the display passed to eglCreateSyncKHR when was created, the behaviour is undefined." -- cgit v1.1 From b21a4e3b5f7f07ed160ca6e1809313e2a8e2a6a4 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Thu, 6 Dec 2012 17:51:53 -0800 Subject: ConsumerBase: free buffers outside the lock This change makes ConsumerBase::onBuffersReleased hold a reference to all its gralloc buffers until after the mutex is unlocked. This prevents slow gralloc::free calls from causing lock contention with rendering threads. Bug: 7675940 Change-Id: I0ec805d1b612afeeecfffec03f982371d27d93be --- libs/gui/ConsumerBase.cpp | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 624d7e0..cfc0293 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -109,21 +109,35 @@ void ConsumerBase::onFrameAvailable() { } void ConsumerBase::onBuffersReleased() { - Mutex::Autolock lock(mMutex); + sp bufRefs[BufferQueue::NUM_BUFFER_SLOTS]; + + { // Scope for the lock + Mutex::Autolock lock(mMutex); + + CB_LOGV("onBuffersReleased"); - CB_LOGV("onBuffersReleased"); + if (mAbandoned) { + // Nothing to do if we're already abandoned. + return; + } + + uint32_t mask = 0; + mBufferQueue->getReleasedBuffers(&mask); + for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + if (mask & (1 << i)) { + // Grab a local reference to the buffers so that they don't + // get freed while the lock is held. + bufRefs[i] = mSlots[i].mGraphicBuffer; - if (mAbandoned) { - // Nothing to do if we're already abandoned. - return; + freeBufferLocked(i); + } + } } - uint32_t mask = 0; - mBufferQueue->getReleasedBuffers(&mask); + // Clear the local buffer references. This would happen automatically + // when the array gets dtor'd, but I'm doing it explicitly for clarity. for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - if (mask & (1 << i)) { - freeBufferLocked(i); - } + bufRefs[i].clear(); } } -- cgit v1.1 From 2e59d2c3fdc0bcfedbe9c5d04d7acadc3eff8887 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Fri, 7 Dec 2012 00:38:36 -0800 Subject: DO NOT MERGE GraphicBufferAllocator: make frees async This change makes GraphicBufferAllocator::free queue a job to another thread to perform the actual free operation. This prevents potentially slow free operations from blocking rendering. Bug: 7675940 Change-Id: Id61099d66bb4c3949d04184e0d7f192ac18076b4 --- include/ui/GraphicBufferAllocator.h | 1 + libs/ui/GraphicBufferAllocator.cpp | 66 ++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/include/ui/GraphicBufferAllocator.h b/include/ui/GraphicBufferAllocator.h index dffa788..479cd3e 100644 --- a/include/ui/GraphicBufferAllocator.h +++ b/include/ui/GraphicBufferAllocator.h @@ -84,6 +84,7 @@ private: static KeyedVector sAllocList; friend class Singleton; + friend class BufferLiberatorThread; GraphicBufferAllocator(); ~GraphicBufferAllocator(); diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp index ff550d9..72acd7d 100644 --- a/libs/ui/GraphicBufferAllocator.cpp +++ b/libs/ui/GraphicBufferAllocator.cpp @@ -129,21 +129,65 @@ status_t GraphicBufferAllocator::alloc(uint32_t w, uint32_t h, PixelFormat forma return err; } -status_t GraphicBufferAllocator::free(buffer_handle_t handle) -{ - ATRACE_CALL(); - status_t err; +class BufferLiberatorThread : public Thread { +public: + + static void queueCaptiveBuffer(buffer_handle_t handle) { + static sp thread(new BufferLiberatorThread); + static bool running = false; + if (!running) { + thread->run("BufferLiberator"); + running = true; + } + { + Mutex::Autolock lock(thread->mMutex); + thread->mQueue.push_back(handle); + thread->mCondition.signal(); + } + } - err = mAllocDev->free(mAllocDev, handle); +private: - ALOGW_IF(err, "free(...) failed %d (%s)", err, strerror(-err)); - if (err == NO_ERROR) { - Mutex::Autolock _l(sLock); - KeyedVector& list(sAllocList); - list.removeItem(handle); + BufferLiberatorThread() {} + + virtual bool threadLoop() { + buffer_handle_t handle; + { + Mutex::Autolock lock(mMutex); + while (mQueue.isEmpty()) { + mCondition.wait(mMutex); + } + handle = mQueue[0]; + mQueue.removeAt(0); + } + + status_t err; + GraphicBufferAllocator& gba(GraphicBufferAllocator::get()); + { // Scope for tracing + ATRACE_NAME("gralloc::free"); + err = gba.mAllocDev->free(gba.mAllocDev, handle); + } + ALOGW_IF(err, "free(...) failed %d (%s)", err, strerror(-err)); + + if (err == NO_ERROR) { + Mutex::Autolock _l(GraphicBufferAllocator::sLock); + KeyedVector& + list(GraphicBufferAllocator::sAllocList); + list.removeItem(handle); + } + + return true; } - return err; + Vector mQueue; + Condition mCondition; + Mutex mMutex; +}; + +status_t GraphicBufferAllocator::free(buffer_handle_t handle) +{ + BufferLiberatorThread::queueCaptiveBuffer(handle); + return NO_ERROR; } // --------------------------------------------------------------------------- -- cgit v1.1 From 72c3f7d88160b7c279f90f0efe3c1cb12cd140ae Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Fri, 7 Dec 2012 00:41:56 -0800 Subject: Revert "ConsumerBase: free buffers outside the lock" This reverts commit b21a4e3b5f7f07ed160ca6e1809313e2a8e2a6a4. --- libs/gui/ConsumerBase.cpp | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index cfc0293..624d7e0 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -109,35 +109,21 @@ void ConsumerBase::onFrameAvailable() { } void ConsumerBase::onBuffersReleased() { - sp bufRefs[BufferQueue::NUM_BUFFER_SLOTS]; - - { // Scope for the lock - Mutex::Autolock lock(mMutex); - - CB_LOGV("onBuffersReleased"); - - if (mAbandoned) { - // Nothing to do if we're already abandoned. - return; - } + Mutex::Autolock lock(mMutex); - uint32_t mask = 0; - mBufferQueue->getReleasedBuffers(&mask); - for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - if (mask & (1 << i)) { - // Grab a local reference to the buffers so that they don't - // get freed while the lock is held. - bufRefs[i] = mSlots[i].mGraphicBuffer; + CB_LOGV("onBuffersReleased"); - freeBufferLocked(i); - } - } + if (mAbandoned) { + // Nothing to do if we're already abandoned. + return; } - // Clear the local buffer references. This would happen automatically - // when the array gets dtor'd, but I'm doing it explicitly for clarity. + uint32_t mask = 0; + mBufferQueue->getReleasedBuffers(&mask); for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - bufRefs[i].clear(); + if (mask & (1 << i)) { + freeBufferLocked(i); + } } } -- cgit v1.1 From f53f9c6d3668490f6c68f5c094c28f645c1b3da3 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 10 Dec 2012 17:06:44 -0800 Subject: [DO NOT MERGE] GraphicBufferAllocator: stall alloc for async frees This change makes GraphicBufferAllocator::alloc wait for pending async frees to complete before attempting to allocate a gralloc buffer if there are more than 8 pending async frees. Bug: 7696861 Change-Id: I1fae86e13edefcaa153b8ce9fd057f335716059e --- libs/ui/GraphicBufferAllocator.cpp | 160 +++++++++++++++++++++++++------------ 1 file changed, 108 insertions(+), 52 deletions(-) diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp index 72acd7d..fb43410 100644 --- a/libs/ui/GraphicBufferAllocator.cpp +++ b/libs/ui/GraphicBufferAllocator.cpp @@ -90,59 +90,36 @@ void GraphicBufferAllocator::dumpToSystemLog() ALOGD("%s", s.string()); } -status_t GraphicBufferAllocator::alloc(uint32_t w, uint32_t h, PixelFormat format, - int usage, buffer_handle_t* handle, int32_t* stride) -{ - ATRACE_CALL(); - // make sure to not allocate a N x 0 or 0 x N buffer, since this is - // allowed from an API stand-point allocate a 1x1 buffer instead. - if (!w || !h) - w = h = 1; +class BufferLiberatorThread : public Thread { +public: - // we have a h/w allocator and h/w buffer is requested - status_t err; - - err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride); + static void queueCaptiveBuffer(buffer_handle_t handle) { + size_t queueSize; + { + Mutex::Autolock lock(sMutex); + if (sThread == NULL) { + sThread = new BufferLiberatorThread; + sThread->run("BufferLiberator"); + } - ALOGW_IF(err, "alloc(%u, %u, %d, %08x, ...) failed %d (%s)", - w, h, format, usage, err, strerror(-err)); - - if (err == NO_ERROR) { - Mutex::Autolock _l(sLock); - KeyedVector& list(sAllocList); - int bpp = bytesPerPixel(format); - if (bpp < 0) { - // probably a HAL custom format. in any case, we don't know - // what its pixel size is. - bpp = 0; + sThread->mQueue.push_back(handle); + sThread->mQueuedCondition.signal(); + queueSize = sThread->mQueue.size(); } - alloc_rec_t rec; - rec.w = w; - rec.h = h; - rec.s = *stride; - rec.format = format; - rec.usage = usage; - rec.size = h * stride[0] * bpp; - list.add(*handle, rec); } - return err; -} + static void waitForLiberation() { + Mutex::Autolock lock(sMutex); -class BufferLiberatorThread : public Thread { -public: + waitForLiberationLocked(); + } - static void queueCaptiveBuffer(buffer_handle_t handle) { - static sp thread(new BufferLiberatorThread); - static bool running = false; - if (!running) { - thread->run("BufferLiberator"); - running = true; - } - { - Mutex::Autolock lock(thread->mMutex); - thread->mQueue.push_back(handle); - thread->mCondition.signal(); + static void maybeWaitForLiberation() { + Mutex::Autolock lock(sMutex); + if (sThread != NULL) { + if (sThread->mQueue.size() > 8) { + waitForLiberationLocked(); + } } } @@ -152,13 +129,12 @@ private: virtual bool threadLoop() { buffer_handle_t handle; - { - Mutex::Autolock lock(mMutex); + { // Scope for mutex + Mutex::Autolock lock(sMutex); while (mQueue.isEmpty()) { - mCondition.wait(mMutex); + mQueuedCondition.wait(sMutex); } handle = mQueue[0]; - mQueue.removeAt(0); } status_t err; @@ -176,14 +152,94 @@ private: list.removeItem(handle); } + { // Scope for mutex + Mutex::Autolock lock(sMutex); + mQueue.removeAt(0); + mFreedCondition.broadcast(); + } + return true; } + static void waitForLiberationLocked() { + if (sThread == NULL) { + return; + } + + const nsecs_t timeout = 500 * 1000 * 1000; + nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC); + nsecs_t timeToStop = now + timeout; + while (!sThread->mQueue.isEmpty() && now < timeToStop) { + sThread->mFreedCondition.waitRelative(sMutex, timeToStop - now); + now = systemTime(SYSTEM_TIME_MONOTONIC); + } + + if (!sThread->mQueue.isEmpty()) { + ALOGW("waitForLiberationLocked timed out"); + } + } + + static Mutex sMutex; + static sp sThread; Vector mQueue; - Condition mCondition; - Mutex mMutex; + Condition mQueuedCondition; + Condition mFreedCondition; }; +Mutex BufferLiberatorThread::sMutex; +sp BufferLiberatorThread::sThread; + +status_t GraphicBufferAllocator::alloc(uint32_t w, uint32_t h, PixelFormat format, + int usage, buffer_handle_t* handle, int32_t* stride) +{ + ATRACE_CALL(); + // make sure to not allocate a N x 0 or 0 x N buffer, since this is + // allowed from an API stand-point allocate a 1x1 buffer instead. + if (!w || !h) + w = h = 1; + + // we have a h/w allocator and h/w buffer is requested + status_t err; + + // If too many async frees are queued up then wait for some of them to + // complete before attempting to allocate more memory. This is exercised + // by the android.opengl.cts.GLSurfaceViewTest CTS test. + BufferLiberatorThread::maybeWaitForLiberation(); + + err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride); + + if (err != NO_ERROR) { + ALOGW("WOW! gralloc alloc failed, waiting for pending frees!"); + BufferLiberatorThread::waitForLiberation(); + err = mAllocDev->alloc(mAllocDev, w, h, format, usage, handle, stride); + } + + ALOGW_IF(err, "alloc(%u, %u, %d, %08x, ...) failed %d (%s)", + w, h, format, usage, err, strerror(-err)); + + if (err == NO_ERROR) { + Mutex::Autolock _l(sLock); + KeyedVector& list(sAllocList); + int bpp = bytesPerPixel(format); + if (bpp < 0) { + // probably a HAL custom format. in any case, we don't know + // what its pixel size is. + bpp = 0; + } + alloc_rec_t rec; + rec.w = w; + rec.h = h; + rec.s = *stride; + rec.format = format; + rec.usage = usage; + rec.size = h * stride[0] * bpp; + list.add(*handle, rec); + } + + return err; +} + + status_t GraphicBufferAllocator::free(buffer_handle_t handle) { BufferLiberatorThread::queueCaptiveBuffer(handle); -- cgit v1.1