commit bf711ebeb6931f5b2094ac4ab4fe710b9dbebbe3
author Drew Davenport <ddavenport@google.com> Wed Feb 19 09:41:20 2025 -0700
committer Drew Davenport <ddavenport@google.com> Fri Feb 21 10:51:42 2025 -0700
tree 1ac7ca1ca7d552ac19b39aa926b35291bd5234e7
parent 35a395720a4d567e1aaf1749963088d9829ef8b8

drm_hwcomposer: Avoid resetting vsync timestamp

last_timestamp_ tracks the most recent timestamp from a vblank event. Do
not reset it when toggling vsync tracking on/off. This allows for
GetPhasedVSync to calculate a more precise approximation of the next
vsync relative to the current time, even if a vsync hasn't happened for
a while.

Reset last_timestamp_ when the vsync period changes, since the phased
vsync calculation is based on the current vsync.

Also change last_timestamp_ to std::optional type.

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

drm/VSyncWorker.cpp

diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 5234f6b..5b3071d 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -55,7 +55,6 @@
   {
     const std::lock_guard<std::mutex> lock(mutex_);
     enabled_ = ShouldEnable();
-    last_timestamp_ = -1;
   }
 
   cv_.notify_all();
@@ -64,6 +63,7 @@
 void VSyncWorker::SetVsyncPeriodNs(uint32_t vsync_period_ns) {
   const std::lock_guard<std::mutex> lock(mutex_);
   vsync_period_ns_ = vsync_period_ns;
+  last_timestamp_ = std::nullopt;
 }
 
 void VSyncWorker::SetVsyncTimestampTracking(bool enabled) {
@@ -121,11 +121,11 @@
  *  timestamp.
  */
 int64_t VSyncWorker::GetPhasedVSync(int64_t frame_ns, int64_t current) const {
-  if (last_timestamp_ < 0)
+  if (!last_timestamp_.has_value())
     return current + frame_ns;
 
-  return (frame_ns * ((current - last_timestamp_) / frame_ns + 1)) +
-         last_timestamp_;
+  return (frame_ns * ((current - *last_timestamp_) / frame_ns + 1)) +
+         *last_timestamp_;
 }
 
 static const int64_t kOneSecondNs = 1LL * 1000 * 1000 * 1000;

drm/VSyncWorker.h

diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index 5b97328..784f85d 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -37,7 +37,8 @@
   auto static CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe)
       -> std::unique_ptr<VSyncWorker>;
 
-  // Set the expected vsync period.
+  // Set the expected vsync period. Resets internal timestamp tracking until the
+  // next vsync event is tracked.
   void SetVsyncPeriodNs(uint32_t vsync_period_ns);
 
   // Set or clear a callback to be fired on vsync.
@@ -68,7 +69,7 @@
 
   bool enabled_ GUARDED_BY(mutex_) = false;
   bool thread_exit_ GUARDED_BY(mutex_) = false;
-  int64_t last_timestamp_ GUARDED_BY(mutex_) = -1;
+  std::optional<int64_t> last_timestamp_ GUARDED_BY(mutex_);
 
   // Default to 60Hz refresh rate
   static constexpr uint32_t kDefaultVSPeriodNs = 16666666;

hwc2_device/HwcDisplay.cpp

diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 121f04f..977ab1a 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -822,7 +822,6 @@
   }
 
   if (new_vsync_period_ns) {
-    vsync_worker_->SetVsyncPeriodNs(new_vsync_period_ns.value());
     staged_mode_config_id_.reset();
 
     vsync_worker_->SetVsyncTimestampTracking(false);
@@ -832,6 +831,7 @@
                                                       last_vsync_ts +
                                                           prev_vperiod_ns);
     }
+    vsync_worker_->SetVsyncPeriodNs(new_vsync_period_ns.value());
   }
 
   return HWC2::Error::None;