NES: FDS Accuracy Improvements

Core/NES/Mappers/FDS/Fds.cpp

@@ -254,6 +254,13 @@ 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
+ - 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();
@@ -389,9 +396,35 @@ void Fds::UpdateCrc(uint8_t value)
 	}
 }
 
+void Fds::SetFdsControlReg(uint8_t value)
+{
+	_motorOn = (value & 0x01) == 0x01;
+	_resetTransfer = (value & 0x02) == 0x02;
+	_readMode = (value & 0x04) == 0x04;
+	SetMirroringType(value & 0x08 ? MirroringType::Horizontal : MirroringType::Vertical);
+	_crcControl = (value & 0x10) == 0x10;
+	//TODO $4025 bit 5 is unknown, all known software sets it to 1
+	_diskReady = (value & 0x40) == 0x40;
+	_diskIrqEnabled = (value & 0x80) == 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);
+}
+
 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;
 	}
 
@@ -417,40 +450,47 @@ void Fds::WriteRegister(uint16_t addr, uint8_t value)
 
 		case 0x4023:
 			_diskRegEnabled = (value & 0x01) == 0x01;
+			//TODO This is actually an audio reset, rename this variable in Fds.h
 			_soundRegEnabled = (value & 0x02) == 0x02;
 
 			if(!_diskRegEnabled) {
 				_irqEnabled = false;
+				//Disabling disk registers forces $4025 = $06 (bit 3 reflected in $4030 reads)
+				SetFdsControlReg(0x06);
+				//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;
-			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;
-
-			//Writing to $4025 clears IRQ according to FCEUX, puNES & Nintendulator
-			//Fixes issues in some unlicensed games (error $20 at power on)
-			_cpu->ClearIrqSource(IRQSource::FdsDisk);
+			SetFdsControlReg(value);
 			break;
 
 		case 0x4026:
-			_extConWriteReg = value;
+			//External connector only wires 7 bits
+			_extConWriteReg = value & 0x7F;
 			break;
 
 		default:
@@ -464,24 +504,29 @@ 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 |= _transferComplete ? 0x02 : 0x00;
 				value |= GetMirroringType() == MirroringType::Horizontal ? 0x08 : 0;
 				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);
+
+				//Byte transfer flag is NOT cleared by this register!
+				//_transferComplete = false;
+				//_cpu->ClearIrqSource(IRQSource::FdsDisk);
 				return value;
 
 			case 0x4031:
@@ -521,7 +566,7 @@ uint8_t Fds::ReadRegister(uint16_t addr)
 
 			case 0x4033:
 				//Always return good battery
-				return _extConWriteReg;
+				return _extConWriteReg | 0x80;
 		}
 	}
 

Core/NES/Mappers/FDS/Fds.h

@@ -85,7 +85,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; }
 
@@ -106,6 +106,7 @@ class Fds : public BaseMapper
 	void ProcessCpuClock() override;
 	void UpdateCrc(uint8_t value);
 
+	void SetFdsControlReg(uint8_t value);
 	void WriteRegister(uint16_t addr, uint8_t value) override;
 	uint8_t ReadRegister(uint16_t addr) override;
 
@@ -127,4 +128,4 @@ class Fds : public BaseMapper
 	bool IsDiskInserted();
 
 	bool IsAutoInsertDiskEnabled();
-};
+};

Core/NES/Mappers/FDS/FdsAudio.cpp

@@ -20,44 +20,64 @@ void FdsAudio::Serialize(Serializer& s)
 	SV(_disableEnvelopes);
 	SV(_haltWaveform);
 	SV(_masterVolume);
-	SV(_waveOverflowCounter);
+	SV(_waveAccumulator);
+	SV(_waveM2Counter);
 	SV(_wavePitch);
 	SV(_wavePosition);
 	SV(_lastOutput);
+	SV(_lastGain);
 }
 
 void FdsAudio::ClockAudio()
 {
-	int frequency = _volume.GetFrequency();
+	uint16_t frequency = _volume.GetFrequency();
 	if(!_haltWaveform && !_disableEnvelopes) {
 		_volume.TickEnvelope();
 		if(_mod.TickEnvelope()) {
 			_mod.UpdateOutput(frequency);
 		}
 	}
 
-	if(_mod.TickModulator()) {
+	//TODO Check if modulator and wave units are ticked on the same M2 cycle
+	//(the FDS stuff is probably ticked by the RAM adapter's crystal - we can't really simulate that here)
+	if(_mod.TickModulator(_haltWaveform)) {
 		//Modulator was ticked, update wave pitch
 		_mod.UpdateOutput(frequency);
 	}
 
-	UpdateOutput();
+	if(++_waveM2Counter == 16) {
+		if(_haltWaveform) {
+			_waveAccumulator = 0;
+		} else {
+			if(!_waveWriteEnabled) {
+				_waveAccumulator += (frequency * _mod.GetOutput()) & 0xFFFFF;
+				if(_waveAccumulator > 0xFFFFFF) {
+					_waveAccumulator -= 0x1000000;
+				}
 
-	if(!_haltWaveform && frequency + _mod.GetOutput() > 0) {
-		_waveOverflowCounter += frequency + _mod.GetOutput();
-		if(_waveOverflowCounter < frequency + _mod.GetOutput()) {
-			_wavePosition = (_wavePosition + 1) & 0x3F;
+				_wavePosition = (_waveAccumulator >> 18) & 0x3F;
+			}
 		}
+		_waveM2Counter = 0;
 	}
+	UpdateOutput();
 }
 
 void FdsAudio::UpdateOutput()
 {
+	//The wave unit continues to run, but the output is held at its previous value
 	if(_waveWriteEnabled) {
 		return;
 	}
 
-	uint32_t level = std::min((int)_volume.GetGain(), 32) * WaveVolumeTable[_masterVolume];
+	//"Changes to the volume envelope only take effect while the wavetable
+	// pointer (top 6 bits of wave accumulator) is 0."
+	if(_wavePosition == 0) {
+		_lastGain = _volume.GetGain();
+	}
+	uint32_t level = std::min(_lastGain, uint8_t(32)) * WaveVolumeTable[_masterVolume];
+	//TODO Volume ramp is actually nonlinear, masked by PWM + 2KHz lowpass
+	//(would likely kill performance for a miniscule audio accuracy improvement)
 	uint8_t outputLevel = (_waveTable[_wavePosition] * level) / 1152;
 
 	if(_lastOutput != outputLevel) {
@@ -81,12 +101,55 @@ uint8_t FdsAudio::ReadRegister(uint16_t addr)
 			//"When writing is disabled ($4089.7), reading anywhere in 4040-407F returns the value at the current wave position"
 			value |= _waveTable[_wavePosition];
 		}
-	} else if(addr == 0x4090) {
-		value &= 0xC0;
-		value |= _volume.GetGain();
-	} else if(addr == 0x4092) {
-		value &= 0xC0;
-		value |= _mod.GetGain();
+	} else if(addr >= 0x4090) {
+		switch(addr) {
+			case 0x4090:
+				value &= 0xC0;
+				value |= _volume.GetGain();
+				break;
+
+			case 0x4091:
+				//Wave accumulator
+				//value &= 0xC0;
+				value = (_waveAccumulator >> 12) & 0xFF;
+				break;
+
+			case 0x4092:
+				value &= 0xC0;
+				value |= _mod.GetGain();
+				break;
+
+			case 0x4093:
+				//Mod accumulator
+				value &= 0x80;
+				value |= (_mod.GetModAccumulator() >> 5) & 0x7F;
+				break;
+
+			case 0x4094:
+				//Wave pitch intermediate result
+				//value &= 0xC0;
+				value = (_mod.GetOutput() >> 4) & 0xFF;
+				break;
+
+			case 0x4095:
+				//Mod counter increment (lower nybble)
+				//TODO Determine upper nybble
+				value &= 0xC0;
+				value |= _mod.GetModIncrement();
+				break;
+
+			case 0x4096:
+				//Wavetable value
+				//TODO PWM masking
+				value &= 0xC0;
+				value |= _waveTable[_wavePosition] & 0x3F;
+				break;
+
+			case 0x4097:
+				value &= 0x80;
+				value |= _mod.GetCounter() & 0x7F;
+				break;
+		}
 	}
 
 	return value;
@@ -174,14 +237,13 @@ void FdsAudio::GetMapperStateEntries(vector<MapperStateEntry>& entries)
 	entries.push_back(MapperStateEntry("$4084.7", "Envelope Disabled", _mod.IsEnvelopeDisabled(), MapperStateValueType::Bool));
 
 	int8_t modCounter = _mod.GetCounter();
-	entries.push_back(MapperStateEntry("$4085.0-6", "Counter", std::to_string(modCounter), modCounter < 0 ? (modCounter + 128) : modCounter));
+	entries.push_back(MapperStateEntry("$4085.0-6", "Counter", std::to_string(modCounter), (modCounter & 0x7F)));
 
 	entries.push_back(MapperStateEntry("$4086/7.0-11", "Frequency", _mod.GetFrequency(), MapperStateValueType::Number16));
 
-	//todo emulation logic + this based on new info
-	//entries.push_back(MapperStateEntry("$4087.6", "???", false, MapperStateValueType::Bool));
+	entries.push_back(MapperStateEntry("$4087.6", "Force Tick Modulator", _mod.GetForceCarryOut(), MapperStateValueType::Bool));
 
-	entries.push_back(MapperStateEntry("$4087.7", "Disabled", _mod.IsModulationDisabled(), MapperStateValueType::Bool));
+	entries.push_back(MapperStateEntry("$4087.7", "Counter Disabled", _mod.IsModCounterDisabled(), MapperStateValueType::Bool));
 	entries.push_back(MapperStateEntry("", "Gain", _mod.GetGain(), MapperStateValueType::Number8));
 	entries.push_back(MapperStateEntry("", "Mod Output", std::to_string(_mod.GetOutput())));
 
@@ -190,4 +252,14 @@ void FdsAudio::GetMapperStateEntries(vector<MapperStateEntry>& entries)
 	entries.push_back(MapperStateEntry("$4089.7", "Wave Write Enabled", _waveWriteEnabled, MapperStateValueType::Bool));
 
 	entries.push_back(MapperStateEntry("$408A", "Envelope Speed Multiplier", _volume.GetMasterSpeed(), MapperStateValueType::Number8));
+
+	entries.push_back(MapperStateEntry("$4090-$4097", "Audio Debug"));
+	entries.push_back(MapperStateEntry("$4090.0-5", "Volume Gain", _volume.GetGain(), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4091", "Wave Accumulator", ((_waveAccumulator >> 12) & 0xFF), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4092.0-5", "Mod Gain", _mod.GetGain(), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4093.0-6", "Mod Accumulator", ((_mod.GetModAccumulator() >> 5) & 0x7F), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4094", "Mod Counter * Gain", ((_mod.GetOutput() >> 4) & 0xFF), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4095.0-3", "Mod Counter Increment", _mod.GetModIncrement(), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4096.0-5", "Wavetable Value", (_waveTable[_wavePosition] & 0x3F), MapperStateValueType::Number8));
+	entries.push_back(MapperStateEntry("$4097.0-6", "Mod Counter Value", std::to_string(modCounter), (modCounter & 0x7F)));
 }

Core/NES/Mappers/FDS/FdsAudio.h

@@ -27,11 +27,13 @@ class FdsAudio : public BaseExpansionAudio
 	uint8_t _masterVolume = 0;
 
 	//Internal values
-	uint16_t _waveOverflowCounter = 0;
+	uint32_t _waveAccumulator = 0; //24-bit accumulator
+	uint8_t _waveM2Counter = 0;
 	int32_t _wavePitch = 0;
 	uint8_t _wavePosition = 0;
 
 	uint8_t _lastOutput = 0;
+	uint8_t _lastGain = 0;
 
 protected:
 	void Serialize(Serializer& s) override;
@@ -46,4 +48,4 @@ class FdsAudio : public BaseExpansionAudio
 	void WriteRegister(uint16_t addr, uint8_t value);
 
 	void GetMapperStateEntries(vector<MapperStateEntry>& entries);
-};
+};