Fixing OAM DMA. Removing logging.

CMakeLists.txt

@@ -26,7 +26,7 @@ add_executable(nes
         src/Controller.cpp
         src/Controller.h
         examples/sdl/SdlKeyboardController.cpp
-        examples/sdl/SdlKeyboardController.h src/Dma.cpp src/Dma.h
+        examples/sdl/SdlKeyboardController.h
         src/Oam.cpp
         src/Oam.h
         src/Mapper/Mapper1.cpp
@@ -38,5 +38,4 @@ add_executable(nes
         examples/sdl/SdlGamepadController.h)
 
 find_package(SDL3 CONFIG REQUIRED)
-find_package(fmt REQUIRED)
+target_link_libraries(nes PRIVATE SDL3::SDL3)
-target_link_libraries(nes PRIVATE SDL3::SDL3 fmt::fmt)

src/Bus.cpp

@@ -3,6 +3,8 @@
 //
 
 #include "Bus.h"
+#include "Cpu.h"
+#include "Ppu.h"
 #include <iostream>
 
 namespace nes {
@@ -12,18 +14,16 @@ namespace nes {
         Ppu *ppu,
         Cartridge *cartridge,
         Controller *controller1,
-        Controller *controller2,
+        Controller *controller2):
-        Dma *dma): _ram(ram),
+            _ram(ram),
-                   _cpu(cpu),
+            _cpu(cpu),
-                   _ppu(ppu),
+            _ppu(ppu),
-                   _cartridge(cartridge),
+            _cartridge(cartridge),
-                   _controller1(controller1),
+            _controller1(controller1),
-                   _controller2(controller2),
+            _controller2(controller2) {
-                   _dma(dma) {
 
         _cpu->connectToBus(this);
         _ppu->connectToBus(this);
-        _dma->connectToBus(this);
     }
 
     void Bus::connectCartridge(Cartridge *cartridge) {
@@ -83,7 +83,7 @@ namespace nes {
             _ppu->write(address & 0x0007, value);
         }
         else if(address == 0x4014) {
-            _dma->start(value);
+            _ppu->startDma(value);
         }
         else if(address == 0x4016) {
             _controller1->poll();

src/Bus.h

@@ -5,15 +5,15 @@
 #ifndef BUS_H
 #define BUS_H
 
-#include "Cpu.h"
-#include "Ppu.h"
 #include "Cartridge.h"
 #include "Controller.h"
-#include "Dma.h"
 #include <cstdint>
 
 namespace nes {
 
+    class Cpu;
+    class Ppu;
+
     class Bus {
     public:
         Bus(uint8_t *ram,
@@ -21,8 +21,7 @@ namespace nes {
             Ppu *ppu,
             Cartridge *cartridge,
             Controller *controller1,
-            Controller *controller2,
+            Controller *controller2);
-            Dma *dma);
 
         void connectCartridge(Cartridge *cartridge);
         void connectController1(Controller *controller1);
@@ -42,7 +41,6 @@ namespace nes {
         Cartridge* _cartridge;
         Controller* _controller1;
         Controller* _controller2;
-        Dma* _dma;
     };
 
 }

src/Cpu.cpp

@@ -6,10 +6,6 @@
 #include "Bus.h"
 #include <iostream>
 
-#ifdef NES_LOGGING
-#include <fmt/format.h>
-#endif
-
 namespace nes {
 
     Cpu::Cpu(): _ticks{}, A{}, X{}, Y{}, PC{}, SP{}, flags{} {
@@ -229,23 +225,6 @@ namespace nes {
         _bus = bus;
     }
 
-#ifdef NES_LOGGING
-    std::string Cpu::state() const {
-        return fmt::format("{} ({:02X}), PC: {:X}, SP: {:X}, A: {:02X}, X: {:02X}, Y: {:02X}, [N:{}, V:{}, B{}, D{}, I{}, Z:{}, C:{}], H: {:08X}",
-                           _instructions[_currentOpcode].name,
-                           _currentOpcode,
-                           PC, SP, A, X, Y,
-                           (int)getFlag(Negative),
-                           (int)getFlag(Overflow),
-                           (int)getFlag(Break),
-                           (int)getFlag(DecimalMode),
-                           (int)getFlag(InterruptDisable),
-                           (int)getFlag(Zero),
-                           (int)getFlag(Carry),
-                           _bus->zpHash());
-    }
-#endif
-
     void Cpu::setFlag(CpuFlags flag, bool value) {
         if(value) {
             flags |= flag;

src/Cpu.h

@@ -51,10 +51,6 @@ namespace nes {
         void nmi();
         void connectToBus(Bus* bus);
 
-#ifdef NES_LOGGING
-        std::string state() const;
-#endif
-
     private:
         size_t _ticks;
         Bus* _bus;

src/Dma.cpp

@@ -1,47 +0,0 @@
-//
-// Created by selim on 9/23/23.
-//
-
-#include "Dma.h"
-#include "Ppu.h"
-#include "Bus.h"
-
-namespace nes {
-
-    Dma::Dma(Ppu *ppu): _ppu{ppu} {
-
-    }
-
-    bool Dma::active() const {
-        return _active;
-    }
-
-    void Dma::tick(uint64_t cycles) {
-        if(_dummy) {
-            if(cycles % 2 == 1) {
-                _dummy = false;
-            }
-        } else {
-            if(cycles % 2 == 0) {
-                _data = _bus->read((_page << 8) | _address);
-            } else {
-                _ppu->writeOam(_address, _data);
-                _address++;
-
-                if(_address == 0) {
-                    _active = false;
-                    _dummy = true;
-                }
-            }
-        }
-    }
-
-    void Dma::start(uint8_t page) {
-        _page = page;
-        _active = true;
-    }
-
-    void Dma::connectToBus(Bus *bus) {
-        _bus = bus;
-    }
-}

src/Dma.h

@@ -1,39 +0,0 @@
-//
-// Created by selim on 9/23/23.
-//
-
-#ifndef NES_DMA_H
-#define NES_DMA_H
-
-#include <cstdint>
-
-namespace nes {
-
-    class Bus;
-    class Ppu;
-
-    class Dma {
-    public:
-        Dma(Ppu* ppu);
-        [[nodiscard]] bool active() const;
-        void tick(uint64_t cycles);
-        void start(uint8_t page);
-        void connectToBus(Bus *bus);
-
-    private:
-        Bus* _bus;
-        Ppu* _ppu;
-
-    private:
-        uint8_t _page = 0;
-        uint8_t _address = 0;
-        uint8_t _data = 0;
-
-    private:
-        bool _active = false;
-        bool _dummy = true;
-    };
-
-}
-
-#endif //NES_DMA_H

src/Logger.cpp

@@ -6,7 +6,7 @@
 
 #include <cstring>
 #include <fstream>
-#include <fmt/format.h>
+#include <format>
 
 namespace nes {
 
@@ -19,7 +19,7 @@ namespace nes {
             return;
         }
 
-        std::string cycleString = fmt::format("[{:06d}] ", cycle);
+        std::string cycleString = std::format("[{:06d}] ", cycle);
         std::memcpy(_data.get() + _offset, cycleString.data(), cycleString.size());
         _offset += cycleString.size();
 

src/Oam.cpp

@@ -6,6 +6,7 @@
 #include "Ppu.h"
 
 #include <cstring>
+#include <print>
 
 namespace nes {
 
@@ -25,10 +26,6 @@ namespace nes {
         _address++;
     }
 
-    void Oam::write(uint8_t address, uint8_t value) {
-        reinterpret_cast<uint8_t*>(_data)[address] = value;
-    }
-
     bool Oam::detectVisibleSprites(int16_t scanline, bool bigSprite) {
         memset(_visibleSprites, 0xFF, MAX_SPRITES_PER_SCANLINE*sizeof(SpriteInfo));
         _visibleSpriteCount = 0;
@@ -142,4 +139,36 @@ namespace nes {
     bool Oam::spriteZeroBeingRendered() const {
         return _spriteZeroVisible && _firstVisibleSpriteBeingRendered;
     }
+
+    void Oam::dmaTick(uint64_t cycles) {
+        if(_dmaDummy) {
+            if(cycles % 2 == 1) {
+                _dmaDummy = false;
+            }
+        } else {
+            if(cycles % 2 == 0) {
+                _dmaData = _ppu->readRam((_dmaPage << 8) | _dmaAddress++);
+            } else {
+                write(_dmaData);
+                if(_dmaAddress == 0) {
+                    _dmaActive = false;
+                    _dmaDummy = true;
+                }
+            }
+        }
+    }
+
+    void Oam::dmaStart(uint8_t page) {
+        _dmaPage = page;
+        _dmaAddress = 0;
+        _dmaActive = true;
+    }
+
+    bool Oam::dmaActive() const {
+        return _dmaActive;
+    }
+
+    uint8_t Oam::address() const {
+        return _address;
+    }
 }

src/Oam.h

@@ -38,7 +38,10 @@ namespace nes {
         void setAddress(uint8_t address);
         [[nodiscard]] uint8_t read() const;
         void write(uint8_t value);
-        void write(uint8_t address, uint8_t value);
+        void dmaTick(uint64_t cycles);
+        void dmaStart(uint8_t page);
+        bool dmaActive() const;
+        uint8_t address() const;
 
     public:
         bool detectVisibleSprites(int16_t scanline, bool bigSprite);
@@ -59,6 +62,13 @@ namespace nes {
         uint8_t _visibleSpriteCount = 0;
         bool _spriteZeroVisible = false;
         bool _firstVisibleSpriteBeingRendered = false;
+
+    private:
+        uint8_t _dmaPage = 0;
+        uint8_t _dmaAddress = 0;
+        uint8_t _dmaData = 0;
+        bool _dmaActive = false;
+        bool _dmaDummy = true;
     };
 
 }

src/Ppu.cpp

@@ -3,11 +3,6 @@
 //
 
 #include "Ppu.h"
-
-#ifdef NES_LOGGING
-#include <fmt/format.h>
-#endif
-
 #include <print>
 
 namespace nes {
@@ -185,6 +180,8 @@ namespace nes {
             case OamData:
                 value = _oam->read();
                 break;
+            case OamAddress:
+                value = _oam->address();
         }
         return value;
     }
@@ -338,6 +335,22 @@ namespace nes {
         }
     }
 
+    uint8_t Ppu::readRam(uint16_t address) const {
+        return _bus->read(address);
+    }
+
+    bool Ppu::dmaActive() const {
+        return _oam->dmaActive();
+    }
+
+    void Ppu::dmaTick(uint64_t cycles) const {
+        _oam->dmaTick(cycles);
+    }
+
+    void Ppu::startDma(uint8_t page) const {
+        _oam->dmaStart(page);
+    }
+
     Pixel Ppu::getColor(uint8_t palette, uint8_t pixel) {
         uint16_t  address = 0x3F00 + palette*4 + pixel;
         return _palette[internalRead(address) & 0x3F];
@@ -432,33 +445,7 @@ namespace nes {
         }
     }
 
-#ifdef NES_LOGGING
+    void Ppu::writeOam(uint8_t data) {
-    std::string Ppu::state() const {
+        _oam->write(data);
-
-        auto palettes = (uint32_t*)_paletteTable.get();
-
-        return fmt::format("X:{:03d}, Y:{:03d}, V:{:d}, vR: [CX:{:02X}, CY:{:02X}, NX:{:d}, NY:{:d}, FY:{:d}], tR: [CX:{:02X}, CY:{:02X}, NX:{:d}, NY:{:d}, FY:{:d}], FX:{:d}, AL:{:d}, ASL:{:04X}, ASH:{:04X}",
-                           _column,
-                           _scanline,
-                           _status.verticalBlank,
-                           _vRamAddress.coarseX,
-                           _vRamAddress.coarseY,
-                           _vRamAddress.nameTableX,
-                           _vRamAddress.nameTableY,
-                           _vRamAddress.fineY,
-                           _tRamAddress.coarseX,
-                           _tRamAddress.coarseY,
-                           _tRamAddress.nameTableX,
-                           _tRamAddress.nameTableY,
-                           _tRamAddress.fineY,
-                           _fineX,
-                           _addressWriteInProgress,
-                           _bgAttribShifter._lo,
-                           _bgAttribShifter._hi);
-    }
-#endif
-
-    void Ppu::writeOam(uint8_t address, uint8_t data) {
-        _oam->write(address, data);
     }
 }

src/Ppu.h

@@ -107,13 +107,13 @@ namespace nes {
         void setPixel(uint16_t row, uint16_t column, Pixel pixel);
         void connectToBus(Bus* bus);
         void reset();
-        void writeOam(uint8_t address, uint8_t data);
+        void writeOam(uint8_t data);
         uint8_t internalRead(uint16_t address);
         void internalWrite(uint16_t address, uint8_t value);
-        
+        [[nodiscard]] uint8_t readRam(uint16_t address) const;
-#ifdef NES_LOGGING
+        [[nodiscard]] bool dmaActive() const;
-        [[nodiscard]] std::string state() const;
+        void dmaTick(uint64_t cycles) const;
-#endif
+        void startDma(uint8_t page) const;
 
     public:
         std::function<void(const Pixel*)> onNewFrame;

src/System.cpp

@@ -9,28 +9,20 @@
 #include "Controller.h"
 
 #include <utility>
-#include <iostream>
 
 namespace nes {
 
-    System::System():
+    System::System(): _cycles{} {
-    _cycles{}
-#ifdef NES_LOGGING
-    ,_logger(500*1024*1024)
-#endif
-    {
         _ram = std::make_unique<uint8_t[]>(2*1024);
         _cpu = std::make_unique<Cpu>();
         _ppu = std::make_unique<Ppu>();
-        _dma = std::make_unique<Dma>(_ppu.get());
         _controller1 = std::make_shared<Controller>();
         _bus = std::make_unique<Bus>(_ram.get(),
             _cpu.get(),
             _ppu.get(),
             _cartridge.get(),
             _controller1.get(),
-            nullptr,
+            nullptr);
-            _dma.get());
     }
 
     void System::insertCartridge(const fs::path &path, std::optional<uint16_t> address) {
@@ -58,37 +50,19 @@ namespace nes {
 
     void System::tick() {
         bool needInterrupt = _ppu->tick();
-#ifdef NES_LOGGING
-        _logger.addLine(_cycles, _ppu->state());
-#endif
 
         if(_cycles % 3 == 0) {
-            if(_dma->active()) {
+            if(_ppu->dmaActive()) {
-                _dma->tick(_cycles);
+                _ppu->dmaTick(_cycles);
             } else {
                 bool instructionExecuted = _cpu->tick();
-#ifdef NES_LOGGING
-                if(instructionExecuted) {
-                    _logger.addLine(_cycles, _cpu->state());
-                }
-#endif
             }
         }
 
         if(needInterrupt) {
             _cpu->nmi();
-#ifdef NES_LOGGING
-            _logger.addLine(_cycles, "NMI");
-            _logger.addLine(_cycles, _cpu->state());
-#endif
         }
 
-#ifdef NES_LOGGING
-        if(_cycles == 3000000) {
-            _logger.dump("/home/selim/Documents/log.txt");
-        }
-#endif
-
         _cycles++;
     }
 

src/System.h

@@ -9,7 +9,6 @@
 #include "Cpu.h"
 #include "Ppu.h"
 #include "Cartridge.h"
-#include "Dma.h"
 #include "Bus.h"
 
 #include <filesystem>
@@ -40,12 +39,7 @@ namespace nes {
         std::unique_ptr<Ppu> _ppu;
         std::unique_ptr<Cartridge> _cartridge;
         std::shared_ptr<Controller> _controller1;
-        std::unique_ptr<Dma> _dma;
         std::unique_ptr<Bus> _bus;
-
-#ifdef NES_LOGGING
-        Logger _logger;
-#endif
     };
 
 }