Revert "Caching between SF and HWC for BufferStateLayers"
This reverts commit b28f007e51fb84ff78ce8b053d80efb0efdbce20.
Reason for revert: Causing pre-submit failure b/126704033
Change-Id: I17c99cd410b3fa3825d01fedd21a52451368b5c0
(cherry picked from commit 5723fbda94dc6c8ecc444360dc2de487585daa78)
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 18b0d58..215dea1 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -366,7 +366,7 @@
uint32_t hwcSlot = 0;
sp<GraphicBuffer> hwcBuffer;
(*outputLayer->editState().hwc)
- .hwcBufferCache.getHwcBuffer(mActiveBuffer, &hwcSlot, &hwcBuffer);
+ .hwcBufferCache.getHwcBuffer(mActiveBufferSlot, 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 a6c090c..f6b69eb 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -565,17 +565,14 @@
void BufferStateLayer::setHwcLayerBuffer(const sp<const DisplayDevice>& display) {
const auto outputLayer = findOutputLayerForDisplay(display);
LOG_FATAL_IF(!outputLayer || !outputLayer->getState().hwc);
- auto& hwcInfo = *outputLayer->editState().hwc;
- auto& hwcLayer = hwcInfo.hwcLayer;
+ auto& hwcLayer = (*outputLayer->getState().hwc).hwcLayer;
const State& s(getDrawingState());
- // obtain slot
+ // TODO(marissaw): support more than one slot
uint32_t hwcSlot = 0;
- sp<GraphicBuffer> buffer;
- hwcInfo.hwcBufferCache.getHwcBuffer(s.buffer, &hwcSlot, &buffer);
- auto error = hwcLayer->setBuffer(hwcSlot, buffer, s.acquireFence);
+ auto error = hwcLayer->setBuffer(hwcSlot, s.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<int32_t>(error));
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h
index 3b4f6e4..b45de5a 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h
@@ -19,7 +19,6 @@
#include <cstdint>
#include <vector>
-#include <gui/BufferQueue.h>
#include <utils/StrongPointer.h>
namespace android {
@@ -40,28 +39,19 @@
class HwcBufferCache {
public:
HwcBufferCache();
- // Given a buffer, return the HWC cache slot and
+
+ // Given a buffer queue slot and 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(const sp<GraphicBuffer>& buffer, uint32_t* outSlot,
+ void getHwcBuffer(int slot, const sp<GraphicBuffer>& buffer, uint32_t* outSlot,
sp<GraphicBuffer>* outBuffer);
-protected:
- int getSlot(const sp<GraphicBuffer>& buffer);
- int getLeastRecentlyUsedSlot();
- uint64_t getCounter();
-
private:
- // 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<uint64_t, wp<GraphicBuffer>> 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;
+ // a vector as we expect "slot" to be in the range of [0, 63] (that is,
+ // less than BufferQueue::NUM_BUFFER_SLOTS).
+ std::vector<sp<GraphicBuffer>> mBuffers;
};
} // namespace compositionengine::impl
diff --git a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp
index 8177c7f..6f340b9 100644
--- a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp
@@ -21,45 +21,31 @@
namespace android::compositionengine::impl {
HwcBufferCache::HwcBufferCache() {
- std::fill(std::begin(mBuffers), std::end(mBuffers),
- std::pair<uint64_t, wp<GraphicBuffer>>(0, nullptr));
+ mBuffers.reserve(BufferQueue::NUM_BUFFER_SLOTS);
}
-int HwcBufferCache::getSlot(const sp<GraphicBuffer>& buffer) {
- // search for cached buffer first
- for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
- if (mBuffers[i].second == buffer) {
- return i;
- }
+
+void HwcBufferCache::getHwcBuffer(int slot, const sp<GraphicBuffer>& buffer, uint32_t* outSlot,
+ sp<GraphicBuffer>* outBuffer) {
+ if (slot == BufferQueue::INVALID_BUFFER_SLOT || slot < 0) {
+ // default to slot 0
+ slot = 0;
}
- // use the least-recently used slot
- return getLeastRecentlyUsedSlot();
-}
+ if (static_cast<size_t>(slot) >= mBuffers.size()) {
+ mBuffers.resize(slot + 1);
+ }
-int HwcBufferCache::getLeastRecentlyUsedSlot() {
- auto iter = std::min_element(std::begin(mBuffers), std::end(mBuffers));
- return std::distance(std::begin(mBuffers), iter);
-}
+ *outSlot = slot;
-void HwcBufferCache::getHwcBuffer(const sp<GraphicBuffer>& buffer, uint32_t* outSlot,
- sp<GraphicBuffer>* outBuffer) {
- *outSlot = getSlot(buffer);
-
- auto& [currentCounter, currentBuffer] = mBuffers[*outSlot];
- if (currentBuffer == buffer) {
+ if (mBuffers[slot] == buffer) {
// already cached in HWC, skip sending the buffer
*outBuffer = nullptr;
- currentCounter = getCounter();
} else {
*outBuffer = buffer;
// update cache
- currentBuffer = buffer;
- currentCounter = getCounter();
+ mBuffers[slot] = buffer;
}
}
-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 d4bbf6d..f2a1aad 100644
--- a/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp
@@ -22,78 +22,60 @@
namespace android::compositionengine {
namespace {
-class TestableHwcBufferCache : public impl::HwcBufferCache {
-public:
- void getHwcBuffer(const sp<GraphicBuffer>& buffer, uint32_t* outSlot,
- sp<GraphicBuffer>* outBuffer) {
- HwcBufferCache::getHwcBuffer(buffer, outSlot, outBuffer);
- }
- int getSlot(const sp<GraphicBuffer>& buffer) { return HwcBufferCache::getSlot(buffer); }
- int getLeastRecentlyUsedSlot() { return HwcBufferCache::getLeastRecentlyUsedSlot(); }
-};
-
class HwcBufferCacheTest : public testing::Test {
public:
~HwcBufferCacheTest() override = default;
- TestableHwcBufferCache mCache;
+ void testSlot(const int inSlot, const uint32_t expectedSlot) {
+ uint32_t outSlot;
+ sp<GraphicBuffer> 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<GraphicBuffer>(), &outSlot, &outBuffer);
+ EXPECT_EQ(expectedSlot, outSlot);
+ EXPECT_EQ(nullptr, outBuffer.get());
+ }
+
+ impl::HwcBufferCache mCache;
sp<GraphicBuffer> mBuffer1{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};
sp<GraphicBuffer> mBuffer2{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};
};
-TEST_F(HwcBufferCacheTest, testSlot) {
- uint32_t outSlot;
- sp<GraphicBuffer> outBuffer;
-
- // The first time, the output is the same as the input
- mCache.getHwcBuffer(mBuffer1, &outSlot, &outBuffer);
- EXPECT_EQ(0, outSlot);
- EXPECT_EQ(mBuffer1, outBuffer);
-
- // 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<GraphicBuffer>(), &outSlot, &outBuffer);
- EXPECT_EQ(2, outSlot);
- EXPECT_EQ(nullptr, outBuffer.get());
+TEST_F(HwcBufferCacheTest, cacheWorksForSlotZero) {
+ testSlot(0, 0);
}
-TEST_F(HwcBufferCacheTest, testGetLeastRecentlyUsedSlot) {
- int slot;
- uint32_t outSlot;
- sp<GraphicBuffer> outBuffer;
+TEST_F(HwcBufferCacheTest, cacheWorksForMaxSlot) {
+ testSlot(BufferQueue::NUM_BUFFER_SLOTS - 1, BufferQueue::NUM_BUFFER_SLOTS - 1);
+}
- // fill up cache
- for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) {
- sp<GraphicBuffer> 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);
- }
+TEST_F(HwcBufferCacheTest, cacheMapsNegativeSlotToZero) {
+ testSlot(-123, 0);
+}
- 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);
+TEST_F(HwcBufferCacheTest, cacheMapsInvalidBufferSlotToZero) {
+ testSlot(BufferQueue::INVALID_BUFFER_SLOT, 0);
}
} // namespace
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index 775fb80..27812f7 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -111,7 +111,8 @@
BufferItem item;
status_t err = acquireBufferLocked(&item, 0);
if (err == BufferQueue::NO_BUFFER_AVAILABLE) {
- mHwcBufferCache.getHwcBuffer(mCurrentBuffer, &outSlot, &outBuffer);
+ mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer,
+ &outSlot, &outBuffer);
return NO_ERROR;
} else if (err != NO_ERROR) {
ALOGE("error acquiring buffer: %s (%d)", strerror(-err), err);
@@ -137,7 +138,8 @@
mCurrentFence = item.mFence;
outFence = item.mFence;
- mHwcBufferCache.getHwcBuffer(mCurrentBuffer, &outSlot, &outBuffer);
+ mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer,
+ &outSlot, &outBuffer);
outDataspace = static_cast<Dataspace>(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 613dc77..1c2853a 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -224,7 +224,8 @@
if (fbBuffer != nullptr) {
uint32_t hwcSlot = 0;
sp<GraphicBuffer> hwcBuffer;
- mHwcBufferCache.getHwcBuffer(fbBuffer, &hwcSlot, &hwcBuffer);
+ mHwcBufferCache.getHwcBuffer(mFbProducerSlot, fbBuffer,
+ &hwcSlot, &hwcBuffer);
// TODO: Correctly propagate the dataspace from GL composition
result = mHwc.setClientTarget(*mDisplayId, hwcSlot, mFbFence, hwcBuffer,
