From 441f5ad90635700e11d3ff0a771d9d70418905e2 Mon Sep 17 00:00:00 2001 From: Michel Heily Date: Tue, 26 May 2020 01:46:05 +0300 Subject: [PATCH] Refactor IRQ signaling Use Rc 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 | 25 ++++++------ rustboyadvance-core/src/gba.rs | 34 +++++++--------- rustboyadvance-core/src/gpu/mod.rs | 58 ++++++++++++++-------------- rustboyadvance-core/src/interrupt.rs | 34 ++++++++++++---- rustboyadvance-core/src/iodev.rs | 20 ++++++---- rustboyadvance-core/src/lib.rs | 2 +- rustboyadvance-core/src/timer.rs | 25 +++++++----- 7 files changed, 115 insertions(+), 83 deletions(-) diff --git a/rustboyadvance-core/src/dma.rs b/rustboyadvance-core/src/dma.rs index 42814d8..43aedd5 100644 --- a/rustboyadvance-core/src/dma.rs +++ b/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(1), - DmaChannel::new(2), - DmaChannel::new(3), + DmaChannel::new(0, interrupt_flags.clone()), + DmaChannel::new(1, interrupt_flags.clone()), + DmaChannel::new(2, interrupt_flags.clone()), + 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; diff --git a/rustboyadvance-core/src/gba.rs b/rustboyadvance-core/src/gba.rs index 432a8f6..7201838 100644 --- a/rustboyadvance-core/src/gba.rs +++ b/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(gpu, sound_controller); + let io = IoDevices::new(intc, gpu, dmac, timers, 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.intc.request_irqs(irqs); + io.dmac.perform_work(&mut self.sysbus); 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 } diff --git a/rustboyadvance-core/src/gpu/mod.rs b/rustboyadvance-core/src/gpu/mod.rs index 1302560..06a181e 100644 --- a/rustboyadvance-core/src/gpu/mod.rs +++ b/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>; #[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( &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( &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); diff --git a/rustboyadvance-core/src/interrupt.rs b/rustboyadvance-core/src/interrupt.rs index 146ae15..8ccbb90 100644 --- a/rustboyadvance-core/src/interrupt.rs +++ b/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) { - self.interrupt_flags.0 |= flags.0; + #[inline] + 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 { - self.interrupt_master_enable & ((self.interrupt_flags.0 & self.interrupt_enable.0) != 0) + #[inline] + 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) { - self.0 |= 1 << (i as usize); + pub fn value(&self) -> u16 { + self.0 } } @@ -83,3 +99,5 @@ bitfield! { #[allow(non_snake_case)] pub GamePak, set_GamePak: 13; } + +pub type SharedInterruptFlags = Rc>; diff --git a/rustboyadvance-core/src/iodev.rs b/rustboyadvance-core/src/iodev.rs index 3f20ce2..70ba1b6 100644 --- a/rustboyadvance-core/src/iodev.rs +++ b/rustboyadvance-core/src/iodev.rs @@ -41,13 +41,19 @@ pub struct IoDevices { } impl IoDevices { - pub fn new(gpu: Box, sound_controller: Box) -> IoDevices { + pub fn new( + intc: InterruptController, + gpu: Box, + dmac: DmaController, + timers: Timers, + sound_controller: Box, + ) -> 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), diff --git a/rustboyadvance-core/src/lib.rs b/rustboyadvance-core/src/lib.rs index 0d2f5bb..62d43aa 100644 --- a/rustboyadvance-core/src/lib.rs +++ b/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; diff --git a/rustboyadvance-core/src/timer.rs b/rustboyadvance-core/src/timer.rs index fb99204..2c980a7 100644 --- a/rustboyadvance-core/src/timer.rs +++ b/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 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 {