Refactor IRQ signaling

Use Rc<Cell> instead of passing `&mut irqs` all around.

The runtime impact is two additional size_t's per each holder of the
shared pointer.


Former-commit-id: afd3188c31608ebcf062256a7ad51575dbc90d8b
Former-commit-id: 22e0e01953968cee592b5408677e557059669c31

rustboyadvance-core/src/dma.rs

@@ -1,7 +1,8 @@
 use super::cartridge::BackupMedia;
+use super::interrupt::{self, Interrupt, SharedInterruptFlags};
 use super::iodev::consts::{REG_FIFO_A, REG_FIFO_B};
 use super::sysbus::SysBus;
-use super::{Bus, Interrupt, IrqBitmask};
+use super::Bus;
 
 use num::FromPrimitive;
 use serde::{Deserialize, Serialize};
@@ -23,6 +24,7 @@ pub struct DmaChannel {
     start_cycles: usize,
     fifo_mode: bool,
     irq: Interrupt,
+    interrupt_flags: SharedInterruptFlags,
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug, Default)]
@@ -33,7 +35,7 @@ struct DmaInternalRegs {
 }
 
 impl DmaChannel {
-    pub fn new(id: usize) -> DmaChannel {
+    pub fn new(id: usize, interrupt_flags: SharedInterruptFlags) -> DmaChannel {
         if id > 3 {
             panic!("invalid dma id {}", id);
         }
@@ -49,6 +51,7 @@ impl DmaChannel {
             start_cycles: 0,
             fifo_mode: false,
             internal: Default::default(),
+            interrupt_flags,
         }
     }
 
@@ -129,7 +132,7 @@ impl DmaChannel {
         }
     }
 
-    fn xfer(&mut self, sb: &mut SysBus, irqs: &mut IrqBitmask) {
+    fn xfer(&mut self, sb: &mut SysBus) {
         let word_size = if self.ctrl.is_32bit() { 4 } else { 2 };
         let count = match self.internal.count {
             0 => match self.id {
@@ -171,7 +174,7 @@ impl DmaChannel {
             }
         }
         if self.ctrl.is_triggering_irq() {
-            irqs.add_irq(self.irq);
+            interrupt::signal_irq(&self.interrupt_flags, self.irq);
         }
         if self.ctrl.repeat() {
             self.start_cycles = self.cycles;
@@ -194,13 +197,13 @@ pub struct DmaController {
 }
 
 impl DmaController {
-    pub fn new() -> DmaController {
+    pub fn new(interrupt_flags: SharedInterruptFlags) -> DmaController {
         DmaController {
             channels: [
-                DmaChannel::new(0),
+                DmaChannel::new(0, interrupt_flags.clone()),
-                DmaChannel::new(1),
+                DmaChannel::new(1, interrupt_flags.clone()),
-                DmaChannel::new(2),
+                DmaChannel::new(2, interrupt_flags.clone()),
-                DmaChannel::new(3),
+                DmaChannel::new(3, interrupt_flags.clone()),
             ],
             pending_set: 0,
             cycles: 0,
@@ -211,10 +214,10 @@ impl DmaController {
         self.pending_set != 0
     }
 
-    pub fn perform_work(&mut self, sb: &mut SysBus, irqs: &mut IrqBitmask) {
+    pub fn perform_work(&mut self, sb: &mut SysBus) {
         for id in 0..4 {
             if self.pending_set & (1 << id) != 0 {
-                self.channels[id].xfer(sb, irqs);
+                self.channels[id].xfer(sb);
             }
         }
         self.pending_set = 0;

rustboyadvance-core/src/gba.rs

@@ -1,5 +1,5 @@
 /// Struct containing everything
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 
 use bincode;
@@ -7,11 +7,13 @@ use serde::{Deserialize, Serialize};
 
 use super::arm7tdmi;
 use super::cartridge::Cartridge;
+use super::dma::DmaController;
 use super::gpu::*;
 use super::interrupt::*;
 use super::iodev::*;
 use super::sound::SoundController;
 use super::sysbus::SysBus;
+use super::timer::Timers;
 
 use super::{AudioInterface, InputInterface, VideoInterface};
 
@@ -60,12 +62,17 @@ impl GameBoyAdvance {
             true => info!("Verified bios rom"),
             false => warn!("This is not the real bios rom, some games may not be compatible"),
         };
-        let gpu = Box::new(Gpu::new());
+
+        let interrupt_flags = Rc::new(Cell::new(IrqBitmask(0)));
+
+        let intc = InterruptController::new(interrupt_flags.clone());
+        let gpu = Box::new(Gpu::new(interrupt_flags.clone()));
+        let dmac = DmaController::new(interrupt_flags.clone());
+        let timers = Timers::new(interrupt_flags.clone());
         let sound_controller = Box::new(SoundController::new(
             audio_device.borrow().get_sample_rate() as f32,
         ));
-
+        let io = IoDevices::new(intc, gpu, dmac, timers, sound_controller);
-        let io = IoDevices::new(gpu, sound_controller);
         let sysbus = Box::new(SysBus::new(io, bios_rom, gamepak));
 
         let cpu = arm7tdmi::Core::new();
@@ -204,14 +211,11 @@ impl GameBoyAdvance {
             &mut (*ptr).io as &mut IoDevices
         };
 
-        let mut irqs = IrqBitmask(0);
-
         let mut cycles_left = self.cycles_to_next_event;
         let mut cycles_to_next_event = std::usize::MAX;
         let mut cycles = 0;
 
         while cycles_left > 0 {
-            let mut irqs = IrqBitmask(0);
             let _cycles = if !io.dmac.is_active() {
                 if HaltState::Running == io.haltcnt {
                     self.step_cpu(io)
@@ -220,8 +224,7 @@ impl GameBoyAdvance {
                     break;
                 }
             } else {
-                io.dmac.perform_work(&mut self.sysbus, &mut irqs);
+                io.dmac.perform_work(&mut self.sysbus);
-                io.intc.request_irqs(irqs);
                 return cycles;
             };
 
@@ -233,10 +236,9 @@ impl GameBoyAdvance {
         }
 
         // update gpu & sound
-        io.timers.update(cycles, &mut self.sysbus, &mut irqs);
+        io.timers.update(cycles, &mut self.sysbus);
         io.gpu.update(
             cycles,
-            &mut irqs,
             &mut cycles_to_next_event,
             self.sysbus.as_mut(),
             &self.video_device,
@@ -244,7 +246,6 @@ impl GameBoyAdvance {
         io.sound
             .update(cycles, &mut cycles_to_next_event, &self.audio_device);
         self.cycles_to_next_event = cycles_to_next_event;
-        io.intc.request_irqs(irqs);
 
         cycles
     }
@@ -260,28 +261,23 @@ impl GameBoyAdvance {
         };
 
         // clear any pending DMAs
-        let mut irqs = IrqBitmask(0);
         while io.dmac.is_active() {
-            io.dmac.perform_work(&mut self.sysbus, &mut irqs);
+            io.dmac.perform_work(&mut self.sysbus);
         }
-        io.intc.request_irqs(irqs);
 
         let cycles = self.step_cpu(io);
         let breakpoint = self.check_breakpoint();
 
-        irqs = IrqBitmask(0);
         let mut _ignored = 0;
         // update gpu & sound
-        io.timers.update(cycles, &mut self.sysbus, &mut irqs);
+        io.timers.update(cycles, &mut self.sysbus);
         io.gpu.update(
             cycles,
-            &mut irqs,
             &mut _ignored,
             self.sysbus.as_mut(),
             &self.video_device,
         );
         io.sound.update(cycles, &mut _ignored, &self.audio_device);
-        io.intc.request_irqs(irqs);
 
         breakpoint
     }

rustboyadvance-core/src/gpu/mod.rs

@@ -5,7 +5,7 @@ use std::rc::Rc;
 use serde::{Deserialize, Serialize};
 
 use super::dma::{DmaNotifer, TIMING_HBLANK, TIMING_VBLANK};
-use super::interrupt::IrqBitmask;
+use super::interrupt::{self, Interrupt, SharedInterruptFlags};
 pub use super::sysbus::consts::*;
 use super::sysbus::BoxedMemory;
 use super::VideoInterface;
@@ -174,6 +174,7 @@ type VideoDeviceRcRefCell = Rc<RefCell<dyn VideoInterface>>;
 #[derive(Serialize, Deserialize, Clone, DebugStub)]
 pub struct Gpu {
     pub state: GpuState,
+    interrupt_flags: SharedInterruptFlags,
 
     /// how many cycles left until next gpu state ?
     cycles_left_for_current_state: usize,
@@ -270,8 +271,9 @@ impl Bus for Gpu {
 }
 
 impl Gpu {
-    pub fn new() -> Gpu {
+    pub fn new(interrupt_flags: SharedInterruptFlags) -> Gpu {
         Gpu {
+            interrupt_flags,
             dispcnt: DisplayControl(0x80),
             dispstat: DisplayStatus(0),
             backgrounds: [
@@ -415,17 +417,6 @@ impl Gpu {
         // self.mosaic_sfx();
     }
 
-    fn update_vcount(&mut self, value: usize, irqs: &mut IrqBitmask) {
-        self.vcount = value;
-        let vcount_setting = self.dispstat.vcount_setting();
-        self.dispstat
-            .set_vcount_flag(vcount_setting == self.vcount as u16);
-
-        if self.dispstat.vcount_irq_enable() && self.dispstat.get_vcount_flag() {
-            irqs.set_LCD_VCounterMatch(true);
-        }
-    }
-
     /// Clears the gpu obj buffer
     pub fn obj_buffer_reset(&mut self) {
         for x in self.obj_buffer.iter_mut() {
@@ -440,12 +431,24 @@ impl Gpu {
     pub fn on_state_completed<D>(
         &mut self,
         completed: GpuState,
-        irqs: &mut IrqBitmask,
         dma_notifier: &mut D,
         video_device: &VideoDeviceRcRefCell,
     ) where
         D: DmaNotifer,
     {
+        macro_rules! update_vcount {
+            ($value:expr) => {
+                self.vcount = $value;
+                let vcount_setting = self.dispstat.vcount_setting();
+                self.dispstat
+                    .set_vcount_flag(vcount_setting == self.vcount as u16);
+
+                if self.dispstat.vcount_irq_enable() && self.dispstat.get_vcount_flag() {
+                    interrupt::signal_irq(&self.interrupt_flags, Interrupt::LCD_VCounterMatch);
+                }
+            };
+        }
+
         match completed {
             HDraw => {
                 // Transition to HBlank
@@ -454,12 +457,12 @@ impl Gpu {
                 self.dispstat.set_hblank_flag(true);
 
                 if self.dispstat.hblank_irq_enable() {
-                    irqs.set_LCD_HBlank(true);
+                    interrupt::signal_irq(&self.interrupt_flags, Interrupt::LCD_HBlank);
                 };
                 dma_notifier.notify(TIMING_HBLANK);
             }
             HBlank => {
-                self.update_vcount(self.vcount + 1, irqs);
+                update_vcount!(self.vcount + 1);
 
                 if self.vcount < DISPLAY_HEIGHT {
                     self.state = HDraw;
@@ -481,7 +484,7 @@ impl Gpu {
                     self.dispstat.set_vblank_flag(true);
                     self.dispstat.set_hblank_flag(false);
                     if self.dispstat.vblank_irq_enable() {
-                        irqs.set_LCD_VBlank(true);
+                        interrupt::signal_irq(&self.interrupt_flags, Interrupt::LCD_VBlank);
                     };
 
                     dma_notifier.notify(TIMING_VBLANK);
@@ -497,17 +500,17 @@ impl Gpu {
 
                 self.dispstat.set_hblank_flag(true);
                 if self.dispstat.hblank_irq_enable() {
-                    irqs.set_LCD_HBlank(true);
+                    interrupt::signal_irq(&self.interrupt_flags, Interrupt::LCD_HBlank);
                 };
             }
             VBlankHBlank => {
                 if self.vcount < DISPLAY_HEIGHT + VBLANK_LINES - 1 {
-                    self.update_vcount(self.vcount + 1, irqs);
+                    update_vcount!(self.vcount + 1);
                     self.dispstat.set_hblank_flag(false);
                     self.cycles_left_for_current_state = CYCLES_HDRAW;
                     self.state = VBlankHDraw;
                 } else {
-                    self.update_vcount(0, irqs);
+                    update_vcount!(0);
                     self.dispstat.set_vblank_flag(false);
                     self.dispstat.set_hblank_flag(false);
                     self.render_scanline();
@@ -521,7 +524,6 @@ impl Gpu {
     pub fn update<D>(
         &mut self,
         mut cycles: usize,
-        irqs: &mut IrqBitmask,
         cycles_to_next_event: &mut usize,
         dma_notifier: &mut D,
         video_device: &VideoDeviceRcRefCell,
@@ -531,7 +533,7 @@ impl Gpu {
         loop {
             if self.cycles_left_for_current_state <= cycles {
                 cycles -= self.cycles_left_for_current_state;
-                self.on_state_completed(self.state, irqs, dma_notifier, video_device);
+                self.on_state_completed(self.state, dma_notifier, video_device);
             } else {
                 self.cycles_left_for_current_state -= cycles;
                 break;
@@ -547,6 +549,8 @@ impl Gpu {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::cell::Cell;
+    use std::rc::Rc;
 
     struct NopDmaNotifer;
     impl DmaNotifer for NopDmaNotifer {
@@ -566,10 +570,9 @@ mod tests {
 
     #[test]
     fn test_gpu_state_machine() {
-        let mut gpu = Gpu::new();
+        let mut gpu = Gpu::new(Rc::new(Cell::new(Default::default())));
         let video = Rc::new(RefCell::new(TestVideoInterface::default()));
         let video_clone: VideoDeviceRcRefCell = video.clone();
-        let mut irqs = IrqBitmask(0);
         let mut dma_notifier = NopDmaNotifer;
         let mut cycles_to_next_event = CYCLES_FULL_REFRESH;
 
@@ -582,7 +585,6 @@ mod tests {
             ($cycles:expr) => {
                 gpu.update(
                     $cycles,
-                    &mut irqs,
                     &mut cycles_to_next_event,
                     &mut dma_notifier,
                     &video_clone,
@@ -607,7 +609,7 @@ mod tests {
 
             update!(CYCLES_HBLANK);
 
-            assert_eq!(irqs.LCD_VCounterMatch(), false);
+            assert_eq!(gpu.interrupt_flags.get().LCD_VCounterMatch(), false);
         }
 
         assert_eq!(video.borrow().frame_counter, 1);
@@ -623,7 +625,7 @@ mod tests {
             assert_eq!(gpu.dispstat.get_hblank_flag(), true);
             assert_eq!(gpu.dispstat.get_vblank_flag(), true);
             assert_eq!(gpu.state, GpuState::VBlankHBlank);
-            assert_eq!(irqs.LCD_VCounterMatch(), false);
+            assert_eq!(gpu.interrupt_flags.get().LCD_VCounterMatch(), false);
 
             update!(CYCLES_HBLANK);
         }
@@ -631,7 +633,7 @@ mod tests {
         assert_eq!(video.borrow().frame_counter, 1);
         assert_eq!(total_cycles, CYCLES_FULL_REFRESH);
 
-        assert_eq!(irqs.LCD_VCounterMatch(), true);
+        assert_eq!(gpu.interrupt_flags.get().LCD_VCounterMatch(), true);
         assert_eq!(gpu.cycles_left_for_current_state, CYCLES_HDRAW);
         assert_eq!(gpu.state, GpuState::HDraw);
         assert_eq!(gpu.vcount, 0);

rustboyadvance-core/src/interrupt.rs

@@ -1,3 +1,6 @@
+use std::cell::Cell;
+use std::rc::Rc;
+
 use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Debug, Primitive, Copy, Clone, PartialEq)]
@@ -23,29 +26,42 @@ pub enum Interrupt {
 pub struct InterruptController {
     pub interrupt_master_enable: bool,
     pub interrupt_enable: IrqBitmask,
-    pub interrupt_flags: IrqBitmask,
+    pub interrupt_flags: SharedInterruptFlags,
 }
 
 impl InterruptController {
-    pub fn new() -> InterruptController {
+    pub fn new(interrupt_flags: SharedInterruptFlags) -> InterruptController {
         InterruptController {
+            interrupt_flags,
             interrupt_master_enable: false,
             ..Default::default()
         }
     }
 
-    pub fn request_irqs(&mut self, flags: IrqBitmask) {
+    #[inline]
-        self.interrupt_flags.0 |= flags.0;
+    pub fn irq_pending(&self) -> bool {
+        self.interrupt_master_enable
+            & ((self.interrupt_flags.get().value() & self.interrupt_enable.0) != 0)
     }
 
-    pub fn irq_pending(&self) -> bool {
+    #[inline]
-        self.interrupt_master_enable & ((self.interrupt_flags.0 & self.interrupt_enable.0) != 0)
+    pub fn clear(&mut self, value: u16) {
+        let _if = self.interrupt_flags.get();
+        let new_if = _if.0 & !value;
+        self.interrupt_flags.set(IrqBitmask(new_if));
     }
 }
 
+#[inline]
+pub fn signal_irq(interrupt_flags: &SharedInterruptFlags, i: Interrupt) {
+    let _if = interrupt_flags.get();
+    let new_if = _if.0 | 1 << (i as usize);
+    interrupt_flags.set(IrqBitmask(new_if));
+}
+
 impl IrqBitmask {
-    pub fn add_irq(&mut self, i: Interrupt) {
+    pub fn value(&self) -> u16 {
-        self.0 |= 1 << (i as usize);
+        self.0
     }
 }
 
@@ -83,3 +99,5 @@ bitfield! {
     #[allow(non_snake_case)]
     pub GamePak, set_GamePak: 13;
 }
+
+pub type SharedInterruptFlags = Rc<Cell<IrqBitmask>>;

rustboyadvance-core/src/iodev.rs

@@ -41,13 +41,19 @@ pub struct IoDevices {
 }
 
 impl IoDevices {
-    pub fn new(gpu: Box<Gpu>, sound_controller: Box<SoundController>) -> IoDevices {
+    pub fn new(
+        intc: InterruptController,
+        gpu: Box<Gpu>,
+        dmac: DmaController,
+        timers: Timers,
+        sound_controller: Box<SoundController>,
+    ) -> IoDevices {
         IoDevices {
-            gpu: gpu,
+            intc,
+            gpu,
+            timers,
+            dmac,
             sound: sound_controller,
-            timers: Timers::new(),
-            dmac: DmaController::new(),
-            intc: InterruptController::new(),
             post_boot_flag: false,
             haltcnt: HaltState::Running,
             keyinput: keypad::KEYINPUT_ALL_RELEASED,
@@ -92,7 +98,7 @@ impl Bus for IoDevices {
 
             REG_IME => io.intc.interrupt_master_enable as u16,
             REG_IE => io.intc.interrupt_enable.0 as u16,
-            REG_IF => io.intc.interrupt_flags.0 as u16,
+            REG_IF => io.intc.interrupt_flags.get().value() as u16,
 
             REG_TM0CNT_L..=REG_TM3CNT_H => io.timers.handle_read(io_addr),
 
@@ -222,7 +228,7 @@ impl Bus for IoDevices {
 
             REG_IME => io.intc.interrupt_master_enable = value != 0,
             REG_IE => io.intc.interrupt_enable.0 = value,
-            REG_IF => io.intc.interrupt_flags.0 &= !value,
+            REG_IF => io.intc.clear(value),
 
             REG_TM0CNT_L..=REG_TM3CNT_H => io.timers.handle_write(io_addr, value),
 

rustboyadvance-core/src/lib.rs

@@ -43,7 +43,7 @@ pub use sysbus::SysBus;
 pub mod interrupt;
 pub mod iodev;
 pub use interrupt::Interrupt;
-pub use interrupt::IrqBitmask;
+pub use interrupt::SharedInterruptFlags;
 pub mod gba;
 pub use gba::GameBoyAdvance;
 pub mod bus;

rustboyadvance-core/src/timer.rs

@@ -1,4 +1,4 @@
-use super::interrupt::{Interrupt, IrqBitmask};
+use super::interrupt::{self, Interrupt, SharedInterruptFlags};
 use super::iodev::consts::*;
 use super::sysbus::SysBus;
 
@@ -15,19 +15,21 @@ pub struct Timer {
     pub initial_data: u16,
 
     irq: Interrupt,
+    interrupt_flags: SharedInterruptFlags,
     timer_id: usize,
     cycles: usize,
     prescalar_shift: usize,
 }
 
 impl Timer {
-    pub fn new(timer_id: usize) -> Timer {
+    pub fn new(timer_id: usize, interrupt_flags: SharedInterruptFlags) -> Timer {
         if timer_id > 3 {
             panic!("invalid timer id {}", timer_id);
         }
         Timer {
             timer_id: timer_id,
             irq: Interrupt::from_usize(timer_id + 3).unwrap(),
+            interrupt_flags,
             data: 0,
             ctl: TimerCtl(0),
             initial_data: 0,
@@ -43,7 +45,7 @@ impl Timer {
 
     /// increments the timer with an amount of ticks
     /// returns the number of times it overflowed
-    fn update(&mut self, ticks: usize, irqs: &mut IrqBitmask) -> usize {
+    fn update(&mut self, ticks: usize) -> usize {
         let mut ticks = ticks as u32;
         let mut num_overflows = 0;
 
@@ -59,7 +61,7 @@ impl Timer {
             ticks = ticks % ticks_remaining;
 
             if self.ctl.irq_enabled() {
-                irqs.add_irq(self.irq);
+                interrupt::signal_irq(&self.interrupt_flags, self.irq);
             }
         }
 
@@ -90,9 +92,14 @@ impl std::ops::IndexMut<usize> for Timers {
 }
 
 impl Timers {
-    pub fn new() -> Timers {
+    pub fn new(interrupt_flags: SharedInterruptFlags) -> Timers {
         Timers {
-            timers: [Timer::new(0), Timer::new(1), Timer::new(2), Timer::new(3)],
+            timers: [
+                Timer::new(0, interrupt_flags.clone()),
+                Timer::new(1, interrupt_flags.clone()),
+                Timer::new(2, interrupt_flags.clone()),
+                Timer::new(3, interrupt_flags.clone()),
+            ],
             running_timers: 0,
             trace: false,
         }
@@ -163,7 +170,7 @@ impl Timers {
         }
     }
 
-    pub fn update(&mut self, cycles: usize, sb: &mut SysBus, irqs: &mut IrqBitmask) {
+    pub fn update(&mut self, cycles: usize, sb: &mut SysBus) {
         for id in 0..4 {
             if self.running_timers & (1 << id) == 0 {
                 continue;
@@ -174,14 +181,14 @@ impl Timers {
 
                 let cycles = timer.cycles + cycles;
                 let inc = cycles >> timer.prescalar_shift;
-                let num_overflows = timer.update(inc, irqs);
+                let num_overflows = timer.update(inc);
                 timer.cycles = cycles & ((1 << timer.prescalar_shift) - 1);
 
                 if num_overflows > 0 {
                     if id != 3 {
                         let next_timer = &mut self.timers[id + 1];
                         if next_timer.ctl.cascade() {
-                            next_timer.update(num_overflows, irqs);
+                            next_timer.update(num_overflows);
                         }
                     }
                     if id == 0 || id == 1 {