drm_hwcomposer: Rework VSyncWorker to work without utils/worker
utils/worker just complicates the logic without providing any benefit.
Change-Id: I7b3c91aee9c0507d9ca35792d24ba6c8c3870033
Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
diff --git a/.ci/Makefile b/.ci/Makefile
index 66922be..34ca915 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -23,7 +23,6 @@
bufferinfo/legacy/BufferInfoMinigbm.cpp:COARSE \
drm/DrmFbImporter.h:FINE \
drm/DrmUnique.h:FINE \
- drm/VSyncWorker.cpp:COARSE \
hwc2_device/DrmHwcTwo.cpp:COARSE \
hwc2_device/DrmHwcTwo.h:COARSE \
hwc2_device/HwcDisplay.cpp:COARSE \
diff --git a/drm/DrmAtomicStateManager.cpp b/drm/DrmAtomicStateManager.cpp
index 16652c1..cd2d69a 100644
--- a/drm/DrmAtomicStateManager.cpp
+++ b/drm/DrmAtomicStateManager.cpp
@@ -175,12 +175,6 @@
active_frame_state_ = std::move(new_frame_state);
}
- if (args.display_mode) {
- /* TODO(nobody): we still need this for synthetic vsync, remove after
- * vsync reworked */
- connector->SetActiveMode(*args.display_mode);
- }
-
args.out_fence = UniqueFd(out_fence);
return 0;
diff --git a/drm/DrmConnector.cpp b/drm/DrmConnector.cpp
index 6fde6fb..07c5d74 100644
--- a/drm/DrmConnector.cpp
+++ b/drm/DrmConnector.cpp
@@ -183,8 +183,4 @@
return 0;
}
-void DrmConnector::SetActiveMode(DrmMode &mode) {
- active_mode_ = mode;
-}
-
} // namespace android
diff --git a/drm/DrmConnector.h b/drm/DrmConnector.h
index 63150e7..f21f598 100644
--- a/drm/DrmConnector.h
+++ b/drm/DrmConnector.h
@@ -82,12 +82,6 @@
return modes_;
}
- auto &GetActiveMode() const {
- return active_mode_;
- }
-
- void SetActiveMode(DrmMode &mode);
-
auto &GetDpmsProperty() const {
return dpms_property_;
}
@@ -123,7 +117,6 @@
const uint32_t index_in_res_array_;
- DrmMode active_mode_{};
std::vector<DrmMode> modes_;
DrmProperty dpms_property_;
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index a2cad28..77b9d98 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -25,28 +25,48 @@
#include <cstring>
#include <ctime>
+#include "drm/ResourceManager.h"
#include "utils/log.h"
namespace android {
-VSyncWorker::VSyncWorker() : Worker("vsync", HAL_PRIORITY_URGENT_DISPLAY){};
+auto VSyncWorker::CreateInstance(DrmDisplayPipeline *pipe,
+ VSyncWorkerCallbacks &callbacks)
+ -> std::shared_ptr<VSyncWorker> {
+ auto vsw = std::shared_ptr<VSyncWorker>(new VSyncWorker());
-auto VSyncWorker::Init(DrmDisplayPipeline *pipe,
- std::function<void(uint64_t /*timestamp*/)> callback)
- -> int {
- pipe_ = pipe;
- callback_ = std::move(callback);
+ vsw->callbacks_ = callbacks;
- return InitWorker();
+ if (pipe != nullptr) {
+ vsw->high_crtc_ = pipe->crtc->Get()->GetIndexInResArray()
+ << DRM_VBLANK_HIGH_CRTC_SHIFT;
+ vsw->drm_fd_ = UniqueFd::Dup(pipe->device->GetFd());
+ }
+
+ std::thread(&VSyncWorker::ThreadFn, vsw.get(), vsw).detach();
+
+ return vsw;
}
void VSyncWorker::VSyncControl(bool enabled) {
- Lock();
- enabled_ = enabled;
- last_timestamp_ = -1;
- Unlock();
+ {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ enabled_ = enabled;
+ last_timestamp_ = -1;
+ }
- Signal();
+ cv_.notify_all();
+}
+
+void VSyncWorker::StopThread() {
+ {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ thread_exit_ = true;
+ enabled_ = false;
+ callbacks_ = {};
+ }
+
+ cv_.notify_all();
}
/*
@@ -73,82 +93,86 @@
static const int64_t kOneSecondNs = 1LL * 1000 * 1000 * 1000;
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);
struct timespec vsync {};
- int ret = clock_gettime(CLOCK_MONOTONIC, &vsync);
- if (ret)
- return ret;
-
- float refresh = 60.0F; // Default to 60Hz refresh rate
- if (pipe_ != nullptr &&
- pipe_->connector->Get()->GetActiveMode().GetVRefresh() != 0.0F) {
- refresh = pipe_->connector->Get()->GetActiveMode().GetVRefresh();
- }
-
- auto phased_timestamp = GetPhasedVSync(kOneSecondNs /
- static_cast<int>(refresh),
- vsync.tv_sec * kOneSecondNs +
- vsync.tv_nsec);
- vsync.tv_sec = phased_timestamp / kOneSecondNs;
+ vsync.tv_sec = int(phased_timestamp / kOneSecondNs);
vsync.tv_nsec = int(phased_timestamp - (vsync.tv_sec * kOneSecondNs));
+
+ int ret = 0;
do {
ret = clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME, &vsync, nullptr);
} while (ret == EINTR);
- if (ret)
+ if (ret != 0)
return ret;
- *timestamp = (int64_t)vsync.tv_sec * kOneSecondNs + (int64_t)vsync.tv_nsec;
+ *timestamp = phased_timestamp;
return 0;
}
-void VSyncWorker::Routine() {
+void VSyncWorker::ThreadFn(const std::shared_ptr<VSyncWorker> &vsw) {
int ret = 0;
- Lock();
- if (!enabled_) {
- ret = WaitForSignalOrExitLocked();
- if (ret == -EINTR) {
- Unlock();
- return;
+ for (;;) {
+ {
+ std::unique_lock<std::mutex> lock(vsw->mutex_);
+ if (thread_exit_)
+ break;
+
+ if (!enabled_)
+ vsw->cv_.wait(lock);
+
+ if (!enabled_)
+ continue;
}
+
+ ret = -EAGAIN;
+ int64_t timestamp = 0;
+ drmVBlank vblank{};
+
+ if (drm_fd_) {
+ vblank.request.type = (drmVBlankSeqType)(DRM_VBLANK_RELATIVE |
+ (high_crtc_ &
+ DRM_VBLANK_HIGH_CRTC_MASK));
+ vblank.request.sequence = 1;
+
+ ret = drmWaitVBlank(drm_fd_.Get(), &vblank);
+ if (ret == -EINTR)
+ continue;
+ }
+
+ if (ret != 0) {
+ ret = SyntheticWaitVBlank(×tamp);
+ if (ret != 0)
+ continue;
+ } else {
+ constexpr int kUsToNsMul = 1000;
+ timestamp = (int64_t)vblank.reply.tval_sec * kOneSecondNs +
+ (int64_t)vblank.reply.tval_usec * kUsToNsMul;
+ }
+
+ decltype(callbacks_.out_event) callback;
+
+ {
+ const std::lock_guard<std::mutex> lock(mutex_);
+ if (!enabled_)
+ continue;
+ callback = callbacks_.out_event;
+ }
+
+ if (callback)
+ callback(timestamp);
+
+ last_timestamp_ = timestamp;
}
- auto *pipe = pipe_;
- Unlock();
-
- ret = -EAGAIN;
- int64_t timestamp = 0;
- drmVBlank vblank{};
-
- if (pipe != nullptr) {
- auto high_crtc = (pipe->crtc->Get()->GetIndexInResArray()
- << DRM_VBLANK_HIGH_CRTC_SHIFT);
-
- vblank.request.type = (drmVBlankSeqType)(DRM_VBLANK_RELATIVE |
- (high_crtc &
- DRM_VBLANK_HIGH_CRTC_MASK));
- vblank.request.sequence = 1;
-
- ret = drmWaitVBlank(pipe->device->GetFd(), &vblank);
- if (ret == -EINTR)
- return;
- }
-
- if (ret) {
- ret = SyntheticWaitVBlank(×tamp);
- if (ret)
- return;
- } else {
- timestamp = (int64_t)vblank.reply.tval_sec * kOneSecondNs +
- (int64_t)vblank.reply.tval_usec * 1000;
- }
-
- if (!enabled_)
- return;
-
- if (callback_) {
- callback_(timestamp);
- }
-
- last_timestamp_ = timestamp;
+ ALOGI("VSyncWorker thread exit");
}
} // namespace android
diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index 1774cc6..2a0b084 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -16,41 +16,51 @@
#pragma once
-#include <hardware/hardware.h>
-#include <hardware/hwcomposer.h>
-#include <hardware/hwcomposer2.h>
-
-#include <atomic>
-#include <cstdint>
+#include <condition_variable>
#include <functional>
#include <map>
+#include <mutex>
+#include <thread>
#include "DrmDevice.h"
-#include "utils/Worker.h"
namespace android {
-class VSyncWorker : public Worker {
- public:
- VSyncWorker();
- ~VSyncWorker() override = default;
+struct VSyncWorkerCallbacks {
+ std::function<void(uint64_t /*timestamp*/)> out_event;
+ std::function<uint32_t()> get_vperiod_ns;
+};
- auto Init(DrmDisplayPipeline *pipe,
- std::function<void(uint64_t /*timestamp*/)> callback) -> int;
+class VSyncWorker {
+ public:
+ ~VSyncWorker() = default;
+
+ auto static CreateInstance(DrmDisplayPipeline *pipe,
+ VSyncWorkerCallbacks &callbacks)
+ -> std::shared_ptr<VSyncWorker>;
void VSyncControl(bool enabled);
-
- protected:
- void Routine() override;
+ void StopThread();
private:
+ VSyncWorker() = default;
+
+ void ThreadFn(const std::shared_ptr<VSyncWorker> &vsw);
+
int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const;
int SyntheticWaitVBlank(int64_t *timestamp);
- std::function<void(uint64_t /*timestamp*/)> callback_;
+ VSyncWorkerCallbacks callbacks_;
- DrmDisplayPipeline *pipe_ = nullptr;
- std::atomic_bool enabled_ = false;
+ UniqueFd drm_fd_;
+ uint32_t high_crtc_ = 0;
+
+ bool enabled_ = false;
+ bool thread_exit_ = false;
int64_t last_timestamp_ = -1;
+
+ std::condition_variable cv_;
+ std::thread vswt_;
+ std::mutex mutex_;
};
} // namespace android
diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp
index 6facac3..707319e 100644
--- a/hwc2_device/DrmHwcTwo.cpp
+++ b/hwc2_device/DrmHwcTwo.cpp
@@ -61,16 +61,9 @@
const int kTimeForSFToDisposeDisplayUs = 200000;
usleep(kTimeForSFToDisposeDisplayUs);
mutex.lock();
- std::vector<std::unique_ptr<HwcDisplay>> for_disposal;
for (auto handle : displays_for_removal_list_) {
- for_disposal.emplace_back(
- std::unique_ptr<HwcDisplay>(displays_[handle].release()));
displays_.erase(handle);
}
- /* Destroy HwcDisplays while unlocked to avoid vsyncworker deadlocks */
- mutex.unlock();
- for_disposal.clear();
- mutex.lock();
}
bool DrmHwcTwo::BindDisplay(DrmDisplayPipeline *pipeline) {
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 115b61a..4948755 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -137,41 +137,57 @@
GetPipe().atomic_state_manager->ExecuteAtomicCommit(a_args);
#endif
- vsync_worker_.Init(nullptr, [](int64_t) {});
current_plan_.reset();
backend_.reset();
}
+ if (vsync_worker_) {
+ vsync_worker_->StopThread();
+ vsync_worker_ = {};
+ }
+
SetClientTarget(nullptr, -1, 0, {});
}
HWC2::Error HwcDisplay::Init() {
ChosePreferredConfig();
- int ret = vsync_worker_.Init(pipeline_, [this](int64_t timestamp) {
- const std::lock_guard<std::mutex> lock(hwc2_->GetResMan().GetMainLock());
- if (vsync_event_en_) {
- uint32_t period_ns{};
- GetDisplayVsyncPeriod(&period_ns);
- hwc2_->SendVsyncEventToClient(handle_, timestamp, period_ns);
- }
- if (vsync_flattening_en_) {
- ProcessFlatenningVsyncInternal();
- }
- if (vsync_tracking_en_) {
- last_vsync_ts_ = timestamp;
- }
- if (!vsync_event_en_ && !vsync_flattening_en_ && !vsync_tracking_en_) {
- vsync_worker_.VSyncControl(false);
- }
- });
- if (ret && ret != -EALREADY) {
- ALOGE("Failed to create event worker for d=%d %d\n", int(handle_), ret);
+ auto vsw_callbacks = (VSyncWorkerCallbacks){
+ .out_event =
+ [this](int64_t timestamp) {
+ const std::lock_guard<std::mutex> lock(
+ hwc2_->GetResMan().GetMainLock());
+ if (vsync_event_en_) {
+ uint32_t period_ns{};
+ GetDisplayVsyncPeriod(&period_ns);
+ hwc2_->SendVsyncEventToClient(handle_, timestamp, period_ns);
+ }
+ if (vsync_flattening_en_) {
+ ProcessFlatenningVsyncInternal();
+ }
+ if (vsync_tracking_en_) {
+ last_vsync_ts_ = timestamp;
+ }
+ if (!vsync_event_en_ && !vsync_flattening_en_ &&
+ !vsync_tracking_en_) {
+ vsync_worker_->VSyncControl(false);
+ }
+ },
+ .get_vperiod_ns = [this]() -> uint32_t {
+ uint32_t outVsyncPeriod = 0;
+ GetDisplayVsyncPeriod(&outVsyncPeriod);
+ return outVsyncPeriod;
+ },
+ };
+
+ vsync_worker_ = VSyncWorker::CreateInstance(pipeline_, vsw_callbacks);
+ if (!vsync_worker_) {
+ ALOGE("Failed to create event worker for d=%d\n", int(handle_));
return HWC2::Error::BadDisplay;
}
if (!IsInHeadlessMode()) {
- ret = BackendManager::GetInstance().SetBackendForDisplay(this);
+ auto ret = BackendManager::GetInstance().SetBackendForDisplay(this);
if (ret) {
ALOGE("Failed to set backend for d=%d %d\n", int(handle_), ret);
return HWC2::Error::BadDisplay;
@@ -450,8 +466,8 @@
return HWC2::Error::None;
}
- auto PrevModeVsyncPeriodNs = static_cast<int>(
- 1E9 / GetPipe().connector->Get()->GetActiveMode().GetVRefresh());
+ uint32_t prev_vperiod_ns = 0;
+ GetDisplayVsyncPeriod(&prev_vperiod_ns);
auto mode_update_commited_ = false;
if (staged_mode_ &&
@@ -542,8 +558,9 @@
staged_mode_.reset();
vsync_tracking_en_ = false;
if (last_vsync_ts_ != 0) {
- hwc2_->SendVsyncPeriodTimingChangedEventToClient(
- handle_, last_vsync_ts_ + PrevModeVsyncPeriodNs);
+ hwc2_->SendVsyncPeriodTimingChangedEventToClient(handle_,
+ last_vsync_ts_ +
+ prev_vperiod_ns);
}
}
@@ -725,7 +742,7 @@
HWC2::Error HwcDisplay::SetVsyncEnabled(int32_t enabled) {
vsync_event_en_ = HWC2_VSYNC_ENABLE == enabled;
if (vsync_event_en_) {
- vsync_worker_.VSyncControl(true);
+ vsync_worker_->VSyncControl(true);
}
return HWC2::Error::None;
}
@@ -820,7 +837,7 @@
last_vsync_ts_ = 0;
vsync_tracking_en_ = true;
- vsync_worker_.VSyncControl(true);
+ vsync_worker_->VSyncControl(true);
return HWC2::Error::None;
}
@@ -961,7 +978,7 @@
}
vsync_flattening_en_ = true;
- vsync_worker_.VSyncControl(true);
+ vsync_worker_->VSyncControl(true);
flattenning_state_ = ClientFlattenningState::VsyncCountdownMax;
return false;
}
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index 0334470..68353e9 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -18,6 +18,7 @@
#include <hardware/hwcomposer2.h>
+#include <atomic>
#include <optional>
#include "HwcDisplayConfigs.h"
@@ -208,7 +209,7 @@
std::unique_ptr<Backend> backend_;
- VSyncWorker vsync_worker_;
+ std::shared_ptr<VSyncWorker> vsync_worker_;
bool vsync_event_en_{};
bool vsync_flattening_en_{};
bool vsync_tracking_en_{};