From 33c544c4d3af8bae518474df6a9da9b7c349b91a Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 4 Feb 2013 14:43:07 -0800 Subject: [PATCH 1/2] SurfaceFlinger: remove a driver bug workaround Change-Id: I7478293e87899d6e467db8c2f9e295935c8b1d4e --- services/surfaceflinger/Layer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index f94a9ba7e..0cc03e130 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -539,9 +539,6 @@ void Layer::onPostComposition() { const HWComposer& hwc = mFlinger->getHwComposer(); sp presentFence = hwc.getDisplayFence(HWC_DISPLAY_PRIMARY); - // XXX: Temporarily don't use the present fence from HWC to work - // around a driver bug. - presentFence.clear(); if (presentFence != NULL) { mFrameTracker.setActualPresentFence(presentFence); } else { From 4b0eba949cc026ffb2c75313042d8a7bcb3fcf86 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Tue, 5 Feb 2013 13:30:24 -0800 Subject: [PATCH 2/2] SurfaceFlinger: add win anim frame time tracking This change makes the 'dumpsys SurfaceFlinger --latency' command with no extra args dump the frame timestamp data for the most recent frames that SurfaceFlinger generated that included window animation transaction changes. Change-Id: I8bded1ea08a4cddefef0aa955401052bb9107c90 --- services/surfaceflinger/FrameTracker.cpp | 17 ++++++-- services/surfaceflinger/FrameTracker.h | 6 ++- services/surfaceflinger/Layer.cpp | 3 -- services/surfaceflinger/Layer.h | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 49 +++++++++++++++++----- services/surfaceflinger/SurfaceFlinger.h | 7 +++- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/services/surfaceflinger/FrameTracker.cpp b/services/surfaceflinger/FrameTracker.cpp index 5c66ff9c9..9b55d4483 100644 --- a/services/surfaceflinger/FrameTracker.cpp +++ b/services/surfaceflinger/FrameTracker.cpp @@ -31,28 +31,34 @@ FrameTracker::FrameTracker() : } void FrameTracker::setDesiredPresentTime(nsecs_t presentTime) { + Mutex::Autolock lock(mMutex); mFrameRecords[mOffset].desiredPresentTime = presentTime; } void FrameTracker::setFrameReadyTime(nsecs_t readyTime) { + Mutex::Autolock lock(mMutex); mFrameRecords[mOffset].frameReadyTime = readyTime; } void FrameTracker::setFrameReadyFence(const sp& readyFence) { + Mutex::Autolock lock(mMutex); mFrameRecords[mOffset].frameReadyFence = readyFence; mNumFences++; } void FrameTracker::setActualPresentTime(nsecs_t presentTime) { + Mutex::Autolock lock(mMutex); mFrameRecords[mOffset].actualPresentTime = presentTime; } void FrameTracker::setActualPresentFence(const sp& readyFence) { + Mutex::Autolock lock(mMutex); mFrameRecords[mOffset].actualPresentFence = readyFence; mNumFences++; } void FrameTracker::advanceFrame() { + Mutex::Autolock lock(mMutex); mOffset = (mOffset+1) % NUM_FRAME_RECORDS; mFrameRecords[mOffset].desiredPresentTime = INT64_MAX; mFrameRecords[mOffset].frameReadyTime = INT64_MAX; @@ -74,10 +80,11 @@ void FrameTracker::advanceFrame() { // Clean up the signaled fences to keep the number of open fence FDs in // this process reasonable. - processFences(); + processFencesLocked(); } void FrameTracker::clear() { + Mutex::Autolock lock(mMutex); for (size_t i = 0; i < NUM_FRAME_RECORDS; i++) { mFrameRecords[i].desiredPresentTime = 0; mFrameRecords[i].frameReadyTime = 0; @@ -86,9 +93,12 @@ void FrameTracker::clear() { mFrameRecords[i].actualPresentFence.clear(); } mNumFences = 0; + mFrameRecords[mOffset].desiredPresentTime = INT64_MAX; + mFrameRecords[mOffset].frameReadyTime = INT64_MAX; + mFrameRecords[mOffset].actualPresentTime = INT64_MAX; } -void FrameTracker::processFences() const { +void FrameTracker::processFencesLocked() const { FrameRecord* records = const_cast(mFrameRecords); int& numFences = const_cast(mNumFences); @@ -116,7 +126,8 @@ void FrameTracker::processFences() const { } void FrameTracker::dump(String8& result) const { - processFences(); + Mutex::Autolock lock(mMutex); + processFencesLocked(); const size_t o = mOffset; for (size_t i = 1; i < NUM_FRAME_RECORDS; i++) { diff --git a/services/surfaceflinger/FrameTracker.h b/services/surfaceflinger/FrameTracker.h index e8a8c481b..3d122c43d 100644 --- a/services/surfaceflinger/FrameTracker.h +++ b/services/surfaceflinger/FrameTracker.h @@ -19,6 +19,7 @@ #include +#include #include #include @@ -96,7 +97,7 @@ private: // This method is const because although it modifies the frame records it // does so in such a way that the information represented should not // change. This allows it to be called from the dump method. - void processFences() const; + void processFencesLocked() const; // mFrameRecords is the circular buffer storing the tracked data for each // frame. @@ -113,6 +114,9 @@ private: // The number of fences is tracked so that the run time of processFences // doesn't grow with NUM_FRAME_RECORDS. int mNumFences; + + // mMutex is used to protect access to all member variables. + mutable Mutex mMutex; }; } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0cc03e130..1feb9dec1 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -757,9 +757,6 @@ void Layer::dump(String8& result, char* buffer, size_t SIZE) const void Layer::dumpStats(String8& result, char* buffer, size_t SIZE) const { LayerBaseClient::dumpStats(result, buffer, SIZE); - const nsecs_t period = - mFlinger->getHwComposer().getRefreshPeriod(HWC_DISPLAY_PRIMARY); - result.appendFormat("%lld\n", period); mFrameTracker.dump(result); } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index ef829edf9..a82767b75 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -129,6 +129,7 @@ private: // thread-safe volatile int32_t mQueuedFrames; + FrameTracker mFrameTracker; // main thread sp mActiveBuffer; @@ -138,7 +139,6 @@ private: bool mCurrentOpacity; bool mRefreshPending; bool mFrameLatencyNeeded; - FrameTracker mFrameTracker; // constants PixelFormat mFormat; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1fa3b8f00..caabf1d71 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -93,6 +93,7 @@ SurfaceFlinger::SurfaceFlinger() mBootTime(systemTime()), mVisibleRegionsDirty(false), mHwWorkListDirty(false), + mAnimCompositionPending(false), mDebugRegion(0), mDebugDDMS(0), mDebugDisableHWC(0), @@ -885,6 +886,22 @@ void SurfaceFlinger::postComposition() for (size_t i=0 ; ionPostComposition(); } + + if (mAnimCompositionPending) { + mAnimCompositionPending = false; + + const HWComposer& hwc = getHwComposer(); + sp presentFence = hwc.getDisplayFence(HWC_DISPLAY_PRIMARY); + if (presentFence != NULL) { + mAnimFrameTracker.setActualPresentFence(presentFence); + } else { + // The HWC doesn't support present fences, so use the refresh + // timestamp instead. + nsecs_t presentTime = hwc.getRefreshTimestamp(HWC_DISPLAY_PRIMARY); + mAnimFrameTracker.setActualPresentTime(presentTime); + } + mAnimFrameTracker.advanceFrame(); + } } void SurfaceFlinger::rebuildLayerStacks() { @@ -1301,6 +1318,10 @@ void SurfaceFlinger::commitTransaction() mLayersPendingRemoval.clear(); } + // If this transaction is part of a window animation then the next frame + // we composite should be considered an animation as well. + mAnimCompositionPending = mAnimTransactionPending; + mDrawingState = mCurrentState; mTransactionPending = false; mAnimTransactionPending = false; @@ -2264,22 +2285,26 @@ void SurfaceFlinger::dumpStatsLocked(const Vector& args, size_t& index index++; } - const LayerVector& currentLayers = mCurrentState.layersSortedByZ; - const size_t count = currentLayers.size(); - for (size_t i=0 ; i& layer(currentLayers[i]); - if (name.isEmpty()) { - snprintf(buffer, SIZE, "%s\n", layer->getName().string()); - result.append(buffer); - } - if (name.isEmpty() || (name == layer->getName())) { - layer->dumpStats(result, buffer, SIZE); + const nsecs_t period = + getHwComposer().getRefreshPeriod(HWC_DISPLAY_PRIMARY); + result.appendFormat("%lld\n", period); + + if (name.isEmpty()) { + mAnimFrameTracker.dump(result); + } else { + const LayerVector& currentLayers = mCurrentState.layersSortedByZ; + const size_t count = currentLayers.size(); + for (size_t i=0 ; i& layer(currentLayers[i]); + if (name == layer->getName()) { + layer->dumpStats(result, buffer, SIZE); + } } } } void SurfaceFlinger::clearStatsLocked(const Vector& args, size_t& index, - String8& result, char* buffer, size_t SIZE) const + String8& result, char* buffer, size_t SIZE) { String8 name; if (index < args.size()) { @@ -2295,6 +2320,8 @@ void SurfaceFlinger::clearStatsLocked(const Vector& args, size_t& inde layer->clearStats(); } } + + mAnimFrameTracker.clear(); } /*static*/ void SurfaceFlinger::appendSfConfigString(String8& result) diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 65fea30e9..72dd6524a 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -45,8 +45,9 @@ #include #include "Barrier.h" -#include "MessageQueue.h" #include "DisplayDevice.h" +#include "FrameTracker.h" +#include "MessageQueue.h" #include "DisplayHardware/HWComposer.h" @@ -390,7 +391,7 @@ private: void dumpStatsLocked(const Vector& args, size_t& index, String8& result, char* buffer, size_t SIZE) const; void clearStatsLocked(const Vector& args, size_t& index, - String8& result, char* buffer, size_t SIZE) const; + String8& result, char* buffer, size_t SIZE); void dumpAllLocked(String8& result, char* buffer, size_t SIZE) const; bool startDdmConnection(); static void appendSfConfigString(String8& result); @@ -432,6 +433,7 @@ private: State mDrawingState; bool mVisibleRegionsDirty; bool mHwWorkListDirty; + bool mAnimCompositionPending; // this may only be written from the main thread with mStateLock held // it may be read from other threads with mStateLock held @@ -451,6 +453,7 @@ private: // these are thread safe mutable MessageQueue mEventQueue; mutable Barrier mReadyToRunBarrier; + FrameTracker mAnimFrameTracker; // protected by mDestroyedLayerLock; mutable Mutex mDestroyedLayerLock;