From 4a66e1076005bf895295fb4270307f674b8f81fa 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 11:35:37 +0000
Subject: [PATCH] chore: better usage of const values

Improved some comments for DMA.
---
 src/consts.rs |  8 ++++++++
 src/dma.rs    | 17 ++++++++++-------
 src/mmu.rs    | 18 ++++++++++++++----
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/consts.rs b/src/consts.rs
index f9479564..8f083a1a 100644
--- a/src/consts.rs
+++ b/src/consts.rs
@@ -3,3 +3,11 @@ pub const DIV_ADDR: u16 = 0xff04;
 pub const TIMA_ADDR: u16 = 0xff05;
 pub const TMA_ADDR: u16 = 0xff06;
 pub const TAC_ADDR: u16 = 0xff07;
+
+// DMA registers
+pub const DMA_ADDR: u16 = 0xff46;
+pub const HDMA1_ADDR: u16 = 0xff51;
+pub const HDMA2_ADDR: u16 = 0xff52;
+pub const HDMA3_ADDR: u16 = 0xff53;
+pub const HDMA4_ADDR: u16 = 0xff54;
+pub const HDMA5_ADDR: u16 = 0xff55;
diff --git a/src/dma.rs b/src/dma.rs
index a7cfb8a9..83261556 100644
--- a/src/dma.rs
+++ b/src/dma.rs
@@ -1,4 +1,7 @@
-use crate::warnln;
+use crate::{
+    consts::{HDMA1_ADDR, HDMA2_ADDR, HDMA3_ADDR, HDMA4_ADDR, HDMA5_ADDR},
+    warnln,
+};
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum DmaMode {
@@ -38,7 +41,7 @@ impl Dma {
     pub fn read(&mut self, addr: u16) -> u8 {
         match addr {
             // 0xFF55 — HDMA5: VRAM DMA length/mode/start (CGB only)
-            0xff45 => ((self.length >> 4) - 1) as u8 | ((self.active as u8) << 7),
+            HDMA5_ADDR => ((self.length >> 4) - 1) as u8 | ((self.active as u8) << 7),
             _ => {
                 warnln!("Reading from unknown DMA location 0x{:04x}", addr);
                 0xff
@@ -49,15 +52,15 @@ impl Dma {
     pub fn write(&mut self, addr: u16, value: u8) {
         match addr {
             // 0xFF51 — HDMA1: VRAM DMA source high (CGB only)
-            0xff51 => self.source = (self.source & 0x00ff) | ((value as u16) << 8),
+            HDMA1_ADDR => self.source = (self.source & 0x00ff) | ((value as u16) << 8),
             // 0xFF52 — HDMA2: VRAM DMA source low (CGB only)
-            0xff52 => self.source = (self.source & 0xff00) | ((value & 0xf0) as u16),
+            HDMA2_ADDR => self.source = (self.source & 0xff00) | ((value & 0xf0) as u16),
             // 0xFF53 — HDMA3: VRAM DMA destination high (CGB only)
-            0xff53 => self.destination = (self.destination & 0x00ff) | ((value as u16) << 8),
+            HDMA3_ADDR => self.destination = (self.destination & 0x00ff) | ((value as u16) << 8),
             // 0xFF54 — HDMA4: VRAM DMA destination low (CGB only)
-            0xff54 => self.destination = (self.destination & 0xff00) | ((value & 0xf0) as u16),
+            HDMA4_ADDR => self.destination = (self.destination & 0xff00) | ((value & 0xf0) as u16),
             // 0xFF55 — HDMA5: VRAM DMA length/mode/start (CGB only)
-            0xff55 => {
+            HDMA5_ADDR => {
                 self.length = (((value & 0x7f) + 0x1) as u16) << 4;
                 self.mode = match (value & 80) >> 7 {
                     0 => DmaMode::General,
diff --git a/src/mmu.rs b/src/mmu.rs
index 757c4b96..082cba81 100644
--- a/src/mmu.rs
+++ b/src/mmu.rs
@@ -4,6 +4,7 @@ use std::sync::Mutex;
 
 use crate::{
     apu::Apu,
+    consts::DMA_ADDR,
     debugln,
     dma::Dma,
     gb::{Components, GameBoyConfig, GameBoyMode, GameBoySpeed},
@@ -268,7 +269,16 @@ impl Mmu {
             return;
         }
 
-        // @TODO: Implement DMA transfer in a better way
+        // @TODO: Implement DMA transfer in a better way, meaning that
+        // the DMA transfer should respect the timings
+
+        // In both Normal Speed and Double Speed Mode it takes about 8 μs
+        // to transfer a block of $10 bytes. That is, 8 M-cycles in Normal
+        // Speed Mode [1], and 16 “fast” M-cycles in Double Speed Mode [2].
+        // Older MBC controllers (like MBC1-3) and slower ROMs are not guaranteed
+        // to support General Purpose or HBlank DMA, that’s because there are
+        // always 2 bytes transferred per microsecond (even if the itself
+        // program runs it Normal Speed Mode).
 
         // only runs the DMA transfer if the system is in CGB mode
         // this avoids issues when writing to DMG unmapped registers
@@ -485,9 +495,9 @@ impl Mmu {
                             0x40 | 0x60 | 0x70 => {
                                 match addr & 0x00ff {
                                     // 0xFF46 — DMA: OAM DMA source address & start
-                                    0x0046 => {
-                                        // @TODO must increment the cycle count by 160
-                                        // and make this a separated dma.rs file
+                                    DMA_ADDR => {
+                                        // @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);
-- 
GitLab