From 9703fed9f8fad34dfb7b35f17897a054ee282e8c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 18 Jun 2023 14:56:44 -0400 Subject: [PATCH 1/8] Explain constant. --- Components/9918/Implementation/LineLayout.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Components/9918/Implementation/LineLayout.hpp b/Components/9918/Implementation/LineLayout.hpp index 460ee9d709..eedf927cf9 100644 --- a/Components/9918/Implementation/LineLayout.hpp +++ b/Components/9918/Implementation/LineLayout.hpp @@ -28,6 +28,9 @@ template struct LineLayout; // * horizontal adjust on the Yamaha VDPs is applied to EndOfLeftBorder and EndOfPixels; // * the Sega VDPs may programatically extend the left border; and // * text mode on all VDPs adjusts border width. +// +// ModeLaytchCycle is the cycle at which the video mode, blank disable/enable and +// sprite enable/disable are latched for the line. template struct LineLayout> { constexpr static int StartOfSync = 0; From 3a93b8059a22699c08ad507e0157852b92771bb7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 10 Jul 2023 22:24:13 -0400 Subject: [PATCH 2/8] Make faulty attempt to introduce skew into fetching. --- Components/9918/Implementation/9918.cpp | 5 +++++ Components/9918/Implementation/ClockConverter.hpp | 12 ++++++++---- Components/9918/Implementation/Storage.hpp | 3 ++- .../xcshareddata/xcschemes/Clock Signal.xcscheme | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index b8a589e35d..c9757f6210 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -61,6 +61,11 @@ Base::Base() : fetch_pointer_ = output_pointer_; fetch_pointer_.column += output_lag; + // The fetch pointer is interpreted such that its zero is at the mode-latch cycle. + // Conversely the output pointer has zero be at start of sync. So the following + // is a change-of-base rather than a measurement of lag (or similar). + output_pointer_.row += LineLayout::ModeLatchCycle; + fetch_line_buffer_ = line_buffers_.begin(); draw_line_buffer_ = line_buffers_.begin(); fetch_sprite_buffer_ = sprite_buffers_.begin(); diff --git a/Components/9918/Implementation/ClockConverter.hpp b/Components/9918/Implementation/ClockConverter.hpp index 2aa9d840f5..242e5e045a 100644 --- a/Components/9918/Implementation/ClockConverter.hpp +++ b/Components/9918/Implementation/ClockConverter.hpp @@ -55,7 +55,10 @@ template constexpr int clock_rate() { /// Statelessly converts @c length to the internal clock for @c personality; applies conversions per the list of clocks in left-to-right order. template constexpr int to_internal(int length) { if constexpr (head == Clock::FromStartOfSync) { - length = (length + LineLayout::StartOfSync) % LineLayout::CyclesPerLine; + length = ( + length + + (LineLayout::ModeLatchCycle - LineLayout::StartOfSync) + ) % LineLayout::CyclesPerLine; } else { length = length * clock_rate() / clock_rate(); } @@ -70,9 +73,10 @@ template constexpr int to_i /// Statelessly converts @c length to @c clock from the the internal clock used by VDPs of @c personality throwing away any remainder. template constexpr int from_internal(int length) { if constexpr (head == Clock::FromStartOfSync) { - length = - (length + LineLayout::CyclesPerLine - LineLayout::StartOfSync) - % LineLayout::CyclesPerLine; + length = ( + length + LineLayout::CyclesPerLine - + (LineLayout::ModeLatchCycle - LineLayout::StartOfSync) + ) % LineLayout::CyclesPerLine; } else { length = length * clock_rate() / clock_rate(); } diff --git a/Components/9918/Implementation/Storage.hpp b/Components/9918/Implementation/Storage.hpp index bb38207204..c8ce576c42 100644 --- a/Components/9918/Implementation/Storage.hpp +++ b/Components/9918/Implementation/Storage.hpp @@ -87,7 +87,8 @@ struct YamahaFetcher { size_t index = 0; for(int c = 0; c < 1368; c++) { // Specific personality doesn't matter here; both Yamahas use the same internal timing. - const auto event = GeneratorT::event(from_internal(c)); + const int mapped_location = from_internal(c); + const auto event = GeneratorT::event(mapped_location); if(!event) { continue; } diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 3534e99265..474fa352ea 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -1,7 +1,7 @@ + version = "1.8"> From 2ab16867cb6abc9f043cc3b9f0a852b4d4e73c5c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 16 Jul 2023 15:12:52 -0400 Subject: [PATCH 3/8] Require explicit origin. --- .../9918/Implementation/ClockConverter.hpp | 57 ++++++++++++------- Components/9918/Implementation/Fetch.hpp | 26 ++++----- Components/9918/Implementation/Storage.hpp | 2 +- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/Components/9918/Implementation/ClockConverter.hpp b/Components/9918/Implementation/ClockConverter.hpp index 242e5e045a..ab7e1209b3 100644 --- a/Components/9918/Implementation/ClockConverter.hpp +++ b/Components/9918/Implementation/ClockConverter.hpp @@ -24,9 +24,15 @@ enum class Clock { TMSMemoryWindow, /// A fixed 1368-cycle/line clock that is used to count output to the CRT. CRT, +}; + +enum class Origin { + /// + ModeLatch, + /// Provides the same clock rate as ::Internal but is relocated so that 0 is the start of horizontal sync — very not coincidentally, /// where Grauw puts 0 on his detailed TMS and Yamaha timing diagrams. - FromStartOfSync, + StartOfSync, }; template constexpr int clock_rate() { @@ -41,7 +47,6 @@ template constexpr int clock_rate() { case Clock::TMSMemoryWindow: return 171; case Clock::CRT: return 1368; case Clock::Internal: - case Clock::FromStartOfSync: if constexpr (is_classic_vdp(personality)) { return 342; } else if constexpr (is_yamaha_vdp(personality)) { @@ -52,16 +57,8 @@ template constexpr int clock_rate() { } } -/// Statelessly converts @c length to the internal clock for @c personality; applies conversions per the list of clocks in left-to-right order. template constexpr int to_internal(int length) { - if constexpr (head == Clock::FromStartOfSync) { - length = ( - length + - (LineLayout::ModeLatchCycle - LineLayout::StartOfSync) - ) % LineLayout::CyclesPerLine; - } else { - length = length * clock_rate() / clock_rate(); - } + length = length * clock_rate() / clock_rate(); if constexpr (!sizeof...(tail)) { return length; @@ -70,16 +67,23 @@ template constexpr int to_i } } -/// Statelessly converts @c length to @c clock from the the internal clock used by VDPs of @c personality throwing away any remainder. -template constexpr int from_internal(int length) { - if constexpr (head == Clock::FromStartOfSync) { - length = ( - length + LineLayout::CyclesPerLine - - (LineLayout::ModeLatchCycle - LineLayout::StartOfSync) +template constexpr int to_internal(int length) { + if constexpr (origin == Origin::ModeLatch) { + return ( + length + LineLayout::CyclesPerLine - LineLayout::ModeLatchCycle ) % LineLayout::CyclesPerLine; - } else { - length = length * clock_rate() / clock_rate(); } + return length; +} + +/// Statelessly converts @c length to the internal clock for @c personality; applies conversions per the list of clocks in left-to-right order. +template constexpr int to_internal(int length) { + length = to_internal(length); + return to_internal(length); +} + +template constexpr int from_internal(int length) { + length = length * clock_rate() / clock_rate(); if constexpr (!sizeof...(tail)) { return length; @@ -88,6 +92,21 @@ template constexpr int from } } +template constexpr int from_internal(int length) { + if constexpr (origin == Origin::ModeLatch) { + return ( + length + LineLayout::ModeLatchCycle + ) % LineLayout::CyclesPerLine; + } + return length; +} + +/// Statelessly converts @c length to @c clock from the the internal clock used by VDPs of @c personality throwing away any remainder. +template constexpr int from_internal(int length) { + length = from_internal(length); + return from_internal(length); +} + /*! Provides a [potentially-]stateful conversion between the external and internal clocks. Unlike the other clock conversions, this may be non-integral, requiring that diff --git a/Components/9918/Implementation/Fetch.hpp b/Components/9918/Implementation/Fetch.hpp index 0457705361..c5e4007324 100644 --- a/Components/9918/Implementation/Fetch.hpp +++ b/Components/9918/Implementation/Fetch.hpp @@ -54,7 +54,7 @@ template void Base::dispatch(Seq #define index(n) \ if(use_end && end == n) return; \ [[fallthrough]]; \ - case n: fetcher.template fetch(n)>(); + case n: fetcher.template fetch(n)>(); switch(start) { default: assert(false); @@ -373,7 +373,7 @@ struct RefreshSequencer { template void fetch() { if(cycle < 26 || (cycle & 1) || cycle >= 154) { - base->do_external_slot(to_internal(cycle)); + base->do_external_slot(to_internal(cycle)); } } @@ -387,7 +387,7 @@ struct TextSequencer { template void fetch() { // The first 30 and the final 4 slots are external. if constexpr (cycle < 30 || cycle >= 150) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); return; } else { // For the 120 slots in between follow a three-step pattern of: @@ -398,7 +398,7 @@ struct TextSequencer { fetcher.fetch_name(column); break; case 1: // (2) external slot. - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); break; case 2: // (3) fetch tile pattern. fetcher.fetch_pattern(column); @@ -419,7 +419,7 @@ struct CharacterSequencer { template void fetch() { if(cycle < 5) { - character_fetcher.base->do_external_slot(to_internal(cycle)); + character_fetcher.base->do_external_slot(to_internal(cycle)); } if(cycle == 5) { @@ -430,7 +430,7 @@ struct CharacterSequencer { } if(cycle > 14 && cycle < 19) { - character_fetcher.base->do_external_slot(to_internal(cycle)); + character_fetcher.base->do_external_slot(to_internal(cycle)); } // Fetch 8 new sprite Y coordinates, to begin selecting sprites for next line. @@ -448,7 +448,7 @@ struct CharacterSequencer { case 0: character_fetcher.fetch_name(block); break; case 1: if(!(block & 3)) { - character_fetcher.base->do_external_slot(to_internal(cycle)); + character_fetcher.base->do_external_slot(to_internal(cycle)); } else { constexpr int sprite = 8 + ((block >> 2) * 3) + ((block & 3) - 1); sprite_fetcher.fetch_y(sprite); @@ -463,7 +463,7 @@ struct CharacterSequencer { } if(cycle >= 155 && cycle < 157) { - character_fetcher.base->do_external_slot(to_internal(cycle)); + character_fetcher.base->do_external_slot(to_internal(cycle)); } if(cycle == 157) { @@ -511,7 +511,7 @@ struct SMSSequencer { // window 0 to HSYNC low. template void fetch() { if(cycle < 3) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); } if(cycle == 3) { @@ -522,7 +522,7 @@ struct SMSSequencer { } if(cycle == 15 || cycle == 16) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); } if(cycle == 17) { @@ -543,7 +543,7 @@ struct SMSSequencer { case 0: fetcher.fetch_tile_name(block); break; case 1: if(!(block & 3)) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); } else { constexpr int sprite = (8 + ((block >> 2) * 3) + ((block & 3) - 1)) << 1; fetcher.posit_sprite(sprite); @@ -555,7 +555,7 @@ struct SMSSequencer { } if(cycle >= 153 && cycle < 157) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); } if(cycle == 157) { @@ -566,7 +566,7 @@ struct SMSSequencer { } if(cycle >= 169) { - fetcher.base->do_external_slot(to_internal(cycle)); + fetcher.base->do_external_slot(to_internal(cycle)); } } diff --git a/Components/9918/Implementation/Storage.hpp b/Components/9918/Implementation/Storage.hpp index c8ce576c42..8ca9215ab4 100644 --- a/Components/9918/Implementation/Storage.hpp +++ b/Components/9918/Implementation/Storage.hpp @@ -87,7 +87,7 @@ struct YamahaFetcher { size_t index = 0; for(int c = 0; c < 1368; c++) { // Specific personality doesn't matter here; both Yamahas use the same internal timing. - const int mapped_location = from_internal(c); + const int mapped_location = from_internal(c); const auto event = GeneratorT::event(mapped_location); if(!event) { continue; From 3b67d48ebf77a6a09bd5140dc9d273401f998644 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 16 Jul 2023 21:23:49 -0400 Subject: [PATCH 4/8] With origin being its own thing, simplify. --- Components/9918/Implementation/9918.cpp | 4 +-- .../9918/Implementation/ClockConverter.hpp | 30 +++++-------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index c9757f6210..f9c29a793d 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -63,8 +63,8 @@ Base::Base() : // The fetch pointer is interpreted such that its zero is at the mode-latch cycle. // Conversely the output pointer has zero be at start of sync. So the following - // is a change-of-base rather than a measurement of lag (or similar). - output_pointer_.row += LineLayout::ModeLatchCycle; + // is a mere change-of-origin. + output_pointer_.row = to_internal(output_pointer_.row); fetch_line_buffer_ = line_buffers_.begin(); draw_line_buffer_ = line_buffers_.begin(); diff --git a/Components/9918/Implementation/ClockConverter.hpp b/Components/9918/Implementation/ClockConverter.hpp index ab7e1209b3..c35ccfdef7 100644 --- a/Components/9918/Implementation/ClockConverter.hpp +++ b/Components/9918/Implementation/ClockConverter.hpp @@ -57,14 +57,8 @@ template constexpr int clock_rate() { } } -template constexpr int to_internal(int length) { - length = length * clock_rate() / clock_rate(); - - if constexpr (!sizeof...(tail)) { - return length; - } else { - return to_internal(length); - } +template constexpr int to_internal(int length) { + return length * clock_rate() / clock_rate(); } template constexpr int to_internal(int length) { @@ -76,20 +70,13 @@ template constexpr int to_internal(int return length; } -/// Statelessly converts @c length to the internal clock for @c personality; applies conversions per the list of clocks in left-to-right order. -template constexpr int to_internal(int length) { - length = to_internal(length); +template constexpr int to_internal(int length) { + length = to_internal(length); return to_internal(length); } -template constexpr int from_internal(int length) { - length = length * clock_rate() / clock_rate(); - - if constexpr (!sizeof...(tail)) { - return length; - } else { - return to_internal(length); - } +template constexpr int from_internal(int length) { + return length * clock_rate() / clock_rate(); } template constexpr int from_internal(int length) { @@ -101,10 +88,9 @@ template constexpr int from_internal(in return length; } -/// Statelessly converts @c length to @c clock from the the internal clock used by VDPs of @c personality throwing away any remainder. -template constexpr int from_internal(int length) { +template constexpr int from_internal(int length) { length = from_internal(length); - return from_internal(length); + return from_internal(length); } /*! From 8662f06ae5917297f3b097b5bfc0793affb3b5ed Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 16 Jul 2023 22:43:50 -0400 Subject: [PATCH 5/8] Add documentation, to persuade myself. --- .../9918/Implementation/ClockConverter.hpp | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/Components/9918/Implementation/ClockConverter.hpp b/Components/9918/Implementation/ClockConverter.hpp index c35ccfdef7..1735468189 100644 --- a/Components/9918/Implementation/ClockConverter.hpp +++ b/Components/9918/Implementation/ClockConverter.hpp @@ -57,28 +57,36 @@ template constexpr int clock_rate() { } } +/// Scales @c length from @c clock to the internal clock rate. template constexpr int to_internal(int length) { return length * clock_rate() / clock_rate(); } -template constexpr int to_internal(int length) { +/// Moves @c position that is relative to @c Origin::StartOfSync so that it is relative to @c origin ; +/// i.e. can be thought of as "to [internal with origin as specified]". +template constexpr int to_internal(int position) { if constexpr (origin == Origin::ModeLatch) { return ( - length + LineLayout::CyclesPerLine - LineLayout::ModeLatchCycle + position + LineLayout::CyclesPerLine - LineLayout::ModeLatchCycle ) % LineLayout::CyclesPerLine; } - return length; + return position; } -template constexpr int to_internal(int length) { - length = to_internal(length); - return to_internal(length); +/// Converts @c position from one that is measured at the rate implied by @c clock and relative to @c Origin::StartOfSync +/// to one that is at the internal clock rate and relative to @c origin. +template constexpr int to_internal(int position) { + position = to_internal(position); + return to_internal(position); } +/// Scales @c length from the internal clock rate to @c clock. template constexpr int from_internal(int length) { return length * clock_rate() / clock_rate(); } +/// Moves @c position that is relative to @c origin so that it is relative to @c Origin::StartOfSync ; +/// i.e. can be thought of as "from [internal with origin as specified]". template constexpr int from_internal(int length) { if constexpr (origin == Origin::ModeLatch) { return ( @@ -88,9 +96,11 @@ template constexpr int from_internal(in return length; } -template constexpr int from_internal(int length) { - length = from_internal(length); - return from_internal(length); +/// Converts @c position from one that is measured at the internal clock rate and relative to @c origin +/// to one that is at the rate implied by @c clock and relative to @c Origin::StartOfSync +template constexpr int from_internal(int position) { + position = from_internal(position); + return from_internal(position); } /*! From 7f48cd6d9d5e8e810d71137e80232535b7861017 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 21 Jul 2023 15:48:52 -0400 Subject: [PATCH 6/8] Fix fetch offset. Breaking interrupt placement. --- Components/9918/Implementation/9918.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index f9c29a793d..8d93c9d7c1 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -30,6 +30,10 @@ Base::Base() : // Unimaginatively, this class just passes RGB through to the shader. Investigation is needed // into whether there's a more natural form. It feels unlikely given the diversity of chips modelled. + // TODO: values in mode_timing_ should be relative to the ModeLatch since the fetch pointer is used pervasively + // as the authoritative current position. But also it's not clear to me that my current approach of treating them + // as runtime constant is really working. Some more thought required. + if constexpr (is_sega_vdp(personality)) { // Cf. https://www.smspower.org/forums/8161-SMSDisplayTiming @@ -50,7 +54,7 @@ Base::Base() : if constexpr (is_yamaha_vdp(personality)) { // TODO: this is used for interrupt _prediction_ but won't handle text modes correctly, and indeed // can't be just a single value where the programmer has changed into or out of text modes during the - // middle of a line, since screen mode is latched (so it'll be one value for that line, another from then onwards).a + // middle of a line, since screen mode is latched (so it'll be one value for that line, another from then onwards). mode_timing_.line_interrupt_position = LineLayout::EndOfPixels; } @@ -64,7 +68,8 @@ Base::Base() : // The fetch pointer is interpreted such that its zero is at the mode-latch cycle. // Conversely the output pointer has zero be at start of sync. So the following // is a mere change-of-origin. - output_pointer_.row = to_internal(output_pointer_.row); + fetch_pointer_.column = to_internal(output_pointer_.column); + ++output_pointer_.row; fetch_line_buffer_ = line_buffers_.begin(); draw_line_buffer_ = line_buffers_.begin(); @@ -229,7 +234,7 @@ void TMS9918::run_for(const HalfCycles cycles) { // TODO: where did this magic constant come from? https://www.smspower.org/forums/17970-RoadRashHow#111000 mentioned in passing // that "the vertical scroll register is latched at the start of the active display" and this is two clocks before that, so it's // not uncompelling. I can just no longer find my source. - constexpr auto latch_time = LineLayout::EndOfLeftBorder - 2; + constexpr auto latch_time = to_internal(LineLayout::EndOfLeftBorder - 2); static_assert(latch_time > 0); if(this->fetch_pointer_.column < latch_time && end_column >= latch_time) { Storage::latched_vertical_scroll_ = Storage::vertical_scroll_; From 1797bab28f9e959c544504e1b1b3a8435ec06926 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 23 Jul 2023 14:26:02 -0400 Subject: [PATCH 7/8] Explain logic. --- Components/9918/Implementation/9918.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index 8d93c9d7c1..720482e1de 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -68,8 +68,18 @@ Base::Base() : // The fetch pointer is interpreted such that its zero is at the mode-latch cycle. // Conversely the output pointer has zero be at start of sync. So the following // is a mere change-of-origin. + // + // Logically, any mode latch time greater than 0 — i.e. beyond the start of sync — will + // cause fetch_pointer_ to **regress**. It will be set to the value it was at the start + // of sync, ready to overflow to 0 upon mode latch. When it overflows, the fetch row + // will be incremented. + // + // Therefore the nominal output row at instantiation needs to be one greater than the + // fetch row, as that's the first row that'll actually be fetched. fetch_pointer_.column = to_internal(output_pointer_.column); - ++output_pointer_.row; + if(LineLayout::ModeLatchCycle) { + ++output_pointer_.row; + } fetch_line_buffer_ = line_buffers_.begin(); draw_line_buffer_ = line_buffers_.begin(); From e2dcb0a8e2401656244d3978f7f14803adcb06e5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 28 Jul 2023 10:34:15 -0400 Subject: [PATCH 8/8] Begin commutation of interrupts. --- Components/9918/Implementation/9918.cpp | 36 +++---------------- Components/9918/Implementation/9918Base.hpp | 10 ------ Components/9918/Implementation/LineLayout.hpp | 29 ++++++++++++++- 3 files changed, 33 insertions(+), 42 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index 720482e1de..984b8e7829 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -30,34 +30,6 @@ Base::Base() : // Unimaginatively, this class just passes RGB through to the shader. Investigation is needed // into whether there's a more natural form. It feels unlikely given the diversity of chips modelled. - // TODO: values in mode_timing_ should be relative to the ModeLatch since the fetch pointer is used pervasively - // as the authoritative current position. But also it's not clear to me that my current approach of treating them - // as runtime constant is really working. Some more thought required. - - if constexpr (is_sega_vdp(personality)) { - // Cf. https://www.smspower.org/forums/8161-SMSDisplayTiming - - // "For a line interrupt, /INT is pulled low 608 mclks into the appropriate scanline relative to pixel 0. - // This is 3 mclks before the rising edge of /HSYNC which starts the next scanline." - // - // i.e. it's 304 internal clocks after the end of the left border. - mode_timing_.line_interrupt_position = (LineLayout::EndOfLeftBorder + 304) % LineLayout::CyclesPerLine; - - // For a frame interrupt, /INT is pulled low 607 mclks into scanline 192 (of scanlines 0 through 261) relative to pixel 0. - // This is 4 mclks before the rising edge of /HSYNC which starts the next scanline. - // - // i.e. it's 1/2 cycle before the line interrupt position, which I have rounded. Ugh. - mode_timing_.end_of_frame_interrupt_position.column = mode_timing_.line_interrupt_position - 1; - mode_timing_.end_of_frame_interrupt_position.row = 192 + (LineLayout::EndOfLeftBorder + 304) / LineLayout::CyclesPerLine; - } - - if constexpr (is_yamaha_vdp(personality)) { - // TODO: this is used for interrupt _prediction_ but won't handle text modes correctly, and indeed - // can't be just a single value where the programmer has changed into or out of text modes during the - // middle of a line, since screen mode is latched (so it'll be one value for that line, another from then onwards). - mode_timing_.line_interrupt_position = LineLayout::EndOfPixels; - } - // Establish that output is delayed after reading by `output_lag` cycles, // i.e. the fetch pointer is currently _ahead_ of the output pointer. output_pointer_.row = output_pointer_.column = 0; @@ -303,13 +275,15 @@ void TMS9918::run_for(const HalfCycles cycles) { // ------------------------------- // Check for interrupt conditions. // ------------------------------- - if constexpr (is_sega_vdp(personality)) { + if constexpr (LineLayout::HasFixedLineInterrupt) { // The Sega VDP offers a decrementing counter for triggering line interrupts; // it is reloaded either when it overflows or upon every non-pixel line after the first. // It is otherwise decremented. + + constexpr int FixedLineInterrupt = to_internal(LineLayout::FixedLineInterrupt); if( - this->fetch_pointer_.column < this->mode_timing_.line_interrupt_position && - end_column >= this->mode_timing_.line_interrupt_position + this->fetch_pointer_.column < FixedLineInterrupt && + end_column >= FixedLineInterrupt ) { if(this->fetch_pointer_.row >= 0 && this->fetch_pointer_.row <= this->mode_timing_.pixel_lines) { if(!this->line_interrupt_counter_) { diff --git a/Components/9918/Implementation/9918Base.hpp b/Components/9918/Implementation/9918Base.hpp index 4b4085f45b..94e9f36d45 100644 --- a/Components/9918/Implementation/9918Base.hpp +++ b/Components/9918/Implementation/9918Base.hpp @@ -170,16 +170,6 @@ template struct Base: public Storage { // then the appropriate status information will be set. int maximum_visible_sprites = 4; - // Set the position, in cycles, of the two interrupts, - // within a line. - // - // TODO: redetermine where this number came from. - struct { - int column = 313; - int row = 192; - } end_of_frame_interrupt_position; - int line_interrupt_position = -1; - // Enables or disabled the recognition of the sprite // list terminator, and sets the terminator value. bool allow_sprite_terminator = true; diff --git a/Components/9918/Implementation/LineLayout.hpp b/Components/9918/Implementation/LineLayout.hpp index eedf927cf9..b858023b2c 100644 --- a/Components/9918/Implementation/LineLayout.hpp +++ b/Components/9918/Implementation/LineLayout.hpp @@ -32,7 +32,7 @@ template struct LineLayout; // ModeLaytchCycle is the cycle at which the video mode, blank disable/enable and // sprite enable/disable are latched for the line. -template struct LineLayout> { +template struct LineLayout> { constexpr static int StartOfSync = 0; constexpr static int EndOfSync = 26; constexpr static int StartOfColourBurst = 29; @@ -51,11 +51,34 @@ template struct LineLayout struct LineLayout> : + public LineLayout { + + // Cf. https://www.smspower.org/forums/8161-SMSDisplayTiming + + // "For a line interrupt, /INT is pulled low 608 mclks into the appropriate scanline relative to pixel 0. + // This is 3 mclks before the rising edge of /HSYNC which starts the next scanline." + // + // i.e. it's 304 internal clocks after the end of the left border. + constexpr static bool HasFixedLineInterrupt = false; + constexpr static int FixedLineInterrupt = (EndOfLeftBorder + 304) % CyclesPerLine; + + // For a frame interrupt, /INT is pulled low 607 mclks into scanline 192 (of scanlines 0 through 261) relative to pixel 0. + // This is 4 mclks before the rising edge of /HSYNC which starts the next scanline. + // + // i.e. it's 1/2 cycle before the line interrupt position, which I have rounded. Ugh. + constexpr static int EndOfFrameInterrupt = 313; +}; + template struct LineLayout> { constexpr static int StartOfSync = 0; constexpr static int EndOfSync = 100; @@ -73,6 +96,10 @@ template struct LineLayout