Merge remote-tracking branch 'aosp/upstream-main' into uprev
* aosp/upstream-main:
drm_hwcomposer: Clean up VsyncWorker destruction
drm_hwcomposer: Remove VSyncWorkerCallbacks
drm_hwcomposer: Enable VSyncWorker thread as needed
drm_hwcomposer: Set vsync callback on demand
drm_hwcomposer: Move vsync tracking into VSyncWorker
drm_hwcomposer: Simplify default vsync period
drm_hwcomposer: Only update vsync period when committed
drm_hwcomposer: Set vsync period for vsync thread
drm_hwcomposer: Define cc_defaults for hwc3 binary
drm_hwcomposer: HWC3: Fix hotplug handling
drm_hwcomposer: treewide: Handle bool properties in the Property class
drm_hwcomposer: Don't erase displays in the internal layer at hotplug
Bug: 368599093
Bug: 384048744
Test: presubmit
Change-Id: I56ef114974a38d288258a9a5d95866f322f04928
diff --git a/Android.bp b/Android.bp
index 24d4d99..2cbdc44 100644
--- a/Android.bp
+++ b/Android.bp
@@ -180,14 +180,13 @@
],
}
-cc_binary {
- name: "android.hardware.composer.hwc3-service.drm",
+cc_defaults {
+ name: "android.hardware.composer.hwc3-service.drm.defaults",
srcs: [
":drm_hwcomposer_common",
":drm_hwcomposer_hwc3",
":drm_hwcomposer_service",
- "bufferinfo/legacy/BufferInfoLibdrm.cpp",
],
defaults: [
@@ -203,16 +202,25 @@
],
cflags: [
+ "-DUSE_IMAPPER4_METADATA_API",
"-Wall",
"-Werror",
-
- "-DUSE_IMAPPER4_METADATA_API",
],
cppflags: [
"-DHWC2_INCLUDE_STRINGIFICATION",
"-DHWC2_USE_CPP11",
],
+}
+
+cc_binary {
+ name: "android.hardware.composer.hwc3-service.drm",
+
+ defaults: [
+ "android.hardware.composer.hwc3-service.drm.defaults",
+ ],
+
+ srcs: ["bufferinfo/legacy/BufferInfoLibdrm.cpp"],
relative_install_path: "hw",
vendor: true,
diff --git a/drm/DrmDisplayPipeline.cpp b/drm/DrmDisplayPipeline.cpp
index 2d81578..7588ee2 100644
--- a/drm/DrmDisplayPipeline.cpp
+++ b/drm/DrmDisplayPipeline.cpp
@@ -158,22 +158,12 @@
return {};
}
-static bool ReadUseOverlayProperty() {
- char use_overlay_planes_prop[PROPERTY_VALUE_MAX];
- property_get("vendor.hwc.drm.use_overlay_planes", use_overlay_planes_prop,
- "1");
- constexpr int kStrtolBase = 10;
- return strtol(use_overlay_planes_prop, nullptr, kStrtolBase) != 0;
-}
-
auto DrmDisplayPipeline::GetUsablePlanes()
-> std::vector<std::shared_ptr<BindingOwner<DrmPlane>>> {
std::vector<std::shared_ptr<BindingOwner<DrmPlane>>> planes;
planes.emplace_back(primary_plane);
- const static bool kUseOverlayPlanes = ReadUseOverlayProperty();
-
- if (kUseOverlayPlanes) {
+ if (Properties::UseOverlayPlanes()) {
for (const auto &plane : device->GetPlanes()) {
if (plane->IsCrtcSupported(*crtc->Get())) {
if (plane->GetType() == DRM_PLANE_TYPE_OVERLAY) {
diff --git a/drm/DrmHwc.cpp b/drm/DrmHwc.cpp
index aaba506..3f30123 100644
--- a/drm/DrmHwc.cpp
+++ b/drm/DrmHwc.cpp
@@ -200,7 +200,7 @@
/* Virtual display is an experimental feature.
* Unless explicitly set to true, return 0 for no support.
*/
- if (0 == property_get_bool("vendor.hwc.drm.enable_virtual_display", 0)) {
+ if (!Properties::EnableVirtualDisplay()) {
return 0;
}
diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp
index 0c23734..9c68816 100644
--- a/drm/ResourceManager.cpp
+++ b/drm/ResourceManager.cpp
@@ -76,10 +76,9 @@
}
}
- char proptext[PROPERTY_VALUE_MAX];
- property_get("vendor.hwc.drm.scale_with_gpu", proptext, "0");
- scale_with_gpu_ = bool(strncmp(proptext, "0", 1));
+ scale_with_gpu_ = Properties::ScaleWithGpu();
+ char proptext[PROPERTY_VALUE_MAX];
constexpr char kDrmOrGpu[] = "DRM_OR_GPU";
constexpr char kDrmOrIgnore[] = "DRM_OR_IGNORE";
property_get("vendor.hwc.drm.ctm", proptext, kDrmOrGpu);
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 963a37b..64152ab 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -30,12 +30,9 @@
namespace android {
-auto VSyncWorker::CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe,
- VSyncWorkerCallbacks &callbacks)
- -> std::shared_ptr<VSyncWorker> {
- auto vsw = std::shared_ptr<VSyncWorker>(new VSyncWorker());
-
- vsw->callbacks_ = callbacks;
+auto VSyncWorker::CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe)
+ -> std::unique_ptr<VSyncWorker> {
+ auto vsw = std::unique_ptr<VSyncWorker>(new VSyncWorker());
if (pipe) {
vsw->high_crtc_ = pipe->crtc->Get()->GetIndexInResArray()
@@ -43,32 +40,73 @@
vsw->drm_fd_ = pipe->device->GetFd();
}
- std::thread(&VSyncWorker::ThreadFn, vsw.get(), vsw).detach();
+ vsw->vswt_ = std::thread(&VSyncWorker::ThreadFn, vsw.get());
return vsw;
}
-void VSyncWorker::VSyncControl(bool enabled) {
+VSyncWorker::~VSyncWorker() {
+ StopThread();
+
+ vswt_.join();
+}
+
+void VSyncWorker::UpdateVSyncControl() {
{
const std::lock_guard<std::mutex> lock(mutex_);
- enabled_ = enabled;
+ enabled_ = ShouldEnable();
last_timestamp_ = -1;
}
cv_.notify_all();
}
+void VSyncWorker::SetVsyncPeriodNs(uint32_t vsync_period_ns) {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ vsync_period_ns_ = vsync_period_ns;
+}
+
+void VSyncWorker::SetVsyncTimestampTracking(bool enabled) {
+ {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ enable_vsync_timestamps_ = enabled;
+ if (enabled) {
+ // Reset the last timestamp so the caller knows if a vsync timestamp is
+ // fresh or not.
+ last_vsync_timestamp_ = 0;
+ }
+ }
+ UpdateVSyncControl();
+}
+
+uint32_t VSyncWorker::GetLastVsyncTimestamp() {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ return last_vsync_timestamp_;
+}
+
+void VSyncWorker::SetTimestampCallback(
+ std::optional<VsyncTimestampCallback> &&callback) {
+ {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ callback_ = std::move(callback);
+ }
+ UpdateVSyncControl();
+}
+
void VSyncWorker::StopThread() {
{
const std::lock_guard<std::mutex> lock(mutex_);
thread_exit_ = true;
enabled_ = false;
- callbacks_ = {};
}
cv_.notify_all();
}
+bool VSyncWorker::ShouldEnable() const {
+ return enable_vsync_timestamps_ || callback_.has_value();
+};
+
/*
* Returns the timestamp of the next vsync in phase with last_timestamp_.
* For example:
@@ -95,13 +133,7 @@
int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) {
auto time_now = ResourceManager::GetTimeMonotonicNs();
- // Default to 60Hz refresh rate
- constexpr uint32_t kDefaultVSPeriodNs = 16666666;
- auto period_ns = kDefaultVSPeriodNs;
- if (callbacks_.get_vperiod_ns && callbacks_.get_vperiod_ns() != 0)
- period_ns = callbacks_.get_vperiod_ns();
-
- auto phased_timestamp = GetPhasedVSync(period_ns, time_now);
+ auto phased_timestamp = GetPhasedVSync(vsync_period_ns_, time_now);
struct timespec vsync {};
vsync.tv_sec = int(phased_timestamp / kOneSecondNs);
vsync.tv_nsec = int(phased_timestamp - (vsync.tv_sec * kOneSecondNs));
@@ -117,17 +149,17 @@
return 0;
}
-void VSyncWorker::ThreadFn(const std::shared_ptr<VSyncWorker> &vsw) {
+void VSyncWorker::ThreadFn() {
int ret = 0;
for (;;) {
{
- std::unique_lock<std::mutex> lock(vsw->mutex_);
+ std::unique_lock<std::mutex> lock(mutex_);
if (thread_exit_)
break;
if (!enabled_)
- vsw->cv_.wait(lock);
+ cv_.wait(lock);
if (!enabled_)
continue;
@@ -158,18 +190,21 @@
(int64_t)vblank.reply.tval_usec * kUsToNsMul;
}
- decltype(callbacks_.out_event) callback;
+ std::optional<VsyncTimestampCallback> vsync_callback;
{
const std::lock_guard<std::mutex> lock(mutex_);
if (!enabled_)
continue;
- callback = callbacks_.out_event;
+ if (enable_vsync_timestamps_) {
+ last_vsync_timestamp_ = timestamp;
+ }
+ vsync_callback = callback_;
}
- if (callback)
- callback(timestamp);
-
+ if (vsync_callback) {
+ vsync_callback.value()(timestamp, vsync_period_ns_);
+ }
last_timestamp_ = timestamp;
}
diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index 2a4c7c8..c76dd14 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -26,31 +26,41 @@
namespace android {
-struct VSyncWorkerCallbacks {
- std::function<void(uint64_t /*timestamp*/)> out_event;
- std::function<uint32_t()> get_vperiod_ns;
-};
-
class VSyncWorker {
public:
- ~VSyncWorker() = default;
+ using VsyncTimestampCallback = std::function<void(int64_t /*timestamp*/,
+ uint32_t /*period*/)>;
- auto static CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe,
- VSyncWorkerCallbacks &callbacks)
- -> std::shared_ptr<VSyncWorker>;
+ ~VSyncWorker();
- void VSyncControl(bool enabled);
+ auto static CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe)
+ -> std::unique_ptr<VSyncWorker>;
+
+ // Set the expected vsync period.
+ void SetVsyncPeriodNs(uint32_t vsync_period_ns);
+
+ // Set or clear a callback to be fired on vsync.
+ void SetTimestampCallback(std::optional<VsyncTimestampCallback> &&callback);
+
+ // Enable vsync timestamp tracking. GetLastVsyncTimestamp will return 0 if
+ // vsync tracking is disabled, or if no vsync has happened since it was
+ // enabled.
+ void SetVsyncTimestampTracking(bool enabled);
+ uint32_t GetLastVsyncTimestamp();
+
void StopThread();
private:
VSyncWorker() = default;
- void ThreadFn(const std::shared_ptr<VSyncWorker> &vsw);
+ void ThreadFn();
int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const;
int SyntheticWaitVBlank(int64_t *timestamp);
- VSyncWorkerCallbacks callbacks_;
+ // Must hold the lock before calling these.
+ void UpdateVSyncControl();
+ bool ShouldEnable() const;
SharedFd drm_fd_;
uint32_t high_crtc_ = 0;
@@ -59,6 +69,14 @@
bool thread_exit_ = false;
int64_t last_timestamp_ = -1;
+ // Default to 60Hz refresh rate
+ static constexpr uint32_t kDefaultVSPeriodNs = 16666666;
+ // Needs to be threadsafe.
+ uint32_t vsync_period_ns_ = kDefaultVSPeriodNs;
+ bool enable_vsync_timestamps_ = false;
+ uint32_t last_vsync_timestamp_ = 0;
+ std::optional<VsyncTimestampCallback> callback_;
+
std::condition_variable cv_;
std::thread vswt_;
std::mutex mutex_;
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 0503f39..60861af 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -232,6 +232,8 @@
ALOGV("Blocking config succeeded.");
configs_.active_config_id = config;
staged_mode_config_id_.reset();
+ vsync_worker_->SetVsyncPeriodNs(new_config->mode.GetVSyncPeriodNs());
+ // set new vsync period
return ConfigError::kNone;
}
@@ -263,9 +265,7 @@
staged_mode_config_id_ = config;
// Enable vsync events until the mode has been applied.
- last_vsync_ts_ = 0;
- vsync_tracking_en_ = true;
- vsync_worker_->VSyncControl(true);
+ vsync_worker_->SetVsyncTimestampTracking(true);
return ConfigError::kNone;
}
@@ -301,8 +301,6 @@
}
if (vsync_worker_) {
- // TODO: There should be a mechanism to wait for this worker to complete,
- // otherwise there is a race condition while destructing the HwcDisplay.
vsync_worker_->StopThread();
vsync_worker_ = {};
}
@@ -313,31 +311,8 @@
HWC2::Error HwcDisplay::Init() {
ChosePreferredConfig();
- auto vsw_callbacks = (VSyncWorkerCallbacks){
- .out_event =
- [this](int64_t timestamp) {
- const std::unique_lock lock(hwc_->GetResMan().GetMainLock());
- if (vsync_event_en_) {
- uint32_t period_ns{};
- GetDisplayVsyncPeriod(&period_ns);
- hwc_->SendVsyncEventToClient(handle_, timestamp, period_ns);
- }
- if (vsync_tracking_en_) {
- last_vsync_ts_ = timestamp;
- }
- if (!vsync_event_en_ && !vsync_tracking_en_) {
- vsync_worker_->VSyncControl(false);
- }
- },
- .get_vperiod_ns = [this]() -> uint32_t {
- uint32_t outVsyncPeriod = 0;
- GetDisplayVsyncPeriod(&outVsyncPeriod);
- return outVsyncPeriod;
- },
- };
-
if (type_ != HWC2::DisplayType::Virtual) {
- vsync_worker_ = VSyncWorker::CreateInstance(pipeline_, vsw_callbacks);
+ vsync_worker_ = VSyncWorker::CreateInstance(pipeline_);
if (!vsync_worker_) {
ALOGE("Failed to create event worker for d=%d\n", int(handle_));
return HWC2::Error::BadDisplay;
@@ -683,7 +658,7 @@
uint32_t prev_vperiod_ns = 0;
GetDisplayVsyncPeriod(&prev_vperiod_ns);
- auto mode_update_commited_ = false;
+ std::optional<uint32_t> new_vsync_period_ns;
if (staged_mode_config_id_ &&
staged_mode_change_time_ <= ResourceManager::GetTimeMonotonicNs()) {
const HwcDisplayConfig *staged_config = GetConfig(
@@ -698,10 +673,9 @@
.bottom = int(staged_config->mode.GetRawMode().vdisplay)});
configs_.active_config_id = staged_mode_config_id_.value();
-
a_args.display_mode = staged_config->mode;
if (!a_args.test_only) {
- mode_update_commited_ = true;
+ new_vsync_period_ns = staged_config->mode.GetVSyncPeriodNs();
}
}
@@ -777,12 +751,15 @@
return HWC2::Error::BadParameter;
}
- if (mode_update_commited_) {
+ if (new_vsync_period_ns) {
+ vsync_worker_->SetVsyncPeriodNs(new_vsync_period_ns.value());
staged_mode_config_id_.reset();
- vsync_tracking_en_ = false;
- if (last_vsync_ts_ != 0) {
+
+ vsync_worker_->SetVsyncTimestampTracking(false);
+ uint32_t last_vsync_ts = vsync_worker_->GetLastVsyncTimestamp();
+ if (last_vsync_ts != 0) {
hwc_->SendVsyncPeriodTimingChangedEventToClient(handle_,
- last_vsync_ts_ +
+ last_vsync_ts +
prev_vperiod_ns);
}
}
@@ -1072,8 +1049,17 @@
}
vsync_event_en_ = HWC2_VSYNC_ENABLE == enabled;
+
if (vsync_event_en_) {
- vsync_worker_->VSyncControl(true);
+ DrmHwc *hwc = hwc_;
+ hwc2_display_t id = handle_;
+ // Callback will be called from the vsync thread.
+ auto callback = [hwc, id](int64_t timestamp, uint32_t period_ns) {
+ hwc->SendVsyncEventToClient(id, timestamp, period_ns);
+ };
+ vsync_worker_->SetTimestampCallback(callback);
+ } else {
+ vsync_worker_->SetTimestampCallback(std::nullopt);
}
return HWC2::Error::None;
}
@@ -1170,9 +1156,7 @@
outTimeline->newVsyncAppliedTimeNanos = vsyncPeriodChangeConstraints
->desiredTimeNanos;
- last_vsync_ts_ = 0;
- vsync_tracking_en_ = true;
- vsync_worker_->VSyncControl(true);
+ vsync_worker_->SetVsyncTimestampTracking(true);
return HWC2::Error::None;
}
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index ecca514..acefff8 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -246,10 +246,8 @@
std::unique_ptr<Backend> backend_;
std::shared_ptr<FlatteningController> flatcon_;
- std::shared_ptr<VSyncWorker> vsync_worker_;
+ std::unique_ptr<VSyncWorker> vsync_worker_;
bool vsync_event_en_{};
- bool vsync_tracking_en_{};
- int64_t last_vsync_ts_{};
const hwc2_display_t handle_;
HWC2::DisplayType type_;
diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index 0d3f9c0..5366943 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -320,17 +320,8 @@
ComposerClient::~ComposerClient() {
DEBUG_FUNC();
{
- // First Deinit the displays to start shutting down the Display's dependent
- // threads such as VSyncWorker.
const std::unique_lock lock(hwc_->GetResMan().GetMainLock());
hwc_->DeinitDisplays();
- }
- // Sleep to wait for threads to complete and exit.
- const int time_for_threads_to_exit_us = 200000;
- usleep(time_for_threads_to_exit_us);
- {
- // Hold the lock while destructing the hwc_ and the objects that it owns.
- const std::unique_lock lock(hwc_->GetResMan().GetMainLock());
hwc_.reset();
}
LOG(DEBUG) << "removed composer client";
diff --git a/hwc3/DrmHwcThree.cpp b/hwc3/DrmHwcThree.cpp
index fb14bc9..9b4ba86 100644
--- a/hwc3/DrmHwcThree.cpp
+++ b/hwc3/DrmHwcThree.cpp
@@ -28,8 +28,6 @@
namespace aidl::android::hardware::graphics::composer3::impl {
-using ::android::HwcDisplay;
-
DrmHwcThree::~DrmHwcThree() {
/* Display deinit routine is handled by resource manager */
GetResMan().DeInit();
@@ -95,67 +93,22 @@
#endif
-void DrmHwcThree::CleanDisplayResources(uint64_t display_id) {
- DEBUG_FUNC();
- HwcDisplay* display = GetDisplay(display_id);
- if (display == nullptr) {
- return;
- }
-
- display->SetPowerMode(static_cast<int32_t>(PowerMode::OFF));
-
- size_t cache_size = 0;
- auto err = composer_resources_->GetDisplayClientTargetCacheSize(display_id,
- &cache_size);
- if (err != hwc3::Error::kNone) {
- ALOGE("%s: Could not clear target buffer cache for display: %" PRIu64,
- __func__, display_id);
- return;
- }
-
- for (size_t slot = 0; slot < cache_size; slot++) {
- buffer_handle_t buffer_handle = nullptr;
- auto buf_releaser = ComposerResources::CreateResourceReleaser(true);
-
- Buffer buf{};
- buf.slot = static_cast<int32_t>(slot);
- err = composer_resources_->GetDisplayClientTarget(display_id, buf,
- &buffer_handle,
- buf_releaser.get());
- if (err != hwc3::Error::kNone) {
- continue;
- }
-
- err = Hwc2toHwc3Error(
- display->SetClientTarget(buffer_handle, -1,
- static_cast<int32_t>(
- common::Dataspace::UNKNOWN),
- {}));
- if (err != hwc3::Error::kNone) {
- ALOGE(
- "%s: Could not clear slot %zu of the target buffer cache for "
- "display %" PRIu64,
- __func__, slot, display_id);
- }
- }
-}
-
void DrmHwcThree::HandleDisplayHotplugEvent(uint64_t display_id,
bool connected) {
DEBUG_FUNC();
if (!connected) {
composer_resources_->RemoveDisplay(display_id);
- Displays().erase(display_id);
return;
}
- if (composer_resources_->HasDisplay(display_id)) {
- /* Cleanup existing display resources */
- CleanDisplayResources(display_id);
- composer_resources_->RemoveDisplay(display_id);
- Displays().erase(display_id);
+ /* The second or any subsequent hotplug event with connected status enabled is
+ * a special way to inform the client (SF) that the display has changed its
+ * dimensions. In this case, the client removes all layers and re-creates
+ * them. In this case, we keep the display resources.
+ */
+ if (!composer_resources_->HasDisplay(display_id)) {
+ composer_resources_->AddPhysicalDisplay(display_id);
}
- composer_resources_->AddPhysicalDisplay(display_id);
}
} // namespace aidl::android::hardware::graphics::composer3::impl
diff --git a/hwc3/DrmHwcThree.h b/hwc3/DrmHwcThree.h
index f020634..3a9a6db 100644
--- a/hwc3/DrmHwcThree.h
+++ b/hwc3/DrmHwcThree.h
@@ -42,7 +42,6 @@
DrmHwc::DisplayStatus display_status) override;
private:
- void CleanDisplayResources(uint64_t display_id);
void HandleDisplayHotplugEvent(uint64_t display_id, bool connected);
std::shared_ptr<IComposerCallback> composer_callback_;
diff --git a/utils/properties.cpp b/utils/properties.cpp
index a855c94..5ba109b 100644
--- a/utils/properties.cpp
+++ b/utils/properties.cpp
@@ -29,3 +29,15 @@
auto Properties::UseConfigGroups() -> bool {
return (property_get_bool("ro.vendor.hwc.drm.use_config_groups", 1) != 0);
}
+
+auto Properties::UseOverlayPlanes() -> bool {
+ return (property_get_bool("ro.vendor.hwc.use_overlay_planes", 1) != 0);
+}
+
+auto Properties::ScaleWithGpu() -> bool {
+ return (property_get_bool("vendor.hwc.drm.scale_with_gpu", 0) != 0);
+}
+
+auto Properties::EnableVirtualDisplay() -> bool {
+ return (property_get_bool("vendor.hwc.drm.enable_virtual_display", 0) != 0);
+}
diff --git a/utils/properties.h b/utils/properties.h
index 15c2fb2..4df79eb 100644
--- a/utils/properties.h
+++ b/utils/properties.h
@@ -78,4 +78,7 @@
public:
static auto IsPresentFenceNotReliable() -> bool;
static auto UseConfigGroups() -> bool;
+ static auto UseOverlayPlanes() -> bool;
+ static auto ScaleWithGpu() -> bool;
+ static auto EnableVirtualDisplay() -> bool;
};