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>

9 files changed, 164 insertions, 137 deletions

diff --git a/.ci/Makefile b/.ci/Makefile
index 66922be..34ca915 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -23,7 +23,6 @@ TIDY_FILES_OVERRIDE := \
     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 @@ auto DrmAtomicStateManager::CommitFrame(AtomicCommitArgs &args) -> int {
     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 @@ int DrmConnector::UpdateModes() {
   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 @@ class DrmConnector : public PipelineBindable<DrmConnector> {
     return modes_;
   }
 
-  auto &GetActiveMode() const {
-    return active_mode_;
-  }
-
-  void SetActiveMode(DrmMode &mode);
-
   auto &GetDpmsProperty() const {
     return dpms_property_;
   }
@@ -123,7 +117,6 @@ class DrmConnector : public PipelineBindable<DrmConnector> {
 
   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;
+  }
+
+  cv_.notify_all();
+}
+
+void VSyncWorker::StopThread() {
+  {
+    const std::lock_guard<std::mutex> lock(mutex_);
+    thread_exit_ = true;
+    enabled_ = false;
+    callbacks_ = {};
+  }
 
-  Signal();
+  cv_.notify_all();
 }
 
 /*
@@ -73,82 +93,86 @@ int64_t VSyncWorker::GetPhasedVSync(int64_t frame_ns, int64_t current) const {
 static const int64_t kOneSecondNs = 1LL * 1000 * 1000 * 1000;
 
 int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) {
-  struct timespec vsync {};
-  int ret = clock_gettime(CLOCK_MONOTONIC, &vsync);
-  if (ret)
-    return ret;
+  auto time_now = ResourceManager::GetTimeMonotonicNs();
 
-  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();
-  }
+  // 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(kOneSecondNs /
-                                             static_cast<int>(refresh),
-                                         vsync.tv_sec * kOneSecondNs +
-                                             vsync.tv_nsec);
-  vsync.tv_sec = phased_timestamp / kOneSecondNs;
+  auto phased_timestamp = GetPhasedVSync(period_ns, time_now);
+  struct timespec vsync {};
+  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;
     }
-  }
 
-  auto *pipe = pipe_;
-  Unlock();
+    ret = -EAGAIN;
+    int64_t timestamp = 0;
+    drmVBlank vblank{};
 
-  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;
 
-  if (pipe != nullptr) {
-    auto high_crtc = (pipe->crtc->Get()->GetIndexInResArray()
-                      << DRM_VBLANK_HIGH_CRTC_SHIFT);
+      ret = drmWaitVBlank(drm_fd_.Get(), &vblank);
+      if (ret == -EINTR)
+        continue;
+    }
 
-    vblank.request.type = (drmVBlankSeqType)(DRM_VBLANK_RELATIVE |
-                                             (high_crtc &
-                                              DRM_VBLANK_HIGH_CRTC_MASK));
-    vblank.request.sequence = 1;
+    if (ret != 0) {
+      ret = SyntheticWaitVBlank(&timestamp);
+      if (ret != 0)
+        continue;
+    } else {
+      constexpr int kUsToNsMul = 1000;
+      timestamp = (int64_t)vblank.reply.tval_sec * kOneSecondNs +
+                  (int64_t)vblank.reply.tval_usec * kUsToNsMul;
+    }
 
-    ret = drmWaitVBlank(pipe->device->GetFd(), &vblank);
-    if (ret == -EINTR)
-      return;
-  }
+    decltype(callbacks_.out_event) callback;
 
-  if (ret) {
-    ret = SyntheticWaitVBlank(&timestamp);
-    if (ret)
-      return;
-  } else {
-    timestamp = (int64_t)vblank.reply.tval_sec * kOneSecondNs +
-                (int64_t)vblank.reply.tval_usec * 1000;
-  }
+    {
+      const std::lock_guard<std::mutex> lock(mutex_);
+      if (!enabled_)
+        continue;
+      callback = callbacks_.out_event;
+    }
 
-  if (!enabled_)
-    return;
+    if (callback)
+      callback(timestamp);
 
-  if (callback_) {
-    callback_(timestamp);
+    last_timestamp_ = 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 {
+struct VSyncWorkerCallbacks {
+  std::function<void(uint64_t /*timestamp*/)> out_event;
+  std::function<uint32_t()> get_vperiod_ns;
+};
+
+class VSyncWorker {
  public:
-  VSyncWorker();
-  ~VSyncWorker() override = default;
+  ~VSyncWorker() = default;
 
-  auto Init(DrmDisplayPipeline *pipe,
-            std::function<void(uint64_t /*timestamp*/)> callback) -> int;
+  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 @@ void DrmHwcTwo::FinalizeDisplayBinding() {
   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 @@ void HwcDisplay::Deinit() {
     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 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) {
     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 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) {
     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::SetPowerMode(int32_t mode_in) {
 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 @@ HWC2::Error HwcDisplay::SetActiveConfigWithConstraints(
 
   last_vsync_ts_ = 0;
   vsync_tracking_en_ = true;
-  vsync_worker_.VSyncControl(true);
+  vsync_worker_->VSyncControl(true);
 
   return HWC2::Error::None;
 }
@@ -961,7 +978,7 @@ bool HwcDisplay::ProcessClientFlatteningState(bool skip) {
   }
 
   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 @@ class HwcDisplay {
 
   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_{};