From facc0a1976a6933742e42008883cd16c20b69a2a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 17 Nov 2019 21:28:51 -0500 Subject: [PATCH 01/10] Amps up the documentation. --- Machines/Atari/ST/Video.cpp | 8 ++--- Machines/Atari/ST/Video.hpp | 60 +++++++++++++++++++++++++++++-------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index 0b23eb34c..fa66029f9 100644 --- a/Machines/Atari/ST/Video.cpp +++ b/Machines/Atari/ST/Video.cpp @@ -33,7 +33,7 @@ const struct VerticalParams { }; /// @returns The correct @c VerticalParams for output at @c frequency. -const VerticalParams &vertical_parameters(FieldFrequency frequency) { +const VerticalParams &vertical_parameters(Video::FieldFrequency frequency) { return vertical_params[int(frequency)]; } @@ -63,7 +63,7 @@ const struct HorizontalParams { {4*2, 164*2, 184*2, 2*2, 224*2} }; -const HorizontalParams &horizontal_parameters(FieldFrequency frequency) { +const HorizontalParams &horizontal_parameters(Video::FieldFrequency frequency) { return horizontal_params[int(frequency)]; } @@ -72,14 +72,14 @@ struct Checker { Checker() { for(int c = 0; c < 3; ++c) { // Expected horizontal order of events: reset blank, enable display, disable display, enable blank (at least 50 before end of line), end of line - const auto horizontal = horizontal_parameters(FieldFrequency(c)); + const auto horizontal = horizontal_parameters(Video::FieldFrequency(c)); assert(horizontal.reset_blank < horizontal.set_enable); assert(horizontal.set_enable < horizontal.reset_enable); assert(horizontal.reset_enable < horizontal.set_blank); assert(horizontal.set_blank+50 < horizontal.length); // Expected vertical order of events: reset blank, enable display, disable display, enable blank (at least 50 before end of line), end of line - const auto vertical = vertical_parameters(FieldFrequency(c)); + const auto vertical = vertical_parameters(Video::FieldFrequency(c)); assert(vertical.set_enable < vertical.reset_enable); assert(vertical.reset_enable < vertical.height); } diff --git a/Machines/Atari/ST/Video.hpp b/Machines/Atari/ST/Video.hpp index 867ab5e84..048938bc3 100644 --- a/Machines/Atari/ST/Video.hpp +++ b/Machines/Atari/ST/Video.hpp @@ -15,14 +15,20 @@ namespace Atari { namespace ST { -enum class FieldFrequency { - Fifty = 0, Sixty = 1, SeventyTwo = 2 -}; - +/*! + Models a combination of the parts of the GLUE, MMU and Shifter that in net + form the video subsystem of the Atari ST. So not accurate to a real chip, but + (hopefully) to a subsystem. +*/ class Video { public: Video(); + /*! + Sets the memory pool that provides video, and its size. + */ + void set_ram(uint16_t *, size_t size); + /*! Sets the target device for video data. */ @@ -33,21 +39,51 @@ class Video { */ void run_for(HalfCycles duration); + /*! @returns the number of cycles until there is next a change in the hsync, vsync or display_enable outputs. */ HalfCycles get_next_sequence_point(); + /*! + @returns @c true if the horizontal sync output is currently active; @c false otherwise. + + @discussion On an Atari ST, this generates a VPA-style interrupt, which is often erroneously + documented as being triggered by horizontal blank. + */ bool hsync(); + + /*! + @returns @c true if the vertical sync output is currently active; @c false otherwise. + + @discussion On an Atari ST, this generates a VPA-style interrupt, which is often erroneously + documented as being triggered by vertical blank. + */ bool vsync(); + + /*! + @returns @c true if the display enabled output is currently active; @c false otherwise. + + @discussion On an Atari ST this is fed to the MFP. The documentation that I've been able to + find implies a total 28-cycle delay between the real delay enabled signal changing and its effect + on the 68000 interrupt input via the MFP. As I have yet to determine how much delay is caused + by the MFP a full 28-cycle delay is applied by this class. This should be dialled down when the + MFP's responsibility is clarified. + */ bool display_enabled(); - void set_ram(uint16_t *, size_t size); - + /// @returns the effect of reading from @c address; only the low 6 bits are decoded. uint16_t read(int address); + + /// Writes @c value to @c address, of which only the low 6 bits are decoded. void write(int address, uint16_t value); + /// Used internally to track state. + enum class FieldFrequency { + Fifty = 0, Sixty = 1, SeventyTwo = 2 + }; + private: Outputs::CRT::CRT crt_; @@ -56,7 +92,7 @@ class Video { int base_address_ = 0; int current_address_ = 0; - uint16_t *ram_; + uint16_t *ram_ = nullptr; uint16_t line_buffer_[256]; int x_ = 0, y_ = 0, next_y_ = 0; @@ -67,7 +103,7 @@ class Video { FieldFrequency field_frequency_ = FieldFrequency::Fifty; enum class OutputBpp { One, Two, Four - } output_bpp_; + } output_bpp_ = OutputBpp::Four; void update_output_mode(); struct HorizontalState { @@ -92,7 +128,7 @@ class Video { int line_length_ = 1024; int data_latch_position_ = 0; - uint16_t data_latch_[4]; + uint16_t data_latch_[4] = {0, 0, 0, 0}; void latch_word(); class Shifter { @@ -110,8 +146,8 @@ class Video { enum class OutputMode { Sync, Blank, Border, Pixels } output_mode_ = OutputMode::Sync; - uint16_t border_colour_; - OutputBpp bpp_; + uint16_t border_colour_ = 0; + OutputBpp bpp_ = OutputBpp::Four; union { uint64_t output_shifter_; uint32_t shifter_halves_[2]; @@ -119,7 +155,7 @@ class Video { void flush_output(OutputMode next_mode); - uint16_t *pixel_buffer_; + uint16_t *pixel_buffer_ = nullptr; size_t pixel_pointer_ = 0; Outputs::CRT::CRT &crt_; From 1202b0a65ffe68f0a622c5aee49829d3f3fdff4d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 17 Nov 2019 23:28:00 -0500 Subject: [PATCH 02/10] Establishes a pipeline for delayed public state visibility. --- Machines/Atari/ST/Video.cpp | 38 ++++++++++++++++++++++------ Machines/Atari/ST/Video.hpp | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index fa66029f9..3c0ed64a7 100644 --- a/Machines/Atari/ST/Video.cpp +++ b/Machines/Atari/ST/Video.cpp @@ -109,8 +109,27 @@ void Video::set_scan_target(Outputs::Display::ScanTarget *scan_target) { void Video::run_for(HalfCycles duration) { const auto horizontal_timings = horizontal_parameters(field_frequency_); const auto vertical_timings = vertical_parameters(field_frequency_); - int integer_duration = int(duration.as_integral()); + + // Effect any changes in visible state out here; they're not relevant in the inner loop. + if(!pending_events_.empty()) { + auto erase_iterator = pending_events_.begin(); + int duration_remaining = integer_duration; + while(erase_iterator != pending_events_.end()) { + erase_iterator->delay -= duration_remaining; + if(erase_iterator->delay < 0) { + duration_remaining = -erase_iterator->delay; + erase_iterator->apply(public_state_); + ++erase_iterator; + } else { + break; + } + } + if(erase_iterator != pending_events_.begin()) { + pending_events_.erase(pending_events_.begin(), erase_iterator); + } + } + while(integer_duration) { // Seed next event to end of line. int next_event = line_length_; @@ -132,6 +151,7 @@ void Video::run_for(HalfCycles duration) { // Determine current output mode and number of cycles to output for. const int run_length = std::min(integer_duration, next_event - x_); + const bool display_enable = vertical_.enable && horizontal_.enable; if(horizontal_.sync || vertical_.sync) { shifter_.output_sync(run_length); @@ -225,6 +245,12 @@ void Video::run_for(HalfCycles duration) { vertical_ = next_vertical_; y_ = next_y_; } + + // Chuck any deferred output changes into the queue. + const bool next_display_enable = vertical_.enable && horizontal_.enable; + if(display_enable != next_display_enable) { + add_event(28*2 - integer_duration, next_display_enable ? Event::Type::SetDisplayEnable : Event::Type::ResetDisplayEnable); + } } } @@ -273,11 +299,13 @@ HalfCycles Video::get_next_sequence_point() { // visible area. const auto horizontal_timings = horizontal_parameters(field_frequency_); -// const auto vertical_timings = vertical_parameters(field_frequency_); // If this is a vertically-enabled line, check for the display enable boundaries. if(vertical_.enable) { - // TODO: what if there's a sync event scheduled for this line? + /* + TODO: what if there's a sync event scheduled for this line? That can happen with the + lower border open. + */ if(x_ < horizontal_timings.set_enable) return HalfCycles(horizontal_timings.set_enable - x_); if(x_ < horizontal_timings.reset_enable) return HalfCycles(horizontal_timings.reset_enable - x_); } else { @@ -290,10 +318,6 @@ HalfCycles Video::get_next_sequence_point() { if(x_ < line_length_ - 50) return HalfCycles(line_length_ - 50 - x_); if(x_ < line_length_ - 10) return HalfCycles(line_length_ - 10 - x_); - // Okay, then, it depends on the next line. If the next line is the start or end of vertical sync, - // it's that. -// if(y_+1 == vertical_timings.height || y_+1 == 3) return HalfCycles(line_length_ - x_); - // It wasn't any of those, so as a temporary expedient, just supply end of line. return HalfCycles(line_length_ - x_); } diff --git a/Machines/Atari/ST/Video.hpp b/Machines/Atari/ST/Video.hpp index 048938bc3..2859650cd 100644 --- a/Machines/Atari/ST/Video.hpp +++ b/Machines/Atari/ST/Video.hpp @@ -12,6 +12,8 @@ #include "../../../Outputs/CRT/CRT.hpp" #include "../../../ClockReceiver/ClockReceiver.hpp" +#include + namespace Atari { namespace ST { @@ -161,6 +163,53 @@ class Video { Outputs::CRT::CRT &crt_; uint16_t *palette_ = nullptr; } shifter_; + + /// Contains copies of the various observeable fields, after the relevant propagation delay. + struct PublicState { + bool display_enable = false; + } public_state_; + + struct Event { + int delay; + enum class Type { + SetDisplayEnable, ResetDisplayEnable + } type; + + Event(Type type, int delay) : delay(delay), type(type) {} + + void apply(PublicState &state) { + apply(type, state); + } + + static void apply(Type type, PublicState &state) { + switch(type) { + default: + case Type::SetDisplayEnable: state.display_enable = true; break; + case Type::ResetDisplayEnable: state.display_enable = false; break; + } + } + }; + + std::vector pending_events_; + void add_event(int delay, Event::Type type) { + // Apply immediately if there's no delay (or a negative delay). + if(delay <= 0) { + Event::apply(type, public_state_); + return; + } + + // Otherwise enqueue, having subtracted the delay for any preceding events, + // and subtracting from the subsequent, if any. + auto insertion_point = pending_events_.begin(); + while(insertion_point != pending_events_.end() && insertion_point->delay > delay) { + delay -= insertion_point->delay; + ++insertion_point; + } + if(insertion_point != pending_events_.end()) { + insertion_point->delay -= delay; + } + pending_events_.emplace(insertion_point, type, delay); + } }; } From 82c984afa441baefccaaae1d07d354450a6e0048 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 20:02:27 -0500 Subject: [PATCH 03/10] Switches the joysticks around. Thereby finally allowing me to control mode games. --- Machines/Atari/ST/IntelligentKeyboard.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Machines/Atari/ST/IntelligentKeyboard.cpp b/Machines/Atari/ST/IntelligentKeyboard.cpp index 17975563e..4b910ceec 100644 --- a/Machines/Atari/ST/IntelligentKeyboard.cpp +++ b/Machines/Atari/ST/IntelligentKeyboard.cpp @@ -72,9 +72,12 @@ void IntelligentKeyboard::run_for(HalfCycles duration) { key_queue_.clear(); } - // Check for joystick changes. + // Check for joystick changes; slight complexity here: the joystick that the emulated + // machine advertises as joystick 1 is mapped to the Atari ST's joystick 2, so as to + // maintain both the normal emulation expections that the first joystick is the primary + // one and the Atari ST's convention that the main joystick is in port 2. for(size_t c = 0; c < 2; ++c) { - const auto joystick = static_cast(joysticks_[c].get()); + const auto joystick = static_cast(joysticks_[c ^ 1].get()); if(joystick->has_event()) { output_bytes({ uint8_t(0xfe | c), @@ -386,8 +389,8 @@ void IntelligentKeyboard::interrogate_joysticks() { output_bytes({ 0xfd, - joystick1->get_state(), - joystick2->get_state() + joystick2->get_state(), + joystick1->get_state() }); } From b98703bd5bcde4d0105089d1ddae0318224f120f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 22:11:52 -0500 Subject: [PATCH 04/10] Corrects lack of `const`. --- ClockReceiver/ClockReceiver.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ClockReceiver/ClockReceiver.hpp b/ClockReceiver/ClockReceiver.hpp index e66b74325..40e0968fb 100644 --- a/ClockReceiver/ClockReceiver.hpp +++ b/ClockReceiver/ClockReceiver.hpp @@ -137,7 +137,7 @@ template class WrappedInt { // bool operator () is not supported because it offers an implicit cast to int, which is prone silently to permit misuse /// @returns The underlying int, cast to an integral type of your choosing. - template forceinline constexpr Type as() { return Type(length_); } + template forceinline constexpr Type as() const { return Type(length_); } /// @returns The underlying int, in its native form. forceinline constexpr IntType as_integral() const { return length_; } From ade8df7217162c7ac1fd3e7631a8a65059bcb5c6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 22:12:24 -0500 Subject: [PATCH 05/10] Permits a delay on DE propagation back to the CPU. Plus tests. Currently set at 28 cycles, but I don't know. --- Machines/Atari/ST/Video.cpp | 44 ++++++++++++------- .../Clock Signal.xcodeproj/project.pbxproj | 4 ++ .../xcschemes/Clock Signal.xcscheme | 2 +- .../AtariStaticAnalyserTests.mm | 6 +-- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index 3c0ed64a7..cc659b6bc 100644 --- a/Machines/Atari/ST/Video.cpp +++ b/Machines/Atari/ST/Video.cpp @@ -87,6 +87,8 @@ struct Checker { } checker; #endif +const int de_delay_period = 28*2; + } Video::Video() : @@ -117,7 +119,7 @@ void Video::run_for(HalfCycles duration) { int duration_remaining = integer_duration; while(erase_iterator != pending_events_.end()) { erase_iterator->delay -= duration_remaining; - if(erase_iterator->delay < 0) { + if(erase_iterator->delay <= 0) { duration_remaining = -erase_iterator->delay; erase_iterator->apply(public_state_); ++erase_iterator; @@ -249,7 +251,7 @@ void Video::run_for(HalfCycles duration) { // Chuck any deferred output changes into the queue. const bool next_display_enable = vertical_.enable && horizontal_.enable; if(display_enable != next_display_enable) { - add_event(28*2 - integer_duration, next_display_enable ? Event::Type::SetDisplayEnable : Event::Type::ResetDisplayEnable); + add_event(de_delay_period - integer_duration, next_display_enable ? Event::Type::SetDisplayEnable : Event::Type::ResetDisplayEnable); } } } @@ -278,7 +280,7 @@ bool Video::vsync() { } bool Video::display_enabled() { - return horizontal_.enable && vertical_.enable; + return public_state_.display_enable; } HalfCycles Video::get_next_sequence_point() { @@ -300,26 +302,34 @@ HalfCycles Video::get_next_sequence_point() { const auto horizontal_timings = horizontal_parameters(field_frequency_); - // If this is a vertically-enabled line, check for the display enable boundaries. + int event_time = line_length_; // Worst case: report end of line. + + // If any events are pending, give the first of those the chance to be next. + if(!pending_events_.empty()) { + event_time = std::min(event_time, x_ + event_time); + } + + // If this is a vertically-enabled line, check for the display enable boundaries, + the standard delay. if(vertical_.enable) { - /* - TODO: what if there's a sync event scheduled for this line? That can happen with the - lower border open. - */ - if(x_ < horizontal_timings.set_enable) return HalfCycles(horizontal_timings.set_enable - x_); - if(x_ < horizontal_timings.reset_enable) return HalfCycles(horizontal_timings.reset_enable - x_); - } else { - if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && (x_ < 30*2)) { - return HalfCycles(30*2 - x_); + if(x_ < horizontal_timings.set_enable + de_delay_period) { + event_time = std::min(event_time, horizontal_timings.set_enable + de_delay_period); + } + else if(x_ < horizontal_timings.reset_enable + de_delay_period) { + event_time = std::min(event_time, horizontal_timings.reset_enable + de_delay_period); } } - // Test for beginning and end of sync. - if(x_ < line_length_ - 50) return HalfCycles(line_length_ - 50 - x_); - if(x_ < line_length_ - 10) return HalfCycles(line_length_ - 10 - x_); + // If a vertical sync event is scheduled, test for that. + if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && (x_ < 30*2)) { + event_time = std::min(event_time, 30*2); + } + + // Test for beginning and end of horizontal sync. + if(x_ < line_length_ - 50*2) event_time = std::min(line_length_ - 50*2, event_time); + else if(x_ < line_length_ - 10*2) event_time = std::min(line_length_ - 10*2, event_time); // It wasn't any of those, so as a temporary expedient, just supply end of line. - return HalfCycles(line_length_ - x_); + return HalfCycles(event_time - x_); } // MARK: - IO dispatch diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index e5320b768..04af4156d 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -709,6 +709,7 @@ 4BDDBA991EF3451200347E61 /* Z80MachineCycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4BDDBA981EF3451200347E61 /* Z80MachineCycleTests.swift */; }; 4BE0A3EE237BB170002AB46F /* ST.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BE0A3EC237BB170002AB46F /* ST.cpp */; }; 4BE0A3EF237BB170002AB46F /* ST.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BE0A3EC237BB170002AB46F /* ST.cpp */; }; + 4BE34438238389E10058E78F /* AtariSTVideoTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4BE34437238389E10058E78F /* AtariSTVideoTests.mm */; }; 4BE76CF922641ED400ACD6FA /* QLTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4BE76CF822641ED300ACD6FA /* QLTests.mm */; }; 4BE90FFD22D5864800FB464D /* MacintoshVideoTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4BE90FFC22D5864800FB464D /* MacintoshVideoTests.mm */; }; 4BE9A6B11EDE293000CBCB47 /* zexdoc.com in Resources */ = {isa = PBXBuildFile; fileRef = 4BE9A6B01EDE293000CBCB47 /* zexdoc.com */; }; @@ -1582,6 +1583,7 @@ 4BE32314205328FF006EF799 /* Target.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Target.hpp; sourceTree = ""; }; 4BE3231520532AA7006EF799 /* Target.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Target.hpp; sourceTree = ""; }; 4BE3231620532BED006EF799 /* Target.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Target.hpp; sourceTree = ""; }; + 4BE34437238389E10058E78F /* AtariSTVideoTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AtariSTVideoTests.mm; sourceTree = ""; }; 4BE76CF822641ED300ACD6FA /* QLTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = QLTests.mm; sourceTree = ""; }; 4BE845201F2FF7F100A5EA22 /* CRTC6845.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; name = CRTC6845.hpp; path = 6845/CRTC6845.hpp; sourceTree = ""; }; 4BE90FFC22D5864800FB464D /* MacintoshVideoTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MacintoshVideoTests.mm; sourceTree = ""; }; @@ -3138,6 +3140,7 @@ 4BB73EB51B587A5100552FC2 /* Clock SignalTests */ = { isa = PBXGroup; children = ( + 4BE34437238389E10058E78F /* AtariSTVideoTests.mm */, 4B85322922778E4200F26553 /* Comparative68000.hpp */, 4B90467222C6FA31000E2074 /* TestRunner68000.hpp */, 4B97ADC722C6FD9B00A22A41 /* 68000ArithmeticTests.mm */, @@ -4492,6 +4495,7 @@ 4BD388882239E198002D14B5 /* 68000Tests.mm in Sources */, 4BA91E1D216D85BA00F79557 /* MasterSystemVDPTests.mm in Sources */, 4B98A0611FFADCDE00ADF63B /* MSXStaticAnalyserTests.mm in Sources */, + 4BE34438238389E10058E78F /* AtariSTVideoTests.mm in Sources */, 4BEF6AAC1D35D1C400E73575 /* DPLLTests.swift in Sources */, 4BE76CF922641ED400ACD6FA /* QLTests.mm in Sources */, 4B3BA0CF1D318B44005DD7A7 /* MOS6522Bridge.mm in Sources */, 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 1465a4f62..47f9c7286 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ #include "../../../Analyser/Static/StaticAnalyser.hpp" -#include "../../../Analyser/Static/Atari/Target.hpp" +#include "../../../Analyser/Static/Atari2600/Target.hpp" -using PagingModel = Analyser::Static::Atari::Target::PagingModel; +using PagingModel = Analyser::Static::Atari2600::Target::PagingModel; @interface AtariROMRecord : NSObject @property(nonatomic, readonly) PagingModel pagingModel; @@ -601,7 +601,7 @@ static NSDictionary *romRecordsBySHA1 = @{ if(!romRecord) continue; // assert equality - Analyser::Static::Atari::Target *atari_target = dynamic_cast(targets.front().get()); + auto *const atari_target = dynamic_cast(targets.front().get()); XCTAssert(atari_target != nullptr); XCTAssert(atari_target->paging_model == romRecord.pagingModel, @"%@; should be %d, is %d", testFile, romRecord.pagingModel, atari_target->paging_model); XCTAssert(atari_target->uses_superchip == romRecord.usesSuperchip, @"%@; should be %@", testFile, romRecord.usesSuperchip ? @"true" : @"false"); From 0ce5057fd9d4f8904b73e34fb8b04be1321ea588 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 22:37:20 -0500 Subject: [PATCH 06/10] Attempts to factor in event counting direction. --- Components/68901/MFP68901.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Components/68901/MFP68901.cpp b/Components/68901/MFP68901.cpp index 956dca00e..e7a783270 100644 --- a/Components/68901/MFP68901.cpp +++ b/Components/68901/MFP68901.cpp @@ -232,7 +232,10 @@ void MFP68901::set_timer_event_input(int channel, bool value) { if(timers_[channel].event_input == value) return; timers_[channel].event_input = value; - if(timers_[channel].mode == TimerMode::EventCount && !value) { /* TODO: which edge is counted? "as defined by the associated Interrupt Channel’s edge bit"? */ + if(timers_[channel].mode == TimerMode::EventCount && (value == !!(gpip_active_edge_ & (0x10 >> channel)))) { + // "The active state of the signal on TAI or TBI is dependent upon the associated + // Interrupt Channel’s edge bit (GPIP 4 for TAI and GPIP 3 for TBI [...] ). + // If the edge bit associated with the TAI or TBI input is a one, it will be active high. decrement_timer(channel, 1); } } From 6990abc0d3a333101da1abc4eb892ecf1e03e7c2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 22:56:40 -0500 Subject: [PATCH 07/10] Tweaks selected output mode when both BPP bits are set. --- Machines/Atari/ST/Video.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index cc659b6bc..8a1b0d6ca 100644 --- a/Machines/Atari/ST/Video.cpp +++ b/Machines/Atari/ST/Video.cpp @@ -391,11 +391,11 @@ void Video::write(int address, uint16_t value) { void Video::update_output_mode() { // If this is black and white mode, that's that. switch((video_mode_ >> 8) & 3) { - default: case 0: output_bpp_ = OutputBpp::Four; break; case 1: output_bpp_ = OutputBpp::Two; break; // 1bpp mode ignores the otherwise-programmed frequency. + default: case 2: output_bpp_ = OutputBpp::One; field_frequency_ = FieldFrequency::SeventyTwo; From c04d2f6c6eab360403639c2a09498f80a5dba05d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 22:57:13 -0500 Subject: [PATCH 08/10] Restricts DTack delay to RAM and Shifter accesses. --- Machines/Atari/ST/AtariST.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index fb9fa8f7f..618431f27 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -170,9 +170,14 @@ class ConcreteMachine: } } + auto address = cycle.word_address(); + // If this is a new strobing of the address signal, test for bus error and pre-DTack delay. + // + // DTack delay rule: if accessing RAM or the shifter, align with the two cycles next available + // for the CPU to access that side of the bus. HalfCycles delay(0); - if(cycle.operation & Microcycle::NewAddress) { + if((cycle.operation & Microcycle::NewAddress) && (address < ram_.size() || (address == (0xff8260 >> 1)))) { // DTack will be implicit; work out how long until that should be, // and apply bus error constraints. const int i_phase = bus_phase_.as() & 7; @@ -184,7 +189,6 @@ class ConcreteMachine: // TODO: presumably test is if(after declared memory size and (not supervisor or before hardware space)) bus_error? } - auto address = cycle.word_address(); uint16_t *memory = nullptr; switch(memory_map_[address >> 15]) { default: From b12136691a2349f1a5ccfd7e02aad949fabaab30 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 23:46:33 -0500 Subject: [PATCH 09/10] Corrects comment. --- Processors/68000/Implementation/68000Storage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Processors/68000/Implementation/68000Storage.cpp b/Processors/68000/Implementation/68000Storage.cpp index 2c0582283..0d479c5cc 100644 --- a/Processors/68000/Implementation/68000Storage.cpp +++ b/Processors/68000/Implementation/68000Storage.cpp @@ -3040,7 +3040,7 @@ struct ProcessorStorageConstructor { // Throw in the interrupt program. const auto interrupt_pointer = storage_.all_micro_ops_.size(); - // WORKAROUND FOR THE BE68000 MAIN LOOP. Hopefully temporary. + // WORKAROUND FOR THE 68000 MAIN LOOP. Hopefully temporary. op(Action::None, seq("")); // Perform a single write and then a cycle that will obtain an interrupt vector, or else dictate an autovector or a spurious interrupt. From e787c03530e44b885dc181d7afda1994ace8e759 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 18 Nov 2019 23:47:27 -0500 Subject: [PATCH 10/10] Slightly shortens NTSC frame. Either: (i) 263 is incorrect; or (ii) my logic as to frame height is incorrect. Given that the horizontal side of things is really well documented, I'm currently guessing (i). Research to do. --- Machines/Atari/ST/Video.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index 8a1b0d6ca..2156b2557 100644 --- a/Machines/Atari/ST/Video.cpp +++ b/Machines/Atari/ST/Video.cpp @@ -28,7 +28,7 @@ const struct VerticalParams { const int height; } vertical_params[3] = { {63, 263, 313}, // 47 rather than 63 on early machines. - {34, 234, 263}, + {34, 234, 262}, // TODO: is 262 correct? If it's 263, how does that interact with opening the bottom border? {1, 401, 500} // 72 Hz mode: who knows? };