Merge pull request emu-russia#442 from ogamespec/main

fixed regdump (again)

Breaknes/BreaksCore/AbstractBoard.cpp

@@ -105,7 +105,6 @@ namespace Breaknes
 			}
 			ppu_regdump = new RegDumper("PPU", GetPHICounter(), filename);
 			prev_phi_counter_for_ppuregdump = GetPHICounter();
-			phi_flush_counter_ppuregdump = 0;
 
 			printf("PPU regdump enabled to file: %s\n", filename);
 		}
@@ -132,7 +131,6 @@ namespace Breaknes
 			}
 			apu_regdump = new RegDumper("APU", GetPHICounter(), filename);
 			prev_phi_counter_for_apuregdump = GetPHICounter();
-			phi_flush_counter_apuregdump = 0;
 
 			printf("APU regdump enabled to file: %s\n", filename);
 		}
@@ -148,50 +146,37 @@ namespace Breaknes
 
 	/// <summary>
 	/// Check that the 6502 core is accessing the mapped APU/PPU registers and add an entry to regdump if necessary.
+	/// The register operation is committed only on the PHI2 phase of the processor (the signal value is obtained directly from the core)
+	/// If you don't do this, you may catch "bogus" register operations when the register address is set during PHI1.
 	/// </summary>
-	void Board::TreatCoreForRegdump(uint16_t addr_bus, uint8_t data_bus, BaseLogic::TriState m2, BaseLogic::TriState rnw)
+	void Board::TreatCoreForRegdump(uint16_t addr_bus, uint8_t data_bus, BaseLogic::TriState phi2, BaseLogic::TriState rnw)
 	{
-		// The reason for checking for delta = 1 is that the first trigger is received during PHI1 of the core when it sets the register address;
-		// And since the PHI counter is posedge, we need to catch its next change (the next PHI2 after the address is set)
-
+		// APU Regdump
 		if (apu_regdump && (addr_bus & ~MappedAPUMask) == MappedAPUBase) {
 
 			uint64_t phi_now = GetPHICounter();
-			uint64_t delta = phi_now - prev_phi_counter_for_apuregdump;
-			if (prev_phi_counter_for_apuregdump != phi_now && delta == 1) {
+			if (prev_phi_counter_for_apuregdump != phi_now && phi2 == BaseLogic::TriState::One) {
 
 				if (rnw == BaseLogic::TriState::One)
 					apu_regdump->LogRegRead(phi_now, addr_bus & MappedAPUMask);
 				else if (rnw == BaseLogic::TriState::Zero)
 					apu_regdump->LogRegWrite(phi_now, addr_bus & MappedAPUMask, data_bus);
-
-				phi_flush_counter_apuregdump += delta;
-				if (phi_flush_counter_apuregdump >= JustAboutOneSecond) {
-					phi_flush_counter_apuregdump = 0;
-					apu_regdump->Flush();
-				}
+				prev_phi_counter_for_apuregdump = phi_now;
 			}
-			prev_phi_counter_for_apuregdump = phi_now;
 		}
+		// PPU Regump (isomorphic)
 		if (ppu_regdump && (addr_bus & ~MappedPPUMask) == MappedPPUBase) {
 
 			uint64_t phi_now = GetPHICounter();
-			uint64_t delta = phi_now - prev_phi_counter_for_ppuregdump;
-			if (prev_phi_counter_for_ppuregdump != phi_now && delta == 1) {
+			if (prev_phi_counter_for_ppuregdump != phi_now && phi2 == BaseLogic::TriState::One) {
 
 				if (rnw == BaseLogic::TriState::One)
 					ppu_regdump->LogRegRead(phi_now, addr_bus & MappedPPUMask);
 				else if (rnw == BaseLogic::TriState::Zero)
 					ppu_regdump->LogRegWrite(phi_now, addr_bus & MappedPPUMask, data_bus);
-
-				phi_flush_counter_ppuregdump += delta;
-				if (phi_flush_counter_ppuregdump >= JustAboutOneSecond) {
-					phi_flush_counter_ppuregdump = 0;
-					ppu_regdump->Flush();
-				}
+				prev_phi_counter_for_ppuregdump = phi_now;
 			}
-			prev_phi_counter_for_ppuregdump = phi_now;
-		}
+}
 	}
 
 	void Board::GetApuSignalFeatures(APUSim::AudioSignalFeatures* features)

Breaknes/BreaksCore/AbstractBoard.h

@@ -54,10 +54,8 @@ namespace Breaknes
 		RegDumper* apu_regdump = nullptr;
 		size_t prev_phi_counter_for_ppuregdump = 0;
 		size_t prev_phi_counter_for_apuregdump = 0;
-		size_t phi_flush_counter_ppuregdump = 0;
-		size_t phi_flush_counter_apuregdump = 0;
 
-		void TreatCoreForRegdump(uint16_t addr_bus, uint8_t data_bus, BaseLogic::TriState m2, BaseLogic::TriState rnw);
+		void TreatCoreForRegdump(uint16_t addr_bus, uint8_t data_bus, BaseLogic::TriState phi2, BaseLogic::TriState rnw);
 
 	public:
 		Board(APUSim::Revision apu_rev, PPUSim::Revision ppu_rev, Mappers::ConnectorType p1);

Breaknes/BreaksCore/FamicomBoard.cpp

@@ -60,7 +60,7 @@ namespace Breaknes
 		M2 = outputs[(size_t)APUSim::APU_Output::M2];
 
 		// Accesses by the embedded core to APU registers are still broadcast to the address bus via the multiplexer.
-		TreatCoreForRegdump(addr_bus, data_bus, M2, CPU_RnW);
+		TreatCoreForRegdump(addr_bus, data_bus, apu->GetPHI2(), CPU_RnW);
 
 		nOE1 = outputs[(size_t)APUSim::APU_Output::n_IN0];		// #RDP0 official
 		nOE2 = outputs[(size_t)APUSim::APU_Output::n_IN1];		// #RDP1 official

Breaknes/BreaksCore/NESBoard.cpp

@@ -61,7 +61,7 @@ namespace Breaknes
 		M2 = outputs[(size_t)APUSim::APU_Output::M2];
 
 		// Accesses by the embedded core to APU registers are still broadcast to the address bus via the multiplexer.
-		TreatCoreForRegdump(addr_bus, data_bus, M2, CPU_RnW);
+		TreatCoreForRegdump(addr_bus, data_bus, apu->GetPHI2(), CPU_RnW);
 
 		nOE1 = outputs[(size_t)APUSim::APU_Output::n_IN0];		// #RDP0 official
 		nOE2 = outputs[(size_t)APUSim::APU_Output::n_IN1];		// #RDP1 official
@@ -226,7 +226,7 @@ namespace Breaknes
 	{
 		if (PPU_nCE == TriState::Zero && CPU_RnW == TriState::Zero) {
 			int reg_sel = addr_bus & 7;
-			printf("Write PPU %d=0x%02X\n", reg_sel, data_bus);
+			printf("Write PPU %d=0x%02X, phi2: %d, phi counter: 0x%llx\n", reg_sel, data_bus, apu->GetPHI2(), GetPHICounter());
 		}
 	}
 }

BreaksAPU/APUSim/apu.cpp

@@ -161,4 +161,9 @@ namespace APUSim
 
 		// TBD: Add other APU
 	}
+
+	TriState APU::GetPHI2()
+	{
+		return wire.PHI2;
+	}
 }

BreaksAPU/APUSim/apu.h

@@ -365,5 +365,7 @@ namespace APUSim
 		/// </summary>
 		/// <param name="features"></param>
 		void GetSignalFeatures(AudioSignalFeatures& features);
+
+		BaseLogic::TriState GetPHI2();
 	};
 }

BreaksAPU/APUSim/core.cpp

@@ -73,10 +73,11 @@ namespace APUSim
 
 		// TBD: Other APU revisions
 
-		// The software PHI counter is triggered by the raising edge.
+		// The software PHI counter is triggered by the falling edge.
 		// This is purely a software design for convenience, and has nothing to do with APU hardware circuitry.
+		// CPU cycles count by falling edge means that it is a counter of FULL cycles (i.e. PHI1+PHI2 of the processor are executed)
 
-		if (IsPosedge(prev_phi, new_phi))
+		if (IsNegedge(prev_phi, new_phi))
 		{
 			apu->phi_counter++;
 		}

Common/BaseBoardLib/RegDumpProcessor.cpp

@@ -23,7 +23,7 @@ namespace BaseBoard
 	{
 		// Increase the cycle counter
 
-		if (IsPosedge(PrevCLK, CLK))
+		if (IsNegedge(PrevCLK, CLK))
 		{
 			clk_counter++;
 			hold = false;