Merge "SF: Use Aidl Parcelable commands"

3 files changed, 96 insertions, 542 deletions

diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
index ba57be5d65..200bb65b29 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
@@ -41,6 +41,8 @@ using aidl::android::hardware::graphics::composer3::Capability;
 using aidl::android::hardware::graphics::composer3::PowerMode;
 using aidl::android::hardware::graphics::composer3::VirtualDisplay;
 
+using aidl::android::hardware::graphics::composer3::command::CommandResultPayload;
+
 using AidlColorMode = aidl::android::hardware::graphics::composer3::ColorMode;
 using AidlContentType = aidl::android::hardware::graphics::composer3::ContentType;
 using AidlDisplayIdentification =
@@ -48,8 +50,6 @@ using AidlDisplayIdentification =
 using AidlDisplayContentSample = aidl::android::hardware::graphics::composer3::DisplayContentSample;
 using AidlDisplayAttribute = aidl::android::hardware::graphics::composer3::DisplayAttribute;
 using AidlDisplayCapability = aidl::android::hardware::graphics::composer3::DisplayCapability;
-using AidlExecuteCommandsStatus =
-        aidl::android::hardware::graphics::composer3::ExecuteCommandsStatus;
 using AidlHdrCapabilities = aidl::android::hardware::graphics::composer3::HdrCapabilities;
 using AidlPerFrameMetadata = aidl::android::hardware::graphics::composer3::PerFrameMetadata;
 using AidlPerFrameMetadataKey = aidl::android::hardware::graphics::composer3::PerFrameMetadataKey;
@@ -174,6 +174,14 @@ IComposerClient::LayerGenericMetadataKey translate(AidlLayerGenericMetadataKey x
     };
 }
 
+template <>
+IComposerClient::ClientTargetProperty translate(ClientTargetProperty x) {
+    return IComposerClient::ClientTargetProperty{
+            .pixelFormat = translate<PixelFormat>(x.pixelFormat),
+            .dataspace = translate<Dataspace>(x.dataspace),
+    };
+}
+
 mat4 makeMat4(std::vector<float> in) {
     return mat4(static_cast<const float*>(in.data()));
 }
@@ -226,7 +234,7 @@ bool AidlComposer::isDeclared(const std::string& serviceName) {
     return AServiceManager_isDeclared(instance(serviceName).c_str());
 }
 
-AidlComposer::AidlComposer(const std::string& serviceName) : mWriter(kWriterInitialSize) {
+AidlComposer::AidlComposer(const std::string& serviceName) {
     // This only waits if the service is actually declared
     mAidlComposer = AidlIComposer::fromBinder(
             ndk::SpAIBinder(AServiceManager_waitForService(instance(serviceName).c_str())));
@@ -327,8 +335,7 @@ Error AidlComposer::destroyVirtualDisplay(Display display) {
 }
 
 Error AidlComposer::acceptDisplayChanges(Display display) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.acceptDisplayChanges();
+    mWriter.acceptDisplayChanges(translate<int64_t>(display));
     return Error::NONE;
 }
 
@@ -369,7 +376,12 @@ Error AidlComposer::getActiveConfig(Display display, Config* outConfig) {
 Error AidlComposer::getChangedCompositionTypes(
         Display display, std::vector<Layer>* outLayers,
         std::vector<IComposerClient::Composition>* outTypes) {
-    mReader.takeChangedCompositionTypes(display, outLayers, outTypes);
+    std::vector<int64_t> layers;
+    std::vector<Composition> types;
+    mReader.takeChangedCompositionTypes(translate<int64_t>(display), &layers, &types);
+
+    *outLayers = translate<Layer>(layers);
+    *outTypes = translate<IComposerClient::Composition>(types);
     return Error::NONE;
 }
 
@@ -422,7 +434,10 @@ Error AidlComposer::getDisplayName(Display display, std::string* outName) {
 Error AidlComposer::getDisplayRequests(Display display, uint32_t* outDisplayRequestMask,
                                        std::vector<Layer>* outLayers,
                                        std::vector<uint32_t>* outLayerRequestMasks) {
-    mReader.takeDisplayRequests(display, outDisplayRequestMask, outLayers, outLayerRequestMasks);
+    std::vector<int64_t> layers;
+    mReader.takeDisplayRequests(translate<int64_t>(display), outDisplayRequestMask, &layers,
+                                outLayerRequestMasks);
+    *outLayers = translate<Layer>(layers);
     return Error::NONE;
 }
 
@@ -456,21 +471,22 @@ Error AidlComposer::getHdrCapabilities(Display display, std::vector<Hdr>* outTyp
 
 Error AidlComposer::getReleaseFences(Display display, std::vector<Layer>* outLayers,
                                      std::vector<int>* outReleaseFences) {
-    mReader.takeReleaseFences(display, outLayers, outReleaseFences);
+    std::vector<int64_t> layers;
+    mReader.takeReleaseFences(translate<int64_t>(display), &layers, outReleaseFences);
+    *outLayers = translate<Layer>(layers);
     return Error::NONE;
 }
 
 Error AidlComposer::presentDisplay(Display display, int* outPresentFence) {
     ATRACE_NAME("HwcPresentDisplay");
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.presentDisplay();
+    mWriter.presentDisplay(translate<int64_t>(display));
 
     Error error = execute();
     if (error != Error::NONE) {
         return error;
     }
 
-    mReader.takePresentFence(display, outPresentFence);
+    mReader.takePresentFence(translate<int64_t>(display), outPresentFence);
 
     return Error::NONE;
 }
@@ -488,14 +504,12 @@ Error AidlComposer::setActiveConfig(Display display, Config config) {
 Error AidlComposer::setClientTarget(Display display, uint32_t slot, const sp<GraphicBuffer>& target,
                                     int acquireFence, Dataspace dataspace,
                                     const std::vector<IComposerClient::Rect>& damage) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-
     const native_handle_t* handle = nullptr;
     if (target.get()) {
         handle = target->getNativeBuffer()->handle;
     }
 
-    mWriter.setClientTarget(slot, handle, acquireFence,
+    mWriter.setClientTarget(translate<int64_t>(display), slot, handle, acquireFence,
                             translate<aidl::android::hardware::graphics::common::Dataspace>(
                                     dataspace),
                             translate<AidlRect>(damage));
@@ -515,15 +529,14 @@ Error AidlComposer::setColorMode(Display display, ColorMode mode, RenderIntent r
 }
 
 Error AidlComposer::setColorTransform(Display display, const float* matrix, ColorTransform hint) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.setColorTransform(matrix, translate<AidlColorTransform>(hint));
+    mWriter.setColorTransform(translate<int64_t>(display), matrix,
+                              translate<AidlColorTransform>(hint));
     return Error::NONE;
 }
 
 Error AidlComposer::setOutputBuffer(Display display, const native_handle_t* buffer,
                                     int releaseFence) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.setOutputBuffer(0, buffer, dup(releaseFence));
+    mWriter.setOutputBuffer(translate<int64_t>(display), 0, buffer, dup(releaseFence));
     return Error::NONE;
 }
 
@@ -562,15 +575,14 @@ Error AidlComposer::setClientTargetSlotCount(Display display) {
 Error AidlComposer::validateDisplay(Display display, uint32_t* outNumTypes,
                                     uint32_t* outNumRequests) {
     ATRACE_NAME("HwcValidateDisplay");
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.validateDisplay();
+    mWriter.validateDisplay(translate<int64_t>(display));
 
     Error error = execute();
     if (error != Error::NONE) {
         return error;
     }
 
-    mReader.hasChanges(display, outNumTypes, outNumRequests);
+    mReader.hasChanges(translate<int64_t>(display), outNumTypes, outNumRequests);
 
     return Error::NONE;
 }
@@ -579,207 +591,154 @@ Error AidlComposer::presentOrValidateDisplay(Display display, uint32_t* outNumTy
                                              uint32_t* outNumRequests, int* outPresentFence,
                                              uint32_t* state) {
     ATRACE_NAME("HwcPresentOrValidateDisplay");
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.presentOrvalidateDisplay();
+    mWriter.presentOrvalidateDisplay(translate<int64_t>(display));
 
     Error error = execute();
     if (error != Error::NONE) {
         return error;
     }
 
-    mReader.takePresentOrValidateStage(display, state);
+    mReader.takePresentOrValidateStage(translate<int64_t>(display), state);
 
     if (*state == 1) { // Present succeeded
-        mReader.takePresentFence(display, outPresentFence);
+        mReader.takePresentFence(translate<int64_t>(display), outPresentFence);
     }
 
     if (*state == 0) { // Validate succeeded.
-        mReader.hasChanges(display, outNumTypes, outNumRequests);
+        mReader.hasChanges(translate<int64_t>(display), outNumTypes, outNumRequests);
     }
 
     return Error::NONE;
 }
 
 Error AidlComposer::setCursorPosition(Display display, Layer layer, int32_t x, int32_t y) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerCursorPosition(x, y);
+    mWriter.setLayerCursorPosition(translate<int64_t>(display), translate<int64_t>(layer), x, y);
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerBuffer(Display display, Layer layer, uint32_t slot,
                                    const sp<GraphicBuffer>& buffer, int acquireFence) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-
     const native_handle_t* handle = nullptr;
     if (buffer.get()) {
         handle = buffer->getNativeBuffer()->handle;
     }
 
-    mWriter.setLayerBuffer(slot, handle, acquireFence);
+    mWriter.setLayerBuffer(translate<int64_t>(display), translate<int64_t>(layer), slot, handle,
+                           acquireFence);
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerSurfaceDamage(Display display, Layer layer,
                                           const std::vector<IComposerClient::Rect>& damage) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerSurfaceDamage(translate<AidlRect>(damage));
+    mWriter.setLayerSurfaceDamage(translate<int64_t>(display), translate<int64_t>(layer),
+                                  translate<AidlRect>(damage));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerBlendMode(Display display, Layer layer,
                                       IComposerClient::BlendMode mode) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerBlendMode(translate<BlendMode>(mode));
+    mWriter.setLayerBlendMode(translate<int64_t>(display), translate<int64_t>(layer),
+                              translate<BlendMode>(mode));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerColor(Display display, Layer layer,
                                   const IComposerClient::Color& color) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerColor(translate<Color>(color));
+    mWriter.setLayerColor(translate<int64_t>(display), translate<int64_t>(layer),
+                          translate<Color>(color));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerCompositionType(Display display, Layer layer,
                                             IComposerClient::Composition type) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerCompositionType(translate<Composition>(type));
+    mWriter.setLayerCompositionType(translate<int64_t>(display), translate<int64_t>(layer),
+                                    translate<Composition>(type));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerDataspace(Display display, Layer layer, Dataspace dataspace) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerDataspace(translate<AidlDataspace>(dataspace));
+    mWriter.setLayerDataspace(translate<int64_t>(display), translate<int64_t>(layer),
+                              translate<AidlDataspace>(dataspace));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerDisplayFrame(Display display, Layer layer,
                                          const IComposerClient::Rect& frame) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerDisplayFrame(translate<AidlRect>(frame));
+    mWriter.setLayerDisplayFrame(translate<int64_t>(display), translate<int64_t>(layer),
+                                 translate<AidlRect>(frame));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerPlaneAlpha(Display display, Layer layer, float alpha) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerPlaneAlpha(alpha);
+    mWriter.setLayerPlaneAlpha(translate<int64_t>(display), translate<int64_t>(layer), alpha);
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerSidebandStream(Display display, Layer layer,
                                            const native_handle_t* stream) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerSidebandStream(stream);
+    mWriter.setLayerSidebandStream(translate<int64_t>(display), translate<int64_t>(layer), stream);
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerSourceCrop(Display display, Layer layer,
                                        const IComposerClient::FRect& crop) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerSourceCrop(translate<AidlFRect>(crop));
+    mWriter.setLayerSourceCrop(translate<int64_t>(display), translate<int64_t>(layer),
+                               translate<AidlFRect>(crop));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerTransform(Display display, Layer layer, Transform transform) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerTransform(translate<AidlTransform>(transform));
+    mWriter.setLayerTransform(translate<int64_t>(display), translate<int64_t>(layer),
+                              translate<AidlTransform>(transform));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerVisibleRegion(Display display, Layer layer,
                                           const std::vector<IComposerClient::Rect>& visible) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerVisibleRegion(translate<AidlRect>(visible));
+    mWriter.setLayerVisibleRegion(translate<int64_t>(display), translate<int64_t>(layer),
+                                  translate<AidlRect>(visible));
     return Error::NONE;
 }
 
 Error AidlComposer::setLayerZOrder(Display display, Layer layer, uint32_t z) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerZOrder(z);
+    mWriter.setLayerZOrder(translate<int64_t>(display), translate<int64_t>(layer), z);
     return Error::NONE;
 }
 
 Error AidlComposer::execute() {
-    // prepare input command queue
-    bool queueChanged = false;
-    int32_t commandLength = 0;
-    std::vector<aidl::android::hardware::common::NativeHandle> commandHandles;
-    if (!mWriter.writeQueue(&queueChanged, &commandLength, &commandHandles)) {
-        mWriter.reset();
-        return Error::NO_RESOURCES;
-    }
-
-    // set up new input command queue if necessary
-    if (queueChanged) {
-        const auto status = mAidlComposerClient->setInputCommandQueue(mWriter.getMQDescriptor());
-        if (!status.isOk()) {
-            ALOGE("setInputCommandQueue failed %s", status.getDescription().c_str());
-            mWriter.reset();
-            return static_cast<Error>(status.getServiceSpecificError());
-        }
-    }
-
-    if (commandLength == 0) {
+    const auto& commands = mWriter.getPendingCommands();
+    if (commands.empty()) {
         mWriter.reset();
         return Error::NONE;
     }
 
-    AidlExecuteCommandsStatus commandStatus;
-    auto status =
-            mAidlComposerClient->executeCommands(commandLength, commandHandles, &commandStatus);
-    // executeCommands can fail because of out-of-fd and we do not want to
-    // abort() in that case
+    std::vector<CommandResultPayload> results;
+    auto status = mAidlComposerClient->executeCommands(commands, &results);
     if (!status.isOk()) {
         ALOGE("executeCommands failed %s", status.getDescription().c_str());
         return static_cast<Error>(status.getServiceSpecificError());
     }
 
-    // set up new output command queue if necessary
-    if (commandStatus.queueChanged) {
-        ::aidl::android::hardware::common::fmq::MQDescriptor<
-                int32_t, ::aidl::android::hardware::common::fmq::SynchronizedReadWrite>
-                outputCommandQueue;
-        status = mAidlComposerClient->getOutputCommandQueue(&outputCommandQueue);
-        if (!status.isOk()) {
-            ALOGE("getOutputCommandQueue failed %s", status.getDescription().c_str());
-            return static_cast<Error>(status.getServiceSpecificError());
+    mReader.parse(results);
+    const auto commandErrors = mReader.takeErrors();
+    Error error = Error::NONE;
+    for (const auto& cmdErr : commandErrors) {
+        const auto index = static_cast<size_t>(cmdErr.commandIndex);
+        if (index < 0 || index >= commands.size()) {
+            ALOGE("invalid command index %zu", index);
+            return Error::BAD_PARAMETER;
         }
 
-        mReader.setMQDescriptor(outputCommandQueue);
-    }
-
-    Error error;
-    if (mReader.readQueue(commandStatus.length, std::move(commandStatus.handles))) {
-        error = static_cast<Error>(mReader.parse());
-        mReader.reset();
-    } else {
-        error = Error::NO_RESOURCES;
-    }
-
-    if (error == Error::NONE) {
-        std::vector<AidlCommandReader::CommandError> commandErrors = mReader.takeErrors();
-
-        for (const auto& cmdErr : commandErrors) {
-            auto command = mWriter.getCommand(cmdErr.location);
-            if (command == Command::VALIDATE_DISPLAY || command == Command::PRESENT_DISPLAY ||
-                command == Command::PRESENT_OR_VALIDATE_DISPLAY) {
-                error = cmdErr.error;
+        const auto& command = commands[index];
+        if (command.getTag() == command::CommandPayload::Tag::displayCommand) {
+            const auto& displayCommand =
+                    command.get<command::CommandPayload::Tag::displayCommand>();
+            if (displayCommand.validateDisplay || displayCommand.presentDisplay ||
+                displayCommand.presentOrValidateDisplay) {
+                error = translate<Error>(cmdErr.errorCode);
             } else {
-                ALOGW("command 0x%x generated error %d", command, cmdErr.error);
+                ALOGW("command '%s' generated error %" PRId32, command.toString().c_str(),
+                      cmdErr.errorCode);
             }
         }
     }
@@ -792,9 +751,8 @@ Error AidlComposer::execute() {
 Error AidlComposer::setLayerPerFrameMetadata(
         Display display, Layer layer,
         const std::vector<IComposerClient::PerFrameMetadata>& perFrameMetadatas) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerPerFrameMetadata(translate<AidlPerFrameMetadata>(perFrameMetadatas));
+    mWriter.setLayerPerFrameMetadata(translate<int64_t>(display), translate<int64_t>(layer),
+                                     translate<AidlPerFrameMetadata>(perFrameMetadatas));
     return Error::NONE;
 }
 
@@ -855,9 +813,7 @@ Error AidlComposer::getDisplayIdentificationData(Display display, uint8_t* outPo
 }
 
 Error AidlComposer::setLayerColorTransform(Display display, Layer layer, const float* matrix) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerColorTransform(matrix);
+    mWriter.setLayerColorTransform(translate<int64_t>(display), translate<int64_t>(layer), matrix);
     return Error::NONE;
 }
 
@@ -921,9 +877,8 @@ Error AidlComposer::getDisplayedContentSample(Display display, uint64_t maxFrame
 Error AidlComposer::setLayerPerFrameMetadataBlobs(
         Display display, Layer layer,
         const std::vector<IComposerClient::PerFrameMetadataBlob>& metadata) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerPerFrameMetadataBlobs(translate<AidlPerFrameMetadataBlob>(metadata));
+    mWriter.setLayerPerFrameMetadataBlobs(translate<int64_t>(display), translate<int64_t>(layer),
+                                          translate<AidlPerFrameMetadataBlob>(metadata));
     return Error::NONE;
 }
 
@@ -1032,9 +987,8 @@ V2_4::Error AidlComposer::setContentType(Display display,
 V2_4::Error AidlComposer::setLayerGenericMetadata(Display display, Layer layer,
                                                   const std::string& key, bool mandatory,
                                                   const std::vector<uint8_t>& value) {
-    mWriter.selectDisplay(translate<int64_t>(display));
-    mWriter.selectLayer(translate<int64_t>(layer));
-    mWriter.setLayerGenericMetadata(key, mandatory, value);
+    mWriter.setLayerGenericMetadata(translate<int64_t>(display), translate<int64_t>(layer), key,
+                                    mandatory, value);
     return V2_4::Error::NONE;
 }
 
@@ -1052,320 +1006,11 @@ V2_4::Error AidlComposer::getLayerGenericMetadataKeys(
 
 Error AidlComposer::getClientTargetProperty(
         Display display, IComposerClient::ClientTargetProperty* outClientTargetProperty) {
-    mReader.takeClientTargetProperty(display, outClientTargetProperty);
+    ClientTargetProperty property;
+    mReader.takeClientTargetProperty(translate<int64_t>(display), &property);
+    *outClientTargetProperty = translate<IComposerClient::ClientTargetProperty>(property);
     return Error::NONE;
 }
 
-AidlCommandReader::~AidlCommandReader() {
-    resetData();
-}
-
-int AidlCommandReader::parse() {
-    resetData();
-
-    Command command;
-    uint16_t length = 0;
-
-    while (!isEmpty()) {
-        if (!beginCommand(&command, &length)) {
-            break;
-        }
-
-        bool parsed = false;
-        switch (command) {
-            case Command::SELECT_DISPLAY:
-                parsed = parseSelectDisplay(length);
-                break;
-            case Command::SET_ERROR:
-                parsed = parseSetError(length);
-                break;
-            case Command::SET_CHANGED_COMPOSITION_TYPES:
-                parsed = parseSetChangedCompositionTypes(length);
-                break;
-            case Command::SET_DISPLAY_REQUESTS:
-                parsed = parseSetDisplayRequests(length);
-                break;
-            case Command::SET_PRESENT_FENCE:
-                parsed = parseSetPresentFence(length);
-                break;
-            case Command::SET_RELEASE_FENCES:
-                parsed = parseSetReleaseFences(length);
-                break;
-            case Command ::SET_PRESENT_OR_VALIDATE_DISPLAY_RESULT:
-                parsed = parseSetPresentOrValidateDisplayResult(length);
-                break;
-            case Command::SET_CLIENT_TARGET_PROPERTY:
-                parsed = parseSetClientTargetProperty(length);
-                break;
-            default:
-                parsed = false;
-                break;
-        }
-
-        endCommand();
-
-        if (!parsed) {
-            ALOGE("failed to parse command 0x%x length %" PRIu16, command, length);
-            break;
-        }
-    }
-
-    return isEmpty() ? 0 : AidlIComposerClient::EX_NO_RESOURCES;
-}
-
-bool AidlCommandReader::parseSelectDisplay(uint16_t length) {
-    if (length != CommandWriterBase::kSelectDisplayLength) {
-        return false;
-    }
-
-    mCurrentReturnData = &mReturnData[read64()];
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetError(uint16_t length) {
-    if (length != CommandWriterBase::kSetErrorLength) {
-        return false;
-    }
-
-    auto location = read();
-    auto error = static_cast<Error>(readSigned());
-
-    mErrors.emplace_back(CommandError{location, error});
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetChangedCompositionTypes(uint16_t length) {
-    // (layer id [64bit], composition type [32bit]) pairs
-    static constexpr int kCommandWords = 3;
-
-    if (length % kCommandWords != 0 || !mCurrentReturnData) {
-        return false;
-    }
-
-    uint32_t count = length / kCommandWords;
-    mCurrentReturnData->changedLayers.reserve(count);
-    mCurrentReturnData->compositionTypes.reserve(count);
-    while (count > 0) {
-        auto layer = read64();
-        auto type = static_cast<IComposerClient::Composition>(readSigned());
-
-        mCurrentReturnData->changedLayers.push_back(layer);
-        mCurrentReturnData->compositionTypes.push_back(type);
-
-        count--;
-    }
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetDisplayRequests(uint16_t length) {
-    // display requests [32 bit] followed by
-    // (layer id [64bit], layer requests [32bit]) pairs
-    static constexpr int kDisplayRequestsWords = 1;
-    static constexpr int kCommandWords = 3;
-    if (length % kCommandWords != kDisplayRequestsWords || !mCurrentReturnData) {
-        return false;
-    }
-
-    mCurrentReturnData->displayRequests = read();
-
-    uint32_t count = (length - kDisplayRequestsWords) / kCommandWords;
-    mCurrentReturnData->requestedLayers.reserve(count);
-    mCurrentReturnData->requestMasks.reserve(count);
-    while (count > 0) {
-        auto layer = read64();
-        auto layerRequestMask = read();
-
-        mCurrentReturnData->requestedLayers.push_back(layer);
-        mCurrentReturnData->requestMasks.push_back(layerRequestMask);
-
-        count--;
-    }
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetPresentFence(uint16_t length) {
-    if (length != CommandWriterBase::kSetPresentFenceLength || !mCurrentReturnData) {
-        return false;
-    }
-
-    if (mCurrentReturnData->presentFence >= 0) {
-        close(mCurrentReturnData->presentFence);
-    }
-    mCurrentReturnData->presentFence = readFence();
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetReleaseFences(uint16_t length) {
-    // (layer id [64bit], release fence index [32bit]) pairs
-    static constexpr int kCommandWords = 3;
-
-    if (length % kCommandWords != 0 || !mCurrentReturnData) {
-        return false;
-    }
-
-    uint32_t count = length / kCommandWords;
-    mCurrentReturnData->releasedLayers.reserve(count);
-    mCurrentReturnData->releaseFences.reserve(count);
-    while (count > 0) {
-        auto layer = read64();
-        auto fence = readFence();
-
-        mCurrentReturnData->releasedLayers.push_back(layer);
-        mCurrentReturnData->releaseFences.push_back(fence);
-
-        count--;
-    }
-
-    return true;
-}
-
-bool AidlCommandReader::parseSetPresentOrValidateDisplayResult(uint16_t length) {
-    if (length != CommandWriterBase::kPresentOrValidateDisplayResultLength || !mCurrentReturnData) {
-        return false;
-    }
-    mCurrentReturnData->presentOrValidateState = read();
-    return true;
-}
-
-bool AidlCommandReader::parseSetClientTargetProperty(uint16_t length) {
-    if (length != CommandWriterBase::kSetClientTargetPropertyLength || !mCurrentReturnData) {
-        return false;
-    }
-    mCurrentReturnData->clientTargetProperty.pixelFormat = static_cast<PixelFormat>(readSigned());
-    mCurrentReturnData->clientTargetProperty.dataspace = static_cast<Dataspace>(readSigned());
-    return true;
-}
-
-void AidlCommandReader::resetData() {
-    mErrors.clear();
-
-    for (auto& data : mReturnData) {
-        if (data.second.presentFence >= 0) {
-            close(data.second.presentFence);
-        }
-        for (auto fence : data.second.releaseFences) {
-            if (fence >= 0) {
-                close(fence);
-            }
-        }
-    }
-
-    mReturnData.clear();
-    mCurrentReturnData = nullptr;
-}
-
-std::vector<AidlCommandReader::CommandError> AidlCommandReader::takeErrors() {
-    return std::move(mErrors);
-}
-
-bool AidlCommandReader::hasChanges(Display display, uint32_t* outNumChangedCompositionTypes,
-                                   uint32_t* outNumLayerRequestMasks) const {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        *outNumChangedCompositionTypes = 0;
-        *outNumLayerRequestMasks = 0;
-        return false;
-    }
-
-    const ReturnData& data = found->second;
-
-    *outNumChangedCompositionTypes = static_cast<uint32_t>(data.compositionTypes.size());
-    *outNumLayerRequestMasks = static_cast<uint32_t>(data.requestMasks.size());
-
-    return !(data.compositionTypes.empty() && data.requestMasks.empty());
-}
-
-void AidlCommandReader::takeChangedCompositionTypes(
-        Display display, std::vector<Layer>* outLayers,
-        std::vector<IComposerClient::Composition>* outTypes) {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        outLayers->clear();
-        outTypes->clear();
-        return;
-    }
-
-    ReturnData& data = found->second;
-
-    *outLayers = std::move(data.changedLayers);
-    *outTypes = std::move(data.compositionTypes);
-}
-
-void AidlCommandReader::takeDisplayRequests(Display display, uint32_t* outDisplayRequestMask,
-                                            std::vector<Layer>* outLayers,
-                                            std::vector<uint32_t>* outLayerRequestMasks) {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        *outDisplayRequestMask = 0;
-        outLayers->clear();
-        outLayerRequestMasks->clear();
-        return;
-    }
-
-    ReturnData& data = found->second;
-
-    *outDisplayRequestMask = data.displayRequests;
-    *outLayers = std::move(data.requestedLayers);
-    *outLayerRequestMasks = std::move(data.requestMasks);
-}
-
-void AidlCommandReader::takeReleaseFences(Display display, std::vector<Layer>* outLayers,
-                                          std::vector<int>* outReleaseFences) {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        outLayers->clear();
-        outReleaseFences->clear();
-        return;
-    }
-
-    ReturnData& data = found->second;
-
-    *outLayers = std::move(data.releasedLayers);
-    *outReleaseFences = std::move(data.releaseFences);
-}
-
-void AidlCommandReader::takePresentFence(Display display, int* outPresentFence) {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        *outPresentFence = -1;
-        return;
-    }
-
-    ReturnData& data = found->second;
-
-    *outPresentFence = data.presentFence;
-    data.presentFence = -1;
-}
-
-void AidlCommandReader::takePresentOrValidateStage(Display display, uint32_t* state) {
-    auto found = mReturnData.find(display);
-    if (found == mReturnData.end()) {
-        *state = static_cast<uint32_t>(-1);
-        return;
-    }
-    ReturnData& data = found->second;
-    *state = data.presentOrValidateState;
-}
-
-void AidlCommandReader::takeClientTargetProperty(
-        Display display, IComposerClient::ClientTargetProperty* outClientTargetProperty) {
-    auto found = mReturnData.find(display);
-
-    // If not found, return the default values.
-    if (found == mReturnData.end()) {
-        outClientTargetProperty->pixelFormat = PixelFormat::RGBA_8888;
-        outClientTargetProperty->dataspace = Dataspace::UNKNOWN;
-        return;
-    }
-
-    ReturnData& data = found->second;
-    *outClientTargetProperty = data.clientTargetProperty;
-}
-
 } // namespace Hwc2
 } // namespace android
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
index a6d95000d2..418a8cc18b 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
@@ -43,95 +43,10 @@ namespace android::Hwc2 {
 
 using AidlCommandWriterBase = aidl::android::hardware::graphics::composer3::CommandWriterBase;
 using AidlCommandReaderBase = aidl::android::hardware::graphics::composer3::CommandReaderBase;
+using aidl::android::hardware::graphics::composer3::command::CommandResultPayload;
 
 class AidlIComposerCallbackWrapper;
 
-class AidlCommandReader : public AidlCommandReaderBase {
-public:
-    ~AidlCommandReader();
-
-    // Parse and execute commands from the command queue.  The commands are
-    // actually return values from the server and will be saved in ReturnData.
-    int parse();
-
-    // Get and clear saved errors.
-    struct CommandError {
-        uint32_t location;
-        Error error;
-    };
-    std::vector<CommandError> takeErrors();
-
-    bool hasChanges(Display display, uint32_t* outNumChangedCompositionTypes,
-                    uint32_t* outNumLayerRequestMasks) const;
-
-    // Get and clear saved changed composition types.
-    void takeChangedCompositionTypes(Display display, std::vector<Layer>* outLayers,
-                                     std::vector<IComposerClient::Composition>* outTypes);
-
-    // Get and clear saved display requests.
-    void takeDisplayRequests(Display display, uint32_t* outDisplayRequestMask,
-                             std::vector<Layer>* outLayers,
-                             std::vector<uint32_t>* outLayerRequestMasks);
-
-    // Get and clear saved release fences.
-    void takeReleaseFences(Display display, std::vector<Layer>* outLayers,
-                           std::vector<int>* outReleaseFences);
-
-    // Get and clear saved present fence.
-    void takePresentFence(Display display, int* outPresentFence);
-
-    // Get what stage succeeded during PresentOrValidate: Present or Validate
-    void takePresentOrValidateStage(Display display, uint32_t* state);
-
-    // Get the client target properties requested by hardware composer.
-    void takeClientTargetProperty(Display display,
-                                  IComposerClient::ClientTargetProperty* outClientTargetProperty);
-
-private:
-    void resetData();
-
-    bool parseSelectDisplay(uint16_t length);
-    bool parseSetError(uint16_t length);
-    bool parseSetChangedCompositionTypes(uint16_t length);
-    bool parseSetDisplayRequests(uint16_t length);
-    bool parseSetPresentFence(uint16_t length);
-    bool parseSetReleaseFences(uint16_t length);
-    bool parseSetPresentOrValidateDisplayResult(uint16_t length);
-    bool parseSetClientTargetProperty(uint16_t length);
-
-    struct ReturnData {
-        uint32_t displayRequests = 0;
-
-        std::vector<Layer> changedLayers;
-        std::vector<IComposerClient::Composition> compositionTypes;
-
-        std::vector<Layer> requestedLayers;
-        std::vector<uint32_t> requestMasks;
-
-        int presentFence = -1;
-
-        std::vector<Layer> releasedLayers;
-        std::vector<int> releaseFences;
-
-        uint32_t presentOrValidateState;
-
-        // Composer 2.4 implementation can return a client target property
-        // structure to indicate the client target properties that hardware
-        // composer requests. The composer client must change the client target
-        // properties to match this request.
-        IComposerClient::ClientTargetProperty clientTargetProperty{PixelFormat::RGBA_8888,
-                                                                   Dataspace::UNKNOWN};
-    };
-
-    std::vector<CommandError> mErrors;
-    std::unordered_map<Display, ReturnData> mReturnData;
-
-    // When SELECT_DISPLAY is parsed, this is updated to point to the
-    // display's return data in mReturnData.  We use it to avoid repeated
-    // map lookups.
-    ReturnData* mCurrentReturnData;
-};
-
 // Composer is a wrapper to IComposer, a proxy to server-side composer.
 class AidlComposer final : public Hwc2::Composer {
 public:
@@ -284,13 +199,6 @@ public:
             IComposerClient::ClientTargetProperty* outClientTargetProperty) override;
 
 private:
-    class AidlCommandWriter : public AidlCommandWriterBase {
-    public:
-        explicit AidlCommandWriter(uint32_t initialMaxSize)
-              : AidlCommandWriterBase(initialMaxSize) {}
-        ~AidlCommandWriter() override {}
-    };
-
     // 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.
@@ -306,8 +214,8 @@ private:
     // 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;
-    AidlCommandWriter mWriter;
-    AidlCommandReader mReader;
+    AidlCommandWriterBase mWriter;
+    AidlCommandReaderBase mReader;
 
     // Aidl interface
     using AidlIComposer = aidl::android::hardware::graphics::composer3::IComposer;
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 561d9e278d..55684187f8 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -119,6 +119,7 @@ cc_test {
     static_libs: [
         "android.hardware.common-V2-ndk",
         "android.hardware.common.fmq-V1-ndk",
+        "android.hardware.graphics.common-V3-ndk",
         "android.hardware.graphics.composer@2.1",
         "android.hardware.graphics.composer@2.2",
         "android.hardware.graphics.composer@2.3",