Merge pull request #1816 from TomHarte/SAMCounting

Implement SAM address calculation.

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) |

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);
@@ -207,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.
@@ -247,6 +248,7 @@ void MC6847<timing, MemoryAccessT, DelegateT, ModeMapperT>::run_for(const Cycles
 			if(line < FrameLayout<timing>::EndOfPixels) {
 				if(!begin && !address_.row()) delegate_.template set_row_preset<true>();
 
+				// Perform meaningful fetches.
 				Numeric::clamp<LineLayout::EndOfLeftBorder, LineLayout::EndOfPixels>(
 					begin,
 					end,
@@ -262,9 +264,39 @@ void MC6847<timing, MemoryAccessT, DelegateT, ModeMapperT>::run_for(const Cycles
 						}
 					}
 				);
+
+				// Fetches actually continue until end of line, but they're not used.
+				Numeric::clamp<LineLayout::EndOfPixels, LineLayout::EndOfLine>(
+					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 = uint16_t(address_.address() + c);
+							(void)memory_[source];
+						}
+					}
+				);
 				pixel_line(begin, end);
 
-				if(end >= LineLayout::EndOfSync) delegate_.template set_row_preset<false>();
+				// 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<false>();
 			} else if(line < FrameLayout<timing>::EndOfBottomBorder) {
 				if(!begin && line == FrameLayout<timing>::EndOfPixels) {
 					delegate_.template set_field_sync<true>();
@@ -302,9 +334,9 @@ bool MC6847<timing, MemoryAccessT, DelegateT, ModeMapperT>::hsync() const {
 }
 
 template <FrameTiming timing, typename MemoryAccessT, typename DelegateT, typename ModeMapperT>
-bool MC6847<timing, MemoryAccessT, DelegateT, ModeMapperT>::vsync() const {
+bool MC6847<timing, MemoryAccessT, DelegateT, ModeMapperT>::fsync() const {
 	const auto segment = position_.segment();
-	return segment >= FrameLayout<timing>::EndOfFrontPorch && segment < FrameLayout<timing>::EndOfSync;
+	return segment >= FrameLayout<timing>::EndOfPixels && segment < FrameLayout<timing>::EndOfSync;
 }
 
 template <FrameTiming timing, typename MemoryAccessT, typename DelegateT, typename ModeMapperT>

Machines/Tandy/CoCo/CoCo.cpp

@@ -24,6 +24,8 @@
 #include "Outputs/Speaker/Implementation/LowpassSpeaker.hpp"
 #include "Outputs/Speaker/Implementation/BufferSource.hpp"
 
+#include <cassert>
+
 using namespace Tandy::CoCo;
 
 namespace {
@@ -177,7 +179,7 @@ public:
 		const auto duration = delay + CPU::M6809::duration<Cycles>(bus_phase);
 		if(m6847_ += duration) {
 			pia0_.set<Motorola::MC6821::Control::CA1>(m6847_.get()->hsync());
-			pia0_.set<Motorola::MC6821::Control::CB1>(m6847_.get()->vsync());
+			pia0_.set<Motorola::MC6821::Control::CB1>(m6847_.get()->fsync());
 		}
 		tape_player_.run_for(duration);
 		if(typer_) {
@@ -372,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
 
 		//
@@ -380,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.
@@ -580,14 +582,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.
+			return memory_.read((address & 1) | b1b3_ | b4_ | b5b15_);
+		}
+		template <bool active> void set_hsync() {
+			if(active) {
+				b1b3_ &= clear_mask_;
+				b4_ &= clear_mask_;
+				previous_6847_address_ = 0;
+			}
+		}
+		template <bool active> void set_field_sync() {
+			if(!active) {
+				b5b15_ = graphics_address_;
+				b4_ = 0;
+				b1b3_ = 0;
+				previous_6847_address_ = 0;
+
+				// 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 <bool active> void set_row_preset() {
+			if(active) {
+//				x_ = y_ = 0;
+			}
 		}
 
 		template <uint16_t address> void access() {
@@ -685,14 +725,6 @@ private:
 			return graphics_address_;
 		}
 
-		template <bool active> void set_hsync() {}
-		template <bool active> void set_field_sync() {
-			if(!active) {
-				address_counter_ = graphics_address_;
-			}
-		}
-		template <bool active> void set_row_preset() {}
-
 	private:
 		MemoryMap &memory_;
 
@@ -710,9 +742,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_;