From 821d40fe745e65eac41cb3e6b6493fe2844b4a10 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 16 May 2018 21:42:05 -0400 Subject: [PATCH 01/19] Reinstitutes the cap on maximum updating time. --- Concurrency/BestEffortUpdater.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Concurrency/BestEffortUpdater.cpp b/Concurrency/BestEffortUpdater.cpp index ce8e6041d..3d452a081 100644 --- a/Concurrency/BestEffortUpdater.cpp +++ b/Concurrency/BestEffortUpdater.cpp @@ -39,7 +39,9 @@ void BestEffortUpdater::update() { const int64_t integer_duration = std::chrono::duration_cast(elapsed).count(); if(integer_duration > 0) { if(delegate_) { - const double duration = static_cast(integer_duration) / 1e9; + // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any + // brief system interruption. + const double duration = std::min(static_cast(integer_duration) / 1e9, 0.2); delegate_->update(this, duration, has_skipped_); } has_skipped_ = false; From 4c49963988c95089b98be8f8aab42a95e4bdfc26 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 16 May 2018 21:44:09 -0400 Subject: [PATCH 02/19] Switches to proper handling of the motor control and write protection. Per Understanding the Apple II the drive looks write protected while phase 1 is enabled. --- Components/DiskII/DiskII.cpp | 14 +++++++++----- Components/DiskII/DiskII.hpp | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index d0f2178af..abb24a99b 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -36,9 +36,8 @@ void DiskII::set_control(Control control, bool on) { case Control::P3: stepper_mask_ = (stepper_mask_ & 0x7) | (on ? 0x8 : 0x0); break; case Control::Motor: - // TODO: does the motor control trigger both motors at once? - drives_[0].set_motor_on(on); - drives_[1].set_motor_on(on); + motor_is_enabled_ = on; + drives_[active_drive_].set_motor_on(on); break; } @@ -71,9 +70,14 @@ void DiskII::set_mode(Mode mode) { void DiskII::select_drive(int drive) { // printf("Select drive %d\n", drive); - active_drive_ = drive & 1; + if((drive&1) == active_drive_) return; + drives_[active_drive_].set_event_delegate(this); drives_[active_drive_^1].set_event_delegate(nullptr); + + drives_[active_drive_].set_motor_on(false); + active_drive_ = drive & 1; + drives_[active_drive_].set_motor_on(motor_is_enabled_); } void DiskII::set_data_register(uint8_t value) { @@ -134,7 +138,7 @@ void DiskII::set_controller_can_sleep() { } bool DiskII::is_write_protected() { - return true; + return !!(stepper_mask_ & 2) | drives_[active_drive_].get_is_read_only(); } void DiskII::set_state_machine(const std::vector &state_machine) { diff --git a/Components/DiskII/DiskII.hpp b/Components/DiskII/DiskII.hpp index 28b4cd2cc..9c5932c84 100644 --- a/Components/DiskII/DiskII.hpp +++ b/Components/DiskII/DiskII.hpp @@ -76,6 +76,7 @@ class DiskII: bool drive_is_sleeping_[2]; bool controller_can_sleep_ = false; int active_drive_ = 0; + bool motor_is_enabled_ = false; void set_controller_can_sleep(); }; From 1aba9f807ef8e391d37143f0631fdfd705dfa19b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 16 May 2018 22:07:54 -0400 Subject: [PATCH 03/19] Ensures proper upward propagation of sleeping from first start. --- Components/DiskII/DiskII.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index abb24a99b..838ce569c 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -25,6 +25,7 @@ DiskII::DiskII() : { drives_[0].set_sleep_observer(this); drives_[1].set_sleep_observer(this); + drives_[active_drive_].set_event_delegate(this); } void DiskII::set_control(Control control, bool on) { @@ -138,6 +139,7 @@ void DiskII::set_controller_can_sleep() { } bool DiskII::is_write_protected() { +return true; return !!(stepper_mask_ & 2) | drives_[active_drive_].get_is_read_only(); } From 8a031b1137d6d1692b3a74260eb21c7db724ec0f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 16 May 2018 22:09:59 -0400 Subject: [PATCH 04/19] Eliminates 'data' register as it doesn't exist; rejigs state machine command set. --- Components/DiskII/DiskII.cpp | 14 +++++++++----- Components/DiskII/DiskII.hpp | 1 - 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 838ce569c..94379045c 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -84,7 +84,7 @@ void DiskII::select_drive(int drive) { void DiskII::set_data_register(uint8_t value) { // printf("Set data register (?)\n"); inputs_ |= input_command; - data_register_ = value; +// shift_register_ = value; set_controller_can_sleep(); } @@ -106,15 +106,20 @@ void DiskII::run_for(const Cycles cycles) { inputs_ |= input_flux; state_ = state_machine_[static_cast(address)]; switch(state_ & 0xf) { - case 0x0: shift_register_ = 0; break; // clear + default: shift_register_ = 0; break; // clear + case 0x8: break; // nop + case 0x9: shift_register_ = static_cast(shift_register_ << 1); break; // shift left, bringing in a zero case 0xd: shift_register_ = static_cast((shift_register_ << 1) | 1); break; // shift left, bringing in a one - case 0xb: shift_register_ = data_register_; break; // load case 0xa: // shift right, bringing in write protected status shift_register_ = (shift_register_ >> 1) | (is_write_protected() ? 0x80 : 0x00); break; - default: break; + case 0xb: + // load data register from data bus... + printf("TODO\n"); + // shift_register_ = data_register_; + break; // load } // TODO: surely there's a less heavyweight solution than this? @@ -139,7 +144,6 @@ void DiskII::set_controller_can_sleep() { } bool DiskII::is_write_protected() { -return true; return !!(stepper_mask_ & 2) | drives_[active_drive_].get_is_read_only(); } diff --git a/Components/DiskII/DiskII.hpp b/Components/DiskII/DiskII.hpp index 9c5932c84..14a714b71 100644 --- a/Components/DiskII/DiskII.hpp +++ b/Components/DiskII/DiskII.hpp @@ -65,7 +65,6 @@ class DiskII: uint8_t state_ = 0; uint8_t inputs_ = 0; uint8_t shift_register_ = 0; - uint8_t data_register_ = 0; int stepper_mask_ = 0; int stepper_position_ = 0; From 908d3b0ee5e45d178bffeb4d26d645720d41f872 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 16 May 2018 22:37:22 -0400 Subject: [PATCH 05/19] Slightly wrong as to the details, but gets the controller trying to output. At an initial look, I think the shift register should end up on the data bus for all odd accesses. Need to investigate more thoroughly. --- Components/DiskII/DiskII.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 94379045c..76cea2486 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -84,7 +84,7 @@ void DiskII::select_drive(int drive) { void DiskII::set_data_register(uint8_t value) { // printf("Set data register (?)\n"); inputs_ |= input_command; -// shift_register_ = value; + shift_register_ = value; set_controller_can_sleep(); } @@ -114,6 +114,15 @@ void DiskII::run_for(const Cycles cycles) { case 0xa: // shift right, bringing in write protected status shift_register_ = (shift_register_ >> 1) | (is_write_protected() ? 0x80 : 0x00); + + // If the controller is in the sense write protect loop but the register will never change, + // short circuit further work and return now. + if(shift_register_ == is_write_protected() ? 0xff : 0x00) { + if(!drive_is_sleeping_[0]) drives_[0].run_for(Cycles(integer_cycles)); + if(!drive_is_sleeping_[1]) drives_[1].run_for(Cycles(integer_cycles)); + set_controller_can_sleep(); + return; + } break; case 0xb: // load data register from data bus... @@ -241,7 +250,9 @@ uint8_t DiskII::trigger_address(int address, uint8_t value) { case 0xc: return get_shift_register(); case 0xd: set_data_register(value); break; - case 0xe: set_mode(Mode::Read); break; + case 0xe: + set_mode(Mode::Read); + return shift_register_; case 0xf: set_mode(Mode::Write); break; } return 0xff; From c46007332a3f71b63260bbf6179ed19806db6654 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 May 2018 20:18:34 -0400 Subject: [PATCH 06/19] Switches to returning the shift register contents on every even read. --- Components/DiskII/DiskII.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 76cea2486..8669829fd 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -247,15 +247,19 @@ uint8_t DiskII::trigger_address(int address, uint8_t value) { case 0xa: select_drive(0); break; case 0xb: select_drive(1); break; - case 0xc: return get_shift_register(); + case 0xc: + inputs_ &= ~input_command; + break; case 0xd: set_data_register(value); break; case 0xe: set_mode(Mode::Read); - return shift_register_; + break; +// return shift_register_; case 0xf: set_mode(Mode::Write); break; } - return 0xff; + set_controller_can_sleep(); + return (address & 1) ? 0xff : shift_register_; } void DiskII::set_activity_observer(Activity::Observer *observer) { From 7b7beb13a3129a17326539b7b8b6d8dcdfdaf935 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 May 2018 21:39:11 -0400 Subject: [PATCH 07/19] Eliminates the fiction of setting and getting registers. The Disk II seems lower level than that; it will read the data bus whenever it likes, it is the programmer's responsibility to keep up with that. It also reserves the right not to load the bus regardless of whether it receives a read or write access. --- Components/DiskII/DiskII.cpp | 60 +++++---------------- Components/DiskII/DiskII.hpp | 41 ++++++++++++-- Machines/AppleII/DiskIICard.cpp | 8 +-- Machines/Oric/Oric.cpp | 23 ++++---- Storage/Disk/DiskImage/Formats/AppleDSK.hpp | 3 ++ 5 files changed, 71 insertions(+), 64 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 8669829fd..6a4b90b60 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -39,11 +39,9 @@ void DiskII::set_control(Control control, bool on) { case Control::Motor: motor_is_enabled_ = on; drives_[active_drive_].set_motor_on(on); - break; + return; } -// printf("%0x: Set control %d %s\n", stepper_mask_, control, on ? "on" : "off"); - // If the stepper magnet selections have changed, and any is on, see how // that moves the head. if(previous_stepper_mask ^ stepper_mask_ && stepper_mask_) { @@ -63,14 +61,7 @@ void DiskII::set_control(Control control, bool on) { } } -void DiskII::set_mode(Mode mode) { -// printf("Set mode %d\n", mode); - inputs_ = (inputs_ & ~input_mode) | ((mode == Mode::Write) ? input_mode : 0); - set_controller_can_sleep(); -} - void DiskII::select_drive(int drive) { -// printf("Select drive %d\n", drive); if((drive&1) == active_drive_) return; drives_[active_drive_].set_event_delegate(this); @@ -81,20 +72,6 @@ void DiskII::select_drive(int drive) { drives_[active_drive_].set_motor_on(motor_is_enabled_); } -void DiskII::set_data_register(uint8_t value) { -// printf("Set data register (?)\n"); - inputs_ |= input_command; - shift_register_ = value; - set_controller_can_sleep(); -} - -uint8_t DiskII::get_shift_register() { -// if(shift_register_ & 0x80) printf("[%02x] ", shift_register_); - inputs_ &= ~input_command; - set_controller_can_sleep(); - return shift_register_; -} - void DiskII::run_for(const Cycles cycles) { if(is_sleeping()) return; @@ -124,11 +101,7 @@ void DiskII::run_for(const Cycles cycles) { return; } break; - case 0xb: - // load data register from data bus... - printf("TODO\n"); - // shift_register_ = data_register_; - break; // load + case 0xb: shift_register_ = data_input_; break; // load data register from data bus } // TODO: surely there's a less heavyweight solution than this? @@ -221,15 +194,11 @@ bool DiskII::is_sleeping() { return controller_can_sleep_ && drive_is_sleeping_[0] && drive_is_sleeping_[1]; } -void DiskII::set_register(int address, uint8_t value) { - trigger_address(address, value); +void DiskII::set_data_input(uint8_t input) { + data_input_ = input; } -uint8_t DiskII::get_register(int address) { - return trigger_address(address, 0xff); -} - -uint8_t DiskII::trigger_address(int address, uint8_t value) { +int DiskII::read_address(int address) { switch(address & 0xf) { default: case 0x0: set_control(Control::P0, false); break; @@ -241,22 +210,19 @@ uint8_t DiskII::trigger_address(int address, uint8_t value) { case 0x6: set_control(Control::P3, false); break; case 0x7: set_control(Control::P3, true); break; - case 0x8: set_control(Control::Motor, false); break; + case 0x8: + shift_register_ = 0; + set_control(Control::Motor, false); + break; case 0x9: set_control(Control::Motor, true); break; case 0xa: select_drive(0); break; case 0xb: select_drive(1); break; - case 0xc: - inputs_ &= ~input_command; - break; - case 0xd: set_data_register(value); break; - - case 0xe: - set_mode(Mode::Read); - break; -// return shift_register_; - case 0xf: set_mode(Mode::Write); break; + case 0xc: inputs_ &= ~input_command; break; + case 0xd: inputs_ |= input_command; break; + case 0xe: inputs_ &= ~input_mode; break; + case 0xf: inputs_ |= input_mode; break; } set_controller_can_sleep(); return (address & 1) ? 0xff : shift_register_; diff --git a/Components/DiskII/DiskII.hpp b/Components/DiskII/DiskII.hpp index 14a714b71..f85a5ab49 100644 --- a/Components/DiskII/DiskII.hpp +++ b/Components/DiskII/DiskII.hpp @@ -33,15 +33,48 @@ class DiskII: public: DiskII(); - void set_register(int address, uint8_t value); - uint8_t get_register(int address); + /// Sets the current external value of the data bus. + void set_data_input(uint8_t input); + /*! + Submits an access to address @c address. + + @returns The 8-bit value loaded to the data bus by the DiskII if any; + @c DidNotLoad otherwise. + */ + int read_address(int address); + + /*! + The value returned by @c read_address if accessing that address + didn't cause the disk II to place anything onto the bus. + */ + const int DidNotLoad = -1; + + /// Advances the controller by @c cycles. void run_for(const Cycles cycles); + + /*! + Supplies the image of the state machine (i.e. P6) ROM, + which dictates how the Disk II will respond to input. + + To reduce processing costs, some assumptions are made by + the implementation as to the content of this ROM. + Including: + + If Q6 is set and Q7 is reset, the controller is testing + for write protect. If and when the shift register has + become full with the state of the write protect value, + no further processing is required. + */ void set_state_machine(const std::vector &); + /// Inserts @c disk into the drive @c drive. void set_disk(const std::shared_ptr &disk, int drive); + + // As per Sleeper. bool is_sleeping() override; + // The Disk II functions as a potential target for @c Activity::Sources. void set_activity_observer(Activity::Observer *observer); private: @@ -55,8 +88,6 @@ class DiskII: void set_control(Control control, bool on); void set_mode(Mode mode); void select_drive(int drive); - void set_data_register(uint8_t value); - uint8_t get_shift_register(); uint8_t trigger_address(int address, uint8_t value); void process_event(const Storage::Disk::Track::Event &event) override; @@ -78,6 +109,8 @@ class DiskII: bool motor_is_enabled_ = false; void set_controller_can_sleep(); + + uint8_t data_input_ = 0; }; } diff --git a/Machines/AppleII/DiskIICard.cpp b/Machines/AppleII/DiskIICard.cpp index e1447d6cc..6a0fb6c0f 100644 --- a/Machines/AppleII/DiskIICard.cpp +++ b/Machines/AppleII/DiskIICard.cpp @@ -25,10 +25,12 @@ void DiskIICard::perform_bus_operation(CPU::MOS6502::BusOperation operation, uin if(address < 0x100) { if(isReadOperation(operation)) *value = boot_[address]; } else { + // TODO: data input really shouldn't happen only upon a write. + diskii_.set_data_input(*value); + const int disk_value = diskii_.read_address(address); if(isReadOperation(operation)) { - *value = diskii_.get_register(address); - } else { - diskii_.set_register(address, *value); + if(disk_value != diskii_.DidNotLoad) + *value = static_cast(disk_value); } } } diff --git a/Machines/Oric/Oric.cpp b/Machines/Oric/Oric.cpp index 43d1dfcc7..b37124de4 100644 --- a/Machines/Oric/Oric.cpp +++ b/Machines/Oric/Oric.cpp @@ -405,9 +405,8 @@ template class Co } } } else { - update_diskii(); - if(isReadOperation(operation)) *value = diskii_.get_register(address); - else diskii_.set_register(address, *value); + const int disk_value = diskii_.read_address(address); + if(isReadOperation(operation) && disk_value != diskii_.DidNotLoad) *value = static_cast(disk_value); } break; } @@ -436,8 +435,13 @@ template class Co tape_player_.run_for(Cycles(1)); switch(disk_interface) { default: break; - case Analyser::Static::Oric::Target::DiskInterface::Microdisc: microdisc_.run_for(Cycles(8)); break; - case Analyser::Static::Oric::Target::DiskInterface::Pravetz: cycles_since_diskii_update_ += 2; break; + case Analyser::Static::Oric::Target::DiskInterface::Microdisc: + microdisc_.run_for(Cycles(8)); + break; + case Analyser::Static::Oric::Target::DiskInterface::Pravetz: + diskii_.set_data_input(*value); + diskii_.run_for(Cycles(2)); + break; } cycles_since_video_update_++; return Cycles(1); @@ -446,7 +450,6 @@ template class Co forceinline void flush() { update_video(); via_port_handler_.flush(); - if(disk_interface == Analyser::Static::Oric::Target::DiskInterface::Pravetz) update_diskii(); } // to satisfy CRTMachine::Machine @@ -605,10 +608,10 @@ template class Co Apple::DiskII diskii_; std::vector pravetz_rom_; std::size_t pravetz_rom_base_pointer_ = 0; - Cycles cycles_since_diskii_update_; - void update_diskii() { - diskii_.run_for(cycles_since_diskii_update_.flush()); - } +// Cycles cycles_since_diskii_update_; +// void update_diskii() { +// diskii_.run_for(cycles_since_diskii_update_.flush()); +// } // Overlay RAM uint16_t ram_top_ = basic_visible_ram_top_; diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp index 3e5ad3321..fb17f359e 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp @@ -35,6 +35,9 @@ class AppleDSK: public DiskImage { HeadPosition get_maximum_head_position() override; std::shared_ptr get_track_at_position(Track::Address address) override; + // TEST! + bool get_is_read_only() override { return false; } + private: Storage::FileHolder file_; int sectors_per_track_ = 16; From ed06533e6045a042014a4d0a8abfaf0d3ed9fee4 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 18 May 2018 22:07:58 -0400 Subject: [PATCH 08/19] Implements write support out of the Disk II. --- Components/DiskII/DiskII.cpp | 25 ++++++++++++++++++++----- Storage/Disk/Track/PCMPatchedTrack.cpp | 3 ++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 6a4b90b60..b71c9238b 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -104,6 +104,13 @@ void DiskII::run_for(const Cycles cycles) { case 0xb: shift_register_ = data_input_; break; // load data register from data bus } + // Currently writing? + if(inputs_&input_mode) { + // state_ & 0x80 should be the current level sent to the disk; + // therefore transitions in that bit should become flux transitions + drives_[active_drive_].write_bit(!!((state_ ^ address) & 0x80)); + } + // TODO: surely there's a less heavyweight solution than this? if(!drive_is_sleeping_[0]) drives_[0].run_for(Cycles(1)); if(!drive_is_sleeping_[1]) drives_[1].run_for(Cycles(1)); @@ -135,9 +142,9 @@ void DiskII::set_state_machine(const std::vector &state_machine) { state b0, state b2, state b3, pulse, Q7, Q6, shift, state b1 - ... and has the top nibble reflected. Beneath Apple Pro-DOS uses a - different order and several of the online copies are reformatted - into that order. + ... and has the top nibble of each value stored in the ROM reflected. + Beneath Apple Pro-DOS uses a different order and several of the + online copies are reformatted into that order. So the code below remaps into Beneath Apple Pro-DOS order if the supplied state machine isn't already in that order. @@ -221,8 +228,16 @@ int DiskII::read_address(int address) { case 0xc: inputs_ &= ~input_command; break; case 0xd: inputs_ |= input_command; break; - case 0xe: inputs_ &= ~input_mode; break; - case 0xf: inputs_ |= input_mode; break; + case 0xe: + if(inputs_ & input_mode) + drives_[active_drive_].end_writing(); + inputs_ &= ~input_mode; + break; + case 0xf: + if(!(inputs_ & input_mode)) + drives_[active_drive_].begin_writing(Storage::Time(1, 2045454), false); + inputs_ |= input_mode; + break; } set_controller_can_sleep(); return (address & 1) ? 0xff : shift_register_; diff --git a/Storage/Disk/Track/PCMPatchedTrack.cpp b/Storage/Disk/Track/PCMPatchedTrack.cpp index 5f2450339..d845749b0 100644 --- a/Storage/Disk/Track/PCMPatchedTrack.cpp +++ b/Storage/Disk/Track/PCMPatchedTrack.cpp @@ -202,7 +202,8 @@ Storage::Time PCMPatchedTrack::seek_to(const Time &time_since_index_hole) { else current_time_ = underlying_track_->seek_to(time_since_index_hole); - assert(current_time_ <= time_since_index_hole); + // The assert below is disabled as it assumes too much about total precision. +// assert(current_time_ <= time_since_index_hole); return current_time_; } From 8263c48a1d491107b54dd92ca1fc9d69fe5ed05f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 18 May 2018 23:03:28 -0400 Subject: [PATCH 09/19] Added a guarantee that the TrackSerialiser won't modify tracks it receives. --- Storage/Disk/Track/PCMPatchedTrack.cpp | 2 +- Storage/Disk/Track/PCMPatchedTrack.hpp | 6 +++--- Storage/Disk/Track/PCMTrack.cpp | 2 +- Storage/Disk/Track/PCMTrack.hpp | 6 +++--- Storage/Disk/Track/Track.hpp | 2 +- Storage/Disk/Track/TrackSerialiser.cpp | 11 +++++++---- Storage/Disk/Track/TrackSerialiser.hpp | 2 +- Storage/Disk/Track/UnformattedTrack.cpp | 2 +- Storage/Disk/Track/UnformattedTrack.hpp | 6 +++--- 9 files changed, 21 insertions(+), 18 deletions(-) diff --git a/Storage/Disk/Track/PCMPatchedTrack.cpp b/Storage/Disk/Track/PCMPatchedTrack.cpp index d845749b0..7c4275171 100644 --- a/Storage/Disk/Track/PCMPatchedTrack.cpp +++ b/Storage/Disk/Track/PCMPatchedTrack.cpp @@ -27,7 +27,7 @@ PCMPatchedTrack::PCMPatchedTrack(const PCMPatchedTrack &original) { active_period_ = periods_.begin(); } -Track *PCMPatchedTrack::clone() { +Track *PCMPatchedTrack::clone() const { return new PCMPatchedTrack(*this); } diff --git a/Storage/Disk/Track/PCMPatchedTrack.hpp b/Storage/Disk/Track/PCMPatchedTrack.hpp index 62d5b2130..710926c77 100644 --- a/Storage/Disk/Track/PCMPatchedTrack.hpp +++ b/Storage/Disk/Track/PCMPatchedTrack.hpp @@ -43,9 +43,9 @@ class PCMPatchedTrack: public Track { void add_segment(const Time &start_time, const PCMSegment &segment, bool clamp_to_index_hole); // To satisfy Storage::Disk::Track - Event get_next_event(); - Time seek_to(const Time &time_since_index_hole); - Track *clone(); + Event get_next_event() override; + Time seek_to(const Time &time_since_index_hole) override; + Track *clone() const override; private: std::shared_ptr underlying_track_; diff --git a/Storage/Disk/Track/PCMTrack.cpp b/Storage/Disk/Track/PCMTrack.cpp index 93da03f17..9252ef643 100644 --- a/Storage/Disk/Track/PCMTrack.cpp +++ b/Storage/Disk/Track/PCMTrack.cpp @@ -46,7 +46,7 @@ PCMTrack::PCMTrack(const PCMTrack &original) : PCMTrack() { segment_event_sources_ = original.segment_event_sources_; } -Track *PCMTrack::clone() { +Track *PCMTrack::clone() const { return new PCMTrack(*this); } diff --git a/Storage/Disk/Track/PCMTrack.hpp b/Storage/Disk/Track/PCMTrack.hpp index 9ec932175..e14f3fd3e 100644 --- a/Storage/Disk/Track/PCMTrack.hpp +++ b/Storage/Disk/Track/PCMTrack.hpp @@ -42,9 +42,9 @@ class PCMTrack: public Track { PCMTrack(const PCMTrack &); // as per @c Track - Event get_next_event(); - Time seek_to(const Time &time_since_index_hole); - Track *clone(); + Event get_next_event() override; + Time seek_to(const Time &time_since_index_hole) override; + Track *clone() const override; private: // storage for the segments that describe this track diff --git a/Storage/Disk/Track/Track.hpp b/Storage/Disk/Track/Track.hpp index 10df77135..bbc0fa96b 100644 --- a/Storage/Disk/Track/Track.hpp +++ b/Storage/Disk/Track/Track.hpp @@ -114,7 +114,7 @@ class Track { /*! The virtual copy constructor pattern; returns a copy of the Track. */ - virtual Track *clone() = 0; + virtual Track *clone() const = 0; }; } diff --git a/Storage/Disk/Track/TrackSerialiser.cpp b/Storage/Disk/Track/TrackSerialiser.cpp index 143dcde1c..4de6a6885 100644 --- a/Storage/Disk/Track/TrackSerialiser.cpp +++ b/Storage/Disk/Track/TrackSerialiser.cpp @@ -8,11 +8,14 @@ #include "TrackSerialiser.hpp" +#include + // TODO: if this is a PCMTrack with only one segment and that segment's bit rate is within tolerance, // just return a copy of that segment. -Storage::Disk::PCMSegment Storage::Disk::track_serialisation(Track &track, Time length_of_a_bit) { +Storage::Disk::PCMSegment Storage::Disk::track_serialisation(const Track &track, Time length_of_a_bit) { unsigned int history_size = 16; DigitalPhaseLockedLoop pll(100, history_size); + std::unique_ptr track_copy(track.clone()); struct ResultAccumulator: public DigitalPhaseLockedLoop::Delegate { PCMSegment result; @@ -29,12 +32,12 @@ Storage::Disk::PCMSegment Storage::Disk::track_serialisation(Track &track, Time length_multiplier.simplify(); // start at the index hole - track.seek_to(Time(0)); + track_copy->seek_to(Time(0)); // grab events until the next index hole Time time_error = Time(0); while(true) { - Track::Event next_event = track.get_next_event(); + Track::Event next_event = track_copy->get_next_event(); if(next_event.type == Track::Event::IndexHole) break; Time extended_length = next_event.length * length_multiplier + time_error; @@ -47,7 +50,7 @@ Storage::Disk::PCMSegment Storage::Disk::track_serialisation(Track &track, Time if(history_size) { history_size--; if(!history_size) { - track.seek_to(Time(0)); + track_copy->seek_to(Time(0)); time_error.set_zero(); pll.set_delegate(&result_accumulator); } diff --git a/Storage/Disk/Track/TrackSerialiser.hpp b/Storage/Disk/Track/TrackSerialiser.hpp index f1d865b37..85b025cfb 100644 --- a/Storage/Disk/Track/TrackSerialiser.hpp +++ b/Storage/Disk/Track/TrackSerialiser.hpp @@ -30,7 +30,7 @@ namespace Disk { @param length_of_a_bit The expected length of a single bit, as a proportion of the track length. */ -PCMSegment track_serialisation(Track &track, Time length_of_a_bit); +PCMSegment track_serialisation(const Track &track, Time length_of_a_bit); } } diff --git a/Storage/Disk/Track/UnformattedTrack.cpp b/Storage/Disk/Track/UnformattedTrack.cpp index d61db09e8..ed0e1669d 100644 --- a/Storage/Disk/Track/UnformattedTrack.cpp +++ b/Storage/Disk/Track/UnformattedTrack.cpp @@ -21,6 +21,6 @@ Storage::Time UnformattedTrack::seek_to(const Time &time_since_index_hole) { return Time(0); } -Track *UnformattedTrack::clone() { +Track *UnformattedTrack::clone() const { return new UnformattedTrack; } diff --git a/Storage/Disk/Track/UnformattedTrack.hpp b/Storage/Disk/Track/UnformattedTrack.hpp index 77b645782..6d66dc82b 100644 --- a/Storage/Disk/Track/UnformattedTrack.hpp +++ b/Storage/Disk/Track/UnformattedTrack.hpp @@ -19,9 +19,9 @@ namespace Disk { */ class UnformattedTrack: public Track { public: - Event get_next_event(); - Time seek_to(const Time &time_since_index_hole); - Track *clone(); + Event get_next_event() override; + Time seek_to(const Time &time_since_index_hole) override; + Track *clone() const override; }; } From 7cee3b744908b70a314af0d8e9e4c1a100e29114 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 19 May 2018 22:28:29 -0400 Subject: [PATCH 10/19] Resolves potential overflow / sign corruption. --- Storage/Disk/Track/TrackSerialiser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/Disk/Track/TrackSerialiser.cpp b/Storage/Disk/Track/TrackSerialiser.cpp index 4de6a6885..b575816a9 100644 --- a/Storage/Disk/Track/TrackSerialiser.cpp +++ b/Storage/Disk/Track/TrackSerialiser.cpp @@ -43,7 +43,7 @@ Storage::Disk::PCMSegment Storage::Disk::track_serialisation(const Track &track, Time extended_length = next_event.length * length_multiplier + time_error; time_error.clock_rate = extended_length.clock_rate; time_error.length = extended_length.length % extended_length.clock_rate; - pll.run_for(Cycles(extended_length.get())); + pll.run_for(Cycles(static_cast(extended_length.get()))); pll.add_pulse(); // If the PLL is now sufficiently primed, restart, and start recording bits this time. From a09fb01d7114dd6b502939451035bd23a8bc38de Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 19 May 2018 22:30:52 -0400 Subject: [PATCH 11/19] Makes an attempt at write support for Apple DSK files. --- Storage/Disk/DiskImage/Formats/AppleDSK.cpp | 49 +++++++++++++++++++-- Storage/Disk/DiskImage/Formats/AppleDSK.hpp | 7 +-- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp index 91882590d..d4184a849 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp @@ -9,7 +9,9 @@ #include "AppleDSK.hpp" #include "../../Track/PCMTrack.hpp" +#include "../../Track/TrackSerialiser.hpp" #include "../../Encodings/AppleGCR/Encoder.hpp" +#include "../../Encodings/AppleGCR/SegmentParser.hpp" using namespace Storage::Disk; @@ -42,10 +44,21 @@ HeadPosition AppleDSK::get_maximum_head_position() { return HeadPosition(number_of_tracks); } +bool AppleDSK::get_is_read_only() { + return file_.get_is_known_read_only(); +} + +long AppleDSK::file_offset(Track::Address address) { + return address.position.as_int() * bytes_per_sector * sectors_per_track_; +} + std::shared_ptr AppleDSK::get_track_at_position(Track::Address address) { - const long file_offset = address.position.as_int() * bytes_per_sector * sectors_per_track_; - file_.seek(file_offset, SEEK_SET); - const std::vector track_data = file_.read(static_cast(bytes_per_sector * sectors_per_track_)); + std::vector track_data; + { + std::lock_guard lock_guard(file_.get_file_access_mutex()); + file_.seek(file_offset(address), SEEK_SET); + track_data = file_.read(static_cast(bytes_per_sector * sectors_per_track_)); + } Storage::Disk::PCMSegment segment; const uint8_t track = static_cast(address.position.as_int()); @@ -71,8 +84,36 @@ std::shared_ptr AppleDSK::get_track_at_position(Track::Address address) { segment += Encodings::AppleGCR::six_and_two_sync((50000 - segment.number_of_bits) / 10); } } else { - + // TODO: 5 and 3, 13-sector format. If DSK actually supports it? } return std::make_shared(segment); } + +void AppleDSK::set_tracks(const std::map> &tracks) { + std::map> tracks_by_address; + for(const auto &pair: tracks) { + // Decode the track. + auto sector_map = Storage::Encodings::AppleGCR::sectors_from_segment( + Storage::Disk::track_serialisation(*pair.second, Storage::Time(1, 50000))); + + // Rearrange sectors into Apple DOS or Pro-DOS order. + std::vector track_contents(static_cast(bytes_per_sector * sectors_per_track_)); + for(const auto §or_pair: sector_map) { + size_t target_address = sector_pair.second.address.sector; + if(target_address != 15) { + target_address = (target_address * (is_prodos_ ? 2 : 13)) % 15; + } + memcpy(&track_contents[target_address*256], sector_pair.second.data.data(), bytes_per_sector); + } + + // Store for later. + tracks_by_address[pair.first] = std::move(track_contents); + } + + std::lock_guard lock_guard(file_.get_file_access_mutex()); + for(const auto &pair: tracks_by_address) { + file_.seek(file_offset(pair.first), SEEK_SET); + file_.write(pair.second); + } +} diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp index fb17f359e..14f1f5735 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp @@ -34,14 +34,15 @@ class AppleDSK: public DiskImage { // implemented to satisfy @c Disk HeadPosition get_maximum_head_position() override; std::shared_ptr get_track_at_position(Track::Address address) override; - - // TEST! - bool get_is_read_only() override { return false; } + void set_tracks(const std::map> &tracks) override; + bool get_is_read_only() override; private: Storage::FileHolder file_; int sectors_per_track_ = 16; bool is_prodos_ = false; + + long file_offset(Track::Address address); }; } From 46fae1a761863f585339b19c0fc65b320261e706 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 19 May 2018 22:50:33 -0400 Subject: [PATCH 12/19] Corrected: now maps in the proper direction. --- Storage/Disk/DiskImage/Formats/AppleDSK.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp index d4184a849..d66320117 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp @@ -102,7 +102,7 @@ void AppleDSK::set_tracks(const std::map> for(const auto §or_pair: sector_map) { size_t target_address = sector_pair.second.address.sector; if(target_address != 15) { - target_address = (target_address * (is_prodos_ ? 2 : 13)) % 15; + target_address = (target_address * (is_prodos_ ? 8 : 7)) % 15; } memcpy(&track_contents[target_address*256], sector_pair.second.data.data(), bytes_per_sector); } From 4952657b315f4569407d4fb68190ab57dbe97e6e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 19 May 2018 22:59:59 -0400 Subject: [PATCH 13/19] Factors out physical to logical sector conversion. --- Storage/Disk/DiskImage/Formats/AppleDSK.cpp | 20 +++++++++----------- Storage/Disk/DiskImage/Formats/AppleDSK.hpp | 3 ++- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp index d66320117..11a7d0463 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.cpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.cpp @@ -52,6 +52,13 @@ long AppleDSK::file_offset(Track::Address address) { return address.position.as_int() * bytes_per_sector * sectors_per_track_; } +size_t AppleDSK::logical_sector_for_physical_sector(size_t physical) { + // DOS and Pro DOS interleave sectors on disk, and they're represented in a disk + // image in physical order rather than logical. + if(physical == 15) return 15; + return (physical * (is_prodos_ ? 8 : 7)) % 15; +} + std::shared_ptr AppleDSK::get_track_at_position(Track::Address address) { std::vector track_data; { @@ -66,17 +73,11 @@ std::shared_ptr AppleDSK::get_track_at_position(Track::Address address) { // In either case below, the code aims for exactly 50,000 bits per track. if(sectors_per_track_ == 16) { // Write the sectors. - std::size_t sector_number_ = 0; for(uint8_t c = 0; c < 16; ++c) { segment += Encodings::AppleGCR::six_and_two_sync(10); segment += Encodings::AppleGCR::header(254, track, c); segment += Encodings::AppleGCR::six_and_two_sync(10); - segment += Encodings::AppleGCR::six_and_two_data(&track_data[sector_number_ * 256]); - - // DOS and Pro DOS interleave sectors on disk, and they're represented in a disk - // image in physical order rather than logical. So that skew needs to be applied here. - sector_number_ += is_prodos_ ? 8 : 7; - if(sector_number_ > 0xf) sector_number_ %= 15; + segment += Encodings::AppleGCR::six_and_two_data(&track_data[logical_sector_for_physical_sector(c) * 256]); } // Pad if necessary. @@ -100,10 +101,7 @@ void AppleDSK::set_tracks(const std::map> // Rearrange sectors into Apple DOS or Pro-DOS order. std::vector track_contents(static_cast(bytes_per_sector * sectors_per_track_)); for(const auto §or_pair: sector_map) { - size_t target_address = sector_pair.second.address.sector; - if(target_address != 15) { - target_address = (target_address * (is_prodos_ ? 8 : 7)) % 15; - } + const size_t target_address = logical_sector_for_physical_sector(sector_pair.second.address.sector); memcpy(&track_contents[target_address*256], sector_pair.second.data.data(), bytes_per_sector); } diff --git a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp index 14f1f5735..9d7403c44 100644 --- a/Storage/Disk/DiskImage/Formats/AppleDSK.hpp +++ b/Storage/Disk/DiskImage/Formats/AppleDSK.hpp @@ -31,7 +31,7 @@ class AppleDSK: public DiskImage { */ AppleDSK(const std::string &file_name); - // implemented to satisfy @c Disk + // Implemented to satisfy @c Disk. HeadPosition get_maximum_head_position() override; std::shared_ptr get_track_at_position(Track::Address address) override; void set_tracks(const std::map> &tracks) override; @@ -43,6 +43,7 @@ class AppleDSK: public DiskImage { bool is_prodos_ = false; long file_offset(Track::Address address); + size_t logical_sector_for_physical_sector(size_t physical); }; } From e482929da8796040369d89e139eae6998c515be9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 19 May 2018 23:15:28 -0400 Subject: [PATCH 14/19] Enhances the Disk II's ability to sleep. Also enables Disk II sleep observation in the Oric. --- Components/DiskII/DiskII.cpp | 15 ++++++++++++--- Components/DiskII/DiskII.hpp | 6 +++++- Machines/Oric/Oric.cpp | 20 ++++++++++++++------ 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index b71c9238b..75c93bc1f 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -126,10 +126,19 @@ void DiskII::run_for(const Cycles cycles) { void DiskII::set_controller_can_sleep() { // Permit the controller to sleep if it's in sense write protect mode, and the shift register // has already filled with the result of shifting eight times. + bool controller_could_sleep = controller_can_sleep_; controller_can_sleep_ = - (inputs_ == (input_command | input_flux)) && - (shift_register_ == (is_write_protected() ? 0xff : 0x00)); - if(is_sleeping()) update_sleep_observer(); + ( + (inputs_ == input_flux) && + !motor_is_enabled_ && + !shift_register_ + ) || + ( + (inputs_ == (input_command | input_flux)) && + (shift_register_ == (is_write_protected() ? 0xff : 0x00)) + ); + if(controller_could_sleep != controller_can_sleep_) + update_sleep_observer(); } bool DiskII::is_write_protected() { diff --git a/Components/DiskII/DiskII.hpp b/Components/DiskII/DiskII.hpp index f85a5ab49..ee81e0892 100644 --- a/Components/DiskII/DiskII.hpp +++ b/Components/DiskII/DiskII.hpp @@ -61,10 +61,14 @@ class DiskII: the implementation as to the content of this ROM. Including: - If Q6 is set and Q7 is reset, the controller is testing + * If Q6 is set and Q7 is reset, the controller is testing for write protect. If and when the shift register has become full with the state of the write protect value, no further processing is required. + + * If both Q6 and Q7 are reset, the drive motor is disabled, + and the shift register is all zeroes, no further processing + is required. */ void set_state_machine(const std::vector &); diff --git a/Machines/Oric/Oric.cpp b/Machines/Oric/Oric.cpp index b37124de4..4e4ebf153 100644 --- a/Machines/Oric/Oric.cpp +++ b/Machines/Oric/Oric.cpp @@ -201,6 +201,7 @@ template class Co public Utility::TypeRecipient, public Storage::Tape::BinaryTapePlayer::Delegate, public Microdisc::Delegate, + public Sleeper::SleepObserver, public Activity::Source, public Machine { @@ -216,6 +217,10 @@ template class Co via_port_handler_.set_interrupt_delegate(this); tape_player_.set_delegate(this); Memory::Fuzz(ram_, sizeof(ram_)); + + if(disk_interface == Analyser::Static::Oric::Target::DiskInterface::Pravetz) { + diskii_.set_sleep_observer(this); + } } ~ConcreteMachine() { @@ -439,8 +444,10 @@ template class Co microdisc_.run_for(Cycles(8)); break; case Analyser::Static::Oric::Target::DiskInterface::Pravetz: - diskii_.set_data_input(*value); - diskii_.run_for(Cycles(2)); + if(!diskii_is_sleeping_) { + diskii_.set_data_input(*value); + diskii_.run_for(Cycles(2)); + } break; } cycles_since_video_update_++; @@ -564,6 +571,10 @@ template class Co } } + void set_component_is_sleeping(Sleeper *component, bool is_sleeping) override final { + diskii_is_sleeping_ = diskii_.is_sleeping(); + } + private: const uint16_t basic_invisible_ram_top_ = 0xffff; const uint16_t basic_visible_ram_top_ = 0xbfff; @@ -608,10 +619,7 @@ template class Co Apple::DiskII diskii_; std::vector pravetz_rom_; std::size_t pravetz_rom_base_pointer_ = 0; -// Cycles cycles_since_diskii_update_; -// void update_diskii() { -// diskii_.run_for(cycles_since_diskii_update_.flush()); -// } + bool diskii_is_sleeping_ = false; // Overlay RAM uint16_t ram_top_ = basic_visible_ram_top_; From 80d34f551141b8bb19f4089d64c0974e36d83f9e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 21 May 2018 20:54:53 -0400 Subject: [PATCH 15/19] Specs out a new `AppleII::Card` interface. Doesn't yet fully implement it on the Apple II side though. --- Machines/AppleII/AppleII.cpp | 36 +++++++------ Machines/AppleII/Card.hpp | 89 +++++++++++++++++++++++++++++++-- Machines/AppleII/DiskIICard.cpp | 26 ++++++---- Machines/AppleII/DiskIICard.hpp | 4 +- 4 files changed, 123 insertions(+), 32 deletions(-) diff --git a/Machines/AppleII/AppleII.cpp b/Machines/AppleII/AppleII.cpp index b5aa63744..60fda9a7d 100644 --- a/Machines/AppleII/AppleII.cpp +++ b/Machines/AppleII/AppleII.cpp @@ -286,25 +286,29 @@ class ConcreteMachine: break; } - if(address >= 0xc100 && address < 0xc800) { - /* - Decode the area conventionally used by cards for ROMs: - 0xCn00 to 0xCnff: card n. - */ - const size_t card_number = (address - 0xc100) >> 8; - if(cards_[card_number]) { - update_cards(); - cards_[card_number]->perform_bus_operation(operation, address & 0xff, value); + if(address >= 0xc090 && address < 0xc800) { + size_t card_number = 0; + AppleII::Card::Select select = AppleII::Card::None; + + if(address >= 0xc100) { + /* + Decode the area conventionally used by cards for ROMs: + 0xCn00 to 0xCnff: card n. + */ + card_number = (address - 0xc100) >> 8; + select = AppleII::Card::Device; + } else { + /* + Decode the area conventionally used by cards for registers: + C0n0 to C0nF: card n - 8. + */ + card_number = (address - 0xc090) >> 4; + select = AppleII::Card::IO; } - } else if(address >= 0xc090 && address < 0xc100) { - /* - Decode the area conventionally used by cards for registers: - C0n0 to C0nF: card n - 8. - */ - const size_t card_number = (address - 0xc090) >> 4; + if(cards_[card_number]) { update_cards(); - cards_[card_number]->perform_bus_operation(operation, 0x100 | (address&0xf), value); + cards_[card_number]->perform_bus_operation(select, isReadOperation(operation), address, value); } } } diff --git a/Machines/AppleII/Card.hpp b/Machines/AppleII/Card.hpp index 4a8d7d864..42fb4fd23 100644 --- a/Machines/AppleII/Card.hpp +++ b/Machines/AppleII/Card.hpp @@ -15,16 +15,97 @@ namespace AppleII { +/*! + This provides a small subset of the interface offered to cards installed in + an Apple II, oriented pragmatically around the cards that are implemented. + + The main underlying rule is as it is elsewhere in the emulator: no + _inaccurate_ simplifications — no provision of information that shouldn't + actually commute, no interfaces that say they do one thing but which by both + both sides are coupled through an unwritten understanding of abuse. + + Special notes: + + Devices that announce a select constraint, being interested in acting only + when their IO or Device select is active will receive just-in-time @c run_for + notifications, as well as being updated at the end of each of the Apple's + @c run_for periods, prior to a @c flush. + + Devices that do not announce a select constraint will prima facie receive a + @c perform_bus_operation every cycle. They'll also receive a @c flush. + It is **highly** recomended that such devices also implement @c Sleeper + as they otherwise prima facie require a virtual method call every + single cycle. +*/ class Card { public: - /*! Advances time by @c cycles, of which @c stretches were stretched. */ + enum Select: int { + None = 0, // No select line is active + IO = 1 << 0, // IO select is active + Device = 1 << 1, // Device select is active + }; + + /*! + Advances time by @c cycles, of which @c stretches were stretched. + + This is posted only to cards that announced a select constraint. Cards with + no constraints, that want to be informed of every machine cycle, will receive + a call to perform_bus_operation every cycle and should use that for time keeping. + */ virtual void run_for(Cycles half_cycles, int stretches) {} - /*! Performs a bus operation; the card is implicitly selected. */ - virtual void perform_bus_operation(CPU::MOS6502::BusOperation operation, uint16_t address, uint8_t *value) = 0; + /// Requests a flush of any pending audio or video output. + virtual void flush() {} - /*! Supplies a target for observers. */ + /*! + Performs a bus operation. + + @param select The state of the card's select lines: indicates whether the Apple II + thinks this card should respond as though this were an IO access, a Device access, + or it thinks that the card shouldn't respond. + @param is_read @c true if this is a read cycle; @c false otherwise. + @param address The current value of the address bus. + @param value A pointer to the value of the data bus, not accounting input from cards. + If this is a read cycle, the card is permitted to replace this value with the value + output by the card, if any. If this is a write cycle, the card should only read + this value. + */ + virtual void perform_bus_operation(Select select, bool is_read, uint16_t address, uint8_t *value) = 0; + + /*! + Returns the type of bus selects this card is actually interested in. + As specified, the default is that cards will ask to receive perform_bus_operation + only when their select lines are active. + + There's a substantial caveat here: cards that register to receive @c None + will receive a perform_bus_operation every cycle. To reduce the number of + virtual method calls, they **will not** receive run_for. run_for will propagate + only to cards that register for IO and/or Device accesses only. + + + */ + int get_select_constraints() { + return select_constraints_; + } + + /*! Cards may supply a target for activity observation if desired. */ virtual void set_activity_observer(Activity::Observer *observer) {} + + struct Delegate { + virtual void card_did_change_select_constraints(Card *card) = 0; + }; + void set_delegate(Delegate *delegate) { + delegate_ = delegate; + } + + protected: + int select_constraints_ = IO | Device; + Delegate *delegate_ = nullptr; + void set_select_constraints(int constraints) { + if(constraints == select_constraints_) return; + select_constraints_ = constraints; + if(delegate_) delegate_->card_did_change_select_constraints(this); + } }; } diff --git a/Machines/AppleII/DiskIICard.cpp b/Machines/AppleII/DiskIICard.cpp index 6a0fb6c0f..b2dd411ba 100644 --- a/Machines/AppleII/DiskIICard.cpp +++ b/Machines/AppleII/DiskIICard.cpp @@ -19,19 +19,23 @@ DiskIICard::DiskIICard(const ROMMachine::ROMFetcher &rom_fetcher, bool is_16_sec }); boot_ = std::move(*roms[0]); diskii_.set_state_machine(*roms[1]); + set_select_constraints(None); } -void DiskIICard::perform_bus_operation(CPU::MOS6502::BusOperation operation, uint16_t address, uint8_t *value) { - if(address < 0x100) { - if(isReadOperation(operation)) *value = boot_[address]; - } else { - // TODO: data input really shouldn't happen only upon a write. - diskii_.set_data_input(*value); - const int disk_value = diskii_.read_address(address); - if(isReadOperation(operation)) { - if(disk_value != diskii_.DidNotLoad) - *value = static_cast(disk_value); - } +void DiskIICard::perform_bus_operation(Select select, bool is_read, uint16_t address, uint8_t *value) { + diskii_.set_data_input(*value); + switch(select) { + default: break; + case IO: { + const int disk_value = diskii_.read_address(address); + if(is_read) { + if(disk_value != diskii_.DidNotLoad) + *value = static_cast(disk_value); + } + } break; + case Device: + if(is_read) *value = boot_[address & 0xff]; + break; } } diff --git a/Machines/AppleII/DiskIICard.hpp b/Machines/AppleII/DiskIICard.hpp index 63953164d..48d30d021 100644 --- a/Machines/AppleII/DiskIICard.hpp +++ b/Machines/AppleII/DiskIICard.hpp @@ -14,6 +14,7 @@ #include "../../Components/DiskII/DiskII.hpp" #include "../../Storage/Disk/Disk.hpp" +#include "../../ClockReceiver/Sleeper.hpp" #include #include @@ -25,8 +26,9 @@ class DiskIICard: public Card { public: DiskIICard(const ROMMachine::ROMFetcher &rom_fetcher, bool is_16_sector); - void perform_bus_operation(CPU::MOS6502::BusOperation operation, uint16_t address, uint8_t *value) override; + void perform_bus_operation(Select select, bool is_read, uint16_t address, uint8_t *value) override; void run_for(Cycles cycles, int stretches) override; + void set_activity_observer(Activity::Observer *observer) override; void set_disk(const std::shared_ptr &disk, int drive); From 015f692bd3561072587b2fb7a30585dd6afa4a97 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 22 May 2018 19:51:39 -0400 Subject: [PATCH 16/19] The Disk II card now commutes Disk II sleep activity to select constraints. --- Machines/AppleII/DiskIICard.cpp | 7 +++++++ Machines/AppleII/DiskIICard.hpp | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Machines/AppleII/DiskIICard.cpp b/Machines/AppleII/DiskIICard.cpp index b2dd411ba..159c062ed 100644 --- a/Machines/AppleII/DiskIICard.cpp +++ b/Machines/AppleII/DiskIICard.cpp @@ -20,6 +20,7 @@ DiskIICard::DiskIICard(const ROMMachine::ROMFetcher &rom_fetcher, bool is_16_sec boot_ = std::move(*roms[0]); diskii_.set_state_machine(*roms[1]); set_select_constraints(None); + diskii_.set_sleep_observer(this); } void DiskIICard::perform_bus_operation(Select select, bool is_read, uint16_t address, uint8_t *value) { @@ -40,6 +41,7 @@ void DiskIICard::perform_bus_operation(Select select, bool is_read, uint16_t add } void DiskIICard::run_for(Cycles cycles, int stretches) { + if(diskii_is_sleeping_) return; diskii_.run_for(Cycles(cycles.as_int() * 2)); } @@ -50,3 +52,8 @@ void DiskIICard::set_disk(const std::shared_ptr &disk, int void DiskIICard::set_activity_observer(Activity::Observer *observer) { diskii_.set_activity_observer(observer); } + +void DiskIICard::set_component_is_sleeping(Sleeper *component, bool is_sleeping) { + diskii_is_sleeping_ = is_sleeping; + set_select_constraints(is_sleeping ? (IO | Device) : 0); +} diff --git a/Machines/AppleII/DiskIICard.hpp b/Machines/AppleII/DiskIICard.hpp index 48d30d021..9d5eaa8ec 100644 --- a/Machines/AppleII/DiskIICard.hpp +++ b/Machines/AppleII/DiskIICard.hpp @@ -22,7 +22,7 @@ namespace AppleII { -class DiskIICard: public Card { +class DiskIICard: public Card, public Sleeper::SleepObserver { public: DiskIICard(const ROMMachine::ROMFetcher &rom_fetcher, bool is_16_sector); @@ -34,8 +34,10 @@ class DiskIICard: public Card { void set_disk(const std::shared_ptr &disk, int drive); private: + void set_component_is_sleeping(Sleeper *component, bool is_sleeping) override; std::vector boot_; Apple::DiskII diskii_; + bool diskii_is_sleeping_ = false; }; } From ea92363e6c0cfb366eae47e818cc01ef5cbc3a37 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 22 May 2018 20:34:59 -0400 Subject: [PATCH 17/19] Attempts to get the Apple II to honour the AppleII::Card select constraints appropriately. --- Machines/AppleII/AppleII.cpp | 95 +++++++++++++++++++++++++++++------- 1 file changed, 77 insertions(+), 18 deletions(-) diff --git a/Machines/AppleII/AppleII.cpp b/Machines/AppleII/AppleII.cpp index 60fda9a7d..eb9433c0c 100644 --- a/Machines/AppleII/AppleII.cpp +++ b/Machines/AppleII/AppleII.cpp @@ -38,7 +38,8 @@ class ConcreteMachine: public CPU::MOS6502::BusHandler, public Inputs::Keyboard, public AppleII::Machine, - public Activity::Source { + public Activity::Source, + public AppleII::Card::Delegate { private: struct VideoBusHandler : public AppleII::Video::BusHandler { public: @@ -66,8 +67,8 @@ class ConcreteMachine: speaker_.run_for(audio_queue_, cycles_since_audio_update_.divide(Cycles(audio_divider))); } void update_cards() { - for(const auto &card : cards_) { - if(card) card->run_for(cycles_since_card_update_, stretched_cycles_since_card_update_); + for(const auto &card : just_in_time_cards_) { + card->run_for(cycles_since_card_update_, stretched_cycles_since_card_update_); } cycles_since_card_update_ = 0; stretched_cycles_since_card_update_ = 0; @@ -84,10 +85,42 @@ class ConcreteMachine: Cycles cycles_since_audio_update_; ROMMachine::ROMFetcher rom_fetcher_; + + // MARK: - Cards std::array, 7> cards_; Cycles cycles_since_card_update_; + std::vector every_cycle_cards_; + std::vector just_in_time_cards_; + int stretched_cycles_since_card_update_ = 0; + void install_card(std::size_t slot, AppleII::Card *card) { + assert(slot >= 1 && slot < 8); + cards_[slot - 1].reset(card); + card->set_delegate(this); + pick_card_messaging_group(card); + } + + bool is_every_cycle_card(AppleII::Card *card) { + return !card->get_select_constraints(); + } + + void pick_card_messaging_group(AppleII::Card *card) { + const bool is_every_cycle = is_every_cycle_card(card); + std::vector &intended = is_every_cycle ? every_cycle_cards_ : just_in_time_cards_; + std::vector &undesired = is_every_cycle ? just_in_time_cards_ : every_cycle_cards_; + + if(std::find(intended.begin(), intended.end(), card) != intended.end()) return; + auto old_membership = std::find(undesired.begin(), undesired.end(), card); + if(old_membership != undesired.end()) undesired.erase(old_membership); + intended.push_back(card); + } + + void card_did_change_select_constraints(AppleII::Card *card) override { + pick_card_messaging_group(card); + } + + // MARK: - Memory Map struct MemoryBlock { uint8_t *read_pointer = nullptr; uint8_t *write_pointer = nullptr; @@ -174,6 +207,19 @@ class ConcreteMachine: ++ cycles_since_card_update_; cycles_since_audio_update_ += Cycles(7); + // The Apple II has a slightly weird timing pattern: every 65th CPU cycle is stretched + // by an extra 1/7th. That's because one cycle lasts 3.5 NTSC colour clocks, so after + // 65 cycles a full line of 227.5 colour clocks have passed. But the high-rate binary + // signal approximation that produces colour needs to be in phase, so a stretch of exactly + // 0.5 further colour cycles is added. The video class handles that implicitly, but it + // needs to be accumulated here for the audio. + cycles_into_current_line_ = (cycles_into_current_line_ + 1) % 65; + const bool is_stretched_cycle = !cycles_into_current_line_; + if(is_stretched_cycle) { + ++ cycles_since_audio_update_; + ++ stretched_cycles_since_card_update_; + } + /* There are five distinct zones of memory on an Apple II: @@ -286,7 +332,13 @@ class ConcreteMachine: break; } + /* + Communication with cards. + */ + if(address >= 0xc090 && address < 0xc800) { + // If this is a card access, figure out which card is at play before determining + // the totality of who needs messaging. size_t card_number = 0; AppleII::Card::Select select = AppleII::Card::None; @@ -306,25 +358,31 @@ class ConcreteMachine: select = AppleII::Card::IO; } - if(cards_[card_number]) { + // If the selected card is a just-in-time card, update the just-in-time cards, + // and then message it specifically. + AppleII::Card *const target = cards_[card_number].get(); + if(target && !is_every_cycle_card(target)) { update_cards(); - cards_[card_number]->perform_bus_operation(select, isReadOperation(operation), address, value); + target->perform_bus_operation(select, isReadOperation(operation), address, value); + } + + // Update all the every-cycle cards regardless, but send them a ::None select if they're + // not the one actually selected. + for(const auto &card: every_cycle_cards_) { + card->run_for(Cycles(1), is_stretched_cycle); + card->perform_bus_operation( + (card == target) ? select : AppleII::Card::None, + isReadOperation(operation), address, value); + } + } else { + // Update all every-cycle cards and give them the cycle. + for(const auto &card: every_cycle_cards_) { + card->run_for(Cycles(1), is_stretched_cycle); + card->perform_bus_operation(AppleII::Card::None, isReadOperation(operation), address, value); } } } - // The Apple II has a slightly weird timing pattern: every 65th CPU cycle is stretched - // by an extra 1/7th. That's because one cycle lasts 3.5 NTSC colour clocks, so after - // 65 cycles a full line of 227.5 colour clocks have passed. But the high-rate binary - // signal approximation that produces colour needs to be in phase, so a stretch of exactly - // 0.5 further colour cycles is added. The video class handles that implicitly, but it - // needs to be accumulated here for the audio. - cycles_into_current_line_ = (cycles_into_current_line_ + 1) % 65; - if(!cycles_into_current_line_) { - ++ cycles_since_audio_update_; - ++ stretched_cycles_since_card_update_; - } - return Cycles(1); } @@ -395,7 +453,8 @@ class ConcreteMachine: auto *const apple_target = dynamic_cast(target); if(apple_target->disk_controller != Target::DiskController::None) { - cards_[5].reset(new AppleII::DiskIICard(rom_fetcher_, apple_target->disk_controller == Target::DiskController::SixteenSector)); + // Apple recommended slot 6 for the (first) Disk II. + install_card(6, new AppleII::DiskIICard(rom_fetcher_, apple_target->disk_controller == Target::DiskController::SixteenSector)); } rom_ = (apple_target->model == Target::Model::II) ? apple2_rom_ : apple2plus_rom_; From f9c25372c2d1d9af0bf36a717e1494aef485f3c1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 22 May 2018 21:49:34 -0400 Subject: [PATCH 18/19] Ensures cards get messaged regardless of memory area. --- Machines/AppleII/AppleII.cpp | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/Machines/AppleII/AppleII.cpp b/Machines/AppleII/AppleII.cpp index eb9433c0c..2389bcb10 100644 --- a/Machines/AppleII/AppleII.cpp +++ b/Machines/AppleII/AppleII.cpp @@ -66,7 +66,7 @@ class ConcreteMachine: void update_audio() { speaker_.run_for(audio_queue_, cycles_since_audio_update_.divide(Cycles(audio_divider))); } - void update_cards() { + void update_just_in_time_cards() { for(const auto &card : just_in_time_cards_) { card->run_for(cycles_since_card_update_, stretched_cycles_since_card_update_); } @@ -238,8 +238,9 @@ class ConcreteMachine: } else if(address < 0xd000) block = nullptr; else if(address < 0xe000) {block = &memory_blocks_[2]; address -= 0xd000; } - else {block = &memory_blocks_[3]; address -= 0xe000; } + else { block = &memory_blocks_[3]; address -= 0xe000; } + bool has_updated_cards = false; if(block) { if(isReadOperation(operation)) *value = block->read_pointer[address]; else if(block->write_pointer) block->write_pointer[address] = *value; @@ -333,7 +334,7 @@ class ConcreteMachine: } /* - Communication with cards. + Communication with cards follows. */ if(address >= 0xc090 && address < 0xc800) { @@ -360,10 +361,11 @@ class ConcreteMachine: // If the selected card is a just-in-time card, update the just-in-time cards, // and then message it specifically. + const bool is_read = isReadOperation(operation); AppleII::Card *const target = cards_[card_number].get(); if(target && !is_every_cycle_card(target)) { - update_cards(); - target->perform_bus_operation(select, isReadOperation(operation), address, value); + update_just_in_time_cards(); + target->perform_bus_operation(select, is_read, address, value); } // Update all the every-cycle cards regardless, but send them a ::None select if they're @@ -372,14 +374,18 @@ class ConcreteMachine: card->run_for(Cycles(1), is_stretched_cycle); card->perform_bus_operation( (card == target) ? select : AppleII::Card::None, - isReadOperation(operation), address, value); - } - } else { - // Update all every-cycle cards and give them the cycle. - for(const auto &card: every_cycle_cards_) { - card->run_for(Cycles(1), is_stretched_cycle); - card->perform_bus_operation(AppleII::Card::None, isReadOperation(operation), address, value); + is_read, address, value); } + has_updated_cards = true; + } + } + + if(!has_updated_cards && !every_cycle_cards_.empty()) { + // Update all every-cycle cards and give them the cycle. + const bool is_read = isReadOperation(operation); + for(const auto &card: every_cycle_cards_) { + card->run_for(Cycles(1), is_stretched_cycle); + card->perform_bus_operation(AppleII::Card::None, is_read, address, value); } } @@ -389,7 +395,7 @@ class ConcreteMachine: void flush() { update_video(); update_audio(); - update_cards(); + update_just_in_time_cards(); audio_queue_.perform(); } From 086b801c29e6702fb48f3cce4247c9f4f72c0822 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 22 May 2018 21:50:07 -0400 Subject: [PATCH 19/19] Mildly rearranges to avoid unnecessary call. --- Components/DiskII/DiskII.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Components/DiskII/DiskII.cpp b/Components/DiskII/DiskII.cpp index 75c93bc1f..a19556b21 100644 --- a/Components/DiskII/DiskII.cpp +++ b/Components/DiskII/DiskII.cpp @@ -75,9 +75,8 @@ void DiskII::select_drive(int drive) { void DiskII::run_for(const Cycles cycles) { if(is_sleeping()) return; - int integer_cycles = cycles.as_int(); - if(!controller_can_sleep_) { + int integer_cycles = cycles.as_int(); while(integer_cycles--) { const int address = (state_ & 0xf0) | inputs_ | ((shift_register_&0x80) >> 6); inputs_ |= input_flux;