commit b39a3f82f97576d20bf7e1c8e17f908241c5debf
author Drew Davenport <ddavenport@google.com> Fri Dec 13 15:04:56 2024 -0700
committer Drew Davenport <ddavenport@google.com> Mon Dec 16 17:53:48 2024 -0700
tree a5076f600bab93680a353a276cc791f48b0fd1f2
parent 63a699e14e0945015d978be16fbd5703d0b370e9

drm_hwcomposer: Enable VSyncWorker thread as needed

Rather than explicitly enabling and disabling the VSyncWorker thread
from HwcDisplay, move the logic to enable/disable the thread into
VSyncWorker, and enable it based on whether HwcDisplay has requested
vsync timeline events, or timeline tracking.

This simplifies the public interface for VSyncWorker, and removes the
need for holding the global lock to check whether the thread should be
enabled or not.

Change-Id: Ic1632a19b268ea1d4ed2d8ec8704ef531481211c
Signed-off-by: Drew Davenport <ddavenport@google.com>

drm/VSyncWorker.cpp

diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 6123e5c..afc3949 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -48,10 +48,10 @@
   return vsw;
 }
 
-void VSyncWorker::VSyncControl(bool enabled) {
+void VSyncWorker::UpdateVSyncControl() {
   {
     const std::lock_guard<std::mutex> lock(mutex_);
-    enabled_ = enabled;
+    enabled_ = ShouldEnable();
     last_timestamp_ = -1;
   }
 
@@ -64,13 +64,16 @@
 }
 
 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;
+  {
+    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() {
@@ -80,8 +83,11 @@
 
 void VSyncWorker::SetTimestampCallback(
     std::optional<VsyncTimestampCallback> &&callback) {
-  const std::lock_guard<std::mutex> lock(mutex_);
-  callback_ = std::move(callback);
+  {
+    const std::lock_guard<std::mutex> lock(mutex_);
+    callback_ = std::move(callback);
+  }
+  UpdateVSyncControl();
 }
 
 void VSyncWorker::StopThread() {
@@ -95,6 +101,10 @@
   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:

drm/VSyncWorker.h

diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index f032d79..f1143e4 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -41,8 +41,6 @@
                              VSyncWorkerCallbacks &callbacks)
       -> std::shared_ptr<VSyncWorker>;
 
-  void VSyncControl(bool enabled);
-
   // Set the expected vsync period.
   void SetVsyncPeriodNs(uint32_t vsync_period_ns);
 
@@ -65,6 +63,10 @@
   int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const;
   int SyntheticWaitVBlank(int64_t *timestamp);
 
+  // Must hold the lock before calling these.
+  void UpdateVSyncControl();
+  bool ShouldEnable() const;
+
   VSyncWorkerCallbacks callbacks_;
 
   SharedFd drm_fd_;