feat: implement more accurate memory bus conflicts with DMA

.forgejo/workflows/action.yml

@@ -30,6 +30,10 @@ jobs:
         if: always()
         run: ./target/release/meowgb-tests test-roms/blargg/roms/mem_timing.gb test -m 100000000 -s meowgb-tests/expected_output/mem_timing.bin
 
+      - name: Run test ROM (mooneye-test-suite add_sp_e_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/add_sp_e_timing.gb test -m 100000000 -s meowgb-tests/expected_output/add_sp_e_timing.bin
+
       - name: Run test ROM (mooneye-test-suite basic)
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/basic.gb test -m 100000000 -s meowgb-tests/expected_output/basic.bin
@@ -42,6 +46,22 @@ jobs:
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/boot_regs-dmgABC.gb test -m 100000000 -s meowgb-tests/expected_output/boot_regs-dmgABC.bin
 
+      - name: Run test ROM (mooneye-test-suite call_cc_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_cc_timing.gb test -m 100000000 -s meowgb-tests/expected_output/call_cc_timing.bin
+
+      - name: Run test ROM (mooneye-test-suite call_cc_timing2)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_cc_timing2.gb test -m 100000000 -s meowgb-tests/expected_output/call_cc_timing2.bin
+
+      - name: Run test ROM (mooneye-test-suite call_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_timing.gb test -m 100000000 -s meowgb-tests/expected_output/call_timing.bin
+
+      - name: Run test ROM (mooneye-test-suite call_timing2)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_timing2.gb test -m 100000000 -s meowgb-tests/expected_output/call_timing2.bin
+
       - name: Run test ROM (mooneye-test-suite daa)
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/daa.gb test -m 100000000 -s meowgb-tests/expected_output/daa.bin

.github/workflows/action.yml

@@ -30,6 +30,10 @@ jobs:
         if: always()
         run: ./target/release/meowgb-tests test-roms/blargg/roms/mem_timing.gb test -m 100000000 -s meowgb-tests/expected_output/mem_timing.bin
 
+      - name: Run test ROM (mooneye-test-suite add_sp_e_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/add_sp_e_timing.gb test -m 100000000 -s meowgb-tests/expected_output/add_sp_e_timing.bin
+
       - name: Run test ROM (mooneye-test-suite basic)
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/basic.gb test -m 100000000 -s meowgb-tests/expected_output/basic.bin
@@ -42,6 +46,22 @@ jobs:
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/boot_regs-dmgABC.gb test -m 100000000 -s meowgb-tests/expected_output/boot_regs-dmgABC.bin
 
+      - name: Run test ROM (mooneye-test-suite call_cc_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_cc_timing.gb test -m 100000000 -s meowgb-tests/expected_output/call_cc_timing.bin
+
+      - name: Run test ROM (mooneye-test-suite call_cc_timing2)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_cc_timing2.gb test -m 100000000 -s meowgb-tests/expected_output/call_cc_timing2.bin
+
+      - name: Run test ROM (mooneye-test-suite call_timing)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_timing.gb test -m 100000000 -s meowgb-tests/expected_output/call_timing.bin
+
+      - name: Run test ROM (mooneye-test-suite call_timing2)
+        if: always()
+        run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/call_timing2.gb test -m 100000000 -s meowgb-tests/expected_output/call_timing2.bin
+
       - name: Run test ROM (mooneye-test-suite daa)
         if: always()
         run: ./target/release/meowgb-tests test-roms/mooneye-test-suite/roms/daa.gb test -m 100000000 -s meowgb-tests/expected_output/daa.bin

meowgb-core/src/gameboy.rs

@@ -106,15 +106,15 @@ impl<S: SerialWriter> Gameboy<S> {
 	pub fn tick(&mut self) -> bool {
 		if self.tick_count == 0 {
 			cpu::tick_cpu(self);
-			let redraw_requested = self.ppu.tick(&mut self.interrupts);
+			let redraw_requested = self.ppu.tick(&self.dma, &mut self.interrupts);
 			self.dma.tick_dma(&mut self.ppu, &self.memory, self.cartridge.as_deref());
 			self.serial.tick(&mut self.interrupts);
 			self.timer.tick(&mut self.interrupts);
 
 			self.tick_count += 1;
 			redraw_requested
 		} else {
-			let redraw_requested = self.ppu.tick(&mut self.interrupts);
+			let redraw_requested = self.ppu.tick(&self.dma, &mut self.interrupts);
 			self.timer.tick(&mut self.interrupts);
 			self.tick_count += 1;
 			self.tick_count %= 4;
@@ -328,7 +328,7 @@ impl<S: SerialWriter> Gameboy<S> {
 		assert!(!self.registers.mem_op_happened);
 		assert!(self.registers.mem_read_hold.is_none());
 		self.registers.mem_op_happened = true;
-		let value = match self.ppu.dma_occuring {
+		let value = match self.dma.is_conflict(address) {
 			true => match address {
 				0..=0xFEFF => 0xFF,
 				0xFF00..=0xFF7F => self.cpu_read_io(address),
@@ -366,7 +366,7 @@ impl<S: SerialWriter> Gameboy<S> {
 		self.registers.mem_op_happened = true;
 		self.last_write = Some((address, value));
 
-		match self.ppu.dma_occuring {
+		match self.dma.is_conflict(address) {
 			true => match address {
 				0..=0xFEFF => {}
 				0xFF00..=0xFF7F => self.cpu_write_io(address, value),

meowgb-core/src/gameboy/dma.rs

@@ -1,16 +1,64 @@
 use super::{mapper::Mapper, memory::Memory, ppu::Ppu};
 
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DmaMemoryBus {
+	External,
+	Video,
+	Other,
+}
+
+impl DmaMemoryBus {
+	pub fn from_base(base: u8) -> Self {
+		match base {
+			0..=0x7F
+			| 0xA0..=0xFD => Self::External,
+			0x80..=0x9F => Self::Video,
+			_ => Self::Other
+		}
+	}
+
+	pub fn conflict_in_range(self, address: u16) -> bool {
+		let base = (address >> 8) as u8;
+
+		if base == 0xFE {
+			true
+		} else if base == 0xFF {
+			false
+		} else {
+			match self {
+				DmaMemoryBus::External => base < 0x7F || (base >= 0xA0 && base <= 0xFD),
+				DmaMemoryBus::Video => base >= 0x80 && base <= 0x9F,
+				DmaMemoryBus::Other => false,
+			}
+		}
+	}
+}
+
+#[derive(Debug, Clone, Copy)]
 pub struct DmaState {
 	original_base: u8,
+	pub dma_in_progress: bool,
 	pub base: u8,
 	pub remaining_cycles: u8,
 	restarting: Option<(u8, bool)>,
 }
 
 impl DmaState {
+	pub fn is_conflict(&self, address: u16) -> bool {
+		self.in_progress().map(|bus| {
+			bus.conflict_in_range(address)
+		}).unwrap_or_default()
+	}
+
+	pub fn in_progress(&self) -> Option<DmaMemoryBus> {
+		match self.dma_in_progress {
+			true => Some(DmaMemoryBus::from_base(self.original_base)),
+			false => None,
+		}
+	}
+
 	pub fn new() -> Self {
-		Self { original_base: 0, base: 0, remaining_cycles: 0, restarting: None }
+		Self { dma_in_progress: false, original_base: 0, base: 0, remaining_cycles: 0, restarting: None }
 	}
 
 	pub fn init_request(&mut self, base: u8) {
@@ -34,7 +82,7 @@ impl DmaState {
 			None => {}
 		}
 
-		ppu.dma_occuring = self.remaining_cycles > 0;
+		self.dma_in_progress = self.remaining_cycles > 0;
 
 		if self.remaining_cycles > 0 {
 			let offset = 0xA0 - self.remaining_cycles;

meowgb-core/src/gameboy/ppu.rs

@@ -1,4 +1,4 @@
-use super::interrupts::Interrupts;
+use super::{interrupts::Interrupts, dma::DmaState};
 
 pub const FB_HEIGHT: u32 = 144;
 pub const FB_WIDTH: u32 = 160;
@@ -298,8 +298,6 @@ pub struct Ppu {
 	sprite_buffer: [Option<OAMEntry>; 10],
 	sprite_count: usize,
 
-	pub dma_occuring: bool,
-
 	current_draw_state: Option<LineDrawingState>,
 	wy_match: bool,
 
@@ -355,7 +353,6 @@ impl Ppu {
 			dot_target: 0,
 			sprite_buffer: [None; 10],
 			sprite_count: 0,
-			dma_occuring: false,
 			current_draw_state: None,
 			wy_match: false,
 			first_frame: true,
@@ -452,15 +449,14 @@ impl Ppu {
 		((high & 0b1) << 1) | low & 0b1
 	}
 
-	fn internal_read_oam(&mut self, offset: usize) -> u8 {
-		match self.dma_occuring && !OVERRIDE_PPU_MEMORY_ACCESS {
+	fn internal_read_oam(&mut self, dma_state: &DmaState, offset: usize) -> u8 {
+		match dma_state.in_progress().is_some() && !OVERRIDE_PPU_MEMORY_ACCESS {
 			true => 0xFF,
 			false => self.oam[offset as usize],
 		}
 	}
 
 	pub fn dma_write_oam(&mut self, offset: u8, value: u8) {
-		assert!(self.dma_occuring);
 		self.oam[offset as usize] = value;
 	}
 
@@ -569,7 +565,7 @@ impl Ppu {
 		self.registers.ly_lyc = self.registers.ly == self.registers.lyc;
 	}
 
-	pub fn tick(&mut self, interrupts: &mut Interrupts) -> bool {
+	pub fn tick(&mut self, dma_state: &DmaState, interrupts: &mut Interrupts) -> bool {
 		if self.enabled() {
 			self.registers.cycles_since_last_ly_increment += 1;
 			match self.mode() {
@@ -587,10 +583,10 @@ impl Ppu {
 						let oam_item_idx: usize = (self.current_dot as usize / 2) * 4;
 
 						let oam_entry = OAMEntry::parse([
-							self.internal_read_oam(oam_item_idx),
-							self.internal_read_oam(oam_item_idx + 1),
-							self.internal_read_oam(oam_item_idx + 2),
-							self.internal_read_oam(oam_item_idx + 3),
+							self.internal_read_oam(dma_state, oam_item_idx),
+							self.internal_read_oam(dma_state, oam_item_idx + 1),
+							self.internal_read_oam(dma_state, oam_item_idx + 2),
+							self.internal_read_oam(dma_state, oam_item_idx + 3),
 						]);
 
 						let sprite_height = self.sprite_height();

meowgb-tests/expected_output/add_sp_e_timing.bin

@@ -0,0 +1 @@
+"

meowgb-tests/expected_output/call_cc_timing.bin

@@ -0,0 +1 @@
+"

meowgb-tests/expected_output/call_cc_timing2.bin

@@ -0,0 +1 @@
+"

meowgb-tests/expected_output/call_timing.bin

@@ -0,0 +1 @@
+"

meowgb-tests/expected_output/call_timing2.bin

@@ -0,0 +1 @@
+"

meowgb/src/main.rs

@@ -193,8 +193,6 @@ pub fn run_gameboy(
 			gameboy.debugging |= bp_triggered;
 
 			if bp_triggered || step {
-				gameboy.debugging = bp_triggered;
-
 				let now = time::OffsetDateTime::now_utc();
 
 				if let Some(debugging_tbf) = debugging_tbf {

meowgb/src/window/overlay.rs

@@ -1,6 +1,6 @@
 /// Provides an [egui] based overlay for debugigng the emulator whilst it is
 /// running
-use egui::{ClippedPrimitive, Context, Grid, TexturesDelta};
+use egui::{ClippedPrimitive, Context, Grid, TexturesDelta, RichText, Color32};
 use egui_wgpu::renderer::{Renderer, ScreenDescriptor};
 use meowgb_core::gameboy::serial::SerialWriter;
 use pixels::{wgpu, PixelsContext};
@@ -28,6 +28,7 @@ pub struct GuiWindowState {
 	pub wram_window_open: bool,
 	pub oam_window_open: bool,
 	pub hram_window_open: bool,
+	pub dma_window_open: bool,
 }
 
 impl GuiWindowState {
@@ -39,6 +40,7 @@ impl GuiWindowState {
 		self.wram_window_open = false;
 		self.oam_window_open = false;
 		self.hram_window_open = false;
+		self.dma_window_open = false;
 	}
 
 	pub fn any_open(&self) -> bool {
@@ -49,6 +51,7 @@ impl GuiWindowState {
 			|| self.wram_window_open
 			|| self.oam_window_open
 			|| self.hram_window_open
+			|| self.dma_window_open
 	}
 }
 
@@ -68,6 +71,7 @@ pub struct Gui {
 	pub is_debugging: bool,
 	pub breakpoints: [[bool; 3]; 0x10000],
 	pub sender: std::sync::mpsc::Sender<EmulatorWindowEvent>,
+	pub dma: meowgb_core::gameboy::dma::DmaState,
 }
 
 impl Framework {
@@ -124,6 +128,7 @@ impl Framework {
 		self.gui.oam = gameboy.gameboy.ppu.oam;
 		self.gui.hram = gameboy.gameboy.memory.hram;
 		self.gui.wram = gameboy.gameboy.memory.wram;
+		self.gui.dma = gameboy.gameboy.dma;
 
 		// Run the egui frame and create all paint jobs to prepare for rendering.
 		let raw_input = self.egui_state.take_egui_input(window);
@@ -189,6 +194,7 @@ impl Gui {
 				wram_window_open: false,
 				oam_window_open: false,
 				hram_window_open: false,
+    			dma_window_open: false,
 			},
 			state_restore: None,
 			registers: gameboy.gameboy.registers,
@@ -203,6 +209,7 @@ impl Gui {
 			wram: gameboy.gameboy.memory.wram,
 			hram: gameboy.gameboy.memory.hram,
 			oam: gameboy.gameboy.ppu.oam,
+			dma: gameboy.gameboy.dma
 		}
 	}
 
@@ -235,6 +242,10 @@ impl Gui {
 			if ui.button("Toggle OAM Window").clicked() {
 				self.state.oam_window_open = !self.state.oam_window_open;
 			}
+
+			if ui.button("Toggle DMA Window").clicked() {
+				self.state.dma_window_open = !self.state.dma_window_open;
+			}
 		});
 
 		egui::Window::new("Register State").open(&mut self.state.register_window_open).show(
@@ -336,6 +347,20 @@ impl Gui {
 			});
 		});
 
+		egui::Window::new("DMA").vscroll(true).open(&mut self.state.dma_window_open).show(ctx, |ui| {
+			if let Some(bus) = self.dma.in_progress() {
+				ui.heading(RichText::new(format!("Active ({:#?} Bus)", bus)).color(Color32::LIGHT_GREEN));
+			} else {
+				ui.heading(RichText::new("Inactive").color(Color32::LIGHT_RED));
+			}
+
+			let offset = (0xA0 - self.dma.remaining_cycles) as u16;
+			ui.label(format!("Read Address:  {:#04X}", ((self.dma.base as u16) << 8) | offset));
+			ui.label(format!("Write Address: {:#04X}", 0xFE00 | offset));
+			ui.label(format!("Base: {:#04X}", (self.dma.base as u16) << 8));
+			ui.label(format!("Remaining Bytes: {:#02X}", self.dma.remaining_cycles));
+		});
+
 		egui::Window::new("HRAM").vscroll(true).open(&mut self.state.hram_window_open).show(ctx, |ui| {
 			egui::Grid::new("memory_ov_hram").show(ui, |ui| {
 				ui.label("ROW: 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F");