Force HALs to explicitly enable legacy method for clearing buffer caches
Some HAL implementations can't support setLayerBuffer multiple times to
clear the per-layer buffer caches. Therefore, default this behavior to
disabled, and allow HALs to explcitily enable this behavior to obtain
the necessary memory savings.
Test: play videos with both true and false on both HIDL and AIDL
Bug: 285561686
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f70d9c4528888134e16003470b76dd1e81b8631d)
Merged-In: I928cef25e35cfc5337db4ceb8581bf5926b4fbe3
Change-Id: I928cef25e35cfc5337db4ceb8581bf5926b4fbe3
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
index f7049b9..c0eb36d 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
@@ -20,6 +20,7 @@
#include "AidlComposerHal.h"
+#include <SurfaceFlingerProperties.h>
#include <android-base/file.h>
#include <android/binder_ibinder_platform.h>
#include <android/binder_manager.h>
@@ -249,15 +250,18 @@
ALOGE("getInterfaceVersion for AidlComposer constructor failed %s",
status.getDescription().c_str());
}
- if (version == 1) {
- mClearSlotBuffer = sp<GraphicBuffer>::make(1, 1, PIXEL_FORMAT_RGBX_8888,
- GraphicBuffer::USAGE_HW_COMPOSER |
- GraphicBuffer::USAGE_SW_READ_OFTEN |
- GraphicBuffer::USAGE_SW_WRITE_OFTEN,
- "AidlComposer");
- if (!mClearSlotBuffer || mClearSlotBuffer->initCheck() != ::android::OK) {
- LOG_ALWAYS_FATAL("Failed to allocate a buffer for clearing layer buffer slots");
- return;
+ mSupportsBufferSlotsToClear = version > 1;
+ if (!mSupportsBufferSlotsToClear) {
+ if (sysprop::clear_slots_with_set_layer_buffer(false)) {
+ mClearSlotBuffer = sp<GraphicBuffer>::make(1, 1, PIXEL_FORMAT_RGBX_8888,
+ GraphicBuffer::USAGE_HW_COMPOSER |
+ GraphicBuffer::USAGE_SW_READ_OFTEN |
+ GraphicBuffer::USAGE_SW_WRITE_OFTEN,
+ "AidlComposer");
+ if (!mClearSlotBuffer || mClearSlotBuffer->initCheck() != ::android::OK) {
+ LOG_ALWAYS_FATAL("Failed to allocate a buffer for clearing layer buffer slots");
+ return;
+ }
}
}
@@ -844,12 +848,12 @@
Error error = Error::NONE;
mMutex.lock_shared();
if (auto writer = getWriter(display)) {
- // Backwards compatible way of clearing buffer is to set the layer buffer with a placeholder
- // buffer, using the slot that needs to cleared... tricky.
- if (mClearSlotBuffer == nullptr) {
+ if (mSupportsBufferSlotsToClear) {
writer->get().setLayerBufferSlotsToClear(translate<int64_t>(display),
translate<int64_t>(layer), slotsToClear);
- } else {
+ // Backwards compatible way of clearing buffer slots is to set the layer buffer with a
+ // placeholder buffer, using the slot that needs to cleared... tricky.
+ } else if (mClearSlotBuffer != nullptr) {
for (uint32_t slot : slotsToClear) {
// Don't clear the active buffer slot because we need to restore the active buffer
// after clearing the requested buffer slots with a placeholder buffer.
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
index ce05b38..b8ae26f 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
@@ -284,6 +284,8 @@
// threading annotations.
ftl::SharedMutex mMutex;
+ // Whether or not explicitly clearing buffer slots is supported.
+ bool mSupportsBufferSlotsToClear;
// Buffer slots for layers are cleared by setting the slot buffer to this buffer.
sp<GraphicBuffer> mClearSlotBuffer;
diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
index e0f6c45..9b41da5 100644
--- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
@@ -24,12 +24,14 @@
#include "HidlComposerHal.h"
+#include <SurfaceFlingerProperties.h>
#include <android/binder_manager.h>
#include <composer-command-buffer/2.2/ComposerCommandBuffer.h>
#include <hidl/HidlTransportSupport.h>
#include <hidl/HidlTransportUtils.h>
#include <log/log.h>
#include <utils/Trace.h>
+
#include "HWC2.h"
#include "Hal.h"
@@ -189,6 +191,9 @@
}
sp<GraphicBuffer> allocateClearSlotBuffer() {
+ if (!sysprop::clear_slots_with_set_layer_buffer(false)) {
+ return nullptr;
+ }
sp<GraphicBuffer> buffer = sp<GraphicBuffer>::make(1, 1, PIXEL_FORMAT_RGBX_8888,
GraphicBuffer::USAGE_HW_COMPOSER |
GraphicBuffer::USAGE_SW_READ_OFTEN |
@@ -246,7 +251,7 @@
LOG_ALWAYS_FATAL("failed to create composer client");
}
- if (!mClearSlotBuffer) {
+ if (!mClearSlotBuffer && sysprop::clear_slots_with_set_layer_buffer(false)) {
LOG_ALWAYS_FATAL("Failed to allocate a buffer for clearing layer buffer slots");
return;
}
@@ -716,7 +721,11 @@
if (slotsToClear.empty()) {
return Error::NONE;
}
- // Backwards compatible way of clearing buffer is to set the layer buffer with a placeholder
+ // This can be null when the HAL hasn't explicitly enabled this feature.
+ if (mClearSlotBuffer == nullptr) {
+ return Error::NONE;
+ }
+ // Backwards compatible way of clearing buffer is to set the layer buffer with a placeholder
// buffer, using the slot that needs to cleared... tricky.
for (uint32_t slot : slotsToClear) {
// Don't clear the active buffer slot because we need to restore the active buffer after
diff --git a/services/surfaceflinger/SurfaceFlingerProperties.cpp b/services/surfaceflinger/SurfaceFlingerProperties.cpp
index 20fa091..96c8b54 100644
--- a/services/surfaceflinger/SurfaceFlingerProperties.cpp
+++ b/services/surfaceflinger/SurfaceFlingerProperties.cpp
@@ -375,5 +375,9 @@
return SurfaceFlingerProperties::ignore_hdr_camera_layers().value_or(defaultValue);
}
+bool clear_slots_with_set_layer_buffer(bool defaultValue) {
+ return SurfaceFlingerProperties::clear_slots_with_set_layer_buffer().value_or(defaultValue);
+}
+
} // namespace sysprop
} // namespace android
diff --git a/services/surfaceflinger/SurfaceFlingerProperties.h b/services/surfaceflinger/SurfaceFlingerProperties.h
index 080feee..951f8f8 100644
--- a/services/surfaceflinger/SurfaceFlingerProperties.h
+++ b/services/surfaceflinger/SurfaceFlingerProperties.h
@@ -102,6 +102,8 @@
bool ignore_hdr_camera_layers(bool defaultValue);
+bool clear_slots_with_set_layer_buffer(bool defaultValue);
+
} // namespace sysprop
} // namespace android
#endif // SURFACEFLINGERPROPERTIES_H_
diff --git a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
index bcbe21a..689f51a 100644
--- a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
+++ b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop
@@ -470,4 +470,18 @@
scope: Public
access: Readonly
prop_name: "ro.surface_flinger.ignore_hdr_camera_layers"
-}
\ No newline at end of file
+}
+
+# When enabled, SurfaceFlinger will attempt to clear the per-layer HAL buffer cache slots for
+# buffers when they are evicted from the app cache by using additional setLayerBuffer commands.
+# Ideally, this behavior would always be enabled to reduce graphics memory consumption. However,
+# Some HAL implementations may not support the additional setLayerBuffer commands used to clear
+# the cache slots.
+prop {
+ api_name: "clear_slots_with_set_layer_buffer"
+ type: Boolean
+ scope: Public
+ access: Readonly
+ prop_name: "ro.surface_flinger.clear_slots_with_set_layer_buffer"
+}
+
diff --git a/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt b/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt
index 348a462..9660ff3 100644
--- a/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt
+++ b/services/surfaceflinger/sysprop/api/SurfaceFlingerProperties-current.txt
@@ -1,6 +1,10 @@
props {
module: "android.sysprop.SurfaceFlingerProperties"
prop {
+ api_name: "clear_slots_with_set_layer_buffer"
+ prop_name: "ro.surface_flinger.clear_slots_with_set_layer_buffer"
+ }
+ prop {
api_name: "color_space_agnostic_dataspace"
type: Long
prop_name: "ro.surface_flinger.color_space_agnostic_dataspace"