From: Valerie Hau Date: Wed, 20 Feb 2019 18:36:29 +0000 (-0800) Subject: Caching between SF and HWC for BufferStateLayers X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=b28f007e51fb84ff78ce8b053d80efb0efdbce20;p=android-x86%2Fframeworks-native.git Caching between SF and HWC for BufferStateLayers Bug: 120434937 Test: build, boot,libcompositionengine_test Change-Id: I4a99faeb7aa88683aac19c7000db87bd75f9ebe9 --- diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 96c49921dd..76764efd7a 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -367,7 +367,7 @@ void BufferQueueLayer::setHwcLayerBuffer(const sp& display) uint32_t hwcSlot = 0; sp hwcBuffer; (*outputLayer->editState().hwc) - .hwcBufferCache.getHwcBuffer(mActiveBufferSlot, mActiveBuffer, &hwcSlot, &hwcBuffer); + .hwcBufferCache.getHwcBuffer(mActiveBuffer, &hwcSlot, &hwcBuffer); auto acquireFence = mConsumer->getCurrentFence(); auto error = hwcLayer->setBuffer(hwcSlot, hwcBuffer, acquireFence); diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index f6b69ebccb..a6c090cfd8 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -565,14 +565,17 @@ status_t BufferStateLayer::updateFrameNumber(nsecs_t /*latchTime*/) { void BufferStateLayer::setHwcLayerBuffer(const sp& display) { const auto outputLayer = findOutputLayerForDisplay(display); LOG_FATAL_IF(!outputLayer || !outputLayer->getState().hwc); - auto& hwcLayer = (*outputLayer->getState().hwc).hwcLayer; + auto& hwcInfo = *outputLayer->editState().hwc; + auto& hwcLayer = hwcInfo.hwcLayer; const State& s(getDrawingState()); - // TODO(marissaw): support more than one slot + // obtain slot uint32_t hwcSlot = 0; + sp buffer; + hwcInfo.hwcBufferCache.getHwcBuffer(s.buffer, &hwcSlot, &buffer); - auto error = hwcLayer->setBuffer(hwcSlot, s.buffer, s.acquireFence); + auto error = hwcLayer->setBuffer(hwcSlot, buffer, s.acquireFence); if (error != HWC2::Error::None) { ALOGE("[%s] Failed to set buffer %p: %s (%d)", mName.string(), s.buffer->handle, to_string(error).c_str(), static_cast(error)); diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h index b45de5a619..3b4f6e4a88 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h @@ -19,6 +19,7 @@ #include #include +#include #include namespace android { @@ -39,19 +40,28 @@ namespace compositionengine::impl { class HwcBufferCache { public: HwcBufferCache(); - - // Given a buffer queue slot and buffer, return the HWC cache slot and + // Given a buffer, return the HWC cache slot and // buffer to be sent to HWC. // // outBuffer is set to buffer when buffer is not in the HWC cache; // otherwise, outBuffer is set to nullptr. - void getHwcBuffer(int slot, const sp& buffer, uint32_t* outSlot, + void getHwcBuffer(const sp& buffer, uint32_t* outSlot, sp* outBuffer); +protected: + int getSlot(const sp& buffer); + int getLeastRecentlyUsedSlot(); + uint64_t getCounter(); + private: - // a vector as we expect "slot" to be in the range of [0, 63] (that is, - // less than BufferQueue::NUM_BUFFER_SLOTS). - std::vector> mBuffers; + // an array where the index corresponds to a slot and the value corresponds to a (counter, + // buffer) pair. "counter" is a unique value that indicates the last time this slot was updated + // or used and allows us to keep track of the least-recently used buffer. + std::pair> mBuffers[BufferQueue::NUM_BUFFER_SLOTS]; + + // The cache increments this counter value when a slot is updated or used. + // Used to track the least recently-used buffer + uint64_t mCounter = 1; }; } // namespace compositionengine::impl diff --git a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp index 6f340b96d0..8177c7f25d 100644 --- a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp +++ b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp @@ -21,31 +21,45 @@ namespace android::compositionengine::impl { HwcBufferCache::HwcBufferCache() { - mBuffers.reserve(BufferQueue::NUM_BUFFER_SLOTS); + std::fill(std::begin(mBuffers), std::end(mBuffers), + std::pair>(0, nullptr)); } - -void HwcBufferCache::getHwcBuffer(int slot, const sp& buffer, uint32_t* outSlot, - sp* outBuffer) { - if (slot == BufferQueue::INVALID_BUFFER_SLOT || slot < 0) { - // default to slot 0 - slot = 0; +int HwcBufferCache::getSlot(const sp& buffer) { + // search for cached buffer first + for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + if (mBuffers[i].second == buffer) { + return i; + } } - if (static_cast(slot) >= mBuffers.size()) { - mBuffers.resize(slot + 1); - } + // use the least-recently used slot + return getLeastRecentlyUsedSlot(); +} - *outSlot = slot; +int HwcBufferCache::getLeastRecentlyUsedSlot() { + auto iter = std::min_element(std::begin(mBuffers), std::end(mBuffers)); + return std::distance(std::begin(mBuffers), iter); +} + +void HwcBufferCache::getHwcBuffer(const sp& buffer, uint32_t* outSlot, + sp* outBuffer) { + *outSlot = getSlot(buffer); - if (mBuffers[slot] == buffer) { + auto& [currentCounter, currentBuffer] = mBuffers[*outSlot]; + if (currentBuffer == buffer) { // already cached in HWC, skip sending the buffer *outBuffer = nullptr; + currentCounter = getCounter(); } else { *outBuffer = buffer; // update cache - mBuffers[slot] = buffer; + currentBuffer = buffer; + currentCounter = getCounter(); } } +uint64_t HwcBufferCache::getCounter() { + return mCounter++; +} } // namespace android::compositionengine::impl diff --git a/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp b/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp index f2a1aad2fc..d4bbf6ddaa 100644 --- a/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp @@ -22,60 +22,78 @@ namespace android::compositionengine { namespace { +class TestableHwcBufferCache : public impl::HwcBufferCache { +public: + void getHwcBuffer(const sp& buffer, uint32_t* outSlot, + sp* outBuffer) { + HwcBufferCache::getHwcBuffer(buffer, outSlot, outBuffer); + } + int getSlot(const sp& buffer) { return HwcBufferCache::getSlot(buffer); } + int getLeastRecentlyUsedSlot() { return HwcBufferCache::getLeastRecentlyUsedSlot(); } +}; + class HwcBufferCacheTest : public testing::Test { public: ~HwcBufferCacheTest() override = default; - void testSlot(const int inSlot, const uint32_t expectedSlot) { - uint32_t outSlot; - sp outBuffer; - - // The first time, the output is the same as the input - mCache.getHwcBuffer(inSlot, mBuffer1, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(mBuffer1, outBuffer); - - // The second time with the same buffer, the outBuffer is nullptr. - mCache.getHwcBuffer(inSlot, mBuffer1, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - - // With a new buffer, the outBuffer is the input. - mCache.getHwcBuffer(inSlot, mBuffer2, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(mBuffer2, outBuffer); - - // Again, the second request with the same buffer sets outBuffer to nullptr. - mCache.getHwcBuffer(inSlot, mBuffer2, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - - // Setting a slot to use nullptr lookslike works, but note that - // the output values make it look like no new buffer is being set.... - mCache.getHwcBuffer(inSlot, sp(), &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - } - - impl::HwcBufferCache mCache; + TestableHwcBufferCache mCache; sp mBuffer1{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)}; sp mBuffer2{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)}; }; -TEST_F(HwcBufferCacheTest, cacheWorksForSlotZero) { - testSlot(0, 0); -} +TEST_F(HwcBufferCacheTest, testSlot) { + uint32_t outSlot; + sp outBuffer; -TEST_F(HwcBufferCacheTest, cacheWorksForMaxSlot) { - testSlot(BufferQueue::NUM_BUFFER_SLOTS - 1, BufferQueue::NUM_BUFFER_SLOTS - 1); -} + // The first time, the output is the same as the input + mCache.getHwcBuffer(mBuffer1, &outSlot, &outBuffer); + EXPECT_EQ(0, outSlot); + EXPECT_EQ(mBuffer1, outBuffer); -TEST_F(HwcBufferCacheTest, cacheMapsNegativeSlotToZero) { - testSlot(-123, 0); + // The second time with the same buffer, the outBuffer is nullptr. + mCache.getHwcBuffer(mBuffer1, &outSlot, &outBuffer); + EXPECT_EQ(0, outSlot); + EXPECT_EQ(nullptr, outBuffer.get()); + + // With a new buffer, the outBuffer is the input. + mCache.getHwcBuffer(mBuffer2, &outSlot, &outBuffer); + EXPECT_EQ(1, outSlot); + EXPECT_EQ(mBuffer2, outBuffer); + + // Again, the second request with the same buffer sets outBuffer to nullptr. + mCache.getHwcBuffer(mBuffer2, &outSlot, &outBuffer); + EXPECT_EQ(1, outSlot); + EXPECT_EQ(nullptr, outBuffer.get()); + + // Setting a slot to use nullptr lookslike works, but note that + // the output values make it look like no new buffer is being set.... + mCache.getHwcBuffer(sp(), &outSlot, &outBuffer); + EXPECT_EQ(2, outSlot); + EXPECT_EQ(nullptr, outBuffer.get()); } -TEST_F(HwcBufferCacheTest, cacheMapsInvalidBufferSlotToZero) { - testSlot(BufferQueue::INVALID_BUFFER_SLOT, 0); +TEST_F(HwcBufferCacheTest, testGetLeastRecentlyUsedSlot) { + int slot; + uint32_t outSlot; + sp outBuffer; + + // fill up cache + for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + sp buf{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)}; + mCache.getHwcBuffer(buf, &outSlot, &outBuffer); + EXPECT_EQ(buf, outBuffer); + EXPECT_EQ(i, outSlot); + } + + slot = mCache.getLeastRecentlyUsedSlot(); + EXPECT_EQ(0, slot); + + mCache.getHwcBuffer(mBuffer1, &outSlot, &outBuffer); + EXPECT_EQ(0, outSlot); + EXPECT_EQ(mBuffer1, outBuffer); + + slot = mCache.getLeastRecentlyUsedSlot(); + EXPECT_EQ(1, slot); } } // namespace diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 27812f7492..775fb806b8 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -111,8 +111,7 @@ status_t FramebufferSurface::nextBuffer(uint32_t& outSlot, BufferItem item; status_t err = acquireBufferLocked(&item, 0); if (err == BufferQueue::NO_BUFFER_AVAILABLE) { - mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer, - &outSlot, &outBuffer); + mHwcBufferCache.getHwcBuffer(mCurrentBuffer, &outSlot, &outBuffer); return NO_ERROR; } else if (err != NO_ERROR) { ALOGE("error acquiring buffer: %s (%d)", strerror(-err), err); @@ -138,8 +137,7 @@ status_t FramebufferSurface::nextBuffer(uint32_t& outSlot, mCurrentFence = item.mFence; outFence = item.mFence; - mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer, - &outSlot, &outBuffer); + mHwcBufferCache.getHwcBuffer(mCurrentBuffer, &outSlot, &outBuffer); outDataspace = static_cast(item.mDataSpace); status_t result = mHwc.setClientTarget(mDisplayId, outSlot, outFence, outBuffer, outDataspace); if (result != NO_ERROR) { diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 1c2853a857..613dc777ee 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -224,8 +224,7 @@ status_t VirtualDisplaySurface::advanceFrame() { if (fbBuffer != nullptr) { uint32_t hwcSlot = 0; sp hwcBuffer; - mHwcBufferCache.getHwcBuffer(mFbProducerSlot, fbBuffer, - &hwcSlot, &hwcBuffer); + mHwcBufferCache.getHwcBuffer(fbBuffer, &hwcSlot, &hwcBuffer); // TODO: Correctly propagate the dataspace from GL composition result = mHwc.setClientTarget(*mDisplayId, hwcSlot, mFbFence, hwcBuffer,