From da944fde926b1c8a4bc4fd50ef0dc60653e4da93 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 10 Mar 2023 21:04:35 -0500 Subject: [PATCH 1/9] Eliminate data-type assumption. --- Components/9918/Implementation/9918.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index a3e2ff7fe..9a2da296f 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -261,10 +261,11 @@ void TMS9918::run_for(const HalfCycles cycles) { // It is otherwise decremented. if constexpr (is_sega_vdp(personality)) { if(this->fetch_pointer_.row >= 0 && this->fetch_pointer_.row <= this->mode_timing_.pixel_lines) { - --this->line_interrupt_counter_; - if(this->line_interrupt_counter_ == 0xff) { + if(!this->line_interrupt_counter_) { this->line_interrupt_pending_ = true; this->line_interrupt_counter_ = this->line_interrupt_target_; + } else { + --this->line_interrupt_counter_; } } else { this->line_interrupt_counter_ = this->line_interrupt_target_; From 9836a108daee3e28f8e9f05c5a75de81fb3594a4 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 10 Mar 2023 21:04:55 -0500 Subject: [PATCH 2/9] Avoid VDP access races. --- Machines/MasterSystem/MasterSystem.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Machines/MasterSystem/MasterSystem.cpp b/Machines/MasterSystem/MasterSystem.cpp index 30c20e672..8136049f6 100644 --- a/Machines/MasterSystem/MasterSystem.cpp +++ b/Machines/MasterSystem/MasterSystem.cpp @@ -181,24 +181,27 @@ template class ConcreteMachine: } void set_scan_target(Outputs::Display::ScanTarget *scan_target) final { - vdp_->set_tv_standard( + vdp_.last_valid()->set_tv_standard( (region_ == Target::Region::Europe) ? TI::TMS::TVStandard::PAL : TI::TMS::TVStandard::NTSC); - time_until_debounce_ = vdp_->get_time_until_line(-1); - vdp_->set_scan_target(scan_target); + // Doing the following would be technically correct, but isn't + // especially thread-safe and won't make a substantial difference. +// time_until_debounce_ = vdp_->get_time_until_line(-1); + + vdp_.last_valid()->set_scan_target(scan_target); } Outputs::Display::ScanStatus get_scaled_scan_status() const final { - return vdp_->get_scaled_scan_status(); + return vdp_.last_valid()->get_scaled_scan_status(); } void set_display_type(Outputs::Display::DisplayType display_type) final { - vdp_->set_display_type(display_type); + vdp_.last_valid()->set_display_type(display_type); } Outputs::Display::DisplayType get_display_type() const final { - return vdp_->get_display_type(); + return vdp_.last_valid()->get_display_type(); } Outputs::Speaker::Speaker *get_speaker() final { From d46f869276fdcb3761fb098d213b78c64cb5486d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 10 Mar 2023 21:14:52 -0500 Subject: [PATCH 3/9] Minor style improvement. --- Components/9918/Implementation/Draw.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Components/9918/Implementation/Draw.hpp b/Components/9918/Implementation/Draw.hpp index 55703f24a..3726066be 100644 --- a/Components/9918/Implementation/Draw.hpp +++ b/Components/9918/Implementation/Draw.hpp @@ -70,8 +70,9 @@ void Base::draw_sprites(LineBuffer &buffer, int start, int end, con ) colour_buffer[c] = sprite_buffer[c]; } - if(sprite_collision) + if(sprite_collision) { status_ |= StatusSpriteCollision; + } return; } From cc04349618b57100e35b8e2715a048f46d87ddbf Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 11 Mar 2023 22:24:11 -0500 Subject: [PATCH 4/9] Reestablish relationship between fetch and output. --- Components/9918/Implementation/9918.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index 9a2da296f..4368f7647 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -48,8 +48,9 @@ Base::Base() : // Start at a random position. output_pointer_.row = rand() % 262; output_pointer_.column = rand() % (Timing::CyclesPerLine - output_lag); - fetch_pointer_.row = fetch_pointer_.row; - fetch_pointer_.column = output_pointer_.column + output_lag; + + fetch_pointer_ = output_pointer_; + fetch_pointer_.column += output_lag; } template From e0125e01775eec0d705305a2db753d0490ed763f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Mar 2023 23:14:24 -0400 Subject: [PATCH 5/9] Add MSX 1 diversion. --- Analyser/Static/MSX/StaticAnalyser.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Analyser/Static/MSX/StaticAnalyser.cpp b/Analyser/Static/MSX/StaticAnalyser.cpp index b2bdf02de..322189ca4 100644 --- a/Analyser/Static/MSX/StaticAnalyser.cpp +++ b/Analyser/Static/MSX/StaticAnalyser.cpp @@ -37,6 +37,11 @@ static std::unique_ptr CartridgeTarget( auto target = std::make_unique(); target->confidence = confidence; + // Observation: all ROMs of 48kb or less are from the MSX 1 era. + if(segment.data.size() < 48*1024) { + target->model = Analyser::Static::MSX::Target::Model::MSX1; + } + if(type == Analyser::Static::MSX::Cartridge::Type::None) { target->media.cartridges.emplace_back(new Storage::Cartridge::Cartridge(output_segments)); } else { @@ -100,6 +105,7 @@ static Analyser::Static::TargetList CartridgeTargetsFrom( // TODO: check for a rational init address? // If this ROM is less than 48kb in size then it's an ordinary ROM. Just emplace it and move on. + // Bonus observation: all such ROMs are from the MSX 1 era. if(data_size <= 0xc000) { targets.emplace_back(CartridgeTarget(segment, start_address, Analyser::Static::MSX::Cartridge::Type::None, 1.0)); continue; From 201a7c17aef28f2a7bd0937b5397bde7cd27d9ca Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Mar 2023 23:20:48 -0400 Subject: [PATCH 6/9] Avoid VDP race condition. --- Machines/ColecoVision/ColecoVision.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Machines/ColecoVision/ColecoVision.cpp b/Machines/ColecoVision/ColecoVision.cpp index 8349bbef6..19ba93adb 100644 --- a/Machines/ColecoVision/ColecoVision.cpp +++ b/Machines/ColecoVision/ColecoVision.cpp @@ -175,19 +175,19 @@ class ConcreteMachine: } void set_scan_target(Outputs::Display::ScanTarget *scan_target) final { - vdp_->set_scan_target(scan_target); + vdp_.last_valid()->set_scan_target(scan_target); } Outputs::Display::ScanStatus get_scaled_scan_status() const final { - return vdp_->get_scaled_scan_status(); + return vdp_.last_valid()->get_scaled_scan_status(); } void set_display_type(Outputs::Display::DisplayType display_type) final { - vdp_->set_display_type(display_type); + vdp_.last_valid()->set_display_type(display_type); } Outputs::Display::DisplayType get_display_type() const final { - return vdp_->get_display_type(); + return vdp_.last_valid()->get_display_type(); } Outputs::Speaker::Speaker *get_speaker() final { From e703fa9cf8952a16ef1f8082d9cfe0fd23c6eebc Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Mar 2023 23:33:29 -0400 Subject: [PATCH 7/9] Fetch colours in TMS character mode. --- Components/9918/Implementation/Fetch.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Components/9918/Implementation/Fetch.hpp b/Components/9918/Implementation/Fetch.hpp index 34d716d20..7c4c007e2 100644 --- a/Components/9918/Implementation/Fetch.hpp +++ b/Components/9918/Implementation/Fetch.hpp @@ -475,7 +475,7 @@ struct CharacterSequencer { break; case 3: character_fetcher.fetch_pattern(block); - character_fetcher.fetch_pattern(block); + character_fetcher.fetch_colour(block); break; default: break; } From 131784d0076cd065045b82f37d1035d2bbcbdc41 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 13 Mar 2023 22:51:01 -0400 Subject: [PATCH 8/9] Generalise PointSet to read or write. --- Components/9918/Implementation/9918.cpp | 24 ++++++++++--------- Components/9918/Implementation/9918Base.hpp | 7 ++++++ Components/9918/Implementation/Storage.hpp | 9 +++++++ .../9918/Implementation/YamahaCommands.hpp | 12 +++++----- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/Components/9918/Implementation/9918.cpp b/Components/9918/Implementation/9918.cpp index 4368f7647..1153467ff 100644 --- a/Components/9918/Implementation/9918.cpp +++ b/Components/9918/Implementation/9918.cpp @@ -924,20 +924,20 @@ void Base::commit_register(int reg, uint8_t value) { default: case 0b0000: Storage::command_ = nullptr; break; // STOP. - case 0b0100: break; // TODO: point. [read a pixel colour] - case 0b0101: Begin(PointSet); break; // PSET [plot a pixel]. + case 0b0100: Begin(Point); break; // POINT [read a pixel colour]. + case 0b0101: Begin(Point); break; // PSET [plot a pixel]. case 0b0110: break; // TODO: srch. [search horizontally for a colour] - case 0b0111: Begin(Line); break; // LINE [draw a Bresenham line]. + case 0b0111: Begin(Line); break; // LINE [draw a Bresenham line]. - case 0b1000: Begin(LogicalFill); break; // LMMV [logical move, VDP to VRAM, i.e. solid-colour fill]. - case 0b1001: Begin(Move); break; // LMMM [logical move, VRAM to VRAM]. + case 0b1000: Begin(LogicalFill); break; // LMMV [logical move, VDP to VRAM, i.e. solid-colour fill]. + case 0b1001: Begin(Move); break; // LMMM [logical move, VRAM to VRAM]. case 0b1010: break; // TODO: lmcm. [logical move, VRAM to CPU] - case 0b1011: Begin(MoveFromCPU); break; // LMMC [logical move, CPU to VRAM]. + case 0b1011: Begin(MoveFromCPU); break; // LMMC [logical move, CPU to VRAM]. - case 0b1100: Begin(HighSpeedFill); break; // HMMV [high-speed move, VDP to VRAM, i.e. single-byte fill]. - case 0b1101: Begin(Move); break; // HMMM [high-speed move, VRAM to VRAM]. - case 0b1110: Begin(Move); break; // YMMM [high-speed move, y only, VRAM to VRAM]. - case 0b1111: Begin(MoveFromCPU); break; // HMMC [high-speed move, CPU to VRAM]. + case 0b1100: Begin(HighSpeedFill); break; // HMMV [high-speed move, VDP to VRAM, i.e. single-byte fill]. + case 0b1101: Begin(Move); break; // HMMM [high-speed move, VRAM to VRAM]. + case 0b1110: Begin(Move); break; // YMMM [high-speed move, y only, VRAM to VRAM]. + case 0b1111: Begin(MoveFromCPU); break; // HMMC [high-speed move, CPU to VRAM]. } #undef Begin @@ -1113,7 +1113,9 @@ uint8_t Base::read_register() { case 4: LOG("TODO: Yamaha status 4"); break; case 5: LOG("TODO: Yamaha status 5"); break; case 6: LOG("TODO: Yamaha status 6"); break; - case 7: LOG("TODO: Yamaha status 7"); break; + + case 7: return Storage::colour_status_; + case 8: LOG("TODO: Yamaha status 8"); break; case 9: LOG("TODO: Yamaha status 9"); break; } diff --git a/Components/9918/Implementation/9918Base.hpp b/Components/9918/Implementation/9918Base.hpp index 88983a646..bdf698b9d 100644 --- a/Components/9918/Implementation/9918Base.hpp +++ b/Components/9918/Implementation/9918Base.hpp @@ -390,6 +390,13 @@ template struct Base: public Storage { case CommandStep::None: break; + case CommandStep::CopySourcePixelToStatus: + Storage::colour_status_ = extract_colour(source[command_address(context.source, context.arguments & 0x10)], context.source); + + Storage::command_->advance(pixels_per_byte(this->underlying_mode_)); + Storage::update_command_step(access_column); + break; + case CommandStep::ReadSourcePixel: context.latched_colour.set(extract_colour(source[command_address(context.source, context.arguments & 0x10)], context.source)); diff --git a/Components/9918/Implementation/Storage.hpp b/Components/9918/Implementation/Storage.hpp index f83d8acac..f88d2eb70 100644 --- a/Components/9918/Implementation/Storage.hpp +++ b/Components/9918/Implementation/Storage.hpp @@ -100,6 +100,9 @@ template struct Storage struct Storage struct Storagecycles; switch(command_->access) { + case Command::AccessType::ReadPoint: + next_command_step_ = CommandStep::CopySourcePixelToStatus; + break; + case Command::AccessType::CopyPoint: next_command_step_ = CommandStep::ReadSourcePixel; break; diff --git a/Components/9918/Implementation/YamahaCommands.hpp b/Components/9918/Implementation/YamahaCommands.hpp index 6e27d67bf..7545180cb 100644 --- a/Components/9918/Implementation/YamahaCommands.hpp +++ b/Components/9918/Implementation/YamahaCommands.hpp @@ -111,7 +111,9 @@ struct Command { /// being a read, a 24-cycle gap, then a write. CopyByte, -// ReadPoint, + /// Copies a single pixel from @c source to the colour status register. + ReadPoint, + // ReadByte, // WaitForColourSend, }; @@ -211,11 +213,11 @@ struct Line: public Command { /// Implements the PSET command, which plots a single pixel. /// /// No timings are documented, so this'll output as quickly as possible. -struct PointSet: public Command { +template struct Point: public Command { public: - PointSet(CommandContext &context) : Command(context) { + Point(CommandContext &context) : Command(context) { cycles = 0; // TODO. - access = AccessType::PlotPoint; + access = is_read ? AccessType::ReadPoint : AccessType::PlotPoint; } bool done() final { @@ -230,8 +232,6 @@ struct PointSet: public Command { bool done_ = false; }; -// TODO: point. - // MARK: - Rectangular base. /// Useful base class for anything that does logical work in a rectangle. From f26dee16bf40ef24898f1055de56a2f6980a1621 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 13 Mar 2023 23:21:19 -0400 Subject: [PATCH 9/9] Update comment. --- Components/9918/Implementation/YamahaCommands.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Components/9918/Implementation/YamahaCommands.hpp b/Components/9918/Implementation/YamahaCommands.hpp index 7545180cb..dea62a229 100644 --- a/Components/9918/Implementation/YamahaCommands.hpp +++ b/Components/9918/Implementation/YamahaCommands.hpp @@ -210,9 +210,9 @@ struct Line: public Command { // MARK: - Single pixel manipulation. -/// Implements the PSET command, which plots a single pixel. +/// Implements the PSET command, which plots a single pixel and POINT, which reads one. /// -/// No timings are documented, so this'll output as quickly as possible. +/// No timings are documented, so this'll output or input as quickly as possible. template struct Point: public Command { public: Point(CommandContext &context) : Command(context) {