src/cpu.rs

@@ -15,7 +15,7 @@ pub struct Cpu {
     flags: Flags,
     ime: ImeState,
     // TODO: Merge halted and state properties
-    halted: Option<HaltKind>,
+    halted: Option<HaltState>,
     state: State,
 }
 
@@ -53,7 +53,7 @@ impl Cpu {
         self.ime = state;
     }
 
-    pub(crate) fn halt(&mut self, state: HaltKind) {
+    pub(crate) fn halt(&mut self, state: HaltState) {
         self.halted = Some(state);
     }
 
@@ -61,8 +61,8 @@ impl Cpu {
         self.halted = None;
     }
 
-    pub(crate) fn halted(&self) -> Option<HaltKind> {
-        self.halted
+    pub(crate) fn halted(&self) -> Option<&HaltState> {
+        self.halted.as_ref()
     }
 
     pub fn load_cartridge(&mut self, path: &str) -> std::io::Result<()> {
@@ -75,8 +75,6 @@ impl Cpu {
 }
 
 impl Cpu {
-    /// Fetch an [Instruction] from the memory bus
-    /// (4 cycles)
     fn fetch(&mut self) -> u8 {
         let byte = self.read_byte(self.reg.pc);
         self.bus.clock();
@@ -84,10 +82,6 @@ impl Cpu {
         byte
     }
 
-    /// Decode a byte into an [SM83](Cpu) [Instruction]
-    ///
-    /// If opcode == 0xCB, then decoding costs 4 cycles.
-    /// Otherwise, decoding is free
     pub(crate) fn decode(&mut self, opcode: u8) -> Instruction {
         if opcode == 0xCB {
             Instruction::decode(self.fetch(), true)
@@ -96,46 +90,38 @@ impl Cpu {
         }
     }
 
-    /// Execute an [Instruction].
-    ///
-    /// The amount of cycles necessary to execute an instruction range from
-    /// 0 to 20 T-cycles
     fn execute(&mut self, instruction: Instruction) -> Cycle {
         Instruction::execute(self, instruction)
     }
 
-    /// Perform the [`Cpu::fetch()`] [`Cpu::decode(opcode)`] [`Cpu::execute(instr)`]
-    /// routine.
-    ///
-    /// Handle HALT state and interrupts.
     pub fn step(&mut self) -> Cycle {
-        // Log instructions
+        // // Log instructions
         // if self.reg.pc > 0xFF {
         //     let out = std::io::stdout();
-        //     let _ = self._print_logs(out.lock());
+        //     let _ = self._print_debug(out.lock());
         // }
 
-        if let Some(elapsed) = self.handle_interrupt() {
-            return elapsed;
-        }
-
-        if let Some(kind) = self.halted() {
-            use HaltKind::*;
+        // FIXME: The Halt instruction takes more cycles than it should in Blargg's 2nd cpu_instrs test
+        let elapsed = match self.halted() {
+            Some(state) => {
+                use HaltState::*;
 
+                match state {
+                    ImeEnabled | NonePending => {
                         self.bus.clock();
-
-            let elapsed = match kind {
-                ImeEnabled | NonePending => Cycle::new(4),
-                SomePending => todo!("Implement HALT bug"),
-            };
-
-            return elapsed;
+                        Cycle::new(4)
                     }
-
+                    SomePending => todo!("Implement HALT bug"),
+                }
+            }
+            None => {
                 let opcode = self.fetch();
                 let instr = self.decode(opcode);
                 let elapsed = self.execute(instr);
-        self.handle_ei();
+                self.check_ime();
+                elapsed
+            }
+        };
 
         // For use in Blargg's Test ROMs
         if self.read_byte(0xFF02) == 0x81 {
@@ -144,6 +130,8 @@ impl Cpu {
             eprint!("{}", c);
         }
 
+        // TODO: Is this in the wrong place?
+        self.handle_interrupts();
         elapsed
     }
 }
@@ -171,11 +159,18 @@ impl Cpu {
         &mut self.bus.joypad
     }
 
-    fn handle_ei(&mut self) {
+    fn check_ime(&mut self) {
         match self.ime {
-            ImeState::EiExecuted => self.ime = ImeState::Pending,
-            ImeState::Pending => self.ime = ImeState::Enabled,
-            ImeState::Disabled | ImeState::Enabled => {}
+            ImeState::Pending => {
+                // This is within the context of the EI instruction, we need to not update EI until the end of the
+                // next executed Instruction
+                self.ime = ImeState::PendingEnd;
+            }
+            ImeState::PendingEnd => {
+                // The Instruction after EI has now been executed, so we want to enable the IME flag here
+                self.ime = ImeState::Enabled;
+            }
+            ImeState::Disabled | ImeState::Enabled => {} // Do Nothing
         }
     }
 
@@ -187,56 +182,54 @@ impl Cpu {
         self.read_byte(0xFFFF)
     }
 
-    fn handle_interrupt(&mut self) -> Option<Cycle> {
-        let irq = self.int_request();
-        let enable = self.int_enable();
+    fn handle_interrupts(&mut self) {
+        let req = self.int_request();
+        let enabled = self.int_enable();
 
-        // TODO: Ensure that this behaviour is correct
         if self.halted.is_some() {
             // When we're here either a HALT with IME set or
             // a HALT with IME not set and No pending Interrupts was called
 
-            if irq & enable != 0 {
+            if req & enabled != 0 {
                 // The if self.ime() below correctly follows the "resuming from HALT" behaviour so
                 // nothing actually needs to be added here. This is just documentation
                 // since it's a bit weird why nothing is being done
 
-                self.resume();
+                self.resume()
             }
         }
 
-        match self.ime() {
-            ImeState::Enabled => {
-                let mut irq: InterruptFlag = irq.into();
-                let enable: InterruptEnable = enable.into();
+        if let ImeState::Enabled = self.ime() {
+            let mut req: InterruptFlag = req.into();
+            let enabled: InterruptEnable = enabled.into();
 
-                let rst_vector = if irq.vblank() && enable.vblank() {
+            let vector = if req.vblank() && enabled.vblank() {
                 // Handle VBlank Interrupt
-                    irq.set_vblank(false);
+                req.set_vblank(false);
 
                 // INT 40h
                 Some(0x40)
-                } else if irq.lcd_stat() && enable.lcd_stat() {
+            } else if req.lcd_stat() && enabled.lcd_stat() {
                 // Handle LCD STAT Interrupt
-                    irq.set_lcd_stat(false);
+                req.set_lcd_stat(false);
 
                 // INT 48h
                 Some(0x48)
-                } else if irq.timer() && enable.timer() {
+            } else if req.timer() && enabled.timer() {
                 // Handle Timer Interrupt
-                    irq.set_timer(false);
+                req.set_timer(false);
 
                 // INT 50h
                 Some(0x50)
-                } else if irq.serial() && enable.serial() {
+            } else if req.serial() && enabled.serial() {
                 // Handle Serial Interrupt
-                    irq.set_serial(false);
+                req.set_serial(false);
 
                 // INT 58h
                 Some(0x58)
-                } else if irq.joypad() && enable.joypad() {
+            } else if req.joypad() && enabled.joypad() {
                 // Handle Joypad Interrupt
-                    irq.set_joypad(false);
+                req.set_joypad(false);
 
                 // INT 60h
                 Some(0x60)
@@ -244,19 +237,17 @@ impl Cpu {
                 None
             };
 
-                match rst_vector {
-                    Some(vector) => {
+            let _ = match vector {
+                Some(address) => {
                     //  Write the Changes to 0xFF0F and 0xFFFF registers
-                        self.write_byte(0xFF0F, irq.into());
+                    self.write_byte(0xFF0F, req.into());
 
                     // Disable all future interrupts
                     self.set_ime(ImeState::Disabled);
-                        Some(Instruction::reset(self, vector))
+                    Instruction::reset(self, address)
                 }
-                    None => None,
-                }
-            }
-            _ => None,
+                None => Cycle::new(0), // NO Interrupts were enabled and / or requested
+            };
         }
     }
 }
@@ -514,7 +505,7 @@ impl From<u8> for Flags {
 }
 
 #[derive(Debug, Clone, Copy)]
-pub(crate) enum HaltKind {
+pub(crate) enum HaltState {
     ImeEnabled,
     NonePending,
     SomePending,
@@ -523,8 +514,8 @@ pub(crate) enum HaltKind {
 #[derive(Debug, Clone, Copy)]
 pub(crate) enum ImeState {
     Disabled,
-    EiExecuted,
     Pending,
+    PendingEnd,
     Enabled,
 }
 

src/instruction.rs

@@ -9,7 +9,7 @@ use self::table::{
 };
 use self::table::{Group1RegisterPair, Group2RegisterPair, Group3RegisterPair, Register};
 use crate::bus::{Bus, BusIo};
-use crate::cpu::{Cpu, Flags, HaltKind, ImeState, Register as CpuRegister, RegisterPair};
+use crate::cpu::{Cpu, Flags, HaltState, ImeState, Register as CpuRegister, RegisterPair};
 
 #[allow(clippy::upper_case_acronyms)]
 #[derive(Clone, Copy)]
@@ -588,14 +588,14 @@ impl Instruction {
             }
             Instruction::HALT => {
                 // HALT | Enter CPU low power consumption mode until interrupt occurs
-                use HaltKind::*;
+                use HaltState::*;
 
-                let kind = match *cpu.ime() {
+                let halt_state = match *cpu.ime() {
                     ImeState::Enabled => ImeEnabled,
                     _ if cpu.int_request() & cpu.int_enable() != 0 => SomePending,
                     _ => NonePending,
                 };
-                cpu.halt(kind);
+                cpu.halt(halt_state);
                 Cycle::new(4)
             }
             Instruction::ADC(source) => match source {
@@ -971,7 +971,7 @@ impl Instruction {
             }
             Instruction::EI => {
                 // EI | Enable IME after the next instruction
-                cpu.set_ime(ImeState::EiExecuted);
+                cpu.set_ime(ImeState::Pending);
                 Cycle::new(4)
             }
             Instruction::CALL(cond) => {