fix(timers): retain shared free-running prescaler phase

crates/gba-core/src/io.rs

@@ -1,6 +1,6 @@
 //! GBA I/O subset: DMA, timers, interrupts, WAITCNT, display, and keypad input.
-//! Timer phase resets on enable or clock-source changes. Hardware startup delays,
-//! shared prescaler phase, audio events, and interrupt delivery delays are not modeled.
+//! Timers share a free-running prescaler phase. Hardware startup/write delays,
+//! audio events, and interrupt delivery delays are not modeled.
 
 mod timer_step;
 pub(crate) use timer_step::TimerStep;
@@ -67,7 +67,6 @@ struct Timer {
     reload: u16,
     counter: u16,
     control: u8,
-    phase: u32,
 }
 
 impl Timer {
@@ -102,25 +101,22 @@ impl Timer {
         if start {
             self.counter = self.reload;
         }
-        if start || (self.control ^ value) & 7 != 0 {
-            self.phase = 0;
-        }
         self.control = value;
     }
 
     /// Next independently clocked overflow. Cascaded timers receive their
     /// pulses at a predecessor's event and cannot overflow earlier.
-    fn next_overflow(&self) -> Option<u32> {
+    fn next_overflow(&self, phase: u16) -> Option<u32> {
         if !self.enabled() || self.count_up() {
             return None;
         }
         let divisor = [1, 64, 256, 1024][usize::from(self.control & 3)];
-        Some((0x1_0000 - u32::from(self.counter)) * divisor - self.phase)
+        Some((0x1_0000 - u32::from(self.counter)) * divisor - u32::from(phase) % divisor)
     }
 
     /// Advance without looping once per cycle or overflow. Return the number of
     /// overflows, not just a boolean, so cascaded timers receive every pulse.
-    fn advance(&mut self, cycles: u32, previous_overflows: u64) -> u64 {
+    fn advance(&mut self, cycles: u32, previous_overflows: u64, phase: u16) -> u64 {
         if !self.enabled() {
             return 0;
         }
@@ -128,9 +124,9 @@ impl Timer {
             previous_overflows
         } else {
             let divisor = [1, 64, 256, 1024][usize::from(self.control & 3)];
-            let elapsed = u64::from(self.phase) + u64::from(cycles);
-            self.phase = (elapsed % divisor) as u32;
-            elapsed / divisor
+            // Count shared divider edges in (start, end], independently of enable time.
+            let start = u64::from(phase);
+            (start + u64::from(cycles)) / divisor - start / divisor
         };
         let until_overflow = 0x1_0000 - u64::from(self.counter);
         if ticks < until_overflow {
@@ -155,6 +151,8 @@ enum PowerState {
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub(crate) struct Io {
     timers: [Timer; 4],
+    // Low ten system-clock bits cover every supported divider. STOP freezes this phase.
+    timer_phase: u16,
     enable: u16,
     pending: u16,
     master_enable: bool,
@@ -320,7 +318,7 @@ impl Io {
     pub(crate) fn next_event_cycles(&self) -> u32 {
         self.timers
             .iter()
-            .filter_map(Timer::next_overflow)
+            .filter_map(|timer| timer.next_overflow(self.timer_phase))
             .fold(self.display.next_event_cycles(), u32::min)
     }
 
@@ -404,19 +402,22 @@ impl Io {
 
     pub(crate) fn commit_timer_step(&mut self, step: TimerStep) {
         self.timers = step.timers;
+        self.timer_phase = step.phase;
         self.pending = (self.pending & !TIMER_IRQ_MASK) | step.pending;
         self.wake_if_requested();
     }
 
-    fn advance_timer_bank(timers: &mut [Timer; 4], cycles: u32) -> u16 {
+    fn advance_timer_bank(timers: &mut [Timer; 4], phase: &mut u16, cycles: u32) -> u16 {
         let mut pending = 0;
         let mut overflows = 0;
         for (index, timer) in timers.iter_mut().enumerate() {
-            overflows = timer.advance(cycles, overflows);
+            overflows = timer.advance(cycles, overflows, *phase);
             if overflows != 0 && timer.control & 0x40 != 0 {
                 pending |= 1 << (3 + index);
             }
         }
+        // Advance even when every timer is disabled; writes never reset this clock.
+        *phase = ((u64::from(*phase) + u64::from(cycles)) & 1023) as u16;
         pending
     }
 
@@ -453,7 +454,8 @@ impl Io {
         self.dma.trigger(vblank, hblank);
         self.pending |= self.display.advance(cycles);
         if advance_timers {
-            self.pending |= Self::advance_timer_bank(&mut self.timers, cycles);
+            self.pending |=
+                Self::advance_timer_bank(&mut self.timers, &mut self.timer_phase, cycles);
         }
         self.wake_if_requested();
     }

crates/gba-core/src/io/timer_step.rs

@@ -7,6 +7,7 @@ pub(super) const TIMER_IRQ_MASK: u16 = 0x78;
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct TimerStep {
     pub(super) timers: [Timer; 4],
+    pub(super) phase: u16,
     pub(super) pending: u16,
     elapsed: u32,
 }
@@ -15,6 +16,7 @@ impl TimerStep {
     pub(super) fn new(io: &Io) -> Self {
         Self {
             timers: io.timers,
+            phase: io.timer_phase,
             pending: io.pending & TIMER_IRQ_MASK,
             elapsed: 0,
         }
@@ -22,7 +24,8 @@ impl TimerStep {
 
     pub(crate) fn advance_to(&mut self, elapsed: u32) {
         assert!(elapsed >= self.elapsed, "timer step time must be monotonic");
-        self.pending |= Io::advance_timer_bank(&mut self.timers, elapsed - self.elapsed);
+        self.pending |=
+            Io::advance_timer_bank(&mut self.timers, &mut self.phase, elapsed - self.elapsed);
         self.elapsed = elapsed;
     }
 

crates/gba-core/src/memory/timer_step_tests.rs

@@ -322,6 +322,78 @@ fn bios_power_store_pays_its_cycles_and_new_timer_irq_wakes_only_halt() {
     }
 }
 
+#[test]
+fn machine_enable_joins_the_shared_phase_and_commits_it_without_duplicate_ticks() {
+    let (cpu, mut memory) = prepared(
+        false,
+        &[0xe581_0000, 0xe591_2000],
+        &[(0, 0x0081_0000), (1, TIMER_BASE)],
+    );
+    memory.advance_cycles(56); // The disabled divider still runs.
+    let mut machine = Machine::new(cpu, memory);
+    machine.step().unwrap(); // Source 6 + store 1: enable at t=63.
+    assert_eq!(machine.cycles(), 63);
+    assert_eq!(machine.memory().read16(TIMER_BASE).unwrap(), 0);
+    machine.step().unwrap(); // N source 8 + read 1 + internal 1: t=73.
+    assert_eq!(machine.cpu().registers()[2], 0x0081_0001);
+    assert_eq!(machine.cycles(), 73);
+    machine.memory_mut().advance_cycles(54);
+    assert_eq!(machine.memory().read16(TIMER_BASE).unwrap(), 1);
+    machine.memory_mut().advance_cycles(1);
+    assert_eq!(machine.memory().read16(TIMER_BASE).unwrap(), 2);
+}
+
+#[test]
+fn failed_cpu_and_dma_work_cannot_consume_a_shared_prescaler_edge() {
+    let (cpu, mut memory) = prepared(false, &[0xe891_0005], &[(1, TIMER_BASE + 12)]);
+    start(&mut memory, 0, 0xffff, 0xc1);
+    memory.advance_cycles(63);
+    let before = memory.io.timer_step();
+    let mut machine = Machine::new(cpu, memory);
+    assert!(machine.step().is_err()); // First data read succeeds after the t=64 overflow; second fails.
+    assert_eq!(machine.memory().io.timer_step(), before);
+    assert_eq!(machine.cycles(), 63);
+    dma(machine.memory_mut(), 0x0e00_0000, RAM, 1);
+    assert!(machine.step().is_err());
+    assert_eq!(machine.memory().io.timer_step(), before);
+    machine.memory_mut().advance_cycles(1);
+    assert_eq!(machine.memory().read16(IF).unwrap(), 8);
+}
+
+#[test]
+fn dma_timer_phases_share_the_divider_with_later_host_clock_advances() {
+    let mut memory = Memory::new(Vec::new()).unwrap();
+    memory.advance_cycles(60);
+    start(&mut memory, 0, 0, 0x81);
+    dma(&mut memory, TIMER_BASE, RAM, 1);
+    memory.step_dma().unwrap().unwrap(); // Read t=63, destination t=64.
+    assert_eq!(memory.read32(RAM).unwrap(), 0x0081_0000);
+    assert_eq!(memory.read16(TIMER_BASE).unwrap(), 1);
+    memory.write32(RAM, 0x0081_0000).unwrap();
+    dma(&mut memory, RAM, TIMER_BASE + 4, 1);
+    memory.step_dma().unwrap().unwrap(); // Enable timer1 at t=68, not at a new divider origin.
+    memory.advance_cycles(59);
+    assert_eq!(memory.cycles(), 127);
+    assert_eq!(memory.read16(TIMER_BASE + 4).unwrap(), 0);
+    memory.advance_cycles(1);
+    assert_eq!(memory.read16(TIMER_BASE).unwrap(), 2);
+    assert_eq!(memory.read16(TIMER_BASE + 4).unwrap(), 1);
+}
+
+#[test]
+fn halt_after_a_machine_timer_start_uses_the_committed_shared_phase() {
+    let (cpu, mut memory) = prepared(false, &[0xe581_0000], &[(0, 0x00c1_ffff), (1, TIMER_BASE)]);
+    memory.advance_cycles(56);
+    memory.write16(IE, 8).unwrap();
+    let mut machine = Machine::new(cpu, memory);
+    machine.step().unwrap(); // Enable at t=63.
+    machine.memory_mut().write8(HALTCNT, 0).unwrap();
+    assert_eq!(machine.step().unwrap(), StepKind::HaltIdle);
+    assert_eq!(machine.last_timing().idle_cycles, 1);
+    assert_eq!(machine.cycles(), 64);
+    assert!(!machine.halted());
+}
+
 #[test]
 fn halt_and_stop_keep_their_clock_ownership() {
     for stop in [false, true] {