dma: refactoring, fixes for Channel 6/OTC

Tested against dma/otc-test

Author: JaCzekanski

src/device/dma/dma.cpp

@@ -68,4 +68,9 @@ void DMA::write(uint32_t address, uint8_t data) {
         }
     }
 }
+
+bool DMA::isChannelEnabled(Channel ch) {
+    uint32_t mask = 0b1000 << ((int)ch * 4);
+    return (control._reg & mask) != 0;
+}
 }  // namespace device::dma

src/device/dma/dma.h

@@ -34,7 +34,7 @@ union DPCR {
     uint32_t _reg;
     uint8_t _byte[4];
 
-    DPCR() : _reg(0) {}
+    DPCR() : _reg(0x07654321) {}
 };
 
 // 0x1f8010f4 - DICR,  DMA Interrupt Register
@@ -108,6 +108,8 @@ class DMA {
     uint8_t read(uint32_t address);
     void write(uint32_t address, uint8_t data);
 
+    bool isChannelEnabled(Channel ch);
+
     template <class Archive>
     void serialize(Archive& ar) {
         ar(control._reg, status._reg, pendingInterrupt);

src/device/dma/dma6_channel.cpp

@@ -1,7 +1,40 @@
 #include "dma6_channel.h"
+#include <fmt/core.h>
+#include <magic_enum.hpp>
+#include "system.h"
 
 namespace device::dma {
-uint32_t DMA6Channel::readDevice() { return 0xffffffff; }
-
 DMA6Channel::DMA6Channel(Channel channel, System* sys) : DMAChannel(channel, sys) {}
+
+void DMA6Channel::maskControl() {
+    control._reg &= ~CHCR::MASK;
+    control.direction = CHCR::Direction::toRam;
+    control.syncMode = CHCR::SyncMode::block;
+    control.memoryAddressStep = CHCR::MemoryAddressStep::backward;
+    control.choppingEnable = false;
+    control.choppingDmaWindowSize = 0;
+    control.choppingCpuWindowSize = 0;
+    control.unknown1 = false;
+}
+
+void DMA6Channel::startTransfer() {
+    control.startTrigger = CHCR::StartTrigger::clear;
+    uint32_t addr = baseAddress.address;
+    uint32_t wordCount = count.syncMode0.wordCount;
+    if (wordCount == 0) {
+        wordCount = 0x10000;
+    }
+
+    if (verbose) {
+        fmt::print("[DMA{}] {:<8} -> RAM @ 0x{:08x}, block, count: 0x{:04x}\n", (int)channel, magic_enum::enum_name(channel), addr,
+                   wordCount);
+    }
+
+    for (int i = wordCount - 1; i >= 0; i--, addr -= 4) {
+        sys->writeMemory32(addr, (i == 0) ? 0xffffff : (addr - 4) & 0xffffff);
+    }
+
+    irqFlag = true;
+    control.enabled = CHCR::Enabled::completed;
+}
 }  // namespace device::dma

src/device/dma/dma6_channel.h

@@ -3,7 +3,8 @@
 
 namespace device::dma {
 class DMA6Channel : public DMAChannel {
-    uint32_t readDevice() override;
+    void maskControl() override;
+    void startTransfer() override;
 
    public:
     DMA6Channel(Channel channel, System* sys);

src/device/dma/dma_channel.cpp

@@ -3,6 +3,7 @@
 #include <magic_enum.hpp>
 #include "config.h"
 #include "system.h"
+#include <unordered_set>
 
 namespace device::dma {
 DMAChannel::DMAChannel(Channel channel, System* sys) : channel(channel), sys(sys) { verbose = config["debug"]["log"]["dma"]; }
@@ -17,112 +18,115 @@ uint8_t DMAChannel::read(uint32_t address) {
     if (address < 0x4) return baseAddress._byte[address];
     if (address >= 0x4 && address < 0x8) return count._byte[address - 4];
     if (address >= 0x8 && address < 0xc) return control._byte[address - 8];
-
-    fmt::print("R Unimplemented DMA%d address 0x%08x\n", (int)channel, address);
     return 0;
 }
 
 void DMAChannel::write(uint32_t address, uint8_t data) {
-    if (address < 0x4)
+    if (address < 0x4) {
         baseAddress._byte[address] = data;
-    else if (address >= 0x4 && address < 0x8)
+        baseAddress._reg &= 0xffffff;
+    } else if (address >= 0x4 && address < 0x8) {
         count._byte[address - 4] = data;
-    else if (address >= 0x8 && address < 0xc) {
-        control._byte[address - 0x8] = data;
-
-        if (address != 0xb) return;
-
-        if (control.enabled != CHCR::Enabled::start) return;
-        control.startTrigger = CHCR::StartTrigger::clear;
-
-        if (control.syncMode == CHCR::SyncMode::startImmediately) {
-            int addr = baseAddress.address;
-            // TODO: Check Transfer Direction
-            // TODO: Check Memory Address Step
-
-            if (verbose) {
-                if (control.direction == CHCR::Direction::toRam) {
-                    fmt::print("[DMA{}] {:<8} -> RAM @ 0x{:08x}, block, count: 0x{:04x}\n", (int)channel, magic_enum::enum_name(channel),
-                               addr, static_cast<int>(count.syncMode0.wordCount));
-                } else if (control.direction == CHCR::Direction::fromRam) {
-                    fmt::print("[DMA{}] {:<8} <- RAM @ 0x{:08x}, block, count: 0x{:04x}\n", (int)channel, magic_enum::enum_name(channel),
-                               addr, static_cast<int>(count.syncMode0.wordCount));
-                }
+    } else if (address >= 0x8 && address < 0xc) {
+        control._byte[address - 8] = data;
+        maskControl();
+
+        if (address == 0xb) {
+            if (!sys->dma->isChannelEnabled(channel)) return;
+            if (control.syncMode == CHCR::SyncMode::block && control.startTrigger != CHCR::StartTrigger::manual) return;
+            if (control.enabled != CHCR::Enabled::start) return;
+
+            startTransfer();
+        }
+    }
+}
+
+void DMAChannel::maskControl() { control._reg &= ~CHCR::MASK; }
+
+void DMAChannel::startTransfer() {
+    using namespace magic_enum;
+
+    int step = control.memoryAddressStep == CHCR::MemoryAddressStep::forward ? 4 : -4;
+    uint32_t addr = baseAddress.address;
+
+    control.startTrigger = CHCR::StartTrigger::clear;
+
+    if (control.syncMode == CHCR::SyncMode::block) {
+        if (verbose) {
+            fmt::print("[DMA{}] {:<8} {} RAM @ 0x{:08x}, {}, count: 0x{:04x}\n", (int)channel, enum_name(channel), control.dir(), addr,
+                       enum_name(control.syncMode), (int)count.syncMode0.wordCount);
+        }
+        if (control.direction == CHCR::Direction::fromRam) {
+            for (size_t i = 0; i < count.syncMode0.wordCount; i++, addr += step) {
+                writeDevice(sys->readMemory32(addr));
+            }
+        } else if (control.direction == CHCR::Direction::toRam) {
+            for (size_t i = 0; i < count.syncMode0.wordCount; i++, addr += step) {
+                sys->writeMemory32(addr, readDevice());
             }
-            if (channel == Channel::CDROM) {
-                for (size_t i = 0; i < count.syncMode0.wordCount; i++) {
+        }
+        control.enabled = CHCR::Enabled::stop;
+    } else if (control.syncMode == CHCR::SyncMode::sync) {
+        int blockCount = count.syncMode1.blockCount;
+        int blockSize = count.syncMode1.blockSize;
+        if (blockCount == 0) blockCount = 0x10000;
+
+        if (verbose) {
+            fmt::print("[DMA{}] {:<8} {} RAM @ 0x{:08x}, {}, BS: 0x{:04x}, BC: 0x{:04x}\n", (int)channel, enum_name(channel), control.dir(),
+                       addr, enum_name(control.syncMode), blockSize, blockCount);
+        }
+
+        // TODO: Execute sync with chopping
+        if (control.direction == CHCR::Direction::toRam) {
+            for (int block = 0; block < blockCount; block++) {
+                for (int i = 0; i < blockSize; i++, addr += step) {
                     sys->writeMemory32(addr, readDevice());
-                    addr += 4;
-                }
-            } else {
-                for (size_t i = 0; i < count.syncMode0.wordCount; i++) {
-                    if (i == count.syncMode0.wordCount - 1) {
-                        sys->writeMemory32(addr, 0xffffff);
-                    } else {
-                        sys->writeMemory32(addr, (addr - 4) & 0xffffff);
-                    }
-                    addr -= 4;
                 }
             }
-            control.enabled = CHCR::Enabled::stop;
-        } else if (control.syncMode == CHCR::SyncMode::syncBlockToDmaRequests) {
-            uint32_t addr = baseAddress.address;
-            int blockCount = count.syncMode1.blockCount;
-            int blockSize = count.syncMode1.blockSize;
-            if (blockCount == 0) blockCount = 0x10000;
-
-            if (control.direction == CHCR::Direction::toRam) {
-                if (verbose) {
-                    fmt::print("[DMA{}] {:<8} -> RAM @ 0x{:08x}, sync, BS: 0x{:04x}, BC: 0x{:04x}\n", (int)channel,
-                               magic_enum::enum_name(channel), addr, blockSize, blockCount);
-                }
-                for (int block = 0; block < blockCount; block++) {
-                    for (int i = 0; i < blockSize; i++, addr += 4) {
-                        sys->writeMemory32(addr, readDevice());
-                    }
-                }
-            } else if (control.direction == CHCR::Direction::fromRam) {
-                if (verbose) {
-                    fmt::print("[DMA{}] {:<8} <- RAM @ 0x{:08x}, sync, BS: 0x{:04x}, BC: 0x{:04x}\n", (int)channel,
-                               magic_enum::enum_name(channel), addr, blockSize, blockCount);
-                }
-
-                for (int block = 0; block < blockCount; block++) {
-                    for (int i = 0; i < blockSize; i++, addr += 4) {
-                        writeDevice(sys->readMemory32(addr));
-                    }
+        } else if (control.direction == CHCR::Direction::fromRam) {
+            for (int block = 0; block < blockCount; block++) {
+                for (int i = 0; i < blockSize; i++, addr += step) {
+                    writeDevice(sys->readMemory32(addr));
                 }
             }
-            baseAddress.address = addr;
-        } else if (control.syncMode == CHCR::SyncMode::linkedListMode) {
-            int addr = baseAddress.address;
+        }
+        baseAddress.address = addr;
+        count.syncMode1.blockCount = 0;
+    } else if (control.syncMode == CHCR::SyncMode::linkedList) {
+        if (verbose) {
+            fmt::print("[DMA{}] {:<8} {} RAM @ 0x{:08x}, {}\n", (int)channel, enum_name(channel), control.dir(), addr,
+                       enum_name(control.syncMode));
+        }
+
+        std::unordered_set<uint32_t> visited;
+        for (;;) {
+            uint32_t blockInfo = sys->readMemory32(addr);
+            int commandCount = blockInfo >> 24;
+            int nextAddr = blockInfo & 0xffffff;
 
-            if (verbose) {
-                fmt::print("[DMA{}] {:<8} <- RAM @ 0x{:08x}, linked list\n", (int)channel, magic_enum::enum_name(channel), addr);
+            if (verbose >= 2) {
+                fmt::print("[DMA{}] {:<8} {} RAM @ 0x{:08x}, {}, count: {}, nextAddr: 0x{:08x}\n", (int)channel, enum_name(channel),
+                           control.dir(), addr, enum_name(control.syncMode), commandCount, nextAddr);
             }
 
-            int breaker = 0;
-            for (;;) {
-                uint32_t blockInfo = sys->readMemory32(addr);
-                int commandCount = blockInfo >> 24;
+            addr += step;
+            for (int i = 0; i < commandCount; i++, addr += step) {
+                writeDevice(sys->readMemory32(addr));
+            }
 
-                addr += 4;
-                for (int i = 0; i < commandCount; i++, addr += 4) {
-                    writeDevice(sys->readMemory32(addr));
-                }
-                addr = blockInfo & 0xffffff;
-                if (addr == 0xffffff || addr == 0) break;
+            addr = nextAddr;
+            if (addr == 0xffffff || addr == 0) break;
 
-                if (++breaker > 0x4000) {
-                    fmt::print("[DMA{}] GPU DMA transfer too long, breaking.\n", (int)channel);
-                    break;
-                }
+            if (visited.find(addr) != visited.end()) {
+                fmt::print("[DMA{}] GPU DMA transfer loop detected, breaking.\n", (int)channel);
+                break;
             }
-            baseAddress.address = addr;
+            visited.insert(addr);
         }
-
-        irqFlag = true;
-        control.enabled = CHCR::Enabled::completed;
+        baseAddress.address = addr;
     }
+
+    irqFlag = true;
+    control.enabled = CHCR::Enabled::completed;
 }
 }  // namespace device::dma

src/device/dma/dma_channel.h

@@ -38,18 +38,18 @@ union BCR {
 
 // DMA Channel Control
 union CHCR {
+    static inline const uint32_t MASK = 0b10001110'10001000'11111000'11111100;
     enum class Direction : uint32_t { toRam = 0, fromRam = 1 };
     enum class MemoryAddressStep : uint32_t { forward = 0, backward = 1 };
-    enum class SyncMode : uint32_t { startImmediately = 0, syncBlockToDmaRequests = 1, linkedListMode = 2 };
-    enum class ChoppingEnable : uint32_t { normal = 0, chopping = 1 };
+    enum class SyncMode : uint32_t { block = 0, sync = 1, linkedList = 2 };
     enum class Enabled : uint32_t { completed = 0, stop = 0, start = 1 };
     enum class StartTrigger : uint32_t { clear = 0, automatic = 0, manual = 1 };
 
     struct {
         Direction direction : 1;
         MemoryAddressStep memoryAddressStep : 1;
         uint32_t : 6;
-        ChoppingEnable choppingEnable : 1;
+        uint32_t choppingEnable : 1;
         SyncMode syncMode : 2;
         uint32_t : 5;
         uint32_t choppingDmaWindowSize : 3;  // Chopping DMA Window Size (1 SHL N words)
@@ -59,15 +59,25 @@ union CHCR {
         Enabled enabled : 1;  // stopped/completed, start/enable/busy
         uint32_t : 3;
         StartTrigger startTrigger : 1;
-        uint32_t : 3;
+        uint32_t unknown1 : 1;
+        uint32_t unknown2 : 1;
+        uint32_t : 1;
     };
     uint32_t _reg;
     uint8_t _byte[4];
 
     CHCR() : _reg(0) {}
+
+    const char* dir() const {
+        if (direction == Direction::fromRam)
+            return "<-";
+        else
+            return "->";
+    }
 };
 
 class DMAChannel {
+   protected:
     int verbose;
     Channel channel;
 
@@ -79,6 +89,8 @@ class DMAChannel {
 
     virtual uint32_t readDevice();
     virtual void writeDevice(uint32_t data);
+    virtual void maskControl();
+    virtual void startTransfer();
 
    public:
     bool irqFlag = false;