From 72891921305012f377e93b26d6cfc244d8b1b04c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 16 Aug 2022 21:51:02 -0400 Subject: [PATCH 1/6] Fix refresh slots: they're taken, not open. --- Machines/Amiga/Chipset.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Machines/Amiga/Chipset.cpp b/Machines/Amiga/Chipset.cpp index c8accc853..8f1c23f25 100644 --- a/Machines/Amiga/Chipset.cpp +++ b/Machines/Amiga/Chipset.cpp @@ -590,7 +590,7 @@ template bool Chipset::perform_cycle() { // Blitter and CPU priority is dealt with below. if constexpr (cycle >= 0x00 && cycle < 0x08) { // Memory refresh, four slots per line. - return true; + return false; } if constexpr (cycle >= 0x08 && cycle < 0x0e) { From 837acdcf601c80cf679f5740b1f280ef529f93b5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 16 Aug 2022 21:51:13 -0400 Subject: [PATCH 2/6] Experimentally decline immediate blits. --- Machines/Amiga/Blitter.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Machines/Amiga/Blitter.cpp b/Machines/Amiga/Blitter.cpp index c4cd54a23..c60acc8bf 100644 --- a/Machines/Amiga/Blitter.cpp +++ b/Machines/Amiga/Blitter.cpp @@ -240,12 +240,12 @@ bool Blitter::advance_dma() { // TODO: eliminate @c complete_immediately and this workaround. // See commentary in Chipset.cpp. - if constexpr (complete_immediately) { - while(get_status() & 0x4000) { - advance_dma(); - } - return true; - } +// if constexpr (complete_immediately) { +// while(get_status() & 0x4000) { +// advance_dma(); +// } +// return true; +// } if(line_mode_) { not_zero_flag_ = false; From a6b8285d9c0a537cf35f71536db6674f6ca7b789 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 19 Aug 2022 16:38:15 -0400 Subject: [PATCH 3/6] Factor out the blitter sequencer. --- Machines/Amiga/Blitter.hpp | 157 +--------------- Machines/Amiga/BlitterSequencer.hpp | 167 ++++++++++++++++++ .../Clock Signal.xcodeproj/project.pbxproj | 2 + 3 files changed, 177 insertions(+), 149 deletions(-) create mode 100644 Machines/Amiga/BlitterSequencer.hpp diff --git a/Machines/Amiga/Blitter.hpp b/Machines/Amiga/Blitter.hpp index 256137ae6..2a4ce8ae4 100644 --- a/Machines/Amiga/Blitter.hpp +++ b/Machines/Amiga/Blitter.hpp @@ -15,159 +15,11 @@ #include #include "../../ClockReceiver/ClockReceiver.hpp" +#include "BlitterSequencer.hpp" #include "DMADevice.hpp" namespace Amiga { -/*! - Statefully provides the next access the Blitter should make. - - TODO: determine the actual logic here, rather than - relying on tables. -*/ -class BlitterSequencer { - public: - enum class Channel { - /// Tells the caller to calculate and load a new piece of output - /// into the output pipeline. - /// - /// If any inputs are enabled then a one-slot output pipeline applies: - /// output will rest in the pipeline for one write phase before being written. - Write, - /// Indicates that a write should occur if anything is in the pipeline, otherwise - /// no activity should occur. - FlushPipeline, - /// The caller should read from channel C. - C, - /// The caller should read from channel B. - B, - /// The caller should read from channel A. - A, - /// Indicates an unused DMA slot. - None - }; - - /// Sets the current control value, which indicates which - /// channels are enabled. - void set_control(int control) { - control_ = control & 0xf; - index_ = 0; // TODO: this probably isn't accurate; case caught is a change - // of control values during a blit. - } - - /// Indicates that blitting should conclude after this step, i.e. - /// whatever is being fetched now is part of the final set of input data; - /// this is safe to call following a fetch request on any channel. - void complete() { - next_phase_ = - (control_ == 0x9 || control_ == 0xb || control_ == 0xd) ? - Phase::PauseAndComplete : Phase::Complete; - } - - /// Begins a blit operation. - void begin() { - phase_ = next_phase_ = Phase::Ongoing; - index_ = loop_ = 0; - } - - /// Provides the next channel to fetch from, or that a write is required, - /// along with a count of complete channel iterations so far completed. - std::pair next() { - switch(phase_) { - default: break; - - case Phase::Complete: - return std::make_pair(Channel::FlushPipeline, loop_); - - case Phase::PauseAndComplete: - phase_ = Phase::Complete; - return std::make_pair(Channel::None, loop_); - } - - Channel next = Channel::None; - - switch(control_) { - default: break; - - case 0: next = next_channel(pattern0); break; - case 1: next = next_channel(pattern1); break; - case 2: next = next_channel(pattern2); break; - case 3: next = next_channel(pattern3); break; - case 4: next = next_channel(pattern4); break; - case 5: next = next_channel(pattern5); break; - case 6: next = next_channel(pattern6); break; - case 7: next = next_channel(pattern7); break; - case 8: next = next_channel(pattern8); break; - case 9: next = next_channel(pattern9); break; - case 10: next = next_channel(patternA); break; - case 11: next = next_channel(patternB); break; - case 12: next = next_channel(patternC); break; - case 13: next = next_channel(patternD); break; - case 14: next = next_channel(patternE); break; - case 15: next = next_channel(patternF); break; - } - - return std::make_pair(next, loop_); - } - - template bool channel_enabled() { - return control_ & (8 >> channel); - } - - private: - static constexpr std::array pattern0 = { Channel::None }; - static constexpr std::array pattern1 = { Channel::Write, Channel::None }; - static constexpr std::array pattern2 = { Channel::C, Channel::None }; - static constexpr std::array pattern3 = { Channel::C, Channel::Write, Channel::None }; - static constexpr std::array pattern4 = { Channel::B, Channel::None, Channel::None }; - static constexpr std::array pattern5 = { Channel::B, Channel::Write, Channel::None }; - static constexpr std::array pattern6 = { Channel::B, Channel::C, Channel::None }; - static constexpr std::array pattern7 = { Channel::B, Channel::C, Channel::Write, Channel::None }; - static constexpr std::array pattern8 = { Channel::A, Channel::None }; - static constexpr std::array pattern9 = { Channel::A, Channel::Write }; - static constexpr std::array patternA = { Channel::A, Channel::C }; - static constexpr std::array patternB = { Channel::A, Channel::C, Channel::Write }; - static constexpr std::array patternC = { Channel::A, Channel::B, Channel::None }; - static constexpr std::array patternD = { Channel::A, Channel::B, Channel::Write }; - static constexpr std::array patternE = { Channel::A, Channel::B, Channel::C }; - static constexpr std::array patternF = { Channel::A, Channel::B, Channel::C, Channel::Write }; - template Channel next_channel(const ArrayT &list) { - loop_ += index_ / list.size(); - index_ %= list.size(); - const Channel result = list[index_]; - ++index_; - if(index_ == list.size()) { - phase_ = next_phase_; - } - return result; - } - - // Current control flags, i.e. which channels are enabled. - int control_ = 0; - - // Index into the pattern table for this blit. - size_t index_ = 0; - - // Number of times the entire pattern table has been completed. - int loop_ = 0; - - enum class Phase { - /// Return the next thing in the pattern table and advance. - /// If looping from the end of the pattern table to the start, - /// set phase_ to next_phase_. - Ongoing, - /// Return a Channel::None and advancce to phase_ = Phase::Complete. - PauseAndComplete, - /// Return Channel::Write indefinitely. - Complete - }; - - // Current sequencer pahse. - Phase phase_ = Phase::Complete; - // Phase to assume at the end of this iteration of the sequence table. - Phase next_phase_ = Phase::Complete; -}; - /*! If @c record_bus is @c true then all bus interactions will be recorded and can subsequently be retrieved. This is included for testing purposes. @@ -176,6 +28,13 @@ template class Blitter: public DMADevice<4, 4> { public: using DMADevice::DMADevice; + template void set_pointer(uint16_t value) { + if(get_status() & 0x4000) { + printf(">>>"); + } + DMADevice<4, 4>::set_pointer(value); + } + // Various setters; it's assumed that address decoding is handled externally. // // In all cases where a channel is identified numerically, it's taken that diff --git a/Machines/Amiga/BlitterSequencer.hpp b/Machines/Amiga/BlitterSequencer.hpp new file mode 100644 index 000000000..c6f0ff642 --- /dev/null +++ b/Machines/Amiga/BlitterSequencer.hpp @@ -0,0 +1,167 @@ +// +// BlitterSequencer.hpp +// Clock Signal +// +// Created by Thomas Harte on 19/08/2022. +// Copyright © 2022 Thomas Harte. All rights reserved. +// + +#ifndef BlitterSequencer_hpp +#define BlitterSequencer_hpp + +#include + +namespace Amiga { + +/*! + Statefully provides the next access the Blitter should make. + + TODO: determine the actual logic here, rather than + relying on tables. +*/ +class BlitterSequencer { + public: + enum class Channel { + /// Tells the caller to calculate and load a new piece of output + /// into the output pipeline. + /// + /// If any inputs are enabled then a one-slot output pipeline applies: + /// output will rest in the pipeline for one write phase before being written. + Write, + /// Indicates that a write should occur if anything is in the pipeline, otherwise + /// no activity should occur. + FlushPipeline, + /// The caller should read from channel C. + C, + /// The caller should read from channel B. + B, + /// The caller should read from channel A. + A, + /// Indicates an unused DMA slot. + None + }; + + /// Sets the current control value, which indicates which + /// channels are enabled. + void set_control(int control) { + control_ = control & 0xf; + index_ = 0; // TODO: this probably isn't accurate; case caught is a change + // of control values during a blit. + } + + /// Indicates that blitting should conclude after this step, i.e. + /// whatever is being fetched now is part of the final set of input data; + /// this is safe to call following a fetch request on any channel. + void complete() { + next_phase_ = + (control_ == 0x9 || control_ == 0xb || control_ == 0xd) ? + Phase::PauseAndComplete : Phase::Complete; + } + + /// Begins a blit operation. + void begin() { + phase_ = next_phase_ = Phase::Ongoing; + index_ = loop_ = 0; + } + + /// Provides the next channel to fetch from, or that a write is required, + /// along with a count of complete channel iterations so far completed. + std::pair next() { + switch(phase_) { + default: break; + + case Phase::Complete: + return std::make_pair(Channel::FlushPipeline, loop_); + + case Phase::PauseAndComplete: + phase_ = Phase::Complete; + return std::make_pair(Channel::None, loop_); + } + + Channel next = Channel::None; + + switch(control_) { + default: break; + + case 0: next = next_channel(pattern0); break; + case 1: next = next_channel(pattern1); break; + case 2: next = next_channel(pattern2); break; + case 3: next = next_channel(pattern3); break; + case 4: next = next_channel(pattern4); break; + case 5: next = next_channel(pattern5); break; + case 6: next = next_channel(pattern6); break; + case 7: next = next_channel(pattern7); break; + case 8: next = next_channel(pattern8); break; + case 9: next = next_channel(pattern9); break; + case 10: next = next_channel(patternA); break; + case 11: next = next_channel(patternB); break; + case 12: next = next_channel(patternC); break; + case 13: next = next_channel(patternD); break; + case 14: next = next_channel(patternE); break; + case 15: next = next_channel(patternF); break; + } + + return std::make_pair(next, loop_); + } + + template bool channel_enabled() { + return control_ & (8 >> channel); + } + + private: + static constexpr std::array pattern0 = { Channel::None }; + static constexpr std::array pattern1 = { Channel::Write, Channel::None }; + static constexpr std::array pattern2 = { Channel::C, Channel::None }; + static constexpr std::array pattern3 = { Channel::C, Channel::Write, Channel::None }; + static constexpr std::array pattern4 = { Channel::B, Channel::None, Channel::None }; + static constexpr std::array pattern5 = { Channel::B, Channel::Write, Channel::None }; + static constexpr std::array pattern6 = { Channel::B, Channel::C, Channel::None }; + static constexpr std::array pattern7 = { Channel::B, Channel::C, Channel::Write, Channel::None }; + static constexpr std::array pattern8 = { Channel::A, Channel::None }; + static constexpr std::array pattern9 = { Channel::A, Channel::Write }; + static constexpr std::array patternA = { Channel::A, Channel::C }; + static constexpr std::array patternB = { Channel::A, Channel::C, Channel::Write }; + static constexpr std::array patternC = { Channel::A, Channel::B, Channel::None }; + static constexpr std::array patternD = { Channel::A, Channel::B, Channel::Write }; + static constexpr std::array patternE = { Channel::A, Channel::B, Channel::C }; + static constexpr std::array patternF = { Channel::A, Channel::B, Channel::C, Channel::Write }; + template Channel next_channel(const ArrayT &list) { + loop_ += index_ / list.size(); + index_ %= list.size(); + const Channel result = list[index_]; + ++index_; + if(index_ == list.size()) { + phase_ = next_phase_; + } + return result; + } + + // Current control flags, i.e. which channels are enabled. + int control_ = 0; + + // Index into the pattern table for this blit. + size_t index_ = 0; + + // Number of times the entire pattern table has been completed. + int loop_ = 0; + + enum class Phase { + /// Return the next thing in the pattern table and advance. + /// If looping from the end of the pattern table to the start, + /// set phase_ to next_phase_. + Ongoing, + /// Return a Channel::None and advancce to phase_ = Phase::Complete. + PauseAndComplete, + /// Return Channel::Write indefinitely. + Complete + }; + + // Current sequencer pahse. + Phase phase_ = Phase::Complete; + // Phase to assume at the end of this iteration of the sequence table. + Phase next_phase_ = Phase::Complete; +}; + +} + +#endif /* BlitterSequencer_hpp */ diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index 378b9f045..d79ee0f69 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -1359,6 +1359,7 @@ 4B4A762F1DB1A3FA007AAE2E /* AY38910.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = AY38910.hpp; sourceTree = ""; }; 4B4B1A3A200198C900A0F866 /* KonamiSCC.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = KonamiSCC.cpp; sourceTree = ""; }; 4B4B1A3B200198C900A0F866 /* KonamiSCC.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = KonamiSCC.hpp; sourceTree = ""; }; + 4B4C81C228B0288B00F84AE9 /* BlitterSequencer.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = BlitterSequencer.hpp; sourceTree = ""; }; 4B4DC81F1D2C2425003C5BF8 /* Vic20.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Vic20.cpp; sourceTree = ""; }; 4B4DC8201D2C2425003C5BF8 /* Vic20.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = Vic20.hpp; sourceTree = ""; }; 4B4DC8271D2C2470003C5BF8 /* C1540.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = C1540.hpp; sourceTree = ""; }; @@ -4440,6 +4441,7 @@ 4B2130E1273A7A0A008A77B4 /* Audio.hpp */, 4B7C681D2751A104001671EC /* Bitplanes.hpp */, 4B9EC0E026AA260C0060A31F /* Blitter.hpp */, + 4B4C81C228B0288B00F84AE9 /* BlitterSequencer.hpp */, 4B9EC0E526AA4A660060A31F /* Chipset.hpp */, 4BC6236B26F4224300F83DFE /* Copper.hpp */, 4BC6236A26F178DA00F83DFE /* DMADevice.hpp */, From bfc77f1606656833f7d022fc1738b25bb7067385 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 19 Aug 2022 16:38:42 -0400 Subject: [PATCH 4/6] Add workaround that further isolates whatever bug Spindizzy reveals. --- Machines/Amiga/Blitter.cpp | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/Machines/Amiga/Blitter.cpp b/Machines/Amiga/Blitter.cpp index c60acc8bf..3eae3c7fc 100644 --- a/Machines/Amiga/Blitter.cpp +++ b/Machines/Amiga/Blitter.cpp @@ -240,12 +240,27 @@ bool Blitter::advance_dma() { // TODO: eliminate @c complete_immediately and this workaround. // See commentary in Chipset.cpp. -// if constexpr (complete_immediately) { -// while(get_status() & 0x4000) { -// advance_dma(); -// } -// return true; -// } + if constexpr (complete_immediately) { + + // HACK! HACK!! HACK!!! + // + // This resolves an issue with loading the particular copy of Spindizzy Worlds + // I am testing against. + // + // TODO: DO NOT PUBLISH THIS. + // + // This is committed solely so that I can continue researching the real, underlying + // issue across machines. It would not be acceptable to me to ship this. + // (and the printf is another reminder-to-self) + if(width_ == 8 && height_ == 32) { + printf("Accelerating %d x %d\n", width_, height_); + + while(get_status() & 0x4000) { + advance_dma(); + } + return true; + } + } if(line_mode_) { not_zero_flag_ = false; From f30f13f0bccb0bab75b4ab3f4da81a7dc1937b8c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 22 Aug 2022 10:03:24 -0400 Subject: [PATCH 5/6] Add overt include. --- OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm b/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm index 9e0dfed68..a969a5235 100644 --- a/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm +++ b/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm @@ -9,6 +9,7 @@ #import #include "Blitter.hpp" +#include "BlitterSequencer.hpp" #include "NSData+dataWithContentsOfGZippedFile.h" #include From 1b197d0bb2ae72d2d81c6d38acf1c008a5e74c75 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 22 Aug 2022 17:01:41 -0400 Subject: [PATCH 6/6] Resolve crash of machines that require the ROM requester. --- Concurrency/AsyncTaskQueue.hpp | 63 +++++++++++-------- .../Mac/Clock Signal/Machine/CSMachine.mm | 5 +- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/Concurrency/AsyncTaskQueue.hpp b/Concurrency/AsyncTaskQueue.hpp index 7a8452734..f0de9f640 100644 --- a/Concurrency/AsyncTaskQueue.hpp +++ b/Concurrency/AsyncTaskQueue.hpp @@ -65,34 +65,14 @@ template <> struct TaskQueueStorage { action occupies the asynchronous thread for long enough. So it is not true that @c perform will be called once per action. */ -template class AsyncTaskQueue: public TaskQueueStorage { +template class AsyncTaskQueue: public TaskQueueStorage { public: template AsyncTaskQueue(Args&&... args) : - TaskQueueStorage(std::forward(args)...), - thread_{ - [this] { - ActionVector actions; - - while(!should_quit_) { - // Wait for new actions to be signalled, and grab them. - std::unique_lock lock(condition_mutex_); - while(actions_.empty()) { - condition_.wait(lock); - } - std::swap(actions, actions_); - lock.unlock(); - - // Update to now (which is possibly a no-op). - TaskQueueStorage::update(); - - // Perform the actions and destroy them. - for(const auto &action: actions) { - action(); - } - actions.clear(); - } - } - } {} + TaskQueueStorage(std::forward(args)...) { + if constexpr (start_immediately) { + start(); + } + } /// Enqueus @c post_action to be performed asynchronously at some point /// in the future. If @c perform_automatically is @c true then the action @@ -136,6 +116,37 @@ template class AsyncTask } } + /// Starts the queue if it has never been started before. + /// + /// This is not guaranteed safely to restart a stopped queue. + void start() { + thread_ = std::move(std::thread{ + [this] { + ActionVector actions; + + // Continue until told to quit. + while(!should_quit_) { + // Wait for new actions to be signalled, and grab them. + std::unique_lock lock(condition_mutex_); + while(actions_.empty()) { + condition_.wait(lock); + } + std::swap(actions, actions_); + lock.unlock(); + + // Update to now (which is possibly a no-op). + TaskQueueStorage::update(); + + // Perform the actions and destroy them. + for(const auto &action: actions) { + action(); + } + actions.clear(); + } + } + }); + } + /// Schedules any remaining unscheduled work, then blocks synchronously /// until all scheduled work has been performed. void flush() { diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index c3912a080..3ad335c17 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -122,7 +122,7 @@ struct ActivityObserver: public Activity::Observer { CSJoystickManager *_joystickManager; NSMutableArray *_leds; - Concurrency::AsyncTaskQueue updater; + Concurrency::AsyncTaskQueue updater; Time::ScanSynchroniser _scanSynchroniser; NSTimer *_joystickTimer; @@ -149,6 +149,9 @@ struct ActivityObserver: public Activity::Observer { return nil; } updater.performer.machine = _machine->timed_machine(); + if(updater.performer.machine) { + updater.start(); + } // Use the keyboard as a joystick if the machine has no keyboard, or if it has a 'non-exclusive' keyboard. _inputMode =