Skip to content

Commit fc2a4a7

Browse files
author
Manasi Navare
committed
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>
1 parent 2721b5c commit fc2a4a7

18 files changed

Lines changed: 152 additions & 101 deletions

‎services/surfaceflinger/Display/DisplayModeController.cpp‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <ftl/concat.h>
2929
#include <ftl/expected.h>
3030
#include <log/log.h>
31+
#include <utils/Errors.h>
3132

3233
namespace android::display {
3334

@@ -177,12 +178,13 @@ void DisplayModeController::clearDesiredMode(PhysicalDisplayId displayId) {
177178
}
178179
}
179180

180-
bool DisplayModeController::initiateModeChange(PhysicalDisplayId displayId,
181-
DisplayModeRequest&& desiredMode,
182-
const hal::VsyncPeriodChangeConstraints& constraints,
183-
hal::VsyncPeriodChangeTimeline& outTimeline) {
181+
auto DisplayModeController::initiateModeChange(
182+
PhysicalDisplayId displayId, DisplayModeRequest&& desiredMode,
183+
const hal::VsyncPeriodChangeConstraints& constraints,
184+
hal::VsyncPeriodChangeTimeline& outTimeline) -> ModeChangeResult {
184185
std::lock_guard lock(mDisplayLock);
185-
const auto& displayPtr = FTL_EXPECT(mDisplays.get(displayId).ok_or(false)).get();
186+
const auto& displayPtr =
187+
FTL_EXPECT(mDisplays.get(displayId).ok_or(ModeChangeResult::Aborted)).get();
186188

187189
// TODO: b/255635711 - Flow the DisplayModeRequest through the desired/pending/active states.
188190
// For now, `desiredMode` and `desiredModeOpt` are one and the same, but the latter is not
@@ -201,13 +203,17 @@ bool DisplayModeController::initiateModeChange(PhysicalDisplayId displayId,
201203

202204
const auto& mode = *displayPtr->pendingModeOpt->mode.modePtr;
203205

204-
if (mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(), constraints,
205-
&outTimeline) != OK) {
206-
return false;
206+
const auto error = mComposerPtr->setActiveModeWithConstraints(displayId, mode.getHwcId(),
207+
constraints, &outTimeline);
208+
switch (error) {
209+
case FAILED_TRANSACTION:
210+
return ModeChangeResult::Rejected;
211+
case OK:
212+
SFTRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());
213+
return ModeChangeResult::Changed;
214+
default:
215+
return ModeChangeResult::Aborted;
207216
}
208-
209-
SFTRACE_INT(displayPtr->pendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());
210-
return true;
211217
}
212218

213219
void DisplayModeController::finalizeModeChange(PhysicalDisplayId displayId, DisplayModeId modeId,

‎services/surfaceflinger/Display/DisplayModeController.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class DisplayModeController {
7070
RefreshRateSelectorPtr selectorPtrFor(PhysicalDisplayId) const EXCLUDES(mDisplayLock);
7171

7272
enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch };
73+
enum class ModeChangeResult { Changed, Rejected, Aborted };
7374

7475
DesiredModeAction setDesiredMode(PhysicalDisplayId, DisplayModeRequest&&)
7576
EXCLUDES(mDisplayLock);
@@ -86,9 +87,9 @@ class DisplayModeController {
8687

8788
scheduler::FrameRateMode getActiveMode(PhysicalDisplayId) const EXCLUDES(mDisplayLock);
8889

89-
bool initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&,
90-
const hal::VsyncPeriodChangeConstraints&,
91-
hal::VsyncPeriodChangeTimeline& outTimeline)
90+
ModeChangeResult initiateModeChange(PhysicalDisplayId, DisplayModeRequest&&,
91+
const hal::VsyncPeriodChangeConstraints&,
92+
hal::VsyncPeriodChangeTimeline& outTimeline)
9293
REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
9394

9495
void finalizeModeChange(PhysicalDisplayId, DisplayModeId, Fps vsyncRate, Fps renderFps)

‎services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,7 @@ V2_4::Error AidlComposer::getDisplayVsyncPeriod(Display display, VsyncPeriodNano
13841384
return V2_4::Error::NONE;
13851385
}
13861386

1387-
V2_4::Error AidlComposer::setActiveConfigWithConstraints(
1387+
Error AidlComposer::setActiveConfigWithConstraints(
13881388
Display display, Config config,
13891389
const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
13901390
VsyncPeriodChangeTimeline* outTimeline) {
@@ -1398,10 +1398,10 @@ V2_4::Error AidlComposer::setActiveConfigWithConstraints(
13981398
&timeline);
13991399
if (!status.isOk()) {
14001400
ALOGE("setActiveConfigWithConstraints failed %s", status.getDescription().c_str());
1401-
return static_cast<V2_4::Error>(status.getServiceSpecificError());
1401+
return static_cast<Error>(status.getServiceSpecificError());
14021402
}
14031403
*outTimeline = translate<VsyncPeriodChangeTimeline>(timeline);
1404-
return V2_4::Error::NONE;
1404+
return Error::NONE;
14051405
}
14061406

14071407
V2_4::Error AidlComposer::setAutoLowLatencyMode(Display display, bool on) {

‎services/surfaceflinger/DisplayHardware/AidlComposerHal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ class AidlComposer final : public Hwc2::Composer {
206206
V2_4::Error getDisplayConnectionType(Display display,
207207
IComposerClient::DisplayConnectionType* outType) override;
208208
V2_4::Error getDisplayVsyncPeriod(Display display, VsyncPeriodNanos* outVsyncPeriod) override;
209-
V2_4::Error setActiveConfigWithConstraints(
209+
Error setActiveConfigWithConstraints(
210210
Display display, Config config,
211211
const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
212212
VsyncPeriodChangeTimeline* outTimeline) override;

‎services/surfaceflinger/DisplayHardware/ComposerHal.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
#include <ui/PictureProfileHandle.h>
3333
#include <utils/StrongPointer.h>
3434

35+
#include "DisplayHardware/Hal.h"
36+
3537
#include <aidl/android/hardware/graphics/common/DisplayDecorationSupport.h>
3638
#include <aidl/android/hardware/graphics/common/HdrConversionCapability.h>
3739
#include <aidl/android/hardware/graphics/common/HdrConversionStrategy.h>
@@ -73,9 +75,9 @@ using types::V1_2::ColorMode;
7375
using types::V1_2::Dataspace;
7476
using types::V1_2::PixelFormat;
7577

78+
using hardware::graphics::composer::hal::Error;
7679
using V2_1::Config;
7780
using V2_1::Display;
78-
using V2_1::Error;
7981
using V2_1::Layer;
8082
using V2_4::CommandReaderBase;
8183
using V2_4::CommandWriterBase;
@@ -261,7 +263,7 @@ class Composer {
261263
Display display, IComposerClient::DisplayConnectionType* outType) = 0;
262264
virtual V2_4::Error getDisplayVsyncPeriod(Display display,
263265
VsyncPeriodNanos* outVsyncPeriod) = 0;
264-
virtual V2_4::Error setActiveConfigWithConstraints(
266+
virtual Error setActiveConfigWithConstraints(
265267
Display display, Config config,
266268
const IComposerClient::VsyncPeriodChangeConstraints& vsyncPeriodChangeConstraints,
267269
VsyncPeriodChangeTimeline* outTimeline) = 0;

‎services/surfaceflinger/DisplayHardware/HWC2.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
#include <ui/GraphicBuffer.h>
3434
#include <ui/PictureProfileHandle.h>
3535

36+
#include "DisplayHardware/Hal.h"
37+
3638
#include <algorithm>
3739
#include <cinttypes>
3840
#include <iterator>

‎services/surfaceflinger/DisplayHardware/HWComposer.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include "HWComposer.h"
2929

30+
#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
3031
#include <android-base/properties.h>
3132
#include <common/trace.h>
3233
#include <compositionengine/Output.h>
@@ -733,7 +734,11 @@ status_t HWComposer::setActiveModeWithConstraints(
733734
auto error = mDisplayData[displayId].hwcDisplay->setActiveConfigWithConstraints(hwcModeId,
734735
constraints,
735736
outTimeline);
736-
RETURN_IF_HWC_ERROR(error, displayId, UNKNOWN_ERROR);
737+
if (error == hal::Error::CONFIG_FAILED) {
738+
RETURN_IF_HWC_ERROR_FOR("setActiveConfigWithConstraints", error, displayId,
739+
FAILED_TRANSACTION);
740+
}
741+
RETURN_IF_HWC_ERROR_FOR("setActiveConfigWithConstraints", error, displayId, UNKNOWN_ERROR);
737742
return NO_ERROR;
738743
}
739744

‎services/surfaceflinger/DisplayHardware/Hal.h‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,21 @@
1717
#pragma once
1818

1919
#include <android/hardware/graphics/common/1.1/types.h>
20+
#include <android/hardware/graphics/composer/2.1/types.h>
2021
#include <android/hardware/graphics/composer/2.4/IComposer.h>
2122
#include <android/hardware/graphics/composer/2.4/IComposerClient.h>
23+
#include <android/hardware/graphics/composer/2.4/types.h>
2224

2325
#include <aidl/android/hardware/graphics/common/DisplayHotplugEvent.h>
2426
#include <aidl/android/hardware/graphics/common/Hdr.h>
2527
#include <aidl/android/hardware/graphics/composer3/Composition.h>
2628
#include <aidl/android/hardware/graphics/composer3/DisplayCapability.h>
2729
#include <aidl/android/hardware/graphics/composer3/DisplayConfiguration.h>
30+
#include <aidl/android/hardware/graphics/composer3/IComposerClient.h>
2831
#include <aidl/android/hardware/graphics/composer3/VrrConfig.h>
2932

33+
#include <ftl/enum.h>
34+
3035
#define ERROR_HAS_CHANGES 5
3136

3237
namespace android {
@@ -46,7 +51,6 @@ using types::V1_2::ColorMode;
4651
using types::V1_2::Dataspace;
4752
using types::V1_2::PixelFormat;
4853

49-
using V2_1::Error;
5054
using V2_4::IComposer;
5155
using V2_4::IComposerCallback;
5256
using V2_4::IComposerClient;
@@ -78,6 +82,22 @@ using Hdr = aidl::android::hardware::graphics::common::Hdr;
7882
using DisplayConfiguration = V3_0::DisplayConfiguration;
7983
using VrrConfig = V3_0::VrrConfig;
8084

85+
enum class Error : int32_t {
86+
NONE = static_cast<int32_t>(V2_1::Error::NONE),
87+
BAD_CONFIG = static_cast<int32_t>(V2_1::Error::BAD_CONFIG),
88+
BAD_DISPLAY = static_cast<int32_t>(V2_1::Error::BAD_DISPLAY),
89+
BAD_LAYER = static_cast<int32_t>(V2_1::Error::BAD_LAYER),
90+
BAD_PARAMETER = static_cast<int32_t>(V2_1::Error::BAD_PARAMETER),
91+
NO_RESOURCES = static_cast<int32_t>(V2_1::Error::NO_RESOURCES),
92+
NOT_VALIDATED = static_cast<int32_t>(V2_1::Error::NOT_VALIDATED),
93+
UNSUPPORTED = static_cast<int32_t>(V2_1::Error::UNSUPPORTED),
94+
SEAMLESS_NOT_ALLOWED = static_cast<int32_t>(V2_4::Error::SEAMLESS_NOT_ALLOWED),
95+
SEAMLESS_NOT_POSSIBLE = static_cast<int32_t>(V2_4::Error::SEAMLESS_NOT_POSSIBLE),
96+
CONFIG_FAILED = V3_0::IComposerClient::EX_CONFIG_FAILED,
97+
PICTURE_PROFILE_MAX_EXCEEDED = V3_0::IComposerClient::EX_PICTURE_PROFILE_MAX_EXCEEDED,
98+
ftl_last = PICTURE_PROFILE_MAX_EXCEEDED
99+
};
100+
81101
} // namespace hardware::graphics::composer::hal
82102

83103
inline bool hasChangesError(hardware::graphics::composer::hal::Error error) {
@@ -210,7 +230,11 @@ inline std::string to_string(hardware::graphics::composer::hal::V2_4::Error erro
210230
}
211231

212232
inline std::string to_string(hardware::graphics::composer::hal::Error error) {
213-
return to_string(static_cast<hardware::graphics::composer::hal::V2_4::Error>(error));
233+
// 5 is reserved for historical reason, during validation 5 means has changes.
234+
if (hasChangesError(error)) {
235+
return "HAS_CHANGES";
236+
}
237+
return ftl::enum_string(error);
214238
}
215239

216240
// For utils::Dumper ADL.

0 commit comments

Comments
 (0)