hwc2: Add missing locks, relax some locks
Currently calls forwarded to CallDisplayFunction and CallLayerFunction
are not protected via a lock, causing races. Add scope lock.
Relax sequence locks in SetPowerMode, Dumpsys and Create/Destroy layer
to scope locks to avoid locks ups in cases where these get called from
the same binder thread that called Validate.
Change-Id: I22bfc13b61dc308208cb20e8b2f3c2f76dbcafd4
CRs-fixed: 2156131
diff --git a/sdm/libs/hwc2/hwc_session.cpp b/sdm/libs/hwc2/hwc_session.cpp
index d139d0c..4138cc6 100644
--- a/sdm/libs/hwc2/hwc_session.cpp
+++ b/sdm/libs/hwc2/hwc_session.cpp
@@ -328,10 +328,6 @@
// Defined in the same order as in the HWC2 header
int32_t HWCSession::AcceptDisplayChanges(hwc2_device_t *device, hwc2_display_t display) {
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
- SCOPE_LOCK(locker_[display]);
return HWCSession::CallDisplayFunction(device, display, &HWCDisplay::AcceptDisplayChanges);
}
@@ -340,10 +336,7 @@
if (!out_layer_id) {
return HWC2_ERROR_BAD_PARAMETER;
}
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display]);
+
return CallDisplayFunction(device, display, &HWCDisplay::CreateLayer, out_layer_id);
}
@@ -373,10 +366,6 @@
int32_t HWCSession::DestroyLayer(hwc2_device_t *device, hwc2_display_t display,
hwc2_layer_t layer) {
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display]);
return CallDisplayFunction(device, display, &HWCDisplay::DestroyLayer, layer);
}
@@ -411,7 +400,7 @@
} else {
std::string s {};
for (int id = HWC_DISPLAY_PRIMARY; id <= HWC_DISPLAY_VIRTUAL; id++) {
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[id]);
+ SCOPE_LOCK(locker_[id]);
if (hwc_session->hwc_display_[id]) {
s += hwc_session->hwc_display_[id]->Dump();
}
@@ -537,6 +526,11 @@
bool notify_hotplug = false;
auto status = HWC2::Error::BadDisplay;
DTRACE_SCOPED();
+
+ if (display >= HWC_NUM_DISPLAY_TYPES) {
+ return HWC2_ERROR_BAD_DISPLAY;
+ }
+
{
SEQUENCE_EXIT_SCOPE_LOCK(locker_[display]);
if (!device) {
@@ -556,6 +550,10 @@
}
}
+ if (status != HWC2::Error::None && status != HWC2::Error::NotValidated) {
+ SEQUENCE_CANCEL_SCOPE_LOCK(locker_[display]);
+ }
+
// Handle Pending external display connection
if (hwc_session->external_pending_connect_ && (display == HWC_DISPLAY_PRIMARY)) {
Locker::ScopeLock lock_e(locker_[HWC_DISPLAY_EXTERNAL]);
@@ -611,28 +609,20 @@
int32_t HWCSession::SetColorMode(hwc2_device_t *device, hwc2_display_t display,
int32_t /*android_color_mode_t*/ int_mode) {
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
if (int_mode < HAL_COLOR_MODE_NATIVE || int_mode > HAL_COLOR_MODE_DISPLAY_P3) {
return HWC2_ERROR_BAD_PARAMETER;
}
auto mode = static_cast<android_color_mode_t>(int_mode);
- SCOPE_LOCK(locker_[display]);
return HWCSession::CallDisplayFunction(device, display, &HWCDisplay::SetColorMode, mode);
}
int32_t HWCSession::SetColorTransform(hwc2_device_t *device, hwc2_display_t display,
const float *matrix,
int32_t /*android_color_transform_t*/ hint) {
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
if (!matrix || hint < HAL_COLOR_TRANSFORM_IDENTITY ||
hint > HAL_COLOR_TRANSFORM_CORRECT_TRITANOPIA) {
return HWC2_ERROR_BAD_PARAMETER;
}
- SCOPE_LOCK(locker_[display]);
android_color_transform_t transform_hint = static_cast<android_color_transform_t>(hint);
return HWCSession::CallDisplayFunction(device, display, &HWCDisplay::SetColorTransform, matrix,
transform_hint);
@@ -719,15 +709,9 @@
visible);
}
-int32_t HWCSession::SetLayerZOrder(hwc2_device_t *device, hwc2_display_t display,
- hwc2_layer_t layer, uint32_t z) {
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return HWC2_ERROR_BAD_DISPLAY;
- }
-
- SCOPE_LOCK(locker_[display]);
-
- return CallDisplayFunction(device, display, &HWCDisplay::SetLayerZOrder, layer, z);
+static int32_t SetLayerZOrder(hwc2_device_t *device, hwc2_display_t display, hwc2_layer_t layer,
+ uint32_t z) {
+ return HWCSession::CallDisplayFunction(device, display, &HWCDisplay::SetLayerZOrder, layer, z);
}
int32_t HWCSession::SetOutputBuffer(hwc2_device_t *device, hwc2_display_t display,
@@ -770,7 +754,6 @@
return HWC2_ERROR_UNSUPPORTED;
}
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display]);
return CallDisplayFunction(device, display, &HWCDisplay::SetPowerMode, mode);
}
@@ -824,9 +807,8 @@
}
}
- // If validate fails, cancel the sequence lock so that other operations
- // (such as Dump or SetPowerMode) may succeed without blocking on the condition
- if (status == HWC2::Error::BadDisplay) {
+ // Sequence locking currently begins on Validate, so cancel the sequence lock on failures
+ if (status != HWC2::Error::None) {
SEQUENCE_CANCEL_SCOPE_LOCK(locker_[display]);
}
@@ -1410,14 +1392,10 @@
auto mode = static_cast<android_color_mode_t>(input_parcel->readInt32());
auto device = static_cast<hwc2_device_t *>(this);
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return -EINVAL;
- }
-
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display]);
auto err = CallDisplayFunction(device, display, &HWCDisplay::SetColorMode, mode);
if (err != HWC2_ERROR_NONE)
return -EINVAL;
+
return 0;
}
@@ -1426,14 +1404,10 @@
auto mode = input_parcel->readInt32();
auto device = static_cast<hwc2_device_t *>(this);
- if (display >= HWC_NUM_DISPLAY_TYPES) {
- return -EINVAL;
- }
-
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display]);
auto err = CallDisplayFunction(device, display, &HWCDisplay::SetColorModeById, mode);
if (err != HWC2_ERROR_NONE)
return -EINVAL;
+
return 0;
}
@@ -1902,4 +1876,12 @@
use_primary_res, &hwc_display_[disp_id]);
}
+#ifdef DISPLAY_CONFIG_1_1
+// Methods from ::vendor::hardware::display::config::V1_1::IDisplayConfig follow.
+Return<int32_t> HWCSession::setDisplayAnimating(uint64_t display_id, bool animating ) {
+ return CallDisplayFunction(static_cast<hwc2_device_t *>(this), display_id,
+ &HWCDisplay::SetDisplayAnimating, animating);
+}
+#endif
+
} // namespace sdm
diff --git a/sdm/libs/hwc2/hwc_session.h b/sdm/libs/hwc2/hwc_session.h
index 84abd7b..8d25989 100644
--- a/sdm/libs/hwc2/hwc_session.h
+++ b/sdm/libs/hwc2/hwc_session.h
@@ -94,6 +94,7 @@
return HWC2_ERROR_BAD_DISPLAY;
}
+ SCOPE_LOCK(locker_[display]);
HWCSession *hwc_session = static_cast<HWCSession *>(device);
auto status = HWC2::Error::BadDisplay;
if (hwc_session->hwc_display_[display]) {
@@ -115,6 +116,7 @@
return HWC2_ERROR_BAD_DISPLAY;
}
+ SCOPE_LOCK(locker_[display]);
HWCSession *hwc_session = static_cast<HWCSession *>(device);
auto status = HWC2::Error::BadDisplay;
if (hwc_session->hwc_display_[display]) {
@@ -147,8 +149,6 @@
hwc2_function_pointer_t pointer);
static int32_t SetOutputBuffer(hwc2_device_t *device, hwc2_display_t display,
buffer_handle_t buffer, int32_t releaseFence);
- static int32_t SetLayerZOrder(hwc2_device_t *device, hwc2_display_t display, hwc2_layer_t layer,
- uint32_t z);
static int32_t SetPowerMode(hwc2_device_t *device, hwc2_display_t display, int32_t int_mode);
static int32_t ValidateDisplay(hwc2_device_t *device, hwc2_display_t display,
uint32_t *out_num_types, uint32_t *out_num_requests);
diff --git a/sdm/libs/hwc2/hwc_session_services.cpp b/sdm/libs/hwc2/hwc_session_services.cpp
index 23a2a6f..4cef884 100644
--- a/sdm/libs/hwc2/hwc_session_services.cpp
+++ b/sdm/libs/hwc2/hwc_session_services.cpp
@@ -510,14 +510,4 @@
return Void();
}
-#ifdef DISPLAY_CONFIG_1_1
-// Methods from ::vendor::hardware::display::config::V1_1::IDisplayConfig follow.
-Return<int32_t> HWCSession::setDisplayAnimating(uint64_t display_id, bool animating ) {
- SEQUENCE_WAIT_SCOPE_LOCK(locker_[display_id]);
- return CallDisplayFunction(static_cast<hwc2_device_t *>(this), display_id,
- &HWCDisplay::SetDisplayAnimating, animating);
-}
-#endif
-
-
} // namespace sdm