Clean ComposerClient cache on hotplug

On subsequent hotplug connected event for a display
SurfaceFlinger destroys the previous framebuffers and
recreates them. When the new buffers are created
ComposerClient still holds a handle to the old buffers and
they are not destroyed. This way the new framebuffers
may get allocated on non continuous memory causing garbled
screens for the user.

Bug: 160112047
Bug: 169255692
Test: 1. limit cma ion memory to 32 MB
      2. flash device
      3. plug hdmi out and in
      4. verify that the display image is not garbled
Change-Id: Idf7cdf7a070ffc83ecec34ac24c8a7d696f68aa6
This commit is contained in:
Marin Shalamanov
2020-10-23 17:42:59 +02:00
parent 0a3d3f3645
commit ac6cc1f97e
3 changed files with 116 additions and 15 deletions

View File

@@ -87,19 +87,28 @@ class ComposerClientImpl : public Interface {
class HalEventCallback : public Hal::EventCallback {
public:
HalEventCallback(const sp<IComposerCallback> callback, ComposerResources* resources)
: mCallback(callback), mResources(resources) {}
HalEventCallback(Hal* hal, const sp<IComposerCallback> callback,
ComposerResources* resources)
: mHal(hal), mCallback(callback), mResources(resources) {}
void onHotplug(Display display, IComposerCallback::Connection connected) {
if (connected == IComposerCallback::Connection::CONNECTED) {
mResources->addPhysicalDisplay(display);
} else if (connected == IComposerCallback::Connection::DISCONNECTED) {
mResources->removeDisplay(display);
}
void onHotplug(Display display, IComposerCallback::Connection connected) {
if (connected == IComposerCallback::Connection::CONNECTED) {
if (mResources->hasDisplay(display)) {
// This is a subsequent hotplug "connected" for a display. This signals a
// display change and thus the framework may want to reallocate buffers. We
// need to free all cached handles, since they are holding a strong reference
// to the underlying buffers.
cleanDisplayResources(display);
mResources->removeDisplay(display);
}
mResources->addPhysicalDisplay(display);
} else if (connected == IComposerCallback::Connection::DISCONNECTED) {
mResources->removeDisplay(display);
}
auto ret = mCallback->onHotplug(display, connected);
ALOGE_IF(!ret.isOk(), "failed to send onHotplug: %s", ret.description().c_str());
}
auto ret = mCallback->onHotplug(display, connected);
ALOGE_IF(!ret.isOk(), "failed to send onHotplug: %s", ret.description().c_str());
}
void onRefresh(Display display) {
mResources->setDisplayMustValidateState(display, true);
@@ -113,13 +122,64 @@ class ComposerClientImpl : public Interface {
}
protected:
const sp<IComposerCallback> mCallback;
ComposerResources* const mResources;
Hal* const mHal;
const sp<IComposerCallback> mCallback;
ComposerResources* const mResources;
void cleanDisplayResources(Display display) {
size_t cacheSize;
Error err = mResources->getDisplayClientTargetCacheSize(display, &cacheSize);
if (err == Error::NONE) {
for (int slot = 0; slot < cacheSize; slot++) {
ComposerResources::ReplacedHandle replacedBuffer(/*isBuffer*/ true);
// Replace the buffer slots with NULLs. Keep the old handle until it is
// replaced in ComposerHal, otherwise we risk leaving a dangling pointer.
const native_handle_t* clientTarget = nullptr;
err = mResources->getDisplayClientTarget(display, slot, /*useCache*/ true,
/*rawHandle*/ nullptr, &clientTarget,
&replacedBuffer);
if (err != Error::NONE) {
continue;
}
const std::vector<hwc_rect_t> damage;
err = mHal->setClientTarget(display, clientTarget, /*fence*/ -1, 0, damage);
ALOGE_IF(err != Error::NONE,
"Can't clean slot %d of the client target buffer"
"cache for display %" PRIu64,
slot, display);
}
} else {
ALOGE("Can't clean client target cache for display %" PRIu64, display);
}
err = mResources->getDisplayOutputBufferCacheSize(display, &cacheSize);
if (err == Error::NONE) {
for (int slot = 0; slot < cacheSize; slot++) {
// Replace the buffer slots with NULLs. Keep the old handle until it is
// replaced in ComposerHal, otherwise we risk leaving a dangling pointer.
ComposerResources::ReplacedHandle replacedBuffer(/*isBuffer*/ true);
const native_handle_t* outputBuffer = nullptr;
err = mResources->getDisplayOutputBuffer(display, slot, /*useCache*/ true,
/*rawHandle*/ nullptr, &outputBuffer,
&replacedBuffer);
if (err != Error::NONE) {
continue;
}
err = mHal->setOutputBuffer(display, outputBuffer, /*fence*/ -1);
ALOGE_IF(err != Error::NONE,
"Can't clean slot %d of the output buffer cache"
"for display %" PRIu64,
slot, display);
}
} else {
ALOGE("Can't clean output buffer cache for display %" PRIu64, display);
}
}
};
Return<void> registerCallback(const sp<IComposerCallback>& callback) override {
// no locking as we require this function to be called only once
mHalEventCallback = std::make_unique<HalEventCallback>(callback, mResources.get());
mHalEventCallback = std::make_unique<HalEventCallback>(mHal, callback, mResources.get());
mHal->registerEventCallback(mHalEventCallback.get());
return Void();
}

View File

@@ -144,6 +144,10 @@ ComposerHandleCache::~ComposerHandleCache() {
}
}
size_t ComposerHandleCache::getCacheSize() const {
return mHandles.size();
}
bool ComposerHandleCache::initCache(HandleType type, uint32_t cacheSize) {
// already initialized
if (mHandleType != HandleType::INVALID) {
@@ -220,6 +224,14 @@ bool ComposerDisplayResource::initClientTargetCache(uint32_t cacheSize) {
return mClientTargetCache.initCache(ComposerHandleCache::HandleType::BUFFER, cacheSize);
}
size_t ComposerDisplayResource::getClientTargetCacheSize() const {
return mClientTargetCache.getCacheSize();
}
size_t ComposerDisplayResource::getOutputBufferCacheSize() const {
return mOutputBufferCache.getCacheSize();
}
bool ComposerDisplayResource::isVirtual() const {
return mType == DisplayType::VIRTUAL;
}
@@ -293,6 +305,10 @@ void ComposerResources::clear(RemoveDisplay removeDisplay) {
mDisplayResources.clear();
}
bool ComposerResources::hasDisplay(Display display) {
return mDisplayResources.count(display) > 0;
}
Error ComposerResources::addPhysicalDisplay(Display display) {
auto displayResource = createDisplayResource(ComposerDisplayResource::DisplayType::PHYSICAL, 0);
@@ -327,6 +343,26 @@ Error ComposerResources::setDisplayClientTargetCacheSize(Display display,
: Error::BAD_PARAMETER;
}
Error ComposerResources::getDisplayClientTargetCacheSize(Display display, size_t* outCacheSize) {
std::lock_guard<std::mutex> lock(mDisplayResourcesMutex);
ComposerDisplayResource* displayResource = findDisplayResourceLocked(display);
if (!displayResource) {
return Error::BAD_DISPLAY;
}
*outCacheSize = displayResource->getClientTargetCacheSize();
return Error::NONE;
}
Error ComposerResources::getDisplayOutputBufferCacheSize(Display display, size_t* outCacheSize) {
std::lock_guard<std::mutex> lock(mDisplayResourcesMutex);
ComposerDisplayResource* displayResource = findDisplayResourceLocked(display);
if (!displayResource) {
return Error::BAD_DISPLAY;
}
*outCacheSize = displayResource->getOutputBufferCacheSize();
return Error::NONE;
}
Error ComposerResources::addLayer(Display display, Layer layer, uint32_t bufferCacheSize) {
auto layerResource = createLayerResource(bufferCacheSize);

View File

@@ -74,6 +74,7 @@ class ComposerHandleCache {
ComposerHandleCache& operator=(const ComposerHandleCache&) = delete;
bool initCache(HandleType type, uint32_t cacheSize);
size_t getCacheSize() const;
Error lookupCache(uint32_t slot, const native_handle_t** outHandle);
Error updateCache(uint32_t slot, const native_handle_t* handle,
const native_handle** outReplacedHandle);
@@ -120,7 +121,8 @@ class ComposerDisplayResource {
uint32_t outputBufferCacheSize);
bool initClientTargetCache(uint32_t cacheSize);
size_t getClientTargetCacheSize() const;
size_t getOutputBufferCacheSize() const;
bool isVirtual() const;
Error getClientTarget(uint32_t slot, bool fromCache, const native_handle_t* inHandle,
@@ -162,12 +164,15 @@ class ComposerResources {
std::function<void(Display display, bool isVirtual, const std::vector<Layer>& layers)>;
void clear(RemoveDisplay removeDisplay);
bool hasDisplay(Display display);
Error addPhysicalDisplay(Display display);
Error addVirtualDisplay(Display display, uint32_t outputBufferCacheSize);
Error removeDisplay(Display display);
Error setDisplayClientTargetCacheSize(Display display, uint32_t clientTargetCacheSize);
Error getDisplayClientTargetCacheSize(Display display, size_t* outCacheSize);
Error getDisplayOutputBufferCacheSize(Display display, size_t* outCacheSize);
Error addLayer(Display display, Layer layer, uint32_t bufferCacheSize);
Error removeLayer(Display display, Layer layer);