From 4f80523828abfcaf5013c0d3d3e898d6de00fd7b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 31 Mar 2021 22:51:20 -0400 Subject: [PATCH 1/7] Tweaks contended timing. --- Machines/Sinclair/ZXSpectrum/Video.hpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/Video.hpp b/Machines/Sinclair/ZXSpectrum/Video.hpp index 8419b882c..2c03545ec 100644 --- a/Machines/Sinclair/ZXSpectrum/Video.hpp +++ b/Machines/Sinclair/ZXSpectrum/Video.hpp @@ -88,9 +88,12 @@ template class Video { .cycles_per_line = 228 * 2, .lines_per_frame = 311, - .interrupt_time = 56545 * 2, + .interrupt_time = 56542 * 2, - .contention_leadin = 2 * 2, // TODO: is this 2? Or 4? Or... ? + // i.e. video fetching begins five cycles after the start of the + // contended memory pattern below; that should put a clear two + // cycles between a Z80 access and the first video fetch. + .contention_leadin = 5 * 2, .contention_duration = 129 * 2, .delays = { @@ -311,7 +314,9 @@ template class Video { } const int time_into_line = delay_time % timings.cycles_per_line; - if(time_into_line >= timings.contention_duration) return 0; + if(time_into_line >= timings.contention_duration) { + return 0; + } return timings.delays[time_into_line & 15]; } From 687c05365e0a4bc562d78df33cc00be0c7858d30 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 31 Mar 2021 22:52:41 -0400 Subject: [PATCH 2/7] Flushes before `set_last_contended_area_access`. --- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 44 ++++++++++++--------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index 10ecf8460..d924a0144 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -185,25 +185,32 @@ template class ConcreteMachine: forceinline HalfCycles perform_machine_cycle(const CPU::Z80::PartialMachineCycle &cycle) { using PartialMachineCycle = CPU::Z80::PartialMachineCycle; - HalfCycles delay(0); const uint16_t address = cycle.address ? *cycle.address : 0x0000; + if( + is_contended_[address >> 14] && + cycle.operation >= PartialMachineCycle::ReadOpcodeStart && + cycle.operation <= PartialMachineCycle::WriteStart) { + // Apply contention if necessary. + // + // Assumption here: the trigger for the ULA inserting a delay is the falling edge + // of MREQ, which is always half a cycle into a read or write. + // + // TODO: somehow provide that information in the PartialMachineCycle? + + const HalfCycles delay = video_.last_valid()->access_delay(video_.time_since_flush() + HalfCycles(1)); + advance(cycle.length + delay); + return delay; + } + + // TODO: for read/write this should advance only until the rising edge of MREQ, then do + // the read/write, then complete the bus cycle. Only via the 48/128k Spectrum contended + // timings am I now learning what happens with MREQ during extended read/write bus cycles + // (i.e. those longer than 3 cycles) + advance(cycle.length); + switch(cycle.operation) { default: break; - case PartialMachineCycle::ReadOpcodeStart: - case PartialMachineCycle::ReadStart: - case PartialMachineCycle::WriteStart: - // Apply contention if necessary. - // - // Assumption here: the trigger for the ULA inserting a delay is the falling edge - // of MREQ, which is always half a cycle into a read or write. - // - // TODO: somehow provide that information in the PartialMachineCycle? - if(is_contended_[address >> 14]) { - delay = video_.last_valid()->access_delay(video_.time_since_flush() + HalfCycles(1)); - } - break; - case PartialMachineCycle::ReadOpcode: // Fast loading: ROM version. // @@ -225,7 +232,7 @@ template class ConcreteMachine: *cycle.value = read_pointers_[address >> 14][address]; if(is_contended_[address >> 14]) { - video_.last_valid()->set_last_contended_area_access(*cycle.value); + video_->set_last_contended_area_access(*cycle.value); } break; @@ -239,7 +246,7 @@ template class ConcreteMachine: // Fill the floating bus buffer if this write is within the contended area. if(is_contended_[address >> 14]) { - video_.last_valid()->set_last_contended_area_access(*cycle.value); + video_->set_last_contended_area_access(*cycle.value); } break; @@ -357,8 +364,7 @@ template class ConcreteMachine: break; } - advance(cycle.length + delay); - return delay; + return HalfCycles(0); } private: From 87317f5673a6a943b6a591a1ec7225e7db80f6bf Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 1 Apr 2021 12:38:58 -0400 Subject: [PATCH 3/7] Improve documentation, pin down read/write times. --- Machines/Sinclair/ZXSpectrum/Video.hpp | 10 ++++++++-- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 9 ++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/Video.hpp b/Machines/Sinclair/ZXSpectrum/Video.hpp index 2c03545ec..49d81325c 100644 --- a/Machines/Sinclair/ZXSpectrum/Video.hpp +++ b/Machines/Sinclair/ZXSpectrum/Video.hpp @@ -88,14 +88,20 @@ template class Video { .cycles_per_line = 228 * 2, .lines_per_frame = 311, - .interrupt_time = 56542 * 2, - // i.e. video fetching begins five cycles after the start of the // contended memory pattern below; that should put a clear two // cycles between a Z80 access and the first video fetch. .contention_leadin = 5 * 2, .contention_duration = 129 * 2, + // i.e. interrupt is first signalled + // 311*228 - 5 - 56543 = 14364 cycles before the beginning of + // contended accesses. + // + // (which, as above, include a bit of guesswork as to the meaning of + // 'time since interrupt' in the commonly-cited documentation) + .interrupt_time = 56543 * 2, + .delays = { 2, 1, 0, 0, diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index d924a0144..2d4050771 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -206,7 +206,11 @@ template class ConcreteMachine: // the read/write, then complete the bus cycle. Only via the 48/128k Spectrum contended // timings am I now learning what happens with MREQ during extended read/write bus cycles // (i.e. those longer than 3 cycles) - advance(cycle.length); + if(cycle.length > HalfCycles(5)) { + advance(HalfCycles(5)); + } else { + advance(cycle.length); + } switch(cycle.operation) { default: break; @@ -364,6 +368,9 @@ template class ConcreteMachine: break; } + if(cycle.length > HalfCycles(5)) { + advance(cycle.length - HalfCycles(5)); + } return HalfCycles(0); } From 044ac949ba877fc7a86568cdce9e4193ae2bb830 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 1 Apr 2021 12:44:00 -0400 Subject: [PATCH 4/7] Rearrange fields. --- Machines/Sinclair/ZXSpectrum/Video.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/Video.hpp b/Machines/Sinclair/ZXSpectrum/Video.hpp index 49d81325c..aa895ef63 100644 --- a/Machines/Sinclair/ZXSpectrum/Video.hpp +++ b/Machines/Sinclair/ZXSpectrum/Video.hpp @@ -53,14 +53,14 @@ template class Video { // Number of lines comprising a whole frame. Will be 311 or 312. int lines_per_frame; - // Number of cycles after first pixel fetch at which interrupt is first signalled. - int interrupt_time; - // Number of cycles before first pixel fetch that contention starts to be applied. int contention_leadin; // Period in a line for which contention is applied. int contention_duration; + // Number of cycles after first pixel fetch at which interrupt is first signalled. + int interrupt_time; + // Contention to apply, in half-cycles, as a function of number of half cycles since // contention began. int delays[16]; From 14663bd06b5366e786e36d1972326e4653dc2237 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 2 Apr 2021 07:36:57 -0400 Subject: [PATCH 5/7] I think 3 is what I'm aiming for here. But this probably isn't correct for IO cycles. --- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index 2d4050771..f8c568c96 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -206,8 +206,8 @@ template class ConcreteMachine: // the read/write, then complete the bus cycle. Only via the 48/128k Spectrum contended // timings am I now learning what happens with MREQ during extended read/write bus cycles // (i.e. those longer than 3 cycles) - if(cycle.length > HalfCycles(5)) { - advance(HalfCycles(5)); + if(cycle.length > HalfCycles(3)) { + advance(HalfCycles(3)); } else { advance(cycle.length); } @@ -368,8 +368,8 @@ template class ConcreteMachine: break; } - if(cycle.length > HalfCycles(5)) { - advance(cycle.length - HalfCycles(5)); + if(cycle.length > HalfCycles(3)) { + advance(cycle.length - HalfCycles(3)); } return HalfCycles(0); } From 1da51bee6c2dca220a1994981c3947743d5ea5f8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 4 Apr 2021 19:52:19 -0400 Subject: [PATCH 6/7] 14368 and six seem to be the proper numbers, per my comprehension of Patrick Rak. --- Machines/Sinclair/ZXSpectrum/Video.hpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/Video.hpp b/Machines/Sinclair/ZXSpectrum/Video.hpp index aa895ef63..41122b219 100644 --- a/Machines/Sinclair/ZXSpectrum/Video.hpp +++ b/Machines/Sinclair/ZXSpectrum/Video.hpp @@ -88,19 +88,14 @@ template class Video { .cycles_per_line = 228 * 2, .lines_per_frame = 311, - // i.e. video fetching begins five cycles after the start of the + // i.e. video fetching begins six cycles after the start of the // contended memory pattern below; that should put a clear two // cycles between a Z80 access and the first video fetch. - .contention_leadin = 5 * 2, + .contention_leadin = 6 * 2, .contention_duration = 129 * 2, - // i.e. interrupt is first signalled - // 311*228 - 5 - 56543 = 14364 cycles before the beginning of - // contended accesses. - // - // (which, as above, include a bit of guesswork as to the meaning of - // 'time since interrupt' in the commonly-cited documentation) - .interrupt_time = 56543 * 2, + // i.e. interrupt is first signalled 14368 cycles before the first video fetch. + .interrupt_time = (228*311 - 14368) * 2, .delays = { 2, 1, From f26bf4b9e4f77fa40ba812df58df6e0faa503b01 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 4 Apr 2021 19:52:38 -0400 Subject: [PATCH 7/7] Splitting here isn't achieving anything. --- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index 7d58d1d66..73f339027 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -185,12 +185,12 @@ template class ConcreteMachine: using PartialMachineCycle = CPU::Z80::PartialMachineCycle; const uint16_t address = cycle.address ? *cycle.address : 0x0000; + + // Apply contention if necessary. if( is_contended_[address >> 14] && cycle.operation >= PartialMachineCycle::ReadOpcodeStart && cycle.operation <= PartialMachineCycle::WriteStart) { - // Apply contention if necessary. - // // Assumption here: the trigger for the ULA inserting a delay is the falling edge // of MREQ, which is always half a cycle into a read or write. // @@ -201,15 +201,9 @@ template class ConcreteMachine: return delay; } - // TODO: for read/write this should advance only until the rising edge of MREQ, then do - // the read/write, then complete the bus cycle. Only via the 48/128k Spectrum contended - // timings am I now learning what happens with MREQ during extended read/write bus cycles - // (i.e. those longer than 3 cycles) - if(cycle.length > HalfCycles(3)) { - advance(HalfCycles(3)); - } else { - advance(cycle.length); - } + // For all other machine cycles, model the action as happening at the end of the machine cycle; + // that means advancing time now. + advance(cycle.length); switch(cycle.operation) { default: break; @@ -367,9 +361,6 @@ template class ConcreteMachine: break; } - if(cycle.length > HalfCycles(3)) { - advance(cycle.length - HalfCycles(3)); - } return HalfCycles(0); }