From cadbec7c7780278d1f9dcb76b0936b5e6e1a5b4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Magalh=C3=A3es?= <joamag@gmail.com>
Date: Sun, 25 Feb 2024 18:28:41 +0000
Subject: [PATCH] feat: improved support for OAM DMA

Made sure that OAM DMA takes 160 cycles to process.
---
 src/dma.rs   | 65 ++++++++++++++++++++++++++++++++++++++++++++--------
 src/mmu.rs   | 39 +++++++++++++++++--------------
 src/state.rs |  2 +-
 3 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/src/dma.rs b/src/dma.rs
index 5d6b0357..532e0c23 100644
--- a/src/dma.rs
+++ b/src/dma.rs
@@ -1,5 +1,5 @@
 use crate::{
-    consts::{HDMA1_ADDR, HDMA2_ADDR, HDMA3_ADDR, HDMA4_ADDR, HDMA5_ADDR},
+    consts::{DMA_ADDR, HDMA1_ADDR, HDMA2_ADDR, HDMA3_ADDR, HDMA4_ADDR, HDMA5_ADDR},
     warnln,
 };
 
@@ -15,7 +15,10 @@ pub struct Dma {
     length: u16,
     pending: u16,
     mode: DmaMode,
-    active: bool,
+    cycles_dma: u16,
+    value_dma: u8,
+    active_dma: bool,
+    active_hdma: bool,
 }
 
 impl Dma {
@@ -26,7 +29,10 @@ impl Dma {
             length: 0x0,
             pending: 0x0,
             mode: DmaMode::General,
-            active: false,
+            cycles_dma: 0x0,
+            value_dma: 0x0,
+            active_dma: false,
+            active_hdma: false,
         }
     }
 
@@ -36,7 +42,10 @@ impl Dma {
         self.length = 0x0;
         self.pending = 0x0;
         self.mode = DmaMode::General;
-        self.active = false;
+        self.cycles_dma = 0x0;
+        self.value_dma = 0x0;
+        self.active_dma = false;
+        self.active_hdma = false;
     }
 
     pub fn clock(&mut self, _cycles: u16) {}
@@ -44,7 +53,9 @@ impl Dma {
     pub fn read(&mut self, addr: u16) -> u8 {
         match addr {
             // 0xFF55 — HDMA5: VRAM DMA length/mode/start (CGB only)
-            HDMA5_ADDR => ((self.pending >> 4) as u8).wrapping_sub(1) | ((!self.active as u8) << 7),
+            HDMA5_ADDR => {
+                ((self.pending >> 4) as u8).wrapping_sub(1) | ((!self.active_hdma as u8) << 7)
+            }
             _ => {
                 warnln!("Reading from unknown DMA location 0x{:04x}", addr);
                 0xff
@@ -54,6 +65,12 @@ impl Dma {
 
     pub fn write(&mut self, addr: u16, value: u8) {
         match addr {
+            // 0xFF46 — DMA: OAM DMA source address & start
+            DMA_ADDR => {
+                self.cycles_dma = 640;
+                self.value_dma = value;
+                self.active_dma = true;
+            }
             // 0xFF51 — HDMA1: VRAM DMA source high (CGB only)
             HDMA1_ADDR => self.source = (self.source & 0x00ff) | ((value as u16) << 8),
             // 0xFF52 — HDMA2: VRAM DMA source low (CGB only)
@@ -71,7 +88,7 @@ impl Dma {
                     _ => DmaMode::General,
                 };
                 self.pending = self.length;
-                self.active = true;
+                self.active_hdma = true;
             }
             _ => warnln!("Writing to unknown DMA location 0x{:04x}", addr),
         }
@@ -117,12 +134,40 @@ impl Dma {
         self.mode = value;
     }
 
-    pub fn active(&self) -> bool {
-        self.active
+    pub fn cycles_dma(&self) -> u16 {
+        self.cycles_dma
+    }
+
+    pub fn set_cycles_dma(&mut self, value: u16) {
+        self.cycles_dma = value;
+    }
+
+    pub fn value_dma(&self) -> u8 {
+        self.value_dma
+    }
+
+    pub fn set_value_dma(&mut self, value: u8) {
+        self.value_dma = value;
     }
 
-    pub fn set_active(&mut self, value: bool) {
-        self.active = value;
+    pub fn active_dma(&self) -> bool {
+        self.active_dma
+    }
+
+    pub fn set_active_dma(&mut self, value: bool) {
+        self.active_dma = value;
+    }
+
+    pub fn active_hdma(&self) -> bool {
+        self.active_hdma
+    }
+
+    pub fn set_active_hdma(&mut self, value: bool) {
+        self.active_hdma = value;
+    }
+
+    pub fn active(&self) -> bool {
+        self.active_dma || self.active_hdma
     }
 }
 
diff --git a/src/mmu.rs b/src/mmu.rs
index de363235..7841fc5d 100644
--- a/src/mmu.rs
+++ b/src/mmu.rs
@@ -263,11 +263,21 @@ impl Mmu {
         self.boot_active = value;
     }
 
-    pub fn clock_dma(&mut self, _cycles: u16) {
+    pub fn clock_dma(&mut self, cycles: u16) {
         if !self.dma.active() {
             return;
         }
 
+        if self.dma.active_dma() {
+            let cycles_dma = self.dma.cycles_dma().saturating_sub(cycles);
+            if cycles_dma == 0x0 {
+                let data = self.read_many((self.dma.value_dma() as u16) << 8, 160);
+                self.write_many(0xfe00, &data);
+                self.dma.set_active_dma(false);
+            }
+            self.dma.set_cycles_dma(cycles_dma);
+        }
+
         // @TODO: Implement DMA transfer in a better way, meaning that
         // the DMA transfer should respect the timings
         //
@@ -280,16 +290,17 @@ impl Mmu {
         // program runs it Normal Speed Mode).
         //
         // we should also respect General-Purpose DMA vs HBlank DMA
-
-        // only runs the DMA transfer if the system is in CGB mode
-        // this avoids issues when writing to DMG unmapped registers
-        // that would otherwise cause the system to crash
-        if self.mode == GameBoyMode::Cgb {
-            let data = self.read_many(self.dma.source(), self.dma.pending());
-            self.write_many(self.dma.destination(), &data);
+        if self.dma.active_hdma() {
+            // only runs the DMA transfer if the system is in CGB mode
+            // this avoids issues when writing to DMG unmapped registers
+            // that would otherwise cause the system to crash
+            if self.mode == GameBoyMode::Cgb {
+                let data = self.read_many(self.dma.source(), self.dma.pending());
+                self.write_many(self.dma.destination(), &data);
+            }
+            self.dma.set_pending(0);
+            self.dma.set_active_hdma(false);
         }
-        self.dma.set_pending(0);
-        self.dma.set_active(false);
     }
 
     pub fn read(&mut self, addr: u16) -> u8 {
@@ -497,13 +508,7 @@ impl Mmu {
                             0x40 | 0x60 | 0x70 => {
                                 match addr & 0x00ff {
                                     // 0xFF46 — DMA: OAM DMA source address & start
-                                    0x0046 => {
-                                        // @TODO must update the data section only after 160 m cycles,
-                                        // making this an immediate operation creates issues
-                                        debugln!("Going to start DMA transfer to 0x{:x}00", value);
-                                        let data = self.read_many((value as u16) << 8, 160);
-                                        self.write_many(0xfe00, &data);
-                                    }
+                                    0x0046 => self.dma.write(addr, value),
 
                                     // VRAM related write
                                     _ => self.ppu.write(addr, value),
diff --git a/src/state.rs b/src/state.rs
index e483f583..68343de2 100644
--- a/src/state.rs
+++ b/src/state.rs
@@ -1305,7 +1305,7 @@ impl State for BessCore {
 
             // need to disable DMA transfer to avoid unwanted
             // DMA transfers when loading the state
-            gb.dma().set_active(false);
+            gb.dma().set_active_hdma(false);
         }
 
         Ok(())
-- 
GitLab