From c10cabd2b3daa5c1c0bebea74e38d66a9551eaee Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 May 2026 10:08:07 -0400 Subject: [PATCH 1/5] Start implementing SAM address calculation. --- Components/6847/6847.hpp | 17 ++++++- Machines/Tandy/CoCo/CoCo.cpp | 93 ++++++++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 20 deletions(-) diff --git a/Components/6847/6847.hpp b/Components/6847/6847.hpp index 100fa0fa6..233f26a38 100644 --- a/Components/6847/6847.hpp +++ b/Components/6847/6847.hpp @@ -264,7 +264,22 @@ void MC6847::run_for(const Cycles ); pixel_line(begin, end); - if(end >= LineLayout::EndOfSync) delegate_.template set_row_preset(); + // Extra accesses required: ten more in 32-byte video modes, and six more in 16-byte modes. + // That's 80 to 96 more cycles of fetches? + // + // If fetching continued until end of sync then that'd be 354 cycles of fetching = 44.25 columns. + // I'm unclear how to make a meaningful case for 10 vs 6 though. Might need to hack this? + // + // static const int EndOfSync = 28; + // static const int EndOfColourBurst = 55; + // static const int EndOfBackPorch = 72; + // static const int EndOfLeftBorder = 130; + // static const int EndOfPixels = 386; + // static const int EndOfRightBorder = 442; + // static const int EndOfLine = 456; + + + if(end >= LineLayout::EndOfSync && !address_.row()) delegate_.template set_row_preset(); } else if(line < FrameLayout::EndOfBottomBorder) { if(!begin && line == FrameLayout::EndOfPixels) { delegate_.template set_field_sync(); diff --git a/Machines/Tandy/CoCo/CoCo.cpp b/Machines/Tandy/CoCo/CoCo.cpp index 7ff15e05b..15458d125 100644 --- a/Machines/Tandy/CoCo/CoCo.cpp +++ b/Machines/Tandy/CoCo/CoCo.cpp @@ -580,14 +580,52 @@ private: SAM(MemoryMap &memory) : memory_(memory) {} uint8_t operator[](const uint16_t address) { - // TODO: this whole thing is phoney; all I'm doing is taking the full 6847-generated - // address and adding an offset. The real SAM does a minimal reimplementation of 6847-style - // address counting. Do that. - // - // This is very much bootstrapping stuff. - // - // TODO: this shouldn't be able to go into ROM, possibly? - return memory_.read(address + graphics_address_); + b1b3_ += ((previous_6847_address_ ^ address) & previous_6847_address_ & 1) << 1; + previous_6847_address_ = address; + + x_ += b1b3_ >> 4; + b1b3_ &= 0b0'1110; + + if(x_ == x_divider_) { + x_ = 0; + + b4_ += 0b1'0000; + y_ += b4_ >> 5; + b4_ &= 0b1'0000; + + if(y_ == y_divider_) { + y_ = 0; + b5b15_ += 0b10'0000; + } + } + + // TODO: determine whether this should be able to read ROM. +// printf("%04x -> %04x\n", address, (address & 1) | b1b3_ | b4_ | b5b15_); + return memory_.read((address & 1) | b1b3_ | b4_ | b5b15_); + } + template void set_hsync() { + if(active) { + b1b3_ &= clear_mask_; + b4_ &= clear_mask_; +// previous_6847_address_ = 0; // TODO: probably not appropriate, might be because I'm not doing border fetches? +// printf("HS\n"); + } + } + template void set_field_sync() { + if(!active) { + b5b15_ = graphics_address_; + b4_ = 0; + b1b3_ = 0; + previous_6847_address_ = 0; +// x_ = y_ = 0; +// printf("FS\n"); + } + } + template void set_row_preset() { + if(active) { + x_ = y_ = 0; +// printf("RP\n"); + } } template void access() { @@ -685,14 +723,6 @@ private: return graphics_address_; } - template void set_hsync() {} - template void set_field_sync() { - if(!active) { - address_counter_ = graphics_address_; - } - } - template void set_row_preset() {} - private: MemoryMap &memory_; @@ -710,9 +740,34 @@ private: bool page1_ = false; int ram_size_ = 0; - uint16_t address_counter_; - uint16_t clear_mask_; - int x_divider_, y_divider_; + // 'X' and 'Y' are the data sheet names for these values, which is unfortunate given that they're involved + // in data fetch but don't correlate with dimensions. + // + // Requirements: + // + // The low bit of the address coming from the 6847 acts as the low bit of the address fetched. + // It also clocks counting. + // + // Above that is a three-bit counter, which it clocks. + // + // That three bit counter feeds into the 'x' divider. So if the x divider is 3 then that counter must overflow + // three times in order to propagate carry upwards. + // + // Carry from that counter is fed into a one-bit counter. + // + // That one bit counter then clocks the 'y' divider, carry from which clocks the remaining 11 bits of output. + // + // At field sync the top seven bits of the counter are reloaded from the programmed base address. + // At horizontal sync the programmed mask is applied to the counters. + // Upon row preset, both x and y dividers are reset to 0. + // + // Observations: the hsync clearing mask never affects the top 11 bits — the portiona after the y divider. + + uint16_t b1b3_, x_, b4_, y_, b5b15_; + uint16_t x_divider_ = 1, y_divider_ = 1; + uint16_t clear_mask_ = 0xffff; + + uint16_t previous_6847_address_ = 0; }; SAM sam_; From 47104dbf86dd2bb1229ecdfe15d72b6773ac20e7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 May 2026 10:42:29 -0400 Subject: [PATCH 2/5] Add border fetches, rearrange SAM logic. --- Components/6847/6847.hpp | 16 ++++++++++++++++ Machines/Tandy/CoCo/CoCo.cpp | 16 +++++++++------- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/Components/6847/6847.hpp b/Components/6847/6847.hpp index 233f26a38..568d94422 100644 --- a/Components/6847/6847.hpp +++ b/Components/6847/6847.hpp @@ -247,6 +247,7 @@ void MC6847::run_for(const Cycles if(line < FrameLayout::EndOfPixels) { if(!begin && !address_.row()) delegate_.template set_row_preset(); + // Perform meaningful fetches. Numeric::clamp( begin, end, @@ -262,6 +263,21 @@ void MC6847::run_for(const Cycles } } ); + + // Fetches actually continue until end of line, but they're not used. + Numeric::clamp( + begin, + end, + [&](const int fetch_begin, const int fetch_end) { + const int column_begin = (fetch_begin - LineLayout::EndOfLeftBorder) >> 3; + const int column_end = (fetch_end - LineLayout::EndOfLeftBorder) >> 3; + + for(int c = column_begin; c < column_end; c++) { + const auto source = address_.address() + c; + (void)memory_[source]; + } + } + ); pixel_line(begin, end); // Extra accesses required: ten more in 32-byte video modes, and six more in 16-byte modes. diff --git a/Machines/Tandy/CoCo/CoCo.cpp b/Machines/Tandy/CoCo/CoCo.cpp index 15458d125..1a2650b40 100644 --- a/Machines/Tandy/CoCo/CoCo.cpp +++ b/Machines/Tandy/CoCo/CoCo.cpp @@ -24,6 +24,8 @@ #include "Outputs/Speaker/Implementation/LowpassSpeaker.hpp" #include "Outputs/Speaker/Implementation/BufferSource.hpp" +#include + using namespace Tandy::CoCo; namespace { @@ -600,15 +602,13 @@ private: } // TODO: determine whether this should be able to read ROM. -// printf("%04x -> %04x\n", address, (address & 1) | b1b3_ | b4_ | b5b15_); return memory_.read((address & 1) | b1b3_ | b4_ | b5b15_); } template void set_hsync() { if(active) { b1b3_ &= clear_mask_; b4_ &= clear_mask_; -// previous_6847_address_ = 0; // TODO: probably not appropriate, might be because I'm not doing border fetches? -// printf("HS\n"); + previous_6847_address_ = 0; } } template void set_field_sync() { @@ -617,14 +617,16 @@ private: b4_ = 0; b1b3_ = 0; previous_6847_address_ = 0; -// x_ = y_ = 0; -// printf("FS\n"); + + // TODO: these should be in row preset per my understanding, but I'm not sure that's + // signalling correctly. It is definitely true that some titles put the 6847 into one vertical height + // and the SAM into a different one, expecting the SAM's setting to win. + x_ = y_ = 0; } } template void set_row_preset() { if(active) { - x_ = y_ = 0; -// printf("RP\n"); +// x_ = y_ = 0; } } From 7a7bd434992781822bdc01419df98707c6781868 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 May 2026 10:51:17 -0400 Subject: [PATCH 3/5] Reset row if exiting graphics mode. --- Components/6847/6847.cpp | 9 +++++++++ Components/6847/6847.hpp | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Components/6847/6847.cpp b/Components/6847/6847.cpp index d765cd44a..e582ba72c 100644 --- a/Components/6847/6847.cpp +++ b/Components/6847/6847.cpp @@ -353,6 +353,10 @@ void MC6847Base::Address::apply_hsync() { address_ &= line_mask_; } +void MC6847Base::Address::reset_row() { + row_ = 0; +} + void MC6847Base::set_mode( const bool graphics, const bool semigraphics, @@ -361,6 +365,11 @@ void MC6847Base::set_mode( const uint8_t graphics_mode, const bool colour_select ) { + // "the font row is reset each time the VDG is switched from graphics to text mode". + if((mode_ & Mode::Graphics) && !graphics) { + address_.reset_row(); + } + mode_ = (graphics ? Mode::Graphics : 0) | (semigraphics ? Mode::Semigraphics : 0) | diff --git a/Components/6847/6847.hpp b/Components/6847/6847.hpp index 568d94422..8b8f49fbb 100644 --- a/Components/6847/6847.hpp +++ b/Components/6847/6847.hpp @@ -168,6 +168,7 @@ struct MC6847Base { void advance(int column); void apply_vertical_preload(); void apply_hsync(); + void reset_row(); uint16_t increment_mask_ = 1; uint16_t line_mask_ = uint16_t(~31); @@ -273,7 +274,7 @@ void MC6847::run_for(const Cycles const int column_end = (fetch_end - LineLayout::EndOfLeftBorder) >> 3; for(int c = column_begin; c < column_end; c++) { - const auto source = address_.address() + c; + const auto source = uint16_t(address_.address() + c); (void)memory_[source]; } } From 85662b026812c1d6869119e2df0c5a863abc2ed2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 May 2026 13:14:37 -0400 Subject: [PATCH 4/5] Ensure ranges are non-overlapping. --- Components/6847/6847.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Components/6847/6847.hpp b/Components/6847/6847.hpp index 8b8f49fbb..d29b23ad2 100644 --- a/Components/6847/6847.hpp +++ b/Components/6847/6847.hpp @@ -266,7 +266,7 @@ void MC6847::run_for(const Cycles ); // Fetches actually continue until end of line, but they're not used. - Numeric::clamp( + Numeric::clamp( begin, end, [&](const int fetch_begin, const int fetch_end) { From 2f7146d0d7919246ff120d5e9d1b0ecff0e8dabd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 May 2026 13:25:25 -0400 Subject: [PATCH 5/5] Switch PIA input to field sync. --- Components/6847/6847.hpp | 6 +++--- Machines/Tandy/CoCo/CoCo.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Components/6847/6847.hpp b/Components/6847/6847.hpp index d29b23ad2..c4d12d0bb 100644 --- a/Components/6847/6847.hpp +++ b/Components/6847/6847.hpp @@ -208,7 +208,7 @@ public: Cycles next_sequence_point() const; bool hsync() const; - bool vsync() const; + bool fsync() const; using MC6847Base::set_mode; // MARK: - ScanTarget entrypoints. @@ -334,9 +334,9 @@ bool MC6847::hsync() const { } template -bool MC6847::vsync() const { +bool MC6847::fsync() const { const auto segment = position_.segment(); - return segment >= FrameLayout::EndOfFrontPorch && segment < FrameLayout::EndOfSync; + return segment >= FrameLayout::EndOfPixels && segment < FrameLayout::EndOfSync; } template diff --git a/Machines/Tandy/CoCo/CoCo.cpp b/Machines/Tandy/CoCo/CoCo.cpp index 1a2650b40..0e7e9e4d0 100644 --- a/Machines/Tandy/CoCo/CoCo.cpp +++ b/Machines/Tandy/CoCo/CoCo.cpp @@ -179,7 +179,7 @@ public: const auto duration = delay + CPU::M6809::duration(bus_phase); if(m6847_ += duration) { pia0_.set(m6847_.get()->hsync()); - pia0_.set(m6847_.get()->vsync()); + pia0_.set(m6847_.get()->fsync()); } tape_player_.run_for(duration); if(typer_) { @@ -374,7 +374,7 @@ private: // 0 and 2 = right joystick; 1 and 3 = left joystick; // 0 and 1 = switch 1; 2 and 3 = switch 2. // - // CA1: hsync + // CA1: horizontal sync // CA2: select line LSB of joystick MUX // @@ -382,7 +382,7 @@ private: // // b0-b7: keyboard column [output?] // - // CB1: vsync + // CB1: field sync // CB2: select line MSB of joystick MUX // Interrupt outputs both connected to IRQ.