diff options
author | Brian Lindahl <blindahl@google.com> | 2023-06-15 14:19:43 -0600 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-06-24 01:56:27 +0000 |
commit | ab84e11cb5f3e3d257f78ceae64dff80e8e83315 (patch) | |
tree | 0aa71e9331185eb61ae0713dd23df4eafe51c720 | |
parent | 5b67578ab3bf095e357e91c49195dfe7907f3c98 (diff) | |
download | native-ab84e11cb5f3e3d257f78ceae64dff80e8e83315.tar.gz |
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
7 files changed, 55 insertions, 16 deletions
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp index f7049b98e7..c0eb36dc02 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 @@ AidlComposer::AidlComposer(const std::string& serviceName) { 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 AidlComposer::setLayerBufferSlotsToClear(Display display, Layer layer, 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 ce05b382cc..b8ae26f879 100644 --- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h +++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h @@ -284,6 +284,8 @@ private: // 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 e0f6c45f70..9b41da5754 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 @@ std::vector<To> translate(const hidl_vec<From>& in) { } 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 @@ HidlComposer::HidlComposer(const std::string& serviceName) 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 @@ Error HidlComposer::setLayerBufferSlotsToClear(Display display, Layer layer, 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 20fa091730..96c8b54005 100644 --- a/services/surfaceflinger/SurfaceFlingerProperties.cpp +++ b/services/surfaceflinger/SurfaceFlingerProperties.cpp @@ -375,5 +375,9 @@ bool ignore_hdr_camera_layers(bool defaultValue) { 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 080feee686..951f8f8cb3 100644 --- a/services/surfaceflinger/SurfaceFlingerProperties.h +++ b/services/surfaceflinger/SurfaceFlingerProperties.h @@ -102,6 +102,8 @@ bool enable_sdr_dimming(bool defaultValue); 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 bcbe21a483..689f51ad5b 100644 --- a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop +++ b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop @@ -470,4 +470,18 @@ prop { 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 348a462038..9660ff3de6 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" |