From 894066984cdae2abc70aea673f528e203b9b12ce Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 19 Nov 2019 20:13:47 -0500 Subject: [PATCH 1/3] Moves beginning and end of vertical sync to what I now believe is its proper place. At least one demo now successfully opens the top border. --- Machines/Atari/ST/Video.cpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/Machines/Atari/ST/Video.cpp b/Machines/Atari/ST/Video.cpp index 2156b2557..2dc861a53 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, 262}, // TODO: is 262 correct? If it's 263, how does that interact with opening the bottom border? + {34, 234, 263}, // 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? }; @@ -87,7 +87,11 @@ struct Checker { } checker; #endif -const int de_delay_period = 28*2; +const int de_delay_period = 28*2; // Number of half cycles after DE that observed DE changes. +const int vsync_x_position = 54*2; // Horizontal cycle on which vertical sync changes happen. + +// "VSYNC starts 104 cycles after the start of the previous line's HSYNC, so that's 4 cycles before DE would be activated. "; +// hsync is at -50, so that's +54 } @@ -147,8 +151,8 @@ void Video::run_for(HalfCycles duration) { if(line_length_ - 10*2 > x_) next_event = std::min(next_event, line_length_ - 10*2); // Also, a vertical sync event might intercede. - if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && x_ < 30*2 && next_event >= 30*2) { - next_event = 30*2; + if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && x_ < vsync_x_position && next_event >= vsync_x_position) { + next_event = vsync_x_position; } // Determine current output mode and number of cycles to output for. @@ -208,16 +212,21 @@ void Video::run_for(HalfCycles duration) { next_vertical_ = vertical_; next_vertical_.sync_schedule = VerticalState::SyncSchedule::None; - // Use vertical_parameters to get parameters for the current output frequency. - if(next_y_ == vertical_timings.set_enable) { + // Use vertical_parameters to get parameters for the current output frequency; + // quick note: things other than the total frame size are counted in terms + // of the line they're evaluated on — i.e. the test is this line, not the next + // one. The total height constraint is obviously whether the next one would be + // too far. + if(y_ == vertical_timings.set_enable) { next_vertical_.enable = true; - } else if(next_y_ == vertical_timings.reset_enable) { + } else if(y_ == vertical_timings.reset_enable) { next_vertical_.enable = false; } else if(next_y_ == vertical_timings.height) { next_y_ = 0; - next_vertical_.sync_schedule = VerticalState::SyncSchedule::Begin; current_address_ = base_address_ >> 1; - } else if(next_y_ == 3) { + } else if(y_ == 0) { + next_vertical_.sync_schedule = VerticalState::SyncSchedule::Begin; + } else if(y_ == 3) { next_vertical_.sync_schedule = VerticalState::SyncSchedule::End; } } @@ -235,7 +244,7 @@ void Video::run_for(HalfCycles duration) { else if(line_length_ - 10*2 == x_) horizontal_.sync = false; // Check vertical events. - if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && x_ == 30*2) { + if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && x_ == vsync_x_position) { vertical_.sync = vertical_.sync_schedule == VerticalState::SyncSchedule::Begin; vertical_.enable &= !vertical_.sync; } @@ -320,8 +329,8 @@ HalfCycles Video::get_next_sequence_point() { } // 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); + if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && (x_ < vsync_x_position)) { + event_time = std::min(event_time, vsync_x_position); } // Test for beginning and end of horizontal sync. From e0ceab6642be42e8c2dfe57ba9a0e6f7acd35c41 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 19 Nov 2019 21:52:50 -0500 Subject: [PATCH 2/3] Pivots towards looking at Timer B as a cause of in-frame inaccuracy. --- Components/68901/MFP68901.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Components/68901/MFP68901.cpp b/Components/68901/MFP68901.cpp index e7a783270..ffe2402cb 100644 --- a/Components/68901/MFP68901.cpp +++ b/Components/68901/MFP68901.cpp @@ -209,6 +209,7 @@ HalfCycles MFP68901::get_next_sequence_point() { // MARK: - Timers void MFP68901::set_timer_mode(int timer, TimerMode mode, int prescale, bool reset_timer) { + LOG("Timer " << timer << " mode set: " << int(mode) << "; prescale: " << prescale); timers_[timer].mode = mode; timers_[timer].prescale = prescale; if(reset_timer) { @@ -314,8 +315,6 @@ void MFP68901::update_interrupts() { // Update the delegate if necessary. if(interrupt_delegate_ && interrupt_line_ != old_interrupt_line) { - if(interrupt_line_) - LOG("Generating interrupt: " << std::hex << interrupt_pending_ << " / " << std::hex << interrupt_mask_ << " : " << std::hex << interrupt_in_service_); interrupt_delegate_->mfp68901_did_change_interrupt_status(this); } } From 72cb3a1cf6916e480cfe4f38ff825fe0bf89cb95 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 19 Nov 2019 21:54:13 -0500 Subject: [PATCH 3/3] Integrates basic unit test for Atari ST video event prediction. --- .../Clock SignalTests/AtariSTVideoTests.mm | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 OSBindings/Mac/Clock SignalTests/AtariSTVideoTests.mm diff --git a/OSBindings/Mac/Clock SignalTests/AtariSTVideoTests.mm b/OSBindings/Mac/Clock SignalTests/AtariSTVideoTests.mm new file mode 100644 index 000000000..078c47718 --- /dev/null +++ b/OSBindings/Mac/Clock SignalTests/AtariSTVideoTests.mm @@ -0,0 +1,60 @@ +// +// MasterSystemVDPTests.m +// Clock SignalTests +// +// Created by Thomas Harte on 09/10/2018. +// Copyright © 2018 Thomas Harte. All rights reserved. +// + +#import + +#include "../../../Machines/Atari/ST/Video.hpp" + +@interface AtariSTVideoTests : XCTestCase +@end + +@implementation AtariSTVideoTests + +- (void)setUp { + [super setUp]; +} + +- (void)tearDown { + [super tearDown]; +} + +- (void)testSequencePoints { + // Establish an instance of video. + Atari::ST::Video video; + uint16_t ram[256*1024]; + video.set_ram(ram, sizeof(ram)); + + // Set 4bpp, 50Hz. + video.write(0x05, 0x0200); + video.write(0x30, 0x0000); + + // Run for [more than] a whole frame making sure that no observeable outputs + // change at any time other than a sequence point. + HalfCycles next_event; + bool display_enable = false; + bool vsync = false; + bool hsync = false; + for(size_t c = 0; c < 10 * 1000 * 1000; ++c) { + const bool is_transition_point = next_event == HalfCycles(0); + + if(is_transition_point) { + display_enable = video.display_enabled(); + vsync = video.vsync(); + hsync = video.hsync(); + next_event = video.get_next_sequence_point(); + } else { + NSAssert(display_enable == video.display_enabled(), @"Unannounced change in display enabled at cycle %zu [%d before next sequence point]", c, next_event.as()); + NSAssert(vsync == video.vsync(), @"Unannounced change in vsync at cycle %zu [%d before next sequence point]", c, next_event.as()); + NSAssert(hsync == video.hsync(), @"Unannounced change in hsync at cycle %zu [%d before next sequence point]", c, next_event.as()); + } + video.run_for(HalfCycles(2)); + next_event -= HalfCycles(2); + } +} + +@end