From af8ec4db5bb2379a3112afc1ef6d50b46e8f3699 Mon Sep 17 00:00:00 2001 From: Rekai Musuka Date: Mon, 31 Oct 2022 06:17:09 -0300 Subject: [PATCH] chore: go through TODOs and FIXMEs mainly deleting / rewording those that no longer apply --- src/core/Bus.zig | 6 ++++++ src/core/apu.zig | 2 -- src/core/apu/signal/Lfsr.zig | 2 +- src/core/bus/GamePak.zig | 3 +-- src/core/bus/backup.zig | 4 ++-- src/core/bus/backup/eeprom.zig | 2 +- src/core/bus/dma.zig | 1 - src/core/bus/gpio.zig | 2 +- src/core/bus/io.zig | 2 +- src/core/cpu/arm/block_data_transfer.zig | 1 - src/util.zig | 22 ---------------------- 11 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/core/Bus.zig b/src/core/Bus.zig index c1dc753..30c0fa2 100644 --- a/src/core/Bus.zig +++ b/src/core/Bus.zig @@ -121,6 +121,12 @@ fn openBus(self: *const Self, comptime T: type, address: u32) T { const word = blk: { // If Arm, get the most recently fetched instruction (PC + 8) + // + // FIXME: This is most likely a faulty assumption. + // I think what *actually* happens is that the Bus has a latch for the most + // recently fetched piece of data, which is then returned during Open Bus (also DMA open bus?) + // I can "get away" with this because it's very statistically likely that the most recently latched value is + // the most recently fetched instruction by the pipeline if (!self.cpu.cpsr.t.read()) break :blk self.cpu.pipe.stage[1].?; const page = @truncate(u8, r15 >> 24); diff --git a/src/core/apu.zig b/src/core/apu.zig index 5095521..897e13a 100644 --- a/src/core/apu.zig +++ b/src/core/apu.zig @@ -407,7 +407,6 @@ pub const Apu = struct { const ext_left = (clamped_left << 5) | (clamped_left >> 6); const ext_right = (clamped_right << 5) | (clamped_right >> 6); - // FIXME: This rarely happens if (self.sampling_cycle != self.bias.sampling_cycle.read()) self.replaceSDLResampler(); _ = SDL.SDL_AudioStreamPut(self.stream, &[2]u16{ ext_left, ext_right }, 2 * @sizeOf(u16)); @@ -507,7 +506,6 @@ pub fn DmaSound(comptime kind: DmaSoundKind) type { } pub fn push(self: *Self, value: u32) void { - // FIXME: I tried to communicate that this is unlikely to the compiler if (!self.enabled) self.enable(); self.fifo.write(&intToBytes(u32, value)) catch |e| log.err("{} Error: {}", .{ kind, e }); diff --git a/src/core/apu/signal/Lfsr.zig b/src/core/apu/signal/Lfsr.zig index 63a9dd4..9a75907 100644 --- a/src/core/apu/signal/Lfsr.zig +++ b/src/core/apu/signal/Lfsr.zig @@ -33,7 +33,7 @@ pub fn reload(self: *Self, poly: io.PolyCounter) void { } /// Scheduler Event Handler for LFSR Timer Expire -/// FIXME: This gets called a lot, clogging up the Scheduler +/// FIXME: This gets called a lot, slowing down the scheduler pub fn onLfsrTimerExpire(self: *Self, poly: io.PolyCounter, late: u64) void { // Obscure: "Using a noise channel clock shift of 14 or 15 // results in the LFSR receiving no clocks." diff --git a/src/core/bus/GamePak.zig b/src/core/bus/GamePak.zig index d7cd605..ac91b67 100644 --- a/src/core/bus/GamePak.zig +++ b/src/core/bus/GamePak.zig @@ -105,14 +105,13 @@ pub fn dbgRead(self: *const Self, comptime T: type, address: u32) T { switch (T) { u32 => switch (address) { - // TODO: Do I even need to implement these? + // FIXME: Do I even need to implement these? 0x0800_00C4 => std.debug.panic("Handle 32-bit GPIO Data/Direction Reads", .{}), 0x0800_00C6 => std.debug.panic("Handle 32-bit GPIO Direction/Control Reads", .{}), 0x0800_00C8 => std.debug.panic("Handle 32-bit GPIO Control Reads", .{}), else => {}, }, u16 => switch (address) { - // FIXME: What do 16-bit GPIO Reads look like? 0x0800_00C4 => return self.gpio.read(.Data), 0x0800_00C6 => return self.gpio.read(.Direction), 0x0800_00C8 => return self.gpio.read(.Control), diff --git a/src/core/bus/backup.zig b/src/core/bus/backup.zig index 9b7bb91..ef57a74 100644 --- a/src/core/bus/backup.zig +++ b/src/core/bus/backup.zig @@ -151,8 +151,8 @@ pub const Backup = struct { const file_path = try self.savePath(allocator, path); defer allocator.free(file_path); - // FIXME: Don't rely on this lol - if (std.mem.eql(u8, file_path[file_path.len - 12 .. file_path.len], "untitled.sav")) { + const expected = "untitled.sav"; + if (std.mem.eql(u8, file_path[file_path.len - expected.len .. file_path.len], expected)) { return log.err("ROM header lacks title, no save loaded", .{}); } diff --git a/src/core/bus/backup/eeprom.zig b/src/core/bus/backup/eeprom.zig index f1d2238..5eecee4 100644 --- a/src/core/bus/backup/eeprom.zig +++ b/src/core/bus/backup/eeprom.zig @@ -63,7 +63,7 @@ pub const Eeprom = struct { } if (self.state == .RequestEnd) { - if (bit != 0) log.debug("EEPROM Request did not end in 0u1. TODO: is this ok?", .{}); + // if (bit != 0) log.debug("EEPROM Request did not end in 0u1. TODO: is this ok?", .{}); self.state = .Ready; return; } diff --git a/src/core/bus/dma.zig b/src/core/bus/dma.zig index 0856041..6207d79 100644 --- a/src/core/bus/dma.zig +++ b/src/core/bus/dma.zig @@ -259,7 +259,6 @@ fn DmaController(comptime id: u2) type { switch (sad_adj) { .Increment => self.sad_latch +%= offset, .Decrement => self.sad_latch -%= offset, - // FIXME: Is just ignoring this ok? .IncrementReload => log.err("{} is a prohibited adjustment on SAD", .{sad_adj}), .Fixed => {}, } diff --git a/src/core/bus/gpio.zig b/src/core/bus/gpio.zig index c9b7891..9b2b2d2 100644 --- a/src/core/bus/gpio.zig +++ b/src/core/bus/gpio.zig @@ -71,7 +71,7 @@ pub const Gpio = struct { self.* = .{ .data = 0b0000, - .direction = 0b1111, // TODO: What is GPIO DIrection set to by default? + .direction = 0b1111, // TODO: What is GPIO Direction set to by default? .cnt = 0b0, .device = switch (kind) { diff --git a/src/core/bus/io.zig b/src/core/bus/io.zig index 11d75f4..d59c4c4 100644 --- a/src/core/bus/io.zig +++ b/src/core/bus/io.zig @@ -213,7 +213,7 @@ pub fn write(bus: *Bus, comptime T: type, address: u32, value: T) void { // Timers 0x0400_0100...0x0400_010E => timer.write(T, &bus.tim, address, value), - 0x0400_0114 => {}, // TODO: Gyakuten Saiban writes 0x8000 to 0x0400_0114 + 0x0400_0114 => {}, 0x0400_0110 => {}, // Not Used, // Serial Communication 1 diff --git a/src/core/cpu/arm/block_data_transfer.zig b/src/core/cpu/arm/block_data_transfer.zig index f94cbf9..ffe87ac 100644 --- a/src/core/cpu/arm/block_data_transfer.zig +++ b/src/core/cpu/arm/block_data_transfer.zig @@ -57,7 +57,6 @@ pub fn blockDataTransfer(comptime P: bool, comptime U: bool, comptime S: bool, c cpu.r[15] = bus.read(u32, und_addr); cpu.pipe.reload(cpu); } else { - // FIXME: Should r15 on write be +12 ahead? bus.write(u32, und_addr, cpu.r[15] + 4); } diff --git a/src/util.zig b/src/util.zig index 4da0b6d..f2d0d6b 100644 --- a/src/util.zig +++ b/src/util.zig @@ -302,8 +302,6 @@ pub inline fn getHalf(byte: u8) u4 { return @truncate(u4, byte & 1) << 3; } -// TODO: Maybe combine SetLo and SetHi, use addr alignment to deduplicate code - pub inline fn setHalf(comptime T: type, left: T, addr: u8, right: HalfInt(T)) T { const offset = @truncate(u1, addr >> if (T == u32) 1 else 0); @@ -320,26 +318,6 @@ pub inline fn setHalf(comptime T: type, left: T, addr: u8, right: HalfInt(T)) T }; } -/// Sets the high bits of an integer to a value -pub inline fn setLo(comptime T: type, left: T, right: HalfInt(T)) T { - return switch (T) { - u32 => (left & 0xFFFF_0000) | right, - u16 => (left & 0xFF00) | right, - u8 => (left & 0xF0) | right, - else => @compileError("unsupported type"), - }; -} - -/// sets the low bits of an integer to a value -pub inline fn setHi(comptime T: type, left: T, right: HalfInt(T)) T { - return switch (T) { - u32 => (left & 0x0000_FFFF) | @as(u32, right) << 16, - u16 => (left & 0x00FF) | @as(u16, right) << 8, - u8 => (left & 0x0F) | @as(u8, right) << 4, - else => @compileError("unsupported type"), - }; -} - /// The Integer type which corresponds to T with exactly half the amount of bits fn HalfInt(comptime T: type) type { const type_info = @typeInfo(T);