From 1cd7ad277a79ef6695a8b155df9e9f40e891f2cb Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Tue, 13 Oct 2026 17:46:31 -0700 Subject: [PATCH 1/2] videoctrl: Avoid two cyclic timers for VBL interval We were relying on two separate cyclic timers for the VBL interval, but in realtime mode we may end up falling behind in timer execution, and the exact phase may not be preserved. In the worst case they would end up exactly aligned, and the VBL interval would be effectively lost (the guest polls for the VBL bit and would never see it) Switch to only one cyclic timer, which repeatedly sets a one-shot timer for the VBL end. --- devices/video/videoctrl.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/devices/video/videoctrl.cpp b/devices/video/videoctrl.cpp index af8aba9d16..58b1342c13 100644 --- a/devices/video/videoctrl.cpp +++ b/devices/video/videoctrl.cpp @@ -102,27 +102,30 @@ void VideoCtrlBase::start_refresh_task() { this->display.configure(this->active_width, this->active_height); uint64_t refresh_interval = static_cast(1.0f / refresh_rate * NS_PER_SEC + 0.5); - TimerManager::get_instance()->add_cyclic_timer(this->refresh_timer, - refresh_interval, - [this](uint64_t, uint64_t) { - // assert VBL interrupt - this->vbl_cb(1); - this->update_screen(); - } - ); - if (vert_blank == 0) { LOG_F(ERROR, "Vertical blank is 0. Using 25 instead."); vert_blank = 25; } uint64_t vbl_duration = static_cast((double)hori_total * vert_blank / this->pixel_clock * NS_PER_SEC + 0.5); - TimerManager::get_instance()-> add_cyclic_timer(this->vbl_end_timer, + TimerManager::get_instance()->add_cyclic_timer( + this->refresh_timer, refresh_interval, - refresh_interval + vbl_duration, - [this](uint64_t, uint64_t) { - // deassert VBL interrupt - this->vbl_cb(0); + [this, vbl_duration](uint64_t, uint64_t) { + this->vbl_cb(1); // assert VBL interrupt + + if (this->vbl_end_timer.active) { + TimerManager::get_instance()->cancel_timer(this->vbl_end_timer); + } + TimerManager::get_instance()->add_oneshot_timer( + this->vbl_end_timer, + vbl_duration, + [this](uint64_t, uint64_t) { + this->vbl_cb(0); // deassert VBL interrupt + } + ); + + this->update_screen(); } ); } From 38ce23d2c821031a6bc10d42b3ff7be289b3c5a4 Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Tue, 13 Oct 2026 17:47:12 -0700 Subject: [PATCH 2/2] timermanager: Preserve phase when rescheduling overdue timers We would reschedule timers based on their interval and the current time, which would cause timers with the same interval to get aligned. This may be not be desired (if a subsystem has two intentionally out-of-phase timers), so we instead advance the timer based on its previously scheduled time and multiples of the interval. Not strictly necessary for the codebase as it stands today (we no longer use two cyclic intervals for the VBL interval) but it still seems like a worthwhile correctness improvement, if we decide to reintroduce that approach. --- core/timermanager.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/timermanager.cpp b/core/timermanager.cpp index b07fa9a3c5..a7f7118d32 100644 --- a/core/timermanager.cpp +++ b/core/timermanager.cpp @@ -101,8 +101,14 @@ uint64_t TimerManager::process_timers() if (cur_timer->interval_ns) { std::lock_guard lk(this->timer_queue.get_mtx()); uint64_t timeout_ns_new = timeout_ns + cur_timer->interval_ns; - if (timeout_ns_new <= time_now) - timeout_ns_new = time_now + cur_timer->interval_ns; + if (timeout_ns_new <= time_now) { + // Skip missed periods without changing the timer's phase. + // Rebasing deadlines on time_now would align timers that + // have the same period. + const uint64_t periods_to_skip = + ((time_now - timeout_ns_new) / cur_timer->interval_ns) + 1; + timeout_ns_new += periods_to_skip * cur_timer->interval_ns; + } cur_timer->timeout_ns = timeout_ns_new; this->timer_queue.push(cur_timer); } else {