Skip to content

NES: FDS Accuracy Improvements #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TakuikaNinja wants to merge 13 commits into
base: master
Choose a base branch
from
Open

NES: FDS Accuracy Improvements #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3c0edfe
FDS $4023 should not affect read-only registers
TakuikaNinja Apr 13, 2026
46da458
FDS external connector improvements
TakuikaNinja Apr 14, 2026
b626a32
FDS: Move byte transfer flag to $4030 bit 7
TakuikaNinja May 10, 2026
e3aa90a
FDS: $4030 read does not clear disk IRQ
TakuikaNinja May 21, 2026
903ee25
FDS: Improve audio reset handling ($4023.D1 = 0)
TakuikaNinja May 21, 2026
5a0a6f6
FDS: Implement $4090-$4097 reads
TakuikaNinja May 21, 2026
7084e67
FDS: Change $4094 to return the plain counter*gain result
TakuikaNinja May 21, 2026
84c897f
FDS: Fix off-by-one in envelope timer reset
TakuikaNinja May 21, 2026
dab3c10
FDS: Add remaining registers to register view
TakuikaNinja May 21, 2026
c23ff1b
FDS: Run clang-format on sources
TakuikaNinja May 21, 2026
d67831f
FDS: Tweak register viewer headings
TakuikaNinja May 21, 2026
9894d84
FDS: Improve $4025 handling
TakuikaNinja May 25, 2026
638540d
FDS: Fix disk auto-insert regression
TakuikaNinja May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Core/NES/Mappers/FDS/BaseFdsChannel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class BaseFdsChannel : public ISerializable
switch(addr & 0x03) {
case 0:
_speed = value & 0x3F;
_volumeIncrease = (value & 0x40) == 0x40;
_envelopeOff = (value & 0x80) == 0x80;
_volumeIncrease = value & 0x40;
_envelopeOff = value & 0x80;

//"Writing to this register immediately resets the clock timer that ticks the volume envelope (delaying the next tick slightly)."
ResetTimer();
Expand Down Expand Up @@ -111,6 +111,6 @@ class BaseFdsChannel : public ISerializable

void ResetTimer()
{
_timer = 8 * (_speed + 1) * _masterSpeed;
_timer = 8 * (_speed + 1) * (_masterSpeed + 1);
}
};
};
145 changes: 115 additions & 30 deletions Core/NES/Mappers/FDS/Fds.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void Fds::ClockIrq()

uint8_t Fds::ReadRam(uint16_t addr)
{
if(addr == 0xE18C && !_gameStarted && (_memoryManager->DebugRead(0x100) & 0xC0) != 0) {
if(addr == 0xE18C && !_gameStarted && (_memoryManager->DebugRead(0x100) & 0xC0)) {
//$E18B is the NMI entry point (using $E18C due to dummy reads)
//When NMI occurs while $100 & $C0 != 0, it typically means that the game is starting.
_gameStarted = true;
Expand Down Expand Up @@ -190,6 +190,7 @@ uint8_t Fds::ReadRam(uint16_t addr)
if(matchCount > 1) {
//More than 1 disk matches, can happen in unlicensed games - disable auto insert logic
_disableAutoInsertDisk = true;
MessageManager::Log("[FDS] Multiple disks match, auto-insert disabled.");
}

if(matchIndex >= 0) {
Expand Down Expand Up @@ -254,6 +255,14 @@ void Fds::ProcessAutoDiskInsert()
}
}

/**TODO:
- Proper byte transfer flag handling (set every 1792 master cycles, or 149+1/3 CPU cycles, under normal conditions)
- Should allow for accurate handling of level 2->3 load bug in Ai Senshi Nicol
- Proper CRC handling using $4025 bits 4 and 6
- Verify End of Head handling (may affect Kosodate Gokko and FMC Disk Card Checker)
- DRAM refresh watchdog implementation (must track PRG-RAM vs external access cycles...)
(Ongoing research, please consult TakuikaNinja for further details)
**/
void Fds::ProcessCpuClock()
{
BaseProcessCpuClock();
Expand All @@ -273,6 +282,13 @@ void Fds::ProcessCpuClock()
//Disk has been ejected
_endOfHead = true;
_scanningDisk = false;

//It appears the BIOS disk I/O routines can stop the motor well before the end of the disk
//(File transfer loop normally runs until accessed files == value in file amount block)
//Wait a bit before ejecting the disk (eject in ~77 frames)
if(_autoDiskEjectCounter < 0) {
_autoDiskEjectCounter = 77;
}
return;
}

Expand Down Expand Up @@ -391,7 +407,16 @@ void Fds::UpdateCrc(uint8_t value)

void Fds::WriteRegister(uint16_t addr, uint8_t value)
{
if((!_diskRegEnabled && addr >= 0x4024 && addr <= 0x4026) || (!_soundRegEnabled && addr >= 0x4040)) {
if(!_diskRegEnabled && addr >= 0x4024 && addr <= 0x4026) {
return;
}

/**Only $4080 (volume envelope) seems to consistently deny writes during audio reset
TODO:
- $4085 (mod counter) denies writes too, but there is an unknown delay before being forced to 0
- Determine $4088 (mod table write) behaviour while in audio reset state
**/
if(!_soundRegEnabled && (addr == 0x4080 || addr == 0x4085 || addr == 0x4088)) {
return;
}

Expand All @@ -405,8 +430,8 @@ void Fds::WriteRegister(uint16_t addr, uint8_t value)
break;

case 0x4022:
_irqRepeatEnabled = (value & 0x01) == 0x01;
_irqEnabled = (value & 0x02) == 0x02 && _diskRegEnabled;
_irqRepeatEnabled = value & 0x01;
_irqEnabled = (value & 0x02) && _diskRegEnabled;

if(_irqEnabled) {
_irqCounter = _irqReloadValue;
Expand All @@ -416,41 +441,72 @@ void Fds::WriteRegister(uint16_t addr, uint8_t value)
break;

case 0x4023:
_diskRegEnabled = (value & 0x01) == 0x01;
_soundRegEnabled = (value & 0x02) == 0x02;
_diskRegEnabled = value & 0x01;
//TODO This is actually an audio reset, rename this variable in Fds.h
_soundRegEnabled = value & 0x02;

if(!_diskRegEnabled) {
_irqEnabled = false;

//Disabling disk registers forces $4025 = $06 (bit 3 reflected in $4030 reads)
_resetTransfer = true;
_motorOn = false;
_readMode = true;
SetMirroringType(MirroringType::Vertical);
_crcControl = false;
//TODO Set $4025 bit 5 = 0 here once implemented

_diskReady = false; //TODO $4025 bit 6 is CRC enable, not disk ready flag
_diskIrqEnabled = false;

//Disabling disk registers forces $4026 (external connector) = 0x7F
_extConWriteReg = 0x7F;

_cpu->ClearIrqSource(IRQSource::External);
_cpu->ClearIrqSource(IRQSource::FdsDisk);
}

/**TODO Determine/implement audio reset behaviour, should probably go in FdsAudio:
- Proper method of resetting modulation state ($4085 write below doesn't always work)
- Reset wave accumulator to 0
- Mod table appears to init with (or decay to) all 0s?
- There seems to be some kind of analogue "resume" window?
(Ongoing research, please consult TakuikaNinja for further details)
**/
if(!_soundRegEnabled) {
_audio->WriteRegister(0x4080, 0x80); //Based on instant muting
_audio->WriteRegister(0x4085, 0x00); //Based on $4097 state
}
break;

case 0x4024:
_writeDataReg = value;
_transferComplete = false;

//Unsure about clearing irq here: FCEUX/Nintendulator don't do this, puNES does.
//Needed by some Super Magic Card games, which use this IRQ for raster effects
_cpu->ClearIrqSource(IRQSource::FdsDisk);
break;

case 0x4025:
_motorOn = (value & 0x01) == 0x01;
_resetTransfer = (value & 0x02) == 0x02;
_readMode = (value & 0x04) == 0x04;
_resetTransfer = !(value & 0x01); // 0 = reset transfer
_motorOn = !(value & 0x02); // 0 = start motor
_readMode = value & 0x04;
SetMirroringType(value & 0x08 ? MirroringType::Horizontal : MirroringType::Vertical);
_crcControl = (value & 0x10) == 0x10;
//Bit 6 is not used, always 1
_diskReady = (value & 0x40) == 0x40;
_diskIrqEnabled = (value & 0x80) == 0x80;
_crcControl = value & 0x10;
//TODO $4025 bit 5 is unknown, all known software sets it to 1

_diskReady = value & 0x40; //TODO $4025 bit 6 is CRC enable, not disk ready flag
_diskIrqEnabled = value & 0x80;

//Writing to $4025 clears IRQ according to FCEUX, puNES & Nintendulator
//Fixes issues in some unlicensed games (error $20 at power on)
//TODO This probably depends on the bits set?
_cpu->ClearIrqSource(IRQSource::FdsDisk);
break;

case 0x4026:
_extConWriteReg = value;
//External connector only wires 7 bits
_extConWriteReg = value & 0x7F;
break;

default:
Expand All @@ -464,24 +520,25 @@ void Fds::WriteRegister(uint16_t addr, uint8_t value)
uint8_t Fds::ReadRegister(uint16_t addr)
{
uint8_t value = _memoryManager->GetOpenBus();
if(_soundRegEnabled && addr >= 0x4040) {
if(addr >= 0x4040) {
return _audio->ReadRegister(addr);
} else if(_diskRegEnabled && addr <= 0x4033) {
} else if(addr <= 0x4033) {
switch(addr) {
case 0x4030:
//These 3 pins are open bus
//These 2 pins are open bus
value &= 0x24;

/**TODO:
- DRAM refresh watchdog IRQ status is returned in bit 1, needs implementation
- End of Head is returned in bit 6, needs verification
**/
value |= _cpu->HasIrqSource(IRQSource::External) ? 0x01 : 0x00;
value |= _transferComplete ? 0x02 : 0x00;
value |= GetMirroringType() == MirroringType::Horizontal ? 0x08 : 0;
//value |= _transferComplete ? 0x02 : 0x00;
value |= GetMirroringType() == MirroringType::Horizontal ? 0x08 : 0x00;
value |= _useQdFormat && _badCrc ? 0x10 : 0x00;
//value |= _endOfHead ? 0x40 : 0x00;
//value |= _diskRegEnabled ? 0x80 : 0x00;
value |= _transferComplete ? 0x80 : 0x00;

_transferComplete = false;
_cpu->ClearIrqSource(IRQSource::External);
_cpu->ClearIrqSource(IRQSource::FdsDisk);
return value;

case 0x4031:
Expand All @@ -494,8 +551,9 @@ uint8_t Fds::ReadRegister(uint16_t addr)
value &= 0xF8;

value |= !IsDiskInserted() ? 0x01 : 0x00; //Disk not in drive
//TODO This should use _diskReady instead
value |= (!IsDiskInserted() || !_scanningDisk) ? 0x02 : 0x00; //Disk not ready
value |= !IsDiskInserted() ? 0x04 : 0x00; //Disk not writable
value |= !IsDiskInserted() ? 0x04 : 0x00; //Simulate inserted disks as always being writable

if(IsAutoInsertDiskEnabled()) {
if(_emu->GetFrameCount() - _lastDiskCheckFrame < 100) {
Expand All @@ -520,8 +578,8 @@ uint8_t Fds::ReadRegister(uint16_t addr)
return value;

case 0x4033:
//Always return good battery
return _extConWriteReg;
//Always return good battery status in bit 7
return _extConWriteReg | 0x80;
Comment thread
TakuikaNinja marked this conversation as resolved.
}
}

Expand Down Expand Up @@ -631,20 +689,47 @@ vector<MapperStateEntry> Fds::GetMapperStateEntries()
{
vector<MapperStateEntry> entries;

entries.push_back(MapperStateEntry("", "Timer IRQ"));
entries.push_back(MapperStateEntry("$4020/1", "IRQ Reload Value", _irqReloadValue, MapperStateValueType::Number16));
entries.push_back(MapperStateEntry("$4022.0", "IRQ Repeat", _irqRepeatEnabled, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4022.1", "IRQ Enabled", _irqEnabled, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("", "IRQ Counter", _irqCounter, MapperStateValueType::Number16));

entries.push_back(MapperStateEntry("", "I/O Control"));
entries.push_back(MapperStateEntry("$4023.0", "Disk Registers Enabled", _diskRegEnabled, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4023.1", "Sound Registers Enabled", _soundRegEnabled, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4023.1", "Audio Enabled", _soundRegEnabled, MapperStateValueType::Bool));

entries.push_back(MapperStateEntry("$4025.0", "Motor Enabled", _motorOn, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4025.1", "Reset Transfer", _resetTransfer, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("", "Disk Drive"));
entries.push_back(MapperStateEntry("$4025", "Drive Control"));
entries.push_back(MapperStateEntry("$4025.0", "Scan Disk", !_resetTransfer, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4025.1", "Stop Motor", !_motorOn, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4025.2", "Read Mode", _readMode, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4025.3", "Mirroring", GetMirroringType() == MirroringType::Horizontal ? "Horizontal" : "Vertical", GetMirroringType() == MirroringType::Horizontal ? 1 : 0));
entries.push_back(MapperStateEntry("$4025.4", "Transfer CRC", _crcControl, MapperStateValueType::Bool));
//entries.push_back(MapperStateEntry("$4025.6", "CRC Enabled", _crcEnabled, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4025.7", "Disk IRQ Enabled", _diskIrqEnabled, MapperStateValueType::Bool));

entries.push_back(MapperStateEntry("$4030", "Drive/IRQ Status"));
entries.push_back(MapperStateEntry("$4030.0", "Timer IRQ", _cpu->HasIrqSource(IRQSource::External), MapperStateValueType::Bool));
//entries.push_back(MapperStateEntry("$4030.1", "DRAM Refresh Watchdog IRQ", false, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4030.3", "Mirroring", GetMirroringType() == MirroringType::Horizontal ? "Horizontal" : "Vertical", GetMirroringType() == MirroringType::Horizontal ? 1 : 0));
entries.push_back(MapperStateEntry("$4030.5", "CRC Error", (_useQdFormat && _badCrc), MapperStateValueType::Bool));
//entries.push_back(MapperStateEntry("$4030.6", "End of Head", _endOfHead, MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4030.7", "Byte Transferred", _transferComplete, MapperStateValueType::Bool));

entries.push_back(MapperStateEntry("$4024/$4031", "Disk Data"));
entries.push_back(MapperStateEntry("$4024", "Disk Data Output", _writeDataReg, MapperStateValueType::Number8));
entries.push_back(MapperStateEntry("$4031", "Disk Data Input", _readDataReg, MapperStateValueType::Number8));

entries.push_back(MapperStateEntry("$4032", "Disk Status"));
entries.push_back(MapperStateEntry("$4032.0", "Disk Inserted", IsDiskInserted(), MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4032.1", "Disk Ready", (IsDiskInserted() & _scanningDisk), MapperStateValueType::Bool));
entries.push_back(MapperStateEntry("$4032.2", "Write Enabled", IsDiskInserted(), MapperStateValueType::Bool));

entries.push_back(MapperStateEntry("", "External Connector"));
entries.push_back(MapperStateEntry("$4026/$4033.0-6", "External Connector Value", _extConWriteReg, MapperStateValueType::Number8));
entries.push_back(MapperStateEntry("$4033.7", "Drive Powered", true, MapperStateValueType::Bool));

_audio->GetMapperStateEntries(entries);

return entries;
Expand Down
6 changes: 4 additions & 2 deletions Core/NES/Mappers/FDS/Fds.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class Fds : public BaseMapper

uint8_t _writeDataReg = 0;

//TODO Change $4025.D0-D1 namings/meanings so we don't have to invert the read/write logic
// (_resetTransfer (bit 0) -> _scanMedia, _motorOn (bit 1) -> _stopMotor)
bool _motorOn = false;
bool _resetTransfer = false;
bool _readMode = false;
Expand Down Expand Up @@ -85,7 +87,7 @@ class Fds : public BaseMapper
uint32_t GetWorkRamPageSize() override { return 0x8000; }
uint32_t GetWorkRamSize() override { return 0x8000; }
uint16_t RegisterStartAddress() override { return 0x4020; }
uint16_t RegisterEndAddress() override { return 0x4092; }
uint16_t RegisterEndAddress() override { return 0x4097; }
bool AllowRegisterRead() override { return true; }
bool EnableCpuClockHook() override { return true; }

Expand Down Expand Up @@ -127,4 +129,4 @@ class Fds : public BaseMapper
bool IsDiskInserted();

bool IsAutoInsertDiskEnabled();
};
};
Loading