AidlComposer: use a reader/writer per display
In order to call HWC from different threads, with one thread per
display, separate commands and their results per display.
AidlComposer:
- Add a maps from display to a ComposerClientReader and a
ComposerClientWriter. Each reader/writer will be used by a single
display. AidlComposer is generally threadsafe, except for these
objects. (The other members are pointers to proxy binders, which can
have their methods safely called from multiple threads.) Use an
ftl::SharedMutex to guard access to the maps. The client is
responsible for ensuring they do not attempt to access the same
reader/writer concurrently from multiple threads. Different threads
can access different readers/writers concurrently, but the mutex
ensures that adding or deleting an entry does not impact access to the
objects.
- Add a `Display` parameter to execute[Commands] and resetCommands, so
that it only affects the intended writer. The callers already know
which Display they care about, so pass it in.
- If no displays support DisplayCapability.MULTI_THREADED_PRESENT, use a
single reader for all displays. This is required for backwards
compatibility.
[Hidl]ComposerHal, MockComposer:
- update APIs for executeCommands and resetCommands
- implement onHotPlug[Connect/Disconnect]
HWComposer, fuzzer:
- pass the display to new APIs
Bug: 241285491
Test: make, boot
Change-Id: I2b62e4965b12b3c653e6c00f9f6ab4f48b506b18
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
index 0e41962..eff5130 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
@@ -230,6 +230,8 @@
return;
}
+ addReader(translate<Display>(kSingleReaderKey));
+
ALOGI("Loaded AIDL composer3 HAL service");
}
@@ -298,12 +300,19 @@
}
}
-void AidlComposer::resetCommands() {
- mWriter.reset();
+void AidlComposer::resetCommands(Display display) {
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().reset();
+ }
+ mMutex.unlock_shared();
}
-Error AidlComposer::executeCommands() {
- return execute();
+Error AidlComposer::executeCommands(Display display) {
+ mMutex.lock_shared();
+ auto error = execute(display);
+ mMutex.unlock_shared();
+ return error;
}
uint32_t AidlComposer::getMaxVirtualDisplayCount() {
@@ -334,6 +343,7 @@
*outDisplay = translate<Display>(virtualDisplay.display);
*format = static_cast<PixelFormat>(virtualDisplay.format);
+ addDisplay(translate<Display>(virtualDisplay.display));
return Error::NONE;
}
@@ -343,12 +353,20 @@
ALOGE("destroyVirtualDisplay failed %s", status.getDescription().c_str());
return static_cast<Error>(status.getServiceSpecificError());
}
+ removeDisplay(display);
return Error::NONE;
}
Error AidlComposer::acceptDisplayChanges(Display display) {
- mWriter.acceptDisplayChanges(translate<int64_t>(display));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().acceptDisplayChanges(translate<int64_t>(display));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::createLayer(Display display, Layer* outLayer) {
@@ -388,7 +406,17 @@
Error AidlComposer::getChangedCompositionTypes(
Display display, std::vector<Layer>* outLayers,
std::vector<aidl::android::hardware::graphics::composer3::Composition>* outTypes) {
- const auto changedLayers = mReader.takeChangedCompositionTypes(translate<int64_t>(display));
+ std::vector<ChangedCompositionLayer> changedLayers;
+ Error error = Error::NONE;
+ {
+ mMutex.lock_shared();
+ if (auto reader = getReader(display)) {
+ changedLayers = reader->get().takeChangedCompositionTypes(translate<int64_t>(display));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ }
outLayers->reserve(changedLayers.size());
outTypes->reserve(changedLayers.size());
@@ -396,7 +424,7 @@
outLayers->emplace_back(translate<Layer>(layer.layer));
outTypes->emplace_back(layer.composition);
}
- return Error::NONE;
+ return error;
}
Error AidlComposer::getColorModes(Display display, std::vector<ColorMode>* outModes) {
@@ -448,7 +476,17 @@
Error AidlComposer::getDisplayRequests(Display display, uint32_t* outDisplayRequestMask,
std::vector<Layer>* outLayers,
std::vector<uint32_t>* outLayerRequestMasks) {
- const auto displayRequests = mReader.takeDisplayRequests(translate<int64_t>(display));
+ Error error = Error::NONE;
+ DisplayRequest displayRequests;
+ {
+ mMutex.lock_shared();
+ if (auto reader = getReader(display)) {
+ displayRequests = reader->get().takeDisplayRequests(translate<int64_t>(display));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ }
*outDisplayRequestMask = translate<uint32_t>(displayRequests.mask);
outLayers->reserve(displayRequests.layerRequests.size());
outLayerRequestMasks->reserve(displayRequests.layerRequests.size());
@@ -457,7 +495,7 @@
outLayers->emplace_back(translate<Layer>(layer.layer));
outLayerRequestMasks->emplace_back(translate<uint32_t>(layer.mask));
}
- return Error::NONE;
+ return error;
}
Error AidlComposer::getDozeSupport(Display display, bool* outSupport) {
@@ -511,7 +549,17 @@
Error AidlComposer::getReleaseFences(Display display, std::vector<Layer>* outLayers,
std::vector<int>* outReleaseFences) {
- auto fences = mReader.takeReleaseFences(translate<int64_t>(display));
+ Error error = Error::NONE;
+ std::vector<ReleaseFences::Layer> fences;
+ {
+ mMutex.lock_shared();
+ if (auto reader = getReader(display)) {
+ fences = reader->get().takeReleaseFences(translate<int64_t>(display));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ }
outLayers->reserve(fences.size());
outReleaseFences->reserve(fences.size());
@@ -522,19 +570,29 @@
*fence.fence.getR() = -1;
outReleaseFences->emplace_back(fenceOwner);
}
- return Error::NONE;
+ return error;
}
Error AidlComposer::presentDisplay(Display display, int* outPresentFence) {
ATRACE_NAME("HwcPresentDisplay");
- mWriter.presentDisplay(translate<int64_t>(display));
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ auto writer = getWriter(display);
+ auto reader = getReader(display);
+ if (writer && reader) {
+ writer->get().presentDisplay(translate<int64_t>(display));
+ error = execute(display);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
- Error error = execute();
if (error != Error::NONE) {
+ mMutex.unlock_shared();
return error;
}
- auto fence = mReader.takePresentFence(translate<int64_t>(display));
+ auto fence = reader->get().takePresentFence(translate<int64_t>(display));
+ mMutex.unlock_shared();
// take ownership
*outPresentFence = fence.get();
*fence.getR() = -1;
@@ -559,11 +617,19 @@
handle = target->getNativeBuffer()->handle;
}
- mWriter.setClientTarget(translate<int64_t>(display), slot, handle, acquireFence,
- translate<aidl::android::hardware::graphics::common::Dataspace>(
- dataspace),
- translate<AidlRect>(damage));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get()
+ .setClientTarget(translate<int64_t>(display), slot, handle, acquireFence,
+ translate<aidl::android::hardware::graphics::common::Dataspace>(
+ dataspace),
+ translate<AidlRect>(damage));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setColorMode(Display display, ColorMode mode, RenderIntent renderIntent) {
@@ -579,14 +645,28 @@
}
Error AidlComposer::setColorTransform(Display display, const float* matrix) {
- mWriter.setColorTransform(translate<int64_t>(display), matrix);
- return Error::NONE;
+ auto error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setColorTransform(translate<int64_t>(display), matrix);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setOutputBuffer(Display display, const native_handle_t* buffer,
int releaseFence) {
- mWriter.setOutputBuffer(translate<int64_t>(display), 0, buffer, dup(releaseFence));
- return Error::NONE;
+ auto error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setOutputBuffer(translate<int64_t>(display), 0, buffer, dup(releaseFence));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setPowerMode(Display display, IComposerClient::PowerMode mode) {
@@ -624,16 +704,26 @@
Error AidlComposer::validateDisplay(Display display, nsecs_t expectedPresentTime,
uint32_t* outNumTypes, uint32_t* outNumRequests) {
ATRACE_NAME("HwcValidateDisplay");
- mWriter.validateDisplay(translate<int64_t>(display),
- ClockMonotonicTimestamp{expectedPresentTime});
+ const auto displayId = translate<int64_t>(display);
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ auto writer = getWriter(display);
+ auto reader = getReader(display);
+ if (writer && reader) {
+ writer->get().validateDisplay(displayId, ClockMonotonicTimestamp{expectedPresentTime});
+ error = execute(display);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
- Error error = execute();
if (error != Error::NONE) {
+ mMutex.unlock_shared();
return error;
}
- mReader.hasChanges(translate<int64_t>(display), outNumTypes, outNumRequests);
+ reader->get().hasChanges(displayId, outNumTypes, outNumRequests);
+ mMutex.unlock_shared();
return Error::NONE;
}
@@ -641,39 +731,59 @@
uint32_t* outNumTypes, uint32_t* outNumRequests,
int* outPresentFence, uint32_t* state) {
ATRACE_NAME("HwcPresentOrValidateDisplay");
- mWriter.presentOrvalidateDisplay(translate<int64_t>(display),
- ClockMonotonicTimestamp{expectedPresentTime});
+ const auto displayId = translate<int64_t>(display);
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ auto writer = getWriter(display);
+ auto reader = getReader(display);
+ if (writer && reader) {
+ writer->get().presentOrvalidateDisplay(displayId,
+ ClockMonotonicTimestamp{expectedPresentTime});
+ error = execute(display);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
- Error error = execute();
if (error != Error::NONE) {
+ mMutex.unlock_shared();
return error;
}
- const auto result = mReader.takePresentOrValidateStage(translate<int64_t>(display));
+ const auto result = reader->get().takePresentOrValidateStage(displayId);
if (!result.has_value()) {
*state = translate<uint32_t>(-1);
+ mMutex.unlock_shared();
return Error::NO_RESOURCES;
}
*state = translate<uint32_t>(*result);
if (*result == PresentOrValidate::Result::Presented) {
- auto fence = mReader.takePresentFence(translate<int64_t>(display));
+ auto fence = reader->get().takePresentFence(displayId);
// take ownership
*outPresentFence = fence.get();
*fence.getR() = -1;
}
if (*result == PresentOrValidate::Result::Validated) {
- mReader.hasChanges(translate<int64_t>(display), outNumTypes, outNumRequests);
+ reader->get().hasChanges(displayId, outNumTypes, outNumRequests);
}
+ mMutex.unlock_shared();
return Error::NONE;
}
Error AidlComposer::setCursorPosition(Display display, Layer layer, int32_t x, int32_t y) {
- mWriter.setLayerCursorPosition(translate<int64_t>(display), translate<int64_t>(layer), x, y);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerCursorPosition(translate<int64_t>(display), translate<int64_t>(layer),
+ x, y);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerBuffer(Display display, Layer layer, uint32_t slot,
@@ -683,90 +793,190 @@
handle = buffer->getNativeBuffer()->handle;
}
- mWriter.setLayerBuffer(translate<int64_t>(display), translate<int64_t>(layer), slot, handle,
- acquireFence);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerBuffer(translate<int64_t>(display), translate<int64_t>(layer), slot,
+ handle, acquireFence);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerSurfaceDamage(Display display, Layer layer,
const std::vector<IComposerClient::Rect>& damage) {
- mWriter.setLayerSurfaceDamage(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlRect>(damage));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerSurfaceDamage(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlRect>(damage));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerBlendMode(Display display, Layer layer,
IComposerClient::BlendMode mode) {
- mWriter.setLayerBlendMode(translate<int64_t>(display), translate<int64_t>(layer),
- translate<BlendMode>(mode));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerBlendMode(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<BlendMode>(mode));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerColor(Display display, Layer layer, const Color& color) {
- mWriter.setLayerColor(translate<int64_t>(display), translate<int64_t>(layer), color);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerColor(translate<int64_t>(display), translate<int64_t>(layer), color);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerCompositionType(
Display display, Layer layer,
aidl::android::hardware::graphics::composer3::Composition type) {
- mWriter.setLayerCompositionType(translate<int64_t>(display), translate<int64_t>(layer), type);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerCompositionType(translate<int64_t>(display),
+ translate<int64_t>(layer), type);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerDataspace(Display display, Layer layer, Dataspace dataspace) {
- mWriter.setLayerDataspace(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlDataspace>(dataspace));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerDataspace(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlDataspace>(dataspace));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerDisplayFrame(Display display, Layer layer,
const IComposerClient::Rect& frame) {
- mWriter.setLayerDisplayFrame(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlRect>(frame));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerDisplayFrame(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlRect>(frame));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerPlaneAlpha(Display display, Layer layer, float alpha) {
- mWriter.setLayerPlaneAlpha(translate<int64_t>(display), translate<int64_t>(layer), alpha);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerPlaneAlpha(translate<int64_t>(display), translate<int64_t>(layer),
+ alpha);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerSidebandStream(Display display, Layer layer,
const native_handle_t* stream) {
- mWriter.setLayerSidebandStream(translate<int64_t>(display), translate<int64_t>(layer), stream);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerSidebandStream(translate<int64_t>(display), translate<int64_t>(layer),
+ stream);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerSourceCrop(Display display, Layer layer,
const IComposerClient::FRect& crop) {
- mWriter.setLayerSourceCrop(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlFRect>(crop));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerSourceCrop(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlFRect>(crop));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerTransform(Display display, Layer layer, Transform transform) {
- mWriter.setLayerTransform(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlTransform>(transform));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerTransform(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlTransform>(transform));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerVisibleRegion(Display display, Layer layer,
const std::vector<IComposerClient::Rect>& visible) {
- mWriter.setLayerVisibleRegion(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlRect>(visible));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerVisibleRegion(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlRect>(visible));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerZOrder(Display display, Layer layer, uint32_t z) {
- mWriter.setLayerZOrder(translate<int64_t>(display), translate<int64_t>(layer), z);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerZOrder(translate<int64_t>(display), translate<int64_t>(layer), z);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
-Error AidlComposer::execute() {
- const auto& commands = mWriter.getPendingCommands();
+Error AidlComposer::execute(Display display) {
+ auto writer = getWriter(display);
+ auto reader = getReader(display);
+ if (!writer || !reader) {
+ return Error::BAD_DISPLAY;
+ }
+
+ const auto& commands = writer->get().getPendingCommands();
if (commands.empty()) {
- mWriter.reset();
+ writer->get().reset();
return Error::NONE;
}
@@ -778,9 +988,9 @@
return static_cast<Error>(status.getServiceSpecificError());
}
- mReader.parse(std::move(results));
+ reader->get().parse(std::move(results));
}
- const auto commandErrors = mReader.takeErrors();
+ const auto commandErrors = reader->get().takeErrors();
Error error = Error::NONE;
for (const auto& cmdErr : commandErrors) {
const auto index = static_cast<size_t>(cmdErr.commandIndex);
@@ -798,7 +1008,7 @@
}
}
- mWriter.reset();
+ writer->get().reset();
return error;
}
@@ -806,9 +1016,17 @@
Error AidlComposer::setLayerPerFrameMetadata(
Display display, Layer layer,
const std::vector<IComposerClient::PerFrameMetadata>& perFrameMetadatas) {
- mWriter.setLayerPerFrameMetadata(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlPerFrameMetadata>(perFrameMetadatas));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerPerFrameMetadata(translate<int64_t>(display),
+ translate<int64_t>(layer),
+ translate<AidlPerFrameMetadata>(perFrameMetadatas));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
std::vector<IComposerClient::PerFrameMetadataKey> AidlComposer::getPerFrameMetadataKeys(
@@ -868,8 +1086,16 @@
}
Error AidlComposer::setLayerColorTransform(Display display, Layer layer, const float* matrix) {
- mWriter.setLayerColorTransform(translate<int64_t>(display), translate<int64_t>(layer), matrix);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerColorTransform(translate<int64_t>(display), translate<int64_t>(layer),
+ matrix);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::getDisplayedContentSamplingAttributes(Display display, PixelFormat* outFormat,
@@ -932,20 +1158,36 @@
Error AidlComposer::setLayerPerFrameMetadataBlobs(
Display display, Layer layer,
const std::vector<IComposerClient::PerFrameMetadataBlob>& metadata) {
- mWriter.setLayerPerFrameMetadataBlobs(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlPerFrameMetadataBlob>(metadata));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerPerFrameMetadataBlobs(translate<int64_t>(display),
+ translate<int64_t>(layer),
+ translate<AidlPerFrameMetadataBlob>(metadata));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setDisplayBrightness(Display display, float brightness, float brightnessNits,
const DisplayBrightnessOptions& options) {
- mWriter.setDisplayBrightness(translate<int64_t>(display), brightness, brightnessNits);
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setDisplayBrightness(translate<int64_t>(display), brightness, brightnessNits);
- if (options.applyImmediately) {
- return execute();
+ if (options.applyImmediately) {
+ error = execute(display);
+ mMutex.unlock_shared();
+ return error;
+ }
+ } else {
+ error = Error::BAD_DISPLAY;
}
-
- return Error::NONE;
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::getDisplayCapabilities(Display display,
@@ -1085,20 +1327,43 @@
Error AidlComposer::getClientTargetProperty(
Display display, ClientTargetPropertyWithBrightness* outClientTargetProperty) {
- *outClientTargetProperty = mReader.takeClientTargetProperty(translate<int64_t>(display));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto reader = getReader(display)) {
+ *outClientTargetProperty =
+ reader->get().takeClientTargetProperty(translate<int64_t>(display));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerBrightness(Display display, Layer layer, float brightness) {
- mWriter.setLayerBrightness(translate<int64_t>(display), translate<int64_t>(layer), brightness);
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerBrightness(translate<int64_t>(display), translate<int64_t>(layer),
+ brightness);
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::setLayerBlockingRegion(Display display, Layer layer,
const std::vector<IComposerClient::Rect>& blocking) {
- mWriter.setLayerBlockingRegion(translate<int64_t>(display), translate<int64_t>(layer),
- translate<AidlRect>(blocking));
- return Error::NONE;
+ Error error = Error::NONE;
+ mMutex.lock_shared();
+ if (auto writer = getWriter(display)) {
+ writer->get().setLayerBlockingRegion(translate<int64_t>(display), translate<int64_t>(layer),
+ translate<AidlRect>(blocking));
+ } else {
+ error = Error::BAD_DISPLAY;
+ }
+ mMutex.unlock_shared();
+ return error;
}
Error AidlComposer::getDisplayDecorationSupport(Display display,
@@ -1136,5 +1401,88 @@
return Error::NONE;
}
+ftl::Optional<std::reference_wrapper<ComposerClientWriter>> AidlComposer::getWriter(Display display)
+ REQUIRES_SHARED(mMutex) {
+ return mWriters.get(display);
+}
+
+ftl::Optional<std::reference_wrapper<ComposerClientReader>> AidlComposer::getReader(Display display)
+ REQUIRES_SHARED(mMutex) {
+ if (mSingleReader) {
+ display = translate<Display>(kSingleReaderKey);
+ }
+ return mReaders.get(display);
+}
+
+void AidlComposer::removeDisplay(Display display) {
+ mMutex.lock();
+ bool wasErased = mWriters.erase(display);
+ ALOGW_IF(!wasErased,
+ "Attempting to remove writer for display %" PRId64 " which is not connected",
+ translate<int64_t>(display));
+ if (!mSingleReader) {
+ removeReader(display);
+ }
+ mMutex.unlock();
+}
+
+void AidlComposer::onHotplugDisconnect(Display display) {
+ removeDisplay(display);
+}
+
+bool AidlComposer::hasMultiThreadedPresentSupport(Display display) {
+ const auto displayId = translate<int64_t>(display);
+ std::vector<AidlDisplayCapability> capabilities;
+ const auto status = mAidlComposerClient->getDisplayCapabilities(displayId, &capabilities);
+ if (!status.isOk()) {
+ ALOGE("getDisplayCapabilities failed %s", status.getDescription().c_str());
+ return false;
+ }
+ return std::find(capabilities.begin(), capabilities.end(),
+ AidlDisplayCapability::MULTI_THREADED_PRESENT) != capabilities.end();
+}
+
+void AidlComposer::addReader(Display display) {
+ const auto displayId = translate<int64_t>(display);
+ std::optional<int64_t> displayOpt;
+ if (displayId != kSingleReaderKey) {
+ displayOpt.emplace(displayId);
+ }
+ auto [it, added] = mReaders.try_emplace(display, std::move(displayOpt));
+ ALOGW_IF(!added, "Attempting to add writer for display %" PRId64 " which is already connected",
+ displayId);
+}
+
+void AidlComposer::removeReader(Display display) {
+ bool wasErased = mReaders.erase(display);
+ ALOGW_IF(!wasErased,
+ "Attempting to remove reader for display %" PRId64 " which is not connected",
+ translate<int64_t>(display));
+}
+
+void AidlComposer::addDisplay(Display display) {
+ const auto displayId = translate<int64_t>(display);
+ mMutex.lock();
+ auto [it, added] = mWriters.try_emplace(display, displayId);
+ ALOGW_IF(!added, "Attempting to add writer for display %" PRId64 " which is already connected",
+ displayId);
+ if (mSingleReader) {
+ if (hasMultiThreadedPresentSupport(display)) {
+ mSingleReader = false;
+ removeReader(translate<Display>(kSingleReaderKey));
+ // Note that this includes the new display.
+ for (const auto& [existingDisplay, _] : mWriters) {
+ addReader(existingDisplay);
+ }
+ }
+ } else {
+ addReader(display);
+ }
+ mMutex.unlock();
+}
+
+void AidlComposer::onHotplugConnect(Display display) {
+ addDisplay(display);
+}
} // namespace Hwc2
} // namespace android
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
index f2a59a5..d84efe7 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
@@ -17,10 +17,12 @@
#pragma once
#include "ComposerHal.h"
+#include <ftl/shared_mutex.h>
+#include <ftl/small_map.h>
+#include <functional>
#include <optional>
#include <string>
-#include <unordered_map>
#include <utility>
#include <vector>
@@ -70,10 +72,10 @@
// Reset all pending commands in the command buffer. Useful if you want to
// skip a frame but have already queued some commands.
- void resetCommands() override;
+ void resetCommands(Display) override;
// Explicitly flush all pending commands in the command buffer.
- Error executeCommands() override;
+ Error executeCommands(Display) override;
uint32_t getMaxVirtualDisplayCount() override;
Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
@@ -228,16 +230,29 @@
Error getPhysicalDisplayOrientation(Display displayId,
AidlTransform* outDisplayOrientation) override;
+ void onHotplugConnect(Display) override;
+ void onHotplugDisconnect(Display) override;
private:
// Many public functions above simply write a command into the command
// queue to batch the calls. validateDisplay and presentDisplay will call
// this function to execute the command queue.
- Error execute();
+ Error execute(Display) REQUIRES_SHARED(mMutex);
// returns the default instance name for the given service
static std::string instance(const std::string& serviceName);
+ ftl::Optional<std::reference_wrapper<ComposerClientWriter>> getWriter(Display)
+ REQUIRES_SHARED(mMutex);
+ ftl::Optional<std::reference_wrapper<ComposerClientReader>> getReader(Display)
+ REQUIRES_SHARED(mMutex);
+ void addDisplay(Display) EXCLUDES(mMutex);
+ void removeDisplay(Display) EXCLUDES(mMutex);
+ void addReader(Display) REQUIRES(mMutex);
+ void removeReader(Display) REQUIRES(mMutex);
+
+ bool hasMultiThreadedPresentSupport(Display);
+
// 64KiB minus a small space for metadata such as read/write pointers
static constexpr size_t kWriterInitialSize = 64 * 1024 / sizeof(uint32_t) - 16;
// Max number of buffers that may be cached for a given layer
@@ -245,8 +260,25 @@
// 1. Tightly coupling this cache to the max size of BufferQueue
// 2. Adding an additional slot for the layer caching feature in SurfaceFlinger (see: Planner.h)
static const constexpr uint32_t kMaxLayerBufferCount = BufferQueue::NUM_BUFFER_SLOTS + 1;
- ComposerClientWriter mWriter;
- ComposerClientReader mReader;
+
+ // Without DisplayCapability::MULTI_THREADED_PRESENT, we use a single reader
+ // for all displays. With the capability, we use a separate reader for each
+ // display.
+ bool mSingleReader = true;
+ // Invalid displayId used as a key to mReaders when mSingleReader is true.
+ static constexpr int64_t kSingleReaderKey = 0;
+
+ // TODO (b/256881188): Use display::PhysicalDisplayMap instead of hard-coded `3`
+ ftl::SmallMap<Display, ComposerClientWriter, 3> mWriters GUARDED_BY(mMutex);
+ ftl::SmallMap<Display, ComposerClientReader, 3> mReaders GUARDED_BY(mMutex);
+ // Protect access to mWriters and mReaders with a shared_mutex. Adding and
+ // removing a display require exclusive access, since the iterator or the
+ // writer/reader may be invalidated. Other calls need shared access while
+ // using the writer/reader, so they can use their display's writer/reader
+ // without it being deleted or the iterator being invalidated.
+ // TODO (b/257958323): Use std::shared_mutex and RAII once they support
+ // threading annotations.
+ ftl::SharedMutex mMutex;
// Aidl interface
using AidlIComposer = aidl::android::hardware::graphics::composer3::IComposer;
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h
index b02f867..a9bf282 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h
@@ -110,10 +110,10 @@
// Reset all pending commands in the command buffer. Useful if you want to
// skip a frame but have already queued some commands.
- virtual void resetCommands() = 0;
+ virtual void resetCommands(Display) = 0;
// Explicitly flush all pending commands in the command buffer.
- virtual Error executeCommands() = 0;
+ virtual Error executeCommands(Display) = 0;
virtual uint32_t getMaxVirtualDisplayCount() = 0;
virtual Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat*,
@@ -283,6 +283,8 @@
virtual Error getPhysicalDisplayOrientation(Display displayId,
AidlTransform* outDisplayOrientation) = 0;
virtual Error getOverlaySupport(V3_0::OverlayProperties* outProperties) = 0;
+ virtual void onHotplugConnect(Display) = 0;
+ virtual void onHotplugDisconnect(Display) = 0;
};
} // namespace Hwc2
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 168e2dd..5f11cb8 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -513,7 +513,7 @@
if (displayData.validateWasSkipped) {
// explicitly flush all pending commands
- auto error = static_cast<hal::Error>(mComposer->executeCommands());
+ auto error = static_cast<hal::Error>(mComposer->executeCommands(hwcDisplay->getId()));
RETURN_IF_HWC_ERROR_FOR("executeCommands", error, displayId, UNKNOWN_ERROR);
RETURN_IF_HWC_ERROR_FOR("present", displayData.presentError, displayId, UNKNOWN_ERROR);
return NO_ERROR;
@@ -933,6 +933,8 @@
: "Secondary display",
.deviceProductInfo = std::nullopt};
}();
+
+ mComposer->onHotplugConnect(hwcDisplayId);
}
if (!isConnected(info->id)) {
@@ -960,6 +962,7 @@
// The display will later be destroyed by a call to HWComposer::disconnectDisplay. For now, mark
// it as disconnected.
mDisplayData.at(*displayId).hwcDisplay->setConnected(false);
+ mComposer->onHotplugDisconnect(hwcDisplayId);
return DisplayIdentificationInfo{.id = *displayId};
}
diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
index a664d2c..f8522e2 100644
--- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
@@ -273,11 +273,11 @@
}
}
-void HidlComposer::resetCommands() {
+void HidlComposer::resetCommands(Display) {
mWriter.reset();
}
-Error HidlComposer::executeCommands() {
+Error HidlComposer::executeCommands(Display) {
return execute();
}
@@ -1357,6 +1357,9 @@
registerCallback(sp<ComposerCallbackBridge>::make(callback, vsyncSwitchingSupported));
}
+void HidlComposer::onHotplugConnect(Display) {}
+void HidlComposer::onHotplugDisconnect(Display) {}
+
CommandReader::~CommandReader() {
resetData();
}
diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
index b436408..48b720c 100644
--- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
@@ -177,10 +177,10 @@
// Reset all pending commands in the command buffer. Useful if you want to
// skip a frame but have already queued some commands.
- void resetCommands() override;
+ void resetCommands(Display) override;
// Explicitly flush all pending commands in the command buffer.
- Error executeCommands() override;
+ Error executeCommands(Display) override;
uint32_t getMaxVirtualDisplayCount() override;
Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
@@ -339,6 +339,8 @@
Error getPhysicalDisplayOrientation(Display displayId,
AidlTransform* outDisplayOrientation) override;
+ void onHotplugConnect(Display) override;
+ void onHotplugDisconnect(Display) override;
private:
class CommandWriter : public CommandWriterBase {
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
index f8fc6f5..8a6af10 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
@@ -326,8 +326,8 @@
invokeComposerHal2_3(&composer, display, outLayer);
invokeComposerHal2_4(&composer, display, outLayer);
- composer.executeCommands();
- composer.resetCommands();
+ composer.executeCommands(display);
+ composer.resetCommands(display);
composer.destroyLayer(display, outLayer);
composer.destroyVirtualDisplay(display);
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 9d8e0a2..342c646 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -73,6 +73,7 @@
EXPECT_CALL(*mHal, setClientTargetSlotCount(_));
EXPECT_CALL(*mHal, setVsyncEnabled(hwcDisplayId, Hwc2::IComposerClient::Vsync::DISABLE));
+ EXPECT_CALL(*mHal, onHotplugConnect(hwcDisplayId));
}
};
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
index 3808487..5ee38ec 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
@@ -56,8 +56,8 @@
std::vector<aidl::android::hardware::graphics::composer3::Capability>());
MOCK_METHOD0(dumpDebugInfo, std::string());
MOCK_METHOD1(registerCallback, void(HWC2::ComposerCallback&));
- MOCK_METHOD0(resetCommands, void());
- MOCK_METHOD0(executeCommands, Error());
+ MOCK_METHOD1(resetCommands, void(Display));
+ MOCK_METHOD1(executeCommands, Error(Display));
MOCK_METHOD0(getMaxVirtualDisplayCount, uint32_t());
MOCK_METHOD4(createVirtualDisplay, Error(uint32_t, uint32_t, PixelFormat*, Display*));
MOCK_METHOD1(destroyVirtualDisplay, Error(Display));
@@ -166,6 +166,8 @@
MOCK_METHOD2(getPhysicalDisplayOrientation, Error(Display, AidlTransform*));
MOCK_METHOD1(getOverlaySupport,
Error(aidl::android::hardware::graphics::composer3::OverlayProperties*));
+ MOCK_METHOD1(onHotplugConnect, void(Display));
+ MOCK_METHOD1(onHotplugDisconnect, void(Display));
};
} // namespace Hwc2::mock