From 3e6b8048969f2087f01191d9492f513ad30e6c19 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:12:44 -0400 Subject: [PATCH 1/7] Switches to linked 1/50/1000 Hz timers, and per-interrupt state toggling. --- Machines/Enterprise/Dave.cpp | 93 ++++++++----------- Machines/Enterprise/Dave.hpp | 12 +-- .../Clock SignalTests/EnterpriseDaveTests.mm | 20 ++-- 3 files changed, 50 insertions(+), 75 deletions(-) diff --git a/Machines/Enterprise/Dave.cpp b/Machines/Enterprise/Dave.cpp index 6bad6d676..cc4bc6c35 100644 --- a/Machines/Enterprise/Dave.cpp +++ b/Machines/Enterprise/Dave.cpp @@ -225,21 +225,11 @@ void TimedInterruptSource::write(uint16_t address, uint8_t value) { channels_[address >> 1].reload = uint16_t((channels_[address >> 1].reload & 0x00ff) | ((value & 0xf) << 8)); break; - case 7: { + case 7: channels_[0].sync = value & 0x01; channels_[1].sync = value & 0x02; - - const InterruptRate rate = InterruptRate((value >> 5) & 3); - if(rate != rate_) { - rate_ = rate; - - if(rate_ >= InterruptRate::ToneGenerator0) { - programmable_level_ = channels_[int(rate_) - int(InterruptRate::ToneGenerator0)].level; - } else { - programmable_offset_ = programmble_reload(rate_); - } - } - } break; + rate_ = InterruptRate((value >> 5) & 3); + break; case 31: global_divider_ = Cycles(2 + ((value >> 1)&1)); @@ -269,6 +259,7 @@ void TimedInterruptSource::update_channel(int c, bool is_linked, int decrement) // from high to low is amongst the included flips. if(is_linked && num_flips + channels_[c].level >= 2) { interrupts_ |= uint8_t(Interrupt::VariableFrequency); + programmable_level_ ^= true; } channels_[c].level ^= (num_flips & 1); @@ -287,67 +278,59 @@ void TimedInterruptSource::run_for(Cycles duration) { return; } - // Update the 1Hz interrupt. - one_hz_offset_ -= cycles; - if(one_hz_offset_ <= Cycles(0)) { + // Update the two-second counter, from which the 1Hz, 50Hz and 1000Hz signals + // are derived. + const int previous_counter = two_second_counter_; + two_second_counter_ = (two_second_counter_ + cycles.as()) % 500'000; + + // Check for a 1Hz rollover. + if(previous_counter / 250'000 != two_second_counter_ / 250'000) { interrupts_ |= uint8_t(Interrupt::OneHz); - one_hz_offset_ += clock_rate; + } + + // Check for 1kHz or 50Hz rollover; + switch(rate_) { + default: break; + case InterruptRate::OnekHz: + if(previous_counter / 250 != two_second_counter_ / 250) { + interrupts_ |= uint8_t(Interrupt::VariableFrequency); + programmable_level_ ^= true; + } + break; + case InterruptRate::FiftyHz: + if(previous_counter / 5'000 != two_second_counter_ / 5'000) { + interrupts_ |= uint8_t(Interrupt::VariableFrequency); + programmable_level_ ^= true; + } + break; } // Update the two tone channels. update_channel(0, rate_ == InterruptRate::ToneGenerator0, cycles.as()); update_channel(1, rate_ == InterruptRate::ToneGenerator1, cycles.as()); - - // Update the programmable-frequency interrupt. - if(rate_ < InterruptRate::ToneGenerator0) { - programmable_offset_ -= cycles.as(); - if(programmable_offset_ <= 0) { - if(programmable_level_) { - interrupts_ |= uint8_t(Interrupt::VariableFrequency); - } - programmable_level_ ^= true; - programmable_offset_ = programmble_reload(rate_); - } - } } Cycles TimedInterruptSource::get_next_sequence_point() const { - int result = one_hz_offset_.as(); - switch(rate_) { - case InterruptRate::OnekHz: - case InterruptRate::FiftyHz: - result = std::min(result, programmable_offset_ + (!programmable_level_) * programmble_reload(rate_)); - break; + default: + case InterruptRate::OnekHz: return Cycles(250 - (two_second_counter_ % 250)); + case InterruptRate::FiftyHz: return Cycles(5000 - (two_second_counter_ % 5000)); + case InterruptRate::ToneGenerator0: case InterruptRate::ToneGenerator1: { const auto &channel = channels_[int(rate_) - int(InterruptRate::ToneGenerator0)]; const int cycles_until_interrupt = channel.value + 1 + (!channel.level) * (channel.reload + 1); - result = std::min(result, cycles_until_interrupt); - } break; - } - return Cycles(result); + int result = 250'000 - (two_second_counter_ % 250'000); + return Cycles(std::min(result, cycles_until_interrupt)); + } + } } uint8_t TimedInterruptSource::get_divider_state() { - bool programmable_flag = false; - switch(rate_) { - case InterruptRate::OnekHz: - case InterruptRate::FiftyHz: - programmable_flag = programmable_level_; - break; - case InterruptRate::ToneGenerator0: - programmable_flag = channels_[0].level; - break; - case InterruptRate::ToneGenerator1: - programmable_flag = channels_[1].level; - break; - } - // one_hz_offset_ counts downwards, so when it crosses the halfway mark // it enters the high part of its wave. return - (one_hz_offset_ < half_clock_rate ? 0x4 : 0x0) | - (programmable_flag ? 0x1 : 0x0); + (two_second_counter_ < 250'000 ? 0x4 : 0x0) | + (programmable_level_ ? 0x1 : 0x0); } diff --git a/Machines/Enterprise/Dave.hpp b/Machines/Enterprise/Dave.hpp index 760fd05a4..e2443779f 100644 --- a/Machines/Enterprise/Dave.hpp +++ b/Machines/Enterprise/Dave.hpp @@ -152,7 +152,7 @@ class TimedInterruptSource { static constexpr Cycles half_clock_rate{125000}; // Global divider (i.e. 8MHz/12Mhz switch). - Cycles global_divider_; + Cycles global_divider_ = Cycles(2); Cycles run_length_; // Interrupts that have fired since get_new_interrupts() @@ -160,7 +160,7 @@ class TimedInterruptSource { uint8_t interrupts_ = 0; // A counter for the 1Hz interrupt. - Cycles one_hz_offset_ = clock_rate; + int two_second_counter_ = 0; // A counter specific to the 1kHz and 50Hz timers, if in use. enum class InterruptRate { @@ -169,7 +169,6 @@ class TimedInterruptSource { ToneGenerator0, ToneGenerator1, } rate_ = InterruptRate::OnekHz; - int programmable_offset_ = programmble_reload(InterruptRate::OnekHz); bool programmable_level_ = false; // A local duplicate of the counting state of the first two audio @@ -181,13 +180,6 @@ class TimedInterruptSource { bool level = false; } channels_[2]; void update_channel(int c, bool is_linked, int decrement); - static constexpr int programmble_reload(InterruptRate rate) { - switch(rate) { - default: return 0; - case InterruptRate::OnekHz: return 125; - case InterruptRate::FiftyHz: return 2500; - } - } }; } diff --git a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm index 5d491c48e..349f7d246 100644 --- a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm +++ b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm @@ -23,17 +23,18 @@ _interruptSource = std::make_unique(); } -- (void)performTestExpectedToggles:(int)expectedToggles expectedInterrupts:(int)expectedInterrupts { +- (void)performTestExpectedToggles:(int)expectedToggles { // Check that the programmable timer flag toggles at a rate // of 2kHz, causing 1000 interrupts, and that sequence points // are properly predicted. int toggles = 0; int interrupts = 0; - uint8_t dividerState = _interruptSource->get_divider_state(); + uint8_t dividerState = _interruptSource->get_divider_state() & 1; int nextSequencePoint = _interruptSource->get_next_sequence_point().as(); + for(int c = 0; c < 250000; c++) { - // Advance one cycle. Clock is 250,000 Hz. - _interruptSource->run_for(Cycles(1)); + // Advance one cycle. Clock is 500,000 Hz. + _interruptSource->run_for(Cycles(2)); const uint8_t newDividerState = _interruptSource->get_divider_state(); if((dividerState^newDividerState)&0x1) { @@ -48,7 +49,6 @@ if(newInterrupts & 0x02) { ++interrupts; XCTAssertEqual(nextSequencePoint, 0); - XCTAssertEqual(dividerState&0x1, 0); nextSequencePoint = _interruptSource->get_next_sequence_point().as(); } @@ -62,19 +62,19 @@ } XCTAssertEqual(toggles, expectedToggles); - XCTAssertEqual(interrupts, expectedInterrupts); + XCTAssertEqual(interrupts, expectedToggles); } - (void)test1kHzTimer { // Set 1kHz timer. _interruptSource->write(7, 0 << 5); - [self performTestExpectedToggles:2000 expectedInterrupts:1000]; + [self performTestExpectedToggles:1000]; } - (void)test50HzTimer { // Set 50Hz timer. _interruptSource->write(7, 1 << 5); - [self performTestExpectedToggles:100 expectedInterrupts:50]; + [self performTestExpectedToggles:50]; } - (void)testTone0Timer { @@ -84,7 +84,7 @@ _interruptSource->write(0, 137); _interruptSource->write(1, 0); - [self performTestExpectedToggles:250000/138 expectedInterrupts:250000/(138*2)]; + [self performTestExpectedToggles:250000/(138 * 2)]; } - (void)testTone1Timer { @@ -94,7 +94,7 @@ _interruptSource->write(2, 961 & 0xff); _interruptSource->write(3, (961 >> 8) & 0xff); - [self performTestExpectedToggles:250000/961 expectedInterrupts:250000/(961*2)]; + [self performTestExpectedToggles:250000/(961 * 2)]; } @end From 33e2a4b21c4b2dc6d0562829ca08467d173b5698 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:20:13 -0400 Subject: [PATCH 2/7] Minor cleanups. --- Machines/Enterprise/Dave.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Machines/Enterprise/Dave.cpp b/Machines/Enterprise/Dave.cpp index cc4bc6c35..00a4c1ad6 100644 --- a/Machines/Enterprise/Dave.cpp +++ b/Machines/Enterprise/Dave.cpp @@ -311,6 +311,8 @@ void TimedInterruptSource::run_for(Cycles duration) { } Cycles TimedInterruptSource::get_next_sequence_point() const { + // Since both the 1kHz and 50Hz timers are integer dividers of the 1Hz timer, there's no need + // to factor that one in when determining the next sequence point for either of those. switch(rate_) { default: case InterruptRate::OnekHz: return Cycles(250 - (two_second_counter_ % 250)); @@ -328,9 +330,5 @@ Cycles TimedInterruptSource::get_next_sequence_point() const { } uint8_t TimedInterruptSource::get_divider_state() { - // one_hz_offset_ counts downwards, so when it crosses the halfway mark - // it enters the high part of its wave. - return - (two_second_counter_ < 250'000 ? 0x4 : 0x0) | - (programmable_level_ ? 0x1 : 0x0); + return uint8_t((two_second_counter_ / 250'000) * 4 | programmable_level_); } From 7a673a24485f7892185b1e25f1a49138f4d2bdc8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:23:09 -0400 Subject: [PATCH 3/7] Avoid confusing temporary storage. --- Machines/Enterprise/Dave.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Machines/Enterprise/Dave.cpp b/Machines/Enterprise/Dave.cpp index 00a4c1ad6..39a152630 100644 --- a/Machines/Enterprise/Dave.cpp +++ b/Machines/Enterprise/Dave.cpp @@ -323,8 +323,10 @@ Cycles TimedInterruptSource::get_next_sequence_point() const { const auto &channel = channels_[int(rate_) - int(InterruptRate::ToneGenerator0)]; const int cycles_until_interrupt = channel.value + 1 + (!channel.level) * (channel.reload + 1); - int result = 250'000 - (two_second_counter_ % 250'000); - return Cycles(std::min(result, cycles_until_interrupt)); + return Cycles(std::min( + 250'000 - (two_second_counter_ % 250'000), + cycles_until_interrupt + )); } } } From 704dc9bdcbd53b2e4252434be217421a6ab045b9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:25:32 -0400 Subject: [PATCH 4/7] Improves test, to assert that state toggles happen at interrupts. --- .../Clock SignalTests/EnterpriseDaveTests.mm | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm index 349f7d246..18bb00193 100644 --- a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm +++ b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm @@ -23,7 +23,7 @@ _interruptSource = std::make_unique(); } -- (void)performTestExpectedToggles:(int)expectedToggles { +- (void)performTestExpectedInterrupts:(int)expectedInterrupts { // Check that the programmable timer flag toggles at a rate // of 2kHz, causing 1000 interrupts, and that sequence points // are properly predicted. @@ -37,7 +37,8 @@ _interruptSource->run_for(Cycles(2)); const uint8_t newDividerState = _interruptSource->get_divider_state(); - if((dividerState^newDividerState)&0x1) { + const bool didToggle = (dividerState^newDividerState)&0x1; + if(didToggle) { ++toggles; } dividerState = newDividerState; @@ -49,6 +50,7 @@ if(newInterrupts & 0x02) { ++interrupts; XCTAssertEqual(nextSequencePoint, 0); + XCTAssertTrue(didToggle); nextSequencePoint = _interruptSource->get_next_sequence_point().as(); } @@ -61,20 +63,20 @@ XCTAssertEqual(nextSequencePoint, _interruptSource->get_next_sequence_point().as(), @"At cycle %d", c); } - XCTAssertEqual(toggles, expectedToggles); - XCTAssertEqual(interrupts, expectedToggles); + XCTAssertEqual(toggles, expectedInterrupts); + XCTAssertEqual(interrupts, expectedInterrupts); } - (void)test1kHzTimer { // Set 1kHz timer. _interruptSource->write(7, 0 << 5); - [self performTestExpectedToggles:1000]; + [self performTestExpectedInterrupts:1000]; } - (void)test50HzTimer { // Set 50Hz timer. _interruptSource->write(7, 1 << 5); - [self performTestExpectedToggles:50]; + [self performTestExpectedInterrupts:50]; } - (void)testTone0Timer { @@ -84,7 +86,7 @@ _interruptSource->write(0, 137); _interruptSource->write(1, 0); - [self performTestExpectedToggles:250000/(138 * 2)]; + [self performTestExpectedInterrupts:250000/(138 * 2)]; } - (void)testTone1Timer { @@ -94,7 +96,7 @@ _interruptSource->write(2, 961 & 0xff); _interruptSource->write(3, (961 >> 8) & 0xff); - [self performTestExpectedToggles:250000/(961 * 2)]; + [self performTestExpectedInterrupts:250000/(961 * 2)]; } @end From 8e0893bd4227451d7b64a3efd478988dfef62668 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:28:32 -0400 Subject: [PATCH 5/7] Clarifies control flow. --- .../Clock SignalTests/EnterpriseDaveTests.mm | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm index 18bb00193..781c57151 100644 --- a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm +++ b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm @@ -35,29 +35,27 @@ for(int c = 0; c < 250000; c++) { // Advance one cycle. Clock is 500,000 Hz. _interruptSource->run_for(Cycles(2)); + --nextSequencePoint; + // Check for a status bit change. const uint8_t newDividerState = _interruptSource->get_divider_state(); const bool didToggle = (dividerState^newDividerState)&0x1; - if(didToggle) { - ++toggles; - } dividerState = newDividerState; - - --nextSequencePoint; + toggles += didToggle; // Check for the relevant interrupt. const uint8_t newInterrupts = _interruptSource->get_new_interrupts(); - if(newInterrupts & 0x02) { - ++interrupts; + if(newInterrupts) { XCTAssertEqual(nextSequencePoint, 0); - XCTAssertTrue(didToggle); nextSequencePoint = _interruptSource->get_next_sequence_point().as(); - } - // Failing that, confirm that the other interrupt happend. - if(!nextSequencePoint) { - XCTAssertTrue(newInterrupts & 0x08); - nextSequencePoint = _interruptSource->get_next_sequence_point().as(); + if(newInterrupts & 0x02) { + ++interrupts; + XCTAssertTrue(didToggle); + } else { + // Failing that, confirm that the other interrupt happend. + XCTAssertTrue(newInterrupts & 0x08); + } } XCTAssertEqual(nextSequencePoint, _interruptSource->get_next_sequence_point().as(), @"At cycle %d", c); From 0085265d1386cdd70472f473b5fdff37fbae69c6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:46:22 -0400 Subject: [PATCH 6/7] Test for a longer period; fix expected tone 1 count. --- .../Clock SignalTests/EnterpriseDaveTests.mm | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm index 781c57151..0927cf799 100644 --- a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm +++ b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm @@ -23,16 +23,24 @@ _interruptSource = std::make_unique(); } -- (void)performTestExpectedInterrupts:(int)expectedInterrupts { - // Check that the programmable timer flag toggles at a rate - // of 2kHz, causing 1000 interrupts, and that sequence points - // are properly predicted. +/// Tests that the programmable timer flag toggles and produces interrupts +/// at the rate specified, and that the flag toggles when interrupts are signalled. +- (void)performTestExpectedInterrupts:(double)expectedInterruptsPerSecond applySync:(BOOL)applySync mode:(int)mode { + // If sync is requested, synchronise both channels. + if(applySync) { + _interruptSource->write(0xa7, 3); + _interruptSource->run_for(Cycles(2)); + } + + // Set mode (and disable sync, if it was applied). + _interruptSource->write(0xa7, mode << 5); + int toggles = 0; int interrupts = 0; uint8_t dividerState = _interruptSource->get_divider_state() & 1; int nextSequencePoint = _interruptSource->get_next_sequence_point().as(); - for(int c = 0; c < 250000; c++) { + for(int c = 0; c < 250000 * 5; c++) { // Advance one cycle. Clock is 500,000 Hz. _interruptSource->run_for(Cycles(2)); --nextSequencePoint; @@ -61,40 +69,34 @@ XCTAssertEqual(nextSequencePoint, _interruptSource->get_next_sequence_point().as(), @"At cycle %d", c); } - XCTAssertEqual(toggles, expectedInterrupts); - XCTAssertEqual(interrupts, expectedInterrupts); + XCTAssertEqual(toggles, int(expectedInterruptsPerSecond * 5.0)); + XCTAssertEqual(interrupts, int(expectedInterruptsPerSecond * 5.0)); } - (void)test1kHzTimer { - // Set 1kHz timer. - _interruptSource->write(7, 0 << 5); - [self performTestExpectedInterrupts:1000]; + [self performTestExpectedInterrupts:1000.0 applySync:NO mode:0]; } - (void)test50HzTimer { - // Set 50Hz timer. - _interruptSource->write(7, 1 << 5); - [self performTestExpectedInterrupts:50]; + [self performTestExpectedInterrupts:50.0 applySync:NO mode:1]; } - (void)testTone0Timer { // Set tone generator 0 as the interrupt source, with a divider of 137; // apply sync momentarily. - _interruptSource->write(7, 2 << 5); _interruptSource->write(0, 137); _interruptSource->write(1, 0); - [self performTestExpectedInterrupts:250000/(138 * 2)]; + [self performTestExpectedInterrupts:250000.0/(138.0 * 2.0) applySync:YES mode:2]; } - (void)testTone1Timer { // Set tone generator 1 as the interrupt source, with a divider of 961; // apply sync momentarily. - _interruptSource->write(7, 3 << 5); _interruptSource->write(2, 961 & 0xff); _interruptSource->write(3, (961 >> 8) & 0xff); - [self performTestExpectedInterrupts:250000/(961 * 2)]; + [self performTestExpectedInterrupts:250000.0/(962.0 * 2.0) applySync:YES mode:3]; } @end From 94907b51aafc2624bdd07cf9b0dbccb28efabc0a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Jul 2021 20:47:49 -0400 Subject: [PATCH 7/7] Remove redundant parameter. --- .../Mac/Clock SignalTests/EnterpriseDaveTests.mm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm index 0927cf799..cd8aa3ba8 100644 --- a/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm +++ b/OSBindings/Mac/Clock SignalTests/EnterpriseDaveTests.mm @@ -25,9 +25,9 @@ /// Tests that the programmable timer flag toggles and produces interrupts /// at the rate specified, and that the flag toggles when interrupts are signalled. -- (void)performTestExpectedInterrupts:(double)expectedInterruptsPerSecond applySync:(BOOL)applySync mode:(int)mode { - // If sync is requested, synchronise both channels. - if(applySync) { +- (void)performTestExpectedInterrupts:(double)expectedInterruptsPerSecond mode:(int)mode { + // If a programmable timer mode is requested, synchronise both channels. + if(mode >= 2) { _interruptSource->write(0xa7, 3); _interruptSource->run_for(Cycles(2)); } @@ -74,11 +74,11 @@ } - (void)test1kHzTimer { - [self performTestExpectedInterrupts:1000.0 applySync:NO mode:0]; + [self performTestExpectedInterrupts:1000.0 mode:0]; } - (void)test50HzTimer { - [self performTestExpectedInterrupts:50.0 applySync:NO mode:1]; + [self performTestExpectedInterrupts:50.0 mode:1]; } - (void)testTone0Timer { @@ -87,7 +87,7 @@ _interruptSource->write(0, 137); _interruptSource->write(1, 0); - [self performTestExpectedInterrupts:250000.0/(138.0 * 2.0) applySync:YES mode:2]; + [self performTestExpectedInterrupts:250000.0/(138.0 * 2.0) mode:2]; } - (void)testTone1Timer { @@ -96,7 +96,7 @@ _interruptSource->write(2, 961 & 0xff); _interruptSource->write(3, (961 >> 8) & 0xff); - [self performTestExpectedInterrupts:250000.0/(962.0 * 2.0) applySync:YES mode:3]; + [self performTestExpectedInterrupts:250000.0/(962.0 * 2.0) mode:3]; } @end