SF: Return EX_CONFIG_FAILED from setActiveConfigWithConstraints()

setActiveConfigWithConstraints() currently returns V2_4 Hal::Error
But with the new Composer3 IcomposerClient AIDL changes, it should
return int32_t corresponding the new EX_CONFIG_FAILED
exception. So define a new enum class Error to handle all V2_1  and 2_4
errors and the new IcomposerClient exception.

Also define a new class to return the result of initiateModeChange
as: Changed, Rejected, Aborted.

So change this plumbing through SF. Catch this error and propagate
up from DisplayModeController to SF using Scheduler

Bug: 374185540
Flag: EXEMPT refactor
Test: manual

Change-Id: I3548fd6bda806055fb071cc364d07d37b2693374
Signed-off-by: Manasi Navare <navaremanasi@google.com>
diff --git a/services/surfaceflinger/Display/DisplayModeController.cpp b/services/surfaceflinger/Display/DisplayModeController.cpp
index f8b6c6e..a086aee 100644
--- a/services/surfaceflinger/Display/DisplayModeController.cpp
+++ b/services/surfaceflinger/Display/DisplayModeController.cpp
@@ -28,6 +28,7 @@
 #include <ftl/concat.h>
 #include <ftl/expected.h>
 #include <log/log.h>
+#include <utils/Errors.h>
 
 namespace android::display {
 
@@ -177,12 +178,13 @@
     }
 }
 
-bool DisplayModeController::initiateModeChange(PhysicalDisplayId displayId,
-                                               DisplayModeRequest&& desiredMode,
-                                               const hal::VsyncPeriodChangeConstraints& constraints,
-                                               hal::VsyncPeriodChangeTimeline& outTimeline) {
+auto DisplayModeController::initiateModeChange(
+        PhysicalDisplayId displayId, DisplayModeRequest&& desiredMode,
+        const hal::VsyncPeriodChangeConstraints& constraints,
+        hal::VsyncPeriodChangeTimeline& outTimeline) -> ModeChangeResult {
     std::lock_guard lock(mDisplayLock);
-    const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get();
+    const auto& displayPtr =
+            FTL_EXPECT(mDisplays.get(displayId).ok_or(ModeChangeResult::Aborted)).get();
 
     // TODO: b/255635711 - Flow the DisplayModeRequest through the desired/pending/active states.
     // For now, `desiredMode` and `desiredModeOpt` are one and the same, but the latter is not
@@ -201,13 +203,17 @@
 
     const auto& mode = *displayPtr->pendingModeOpt->mode.modePtr;
 
-    if (mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(), constraints,
-                                                   &outTimeline) != OK) {
-        return false;
+    const auto error = mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(),
+                                                                  constraints, &outTimeline);
+    switch (error) {
+        case FAILED_TRANSACTION:
+            return ModeChangeResult::Rejected;
+        case OK:
+            SFTRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());
+            return ModeChangeResult::Changed;
+        default:
+            return ModeChangeResult::Aborted;
     }
-
-    SFTRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());
-    return true;
 }
 
 void DisplayModeController::finalizeModeChange(PhysicalDisplayId displayId, DisplayModeId modeId,
diff --git a/services/surfaceflinger/Display/DisplayModeController.h b/services/surfaceflinger/Display/DisplayModeController.h
index 9ec603d..af3e909 100644
--- a/services/surfaceflinger/Display/DisplayModeController.h
+++ b/services/surfaceflinger/Display/DisplayModeController.h
@@ -70,6 +70,7 @@
     RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) const EXCLUDES(mDisplayLock);
 
     enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch };
+    enum class ModeChangeResult { Changed, Rejected, Aborted };
 
     DesiredModeAction setDesiredMode(PhysicalDisplayId, DisplayModeRequest&&)
             EXCLUDES(mDisplayLock);
@@ -86,9 +87,9 @@
 
     scheduler::FrameRateMode getActiveMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock);
 
-    bool initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&,
-                            const hal::VsyncPeriodChangeConstraints&,
-                            hal::VsyncPeriodChangeTimeline& outTimeline)
+    ModeChangeResult initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&,
+                                        const hal::VsyncPeriodChangeConstraints&,
+                                        hal::VsyncPeriodChangeTimeline& outTimeline)
             REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
 
     void finalizeModeChange(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps)
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
index 4c8ff58..b83f2ab 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp
@@ -1384,7 +1384,7 @@
     return V2_4::Error::NONE;
 }
 
-V2_4::Error AidlComposer::setActiveConfigWithConstraints(
+Error AidlComposer::setActiveConfigWithConstraints(
         Display display, Config config,
         const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
         VsyncPeriodChangeTimeline* outTimeline) {
@@ -1398,10 +1398,10 @@
                                                      &timeline);
     if (!status.isOk()) {
         ALOGE("setActiveConfigWithConstraints failed %s", status.getDescription().c_str());
-        return static_cast<V2_4::Error>(status.getServiceSpecificError());
+        return static_cast<Error>(status.getServiceSpecificError());
     }
     *outTimeline = translate<VsyncPeriodChangeTimeline>(timeline);
-    return V2_4::Error::NONE;
+    return Error::NONE;
 }
 
 V2_4::Error AidlComposer::setAutoLowLatencyMode(Display display, bool on) {
diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
index 933e8d1..db63d3e 100644
--- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h
@@ -206,7 +206,7 @@
     V2_4::Error getDisplayConnectionType(Display display,
                                          IComposerClient::DisplayConnectionType* outType) override;
     V2_4::Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) override;
-    V2_4::Error setActiveConfigWithConstraints(
+    Error setActiveConfigWithConstraints(
             Display display, Config config,
             const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
             VsyncPeriodChangeTimeline* outTimeline) override;
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h
index c1333c2..ff292fa 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h
@@ -32,6 +32,8 @@
 #include <ui/PictureProfileHandle.h>
 #include <utils/StrongPointer.h>
 
+#include "DisplayHardware/Hal.h"
+
 #include <aidl/android/hardware/graphics/common/DisplayDecorationSupport.h>
 #include <aidl/android/hardware/graphics/common/HdrConversionCapability.h>
 #include <aidl/android/hardware/graphics/common/HdrConversionStrategy.h>
@@ -73,9 +75,9 @@
 using types::V1_2::Dataspace;
 using types::V1_2::PixelFormat;
 
+using hardware::graphics::composer::hal::Error;
 using V2_1::Config;
 using V2_1::Display;
-using V2_1::Error;
 using V2_1::Layer;
 using V2_4::CommandReaderBase;
 using V2_4::CommandWriterBase;
@@ -261,7 +263,7 @@
             Display display, IComposerClient::DisplayConnectionType* outType) = 0;
     virtual V2_4::Error getDisplayVsyncPeriod(Display display,
                                               VsyncPeriodNanos* outVsyncPeriod) = 0;
-    virtual V2_4::Error setActiveConfigWithConstraints(
+    virtual Error setActiveConfigWithConstraints(
             Display display, Config config,
             const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
             VsyncPeriodChangeTimeline* outTimeline) = 0;
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index a274995..081f4aa 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -33,6 +33,8 @@
 #include <ui/GraphicBuffer.h>
 #include <ui/PictureProfileHandle.h>
 
+#include "DisplayHardware/Hal.h"
+
 #include <algorithm>
 #include <cinttypes>
 #include <iterator>
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 61d4541..9943856 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -27,6 +27,7 @@
 
 #include "HWComposer.h"
 
+#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
 #include <android-base/properties.h>
 #include <common/trace.h>
 #include <compositionengine/Output.h>
@@ -733,7 +734,11 @@
     auto error = mDisplayData[displayId].hwcDisplay->setActiveConfigWithConstraints(hwcModeId,
                                                                                     constraints,
                                                                                     outTimeline);
-    RETURN_IF_HWC_ERROR(error, displayId, UNKNOWN_ERROR);
+    if (error == hal::Error::CONFIG_FAILED) {
+        RETURN_IF_HWC_ERROR_FOR("setActiveConfigWithConstraints", error, displayId,
+                                FAILED_TRANSACTION);
+    }
+    RETURN_IF_HWC_ERROR_FOR("setActiveConfigWithConstraints", error, displayId, UNKNOWN_ERROR);
     return NO_ERROR;
 }
 
diff --git a/services/surfaceflinger/DisplayHardware/Hal.h b/services/surfaceflinger/DisplayHardware/Hal.h
index e3d9622..568d758 100644
--- a/services/surfaceflinger/DisplayHardware/Hal.h
+++ b/services/surfaceflinger/DisplayHardware/Hal.h
@@ -17,16 +17,21 @@
 #pragma once
 
 #include <android/hardware/graphics/common/1.1/types.h>
+#include <android/hardware/graphics/composer/2.1/types.h>
 #include <android/hardware/graphics/composer/2.4/IComposer.h>
 #include <android/hardware/graphics/composer/2.4/IComposerClient.h>
+#include <android/hardware/graphics/composer/2.4/types.h>
 
 #include <aidl/android/hardware/graphics/common/DisplayHotplugEvent.h>
 #include <aidl/android/hardware/graphics/common/Hdr.h>
 #include <aidl/android/hardware/graphics/composer3/Composition.h>
 #include <aidl/android/hardware/graphics/composer3/DisplayCapability.h>
 #include <aidl/android/hardware/graphics/composer3/DisplayConfiguration.h>
+#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
 #include <aidl/android/hardware/graphics/composer3/VrrConfig.h>
 
+#include <ftl/enum.h>
+
 #define ERROR_HAS_CHANGES 5
 
 namespace android {
@@ -46,7 +51,6 @@
 using types::V1_2::Dataspace;
 using types::V1_2::PixelFormat;
 
-using V2_1::Error;
 using V2_4::IComposer;
 using V2_4::IComposerCallback;
 using V2_4::IComposerClient;
@@ -78,6 +82,22 @@
 using DisplayConfiguration = V3_0::DisplayConfiguration;
 using VrrConfig = V3_0::VrrConfig;
 
+enum class Error : int32_t {
+    NONE = static_cast<int32_t>(V2_1::Error::NONE),
+    BAD_CONFIG = static_cast<int32_t>(V2_1::Error::BAD_CONFIG),
+    BAD_DISPLAY = static_cast<int32_t>(V2_1::Error::BAD_DISPLAY),
+    BAD_LAYER = static_cast<int32_t>(V2_1::Error::BAD_LAYER),
+    BAD_PARAMETER = static_cast<int32_t>(V2_1::Error::BAD_PARAMETER),
+    NO_RESOURCES = static_cast<int32_t>(V2_1::Error::NO_RESOURCES),
+    NOT_VALIDATED = static_cast<int32_t>(V2_1::Error::NOT_VALIDATED),
+    UNSUPPORTED = static_cast<int32_t>(V2_1::Error::UNSUPPORTED),
+    SEAMLESS_NOT_ALLOWED = static_cast<int32_t>(V2_4::Error::SEAMLESS_NOT_ALLOWED),
+    SEAMLESS_NOT_POSSIBLE = static_cast<int32_t>(V2_4::Error::SEAMLESS_NOT_POSSIBLE),
+    CONFIG_FAILED = V3_0::IComposerClient::EX_CONFIG_FAILED,
+    PICTURE_PROFILE_MAX_EXCEEDED = V3_0::IComposerClient::EX_PICTURE_PROFILE_MAX_EXCEEDED,
+    ftl_last = PICTURE_PROFILE_MAX_EXCEEDED
+};
+
 } // namespace hardware::graphics::composer::hal
 
 inline bool hasChangesError(hardware::graphics::composer::hal::Error error) {
@@ -210,7 +230,11 @@
 }
 
 inline std::string to_string(hardware::graphics::composer::hal::Error error) {
-    return to_string(static_cast<hardware::graphics::composer::hal::V2_4::Error>(error));
+    // 5 is reserved for historical reason, during validation 5 means has changes.
+    if (hasChangesError(error)) {
+        return "HAS_CHANGES";
+    }
+    return ftl::enum_string(error);
 }
 
 // For utils::Dumper ADL.
diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
index e359a26..5703a2d 100644
--- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp
@@ -27,6 +27,7 @@
 #include <SurfaceFlingerProperties.h>
 #include <aidl/android/hardware/graphics/common/DisplayHotplugEvent.h>
 #include <android/binder_manager.h>
+#include <android/hardware/graphics/composer/2.1/types.h>
 #include <common/trace.h>
 #include <composer-command-buffer/2.2/ComposerCommandBuffer.h>
 #include <hidl/HidlTransportSupport.h>
@@ -173,7 +174,7 @@
 };
 
 // assume NO_RESOURCES when Status::isOk returns false
-constexpr Error kDefaultError = Error::NO_RESOURCES;
+constexpr V2_1::Error kDefaultError = V2_1::Error::NO_RESOURCES;
 constexpr V2_4::Error kDefaultError_2_4 = static_cast<V2_4::Error>(kDefaultError);
 
 template <typename T, typename U>
@@ -181,7 +182,7 @@
     return (ret.isOk()) ? static_cast<T>(ret) : static_cast<T>(default_val);
 }
 
-Error unwrapRet(Return<Error>& ret) {
+V2_1::Error unwrapRet(Return<V2_1::Error>& ret) {
     return unwrapRet(ret, kDefaultError);
 }
 
@@ -235,7 +236,7 @@
         });
     } else if (sp<V2_3::IComposer> composer_2_3 = V2_3::IComposer::castFrom(mComposer)) {
         composer_2_3->createClient_2_3([&](const auto& tmpError, const auto& tmpClient) {
-            if (tmpError == Error::NONE) {
+            if (tmpError == V2_1::Error::NONE) {
                 mClient = tmpClient;
                 mClient_2_2 = tmpClient;
                 mClient_2_3 = tmpClient;
@@ -243,7 +244,7 @@
         });
     } else {
         mComposer->createClient([&](const auto& tmpError, const auto& tmpClient) {
-            if (tmpError != Error::NONE) {
+            if (tmpError != V2_1::Error::NONE) {
                 return;
             }
 
@@ -325,14 +326,14 @@
 Error HidlComposer::createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
                                          Display* outDisplay) {
     const uint32_t bufferSlotCount = 1;
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     if (mClient_2_2) {
         mClient_2_2->createVirtualDisplay_2_2(width, height,
                                               static_cast<types::V1_1::PixelFormat>(*format),
                                               bufferSlotCount,
                                               [&](const auto& tmpError, const auto& tmpDisplay,
                                                   const auto& tmpFormat) {
-                                                  error = tmpError;
+                                                  error = static_cast<Error>(tmpError);
                                                   if (error != Error::NONE) {
                                                       return;
                                                   }
@@ -346,7 +347,7 @@
                                       bufferSlotCount,
                                       [&](const auto& tmpError, const auto& tmpDisplay,
                                           const auto& tmpFormat) {
-                                          error = tmpError;
+                                          error = static_cast<Error>(tmpError);
                                           if (error != Error::NONE) {
                                               return;
                                           }
@@ -361,7 +362,7 @@
 
 Error HidlComposer::destroyVirtualDisplay(Display display) {
     auto ret = mClient->destroyVirtualDisplay(display);
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::acceptDisplayChanges(Display display) {
@@ -371,10 +372,10 @@
 }
 
 Error HidlComposer::createLayer(Display display, Layer* outLayer) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient->createLayer(display, kMaxLayerBufferCount,
                          [&](const auto& tmpError, const auto& tmpLayer) {
-                             error = tmpError;
+                             error = static_cast<Error>(tmpError);
                              if (error != Error::NONE) {
                                  return;
                              }
@@ -387,13 +388,13 @@
 
 Error HidlComposer::destroyLayer(Display display, Layer layer) {
     auto ret = mClient->destroyLayer(display, layer);
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::getActiveConfig(Display display, Config* outConfig) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient->getActiveConfig(display, [&](const auto& tmpError, const auto& tmpConfig) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
         if (error != Error::NONE) {
             return;
         }
@@ -412,11 +413,11 @@
 }
 
 Error HidlComposer::getColorModes(Display display, std::vector<ColorMode>* outModes) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
 
     if (mClient_2_3) {
         mClient_2_3->getColorModes_2_3(display, [&](const auto& tmpError, const auto& tmpModes) {
-            error = tmpError;
+            error = static_cast<Error>(tmpError);
             if (error != Error::NONE) {
                 return;
             }
@@ -425,7 +426,7 @@
         });
     } else if (mClient_2_2) {
         mClient_2_2->getColorModes_2_2(display, [&](const auto& tmpError, const auto& tmpModes) {
-            error = tmpError;
+            error = static_cast<Error>(tmpError);
             if (error != Error::NONE) {
                 return;
             }
@@ -436,7 +437,7 @@
         });
     } else {
         mClient->getColorModes(display, [&](const auto& tmpError, const auto& tmpModes) {
-            error = tmpError;
+            error = static_cast<Error>(tmpError);
             if (error != Error::NONE) {
                 return;
             }
@@ -451,7 +452,7 @@
 
 Error HidlComposer::getDisplayAttribute(Display display, Config config,
                                         IComposerClient::Attribute attribute, int32_t* outValue) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     if (mClient_2_4) {
         mClient_2_4->getDisplayAttribute_2_4(display, config, attribute,
                                              [&](const auto& tmpError, const auto& tmpValue) {
@@ -466,7 +467,7 @@
         mClient->getDisplayAttribute(display, config,
                                      static_cast<V2_1::IComposerClient::Attribute>(attribute),
                                      [&](const auto& tmpError, const auto& tmpValue) {
-                                         error = tmpError;
+                                         error = static_cast<Error>(tmpError);
                                          if (error != Error::NONE) {
                                              return;
                                          }
@@ -479,9 +480,9 @@
 }
 
 Error HidlComposer::getDisplayConfigs(Display display, std::vector<Config>* outConfigs) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient->getDisplayConfigs(display, [&](const auto& tmpError, const auto& tmpConfigs) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
         if (error != Error::NONE) {
             return;
         }
@@ -499,9 +500,9 @@
 }
 
 Error HidlComposer::getDisplayName(Display display, std::string* outName) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient->getDisplayName(display, [&](const auto& tmpError, const auto& tmpName) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
         if (error != Error::NONE) {
             return;
         }
@@ -520,9 +521,9 @@
 }
 
 Error HidlComposer::getDozeSupport(Display display, bool* outSupport) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient->getDozeSupport(display, [&](const auto& tmpError, const auto& tmpSupport) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
         if (error != Error::NONE) {
             return;
         }
@@ -541,14 +542,14 @@
 Error HidlComposer::getHdrCapabilities(Display display, std::vector<Hdr>* outHdrTypes,
                                        float* outMaxLuminance, float* outMaxAverageLuminance,
                                        float* outMinLuminance) {
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     if (mClient_2_3) {
         mClient_2_3->getHdrCapabilities_2_3(display,
                                             [&](const auto& tmpError, const auto& tmpHdrTypes,
                                                 const auto& tmpMaxLuminance,
                                                 const auto& tmpMaxAverageLuminance,
                                                 const auto& tmpMinLuminance) {
-                                                error = tmpError;
+                                                error = static_cast<Error>(tmpError);
                                                 if (error != Error::NONE) {
                                                     return;
                                                 }
@@ -564,7 +565,7 @@
                                         const auto& tmpMaxLuminance,
                                         const auto& tmpMaxAverageLuminance,
                                         const auto& tmpMinLuminance) {
-                                        error = tmpError;
+                                        error = static_cast<Error>(tmpError);
                                         if (error != Error::NONE) {
                                             return;
                                         }
@@ -606,7 +607,7 @@
 
 Error HidlComposer::setActiveConfig(Display display, Config config) {
     auto ret = mClient->setActiveConfig(display, config);
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::setClientTarget(Display display, uint32_t slot, const sp<GraphicBuffer>& target,
@@ -625,7 +626,7 @@
 }
 
 Error HidlComposer::setColorMode(Display display, ColorMode mode, RenderIntent renderIntent) {
-    hardware::Return<Error> ret(kDefaultError);
+    hardware::Return<V2_1::Error> ret(kDefaultError);
     if (mClient_2_3) {
         ret = mClient_2_3->setColorMode_2_3(display, mode, renderIntent);
     } else if (mClient_2_2) {
@@ -634,7 +635,7 @@
     } else {
         ret = mClient->setColorMode(display, static_cast<types::V1_0::ColorMode>(mode));
     }
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::setColorTransform(Display display, const float* matrix) {
@@ -654,25 +655,25 @@
 }
 
 Error HidlComposer::setPowerMode(Display display, IComposerClient::PowerMode mode) {
-    Return<Error> ret(Error::UNSUPPORTED);
+    Return<V2_1::Error> ret(V2_1::Error::UNSUPPORTED);
     if (mClient_2_2) {
         ret = mClient_2_2->setPowerMode_2_2(display, mode);
     } else if (mode != IComposerClient::PowerMode::ON_SUSPEND) {
         ret = mClient->setPowerMode(display, static_cast<V2_1::IComposerClient::PowerMode>(mode));
     }
 
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::setVsyncEnabled(Display display, IComposerClient::Vsync enabled) {
     auto ret = mClient->setVsyncEnabled(display, enabled);
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::setClientTargetSlotCount(Display display) {
     const uint32_t bufferSlotCount = BufferQueue::NUM_BUFFER_SLOTS;
     auto ret = mClient->setClientTargetSlotCount(display, bufferSlotCount);
-    return unwrapRet(ret);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::validateDisplay(Display display, nsecs_t /*expectedPresentTime*/,
@@ -903,7 +904,7 @@
     // set up new input command queue if necessary
     if (queueChanged) {
         auto ret = mClient->setInputCommandQueue(*mWriter.getMQDescriptor());
-        auto error = unwrapRet(ret);
+        auto error = static_cast<Error>(unwrapRet(ret));
         if (error != Error::NONE) {
             mWriter.reset();
             return error;
@@ -915,17 +916,17 @@
         return Error::NONE;
     }
 
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     hardware::Return<void> ret;
     auto hidl_callback = [&](const auto& tmpError, const auto& tmpOutChanged,
                              const auto& tmpOutLength, const auto& tmpOutHandles) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
 
         // set up new output command queue if necessary
         if (error == Error::NONE && tmpOutChanged) {
-            error = kDefaultError;
+            error = static_cast<Error>(kDefaultError);
             mClient->getOutputCommandQueue([&](const auto& tmpError, const auto& tmpDescriptor) {
-                error = tmpError;
+                error = static_cast<Error>(tmpError);
                 if (error != Error::NONE) {
                     return;
                 }
@@ -1000,11 +1001,11 @@
         return keys;
     }
 
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     if (mClient_2_3) {
         mClient_2_3->getPerFrameMetadataKeys_2_3(display,
                                                  [&](const auto& tmpError, const auto& tmpKeys) {
-                                                     error = tmpError;
+                                                     error = static_cast<Error>(tmpError);
                                                      if (error != Error::NONE) {
                                                          ALOGW("getPerFrameMetadataKeys failed "
                                                                "with %d",
@@ -1016,7 +1017,7 @@
     } else {
         mClient_2_2
                 ->getPerFrameMetadataKeys(display, [&](const auto& tmpError, const auto& tmpKeys) {
-                    error = tmpError;
+                    error = static_cast<Error>(tmpError);
                     if (error != Error::NONE) {
                         ALOGW("getPerFrameMetadataKeys failed with %d", tmpError);
                         return;
@@ -1039,10 +1040,10 @@
         return Error::NONE;
     }
 
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
 
     auto getRenderIntentsLambda = [&](const auto& tmpError, const auto& tmpKeys) {
-        error = tmpError;
+        error = static_cast<Error>(tmpError);
         if (error != Error::NONE) {
             return;
         }
@@ -1066,10 +1067,10 @@
         return Error::NONE;
     }
 
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient_2_2->getDataspaceSaturationMatrix(static_cast<types::V1_1::Dataspace>(dataspace),
                                               [&](const auto& tmpError, const auto& tmpMatrix) {
-                                                  error = tmpError;
+                                                  error = static_cast<Error>(tmpError);
                                                   if (error != Error::NONE) {
                                                       return;
                                                   }
@@ -1087,11 +1088,11 @@
         return Error::UNSUPPORTED;
     }
 
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient_2_3->getDisplayIdentificationData(display,
                                               [&](const auto& tmpError, const auto& tmpPort,
                                                   const auto& tmpData) {
-                                                  error = tmpError;
+                                                  error = static_cast<Error>(tmpError);
                                                   if (error != Error::NONE) {
                                                       return;
                                                   }
@@ -1123,13 +1124,13 @@
     if (!mClient_2_3) {
         return Error::UNSUPPORTED;
     }
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient_2_3->getDisplayedContentSamplingAttributes(display,
                                                        [&](const auto tmpError,
                                                            const auto& tmpFormat,
                                                            const auto& tmpDataspace,
                                                            const auto& tmpComponentMask) {
-                                                           error = tmpError;
+                                                           error = static_cast<Error>(tmpError);
                                                            if (error == Error::NONE) {
                                                                *outFormat = tmpFormat;
                                                                *outDataspace = tmpDataspace;
@@ -1149,8 +1150,9 @@
 
     auto enable = enabled ? V2_3::IComposerClient::DisplayedContentSampling::ENABLE
                           : V2_3::IComposerClient::DisplayedContentSampling::DISABLE;
-    return mClient_2_3->setDisplayedContentSamplingEnabled(display, enable, componentMask,
-                                                           maxFrames);
+    auto ret = mClient_2_3->setDisplayedContentSamplingEnabled(display, enable, componentMask,
+                                                               maxFrames);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 Error HidlComposer::getDisplayedContentSample(Display display, uint64_t maxFrames,
@@ -1161,12 +1163,12 @@
     if (!mClient_2_3) {
         return Error::UNSUPPORTED;
     }
-    Error error = kDefaultError;
+    Error error = static_cast<Error>(kDefaultError);
     mClient_2_3->getDisplayedContentSample(display, maxFrames, timestamp,
                                            [&](const auto tmpError, auto tmpNumFrames,
                                                const auto& tmpSamples0, const auto& tmpSamples1,
                                                const auto& tmpSamples2, const auto& tmpSamples3) {
-                                               error = tmpError;
+                                               error = static_cast<Error>(tmpError);
                                                if (error == Error::NONE) {
                                                    outStats->numFrames = tmpNumFrames;
                                                    outStats->component_0_sample = tmpSamples0;
@@ -1196,7 +1198,8 @@
     if (!mClient_2_3) {
         return Error::UNSUPPORTED;
     }
-    return mClient_2_3->setDisplayBrightness(display, brightness);
+    auto ret = mClient_2_3->setDisplayBrightness(display, brightness);
+    return static_cast<Error>(unwrapRet(ret));
 }
 
 // Composer HAL 2.4
@@ -1273,19 +1276,18 @@
     return error;
 }
 
-V2_4::Error HidlComposer::setActiveConfigWithConstraints(
+Error HidlComposer::setActiveConfigWithConstraints(
         Display display, Config config,
         const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
         VsyncPeriodChangeTimeline* outTimeline) {
-    using Error = V2_4::Error;
     if (!mClient_2_4) {
         return Error::UNSUPPORTED;
     }
 
-    Error error = kDefaultError_2_4;
+    Error error = static_cast<Error>(kDefaultError_2_4);
     mClient_2_4->setActiveConfigWithConstraints(display, config, vsyncPeriodChangeConstraints,
                                                 [&](const auto& tmpError, const auto& tmpTimeline) {
-                                                    error = tmpError;
+                                                    error = static_cast<Error>(tmpError);
                                                     if (error != Error::NONE) {
                                                         return;
                                                     }
diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
index 9a89dba..42ba9a9 100644
--- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h
@@ -60,7 +60,6 @@
 
 using V2_1::Config;
 using V2_1::Display;
-using V2_1::Error;
 using V2_1::Layer;
 using V2_4::CommandReaderBase;
 using V2_4::CommandWriterBase;
@@ -308,7 +307,7 @@
     V2_4::Error getDisplayConnectionType(Display display,
                                          IComposerClient::DisplayConnectionType* outType) override;
     V2_4::Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) override;
-    V2_4::Error setActiveConfigWithConstraints(
+    Error setActiveConfigWithConstraints(
             Display display, Config config,
             const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
             VsyncPeriodChangeTimeline* outTimeline) override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f9e59b9..97c8623 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1532,8 +1532,9 @@
         constraints.seamlessRequired = false;
         hal::VsyncPeriodChangeTimeline outTimeline;
 
-        if (!mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt),
-                                                       constraints, outTimeline)) {
+        if (mDisplayModeController.initiateModeChange(displayId, std::move(*desiredModeOpt),
+                                                      constraints, outTimeline) !=
+            display::DisplayModeController::ModeChangeResult::Changed) {
             continue;
         }
 
diff --git a/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp
index d971150..29a1fab 100644
--- a/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp
@@ -20,6 +20,7 @@
 #include "Display/DisplayModeController.h"
 #include "Display/DisplaySnapshot.h"
 #include "DisplayHardware/HWComposer.h"
+#include "DisplayHardware/Hal.h"
 #include "DisplayIdentificationTestHelpers.h"
 #include "FpsOps.h"
 #include "mock/DisplayHardware/MockComposer.h"
@@ -103,7 +104,7 @@
 
         EXPECT_CALL(*mComposerHal,
                     setActiveConfigWithConstraints(kHwcDisplayId, hwcModeId, constraints, _))
-                .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::V2_4::Error::NONE)));
+                .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::Error::NONE)));
 
         return constraints;
     }
@@ -183,7 +184,8 @@
     hal::VsyncPeriodChangeTimeline timeline;
     const auto constraints = expectModeSet(modeRequest, timeline);
 
-    EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+    EXPECT_EQ(DisplayModeController::ModeChangeResult::Changed,
+              mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
     EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId));
 
     mDmc.clearDesiredMode(mDisplayId);
@@ -210,7 +212,8 @@
     hal::VsyncPeriodChangeTimeline timeline;
     auto constraints = expectModeSet(modeRequest, timeline);
 
-    EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+    EXPECT_EQ(DisplayModeController::ModeChangeResult::Changed,
+              mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
     EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId));
 
     // No action since a mode switch has already been initiated.
@@ -223,7 +226,8 @@
     constexpr bool kSubsequent = true;
     constraints = expectModeSet(modeRequest, timeline, kSubsequent);
 
-    EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+    EXPECT_EQ(DisplayModeController::ModeChangeResult::Changed,
+              mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
     EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getPendingMode(mDisplayId));
 
     mDmc.clearDesiredMode(mDisplayId);
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index e0753a3..30bce2e 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -69,7 +69,7 @@
 using ::testing::StrictMock;
 
 struct HWComposerTest : testing::Test {
-    using HalError = hardware::graphics::composer::V2_1::Error;
+    using HalError = hal::Error;
 
     Hwc2::mock::Composer* const mHal = new StrictMock<Hwc2::mock::Composer>();
     impl::HWComposer mHwc{std::unique_ptr<Hwc2::Composer>(mHal)};
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 8699621..b0dd5c2 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -34,7 +34,7 @@
                                                static_cast<hal::HWConfigId>(        \
                                                        ftl::to_underlying(modeId)), \
                                                _, _))                               \
-            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)))
+            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::Error::NONE)))
 
 namespace android {
 namespace {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp
index 20a3315..6cc6322 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp
@@ -18,12 +18,13 @@
 #define LOG_TAG "LibSurfaceFlingerUnittests"
 
 #include <gui/SurfaceComposerClient.h>
+#include "DisplayHardware/Hal.h"
 #include "DisplayTransactionTestHelpers.h"
 
 namespace android {
 
 using FakeHwcDisplayInjector = TestableSurfaceFlinger::FakeHwcDisplayInjector;
-using android::hardware::graphics::composer::V2_1::Error;
+using android::hardware::graphics::composer::hal::Error;
 
 class NotifyExpectedPresentTest : public DisplayTransactionTest {
 public:
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
index c3934e6..3ae5ed9 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
@@ -21,6 +21,8 @@
 
 #include "CommitAndCompositeTest.h"
 
+#include "DisplayHardware/Hal.h"
+
 using namespace std::chrono_literals;
 using testing::_;
 using testing::Return;
@@ -40,7 +42,7 @@
     EXPECT_CALL(*mComposer, presentOrValidateDisplay(HWC_DISPLAY, _, _, _, _, _, _)).WillOnce([] {
         constexpr Duration kMockHwcRunTime = 20ms;
         std::this_thread::sleep_for(kMockHwcRunTime);
-        return hardware::graphics::composer::V2_1::Error::NONE;
+        return hardware::graphics::composer::hal::Error::NONE;
     });
     EXPECT_CALL(*mPowerAdvisor, reportActualWorkDuration()).Times(1);
 
@@ -61,7 +63,7 @@
     EXPECT_CALL(*mComposer, presentOrValidateDisplay(HWC_DISPLAY, _, _, _, _, _, _)).WillOnce([] {
         constexpr Duration kMockHwcRunTime = 20ms;
         std::this_thread::sleep_for(kMockHwcRunTime);
-        return hardware::graphics::composer::V2_1::Error::NONE;
+        return hardware::graphics::composer::hal::Error::NONE;
     });
     EXPECT_CALL(*mPowerAdvisor, reportActualWorkDuration()).Times(0);
 
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
index 88052db..0d5266e 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
@@ -20,6 +20,7 @@
 
 #include "DisplayHardware/ComposerHal.h"
 #include "DisplayHardware/HWC2.h"
+#include "DisplayHardware/Hal.h"
 
 namespace android {
 
@@ -34,9 +35,9 @@
 using android::hardware::graphics::common::V1_2::Dataspace;
 using android::hardware::graphics::common::V1_2::PixelFormat;
 
+using android::hardware::graphics::composer::hal::Error;
 using android::hardware::graphics::composer::V2_1::Config;
 using android::hardware::graphics::composer::V2_1::Display;
-using android::hardware::graphics::composer::V2_1::Error;
 using android::hardware::graphics::composer::V2_1::IComposer;
 using android::hardware::graphics::composer::V2_1::Layer;
 using android::hardware::graphics::composer::V2_4::IComposerCallback;
@@ -142,8 +143,8 @@
                  V2_4::Error(Display, Config, std::vector<VsyncPeriodNanos>*));
     MOCK_METHOD2(getDisplayVsyncPeriod, V2_4::Error(Display, VsyncPeriodNanos*));
     MOCK_METHOD4(setActiveConfigWithConstraints,
-                 V2_4::Error(Display, Config, const IComposerClient::VsyncPeriodChangeConstraints&,
-                             VsyncPeriodChangeTimeline*));
+                 Error(Display, Config, const IComposerClient::VsyncPeriodChangeConstraints&,
+                       VsyncPeriodChangeTimeline*));
     MOCK_METHOD2(setAutoLowLatencyMode, V2_4::Error(Display, bool));
     MOCK_METHOD2(setBootDisplayConfig, Error(Display, Config));
     MOCK_METHOD1(clearBootDisplayConfig, Error(Display));
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
index fa74492..ec065a7 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <gmock/gmock.h>
+#include <cstdint>
 
 #include "DisplayHardware/HWC2.h"
 
@@ -85,8 +86,8 @@
     MOCK_METHOD(ftl::Future<hal::Error>, setDisplayBrightness,
                 (float, float, const Hwc2::Composer::DisplayBrightnessOptions &), (override));
     MOCK_METHOD(hal::Error, setActiveConfigWithConstraints,
-                (hal::HWConfigId, const hal::VsyncPeriodChangeConstraints &,
-                 hal::VsyncPeriodChangeTimeline *),
+                (hal::HWConfigId, const hal::VsyncPeriodChangeConstraints&,
+                 hal::VsyncPeriodChangeTimeline*),
                 (override));
     MOCK_METHOD(hal::Error, setBootDisplayConfig, (hal::HWConfigId), (override));
     MOCK_METHOD(hal::Error, clearBootDisplayConfig, (), (override));