From bb54ac14b88fcfcdcf6c9b658cdd79d30e034a93 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Mon, 15 Aug 2022 11:10:17 -0400
Subject: [PATCH] Prove that new output errors are [probably] external to the
 Blitter.

---
 Machines/Amiga/Blitter.cpp                         | 14 ++++++++++++++
 Machines/Amiga/Blitter.hpp                         |  2 +-
 Machines/Amiga/Chipset.cpp                         |  7 ++++++-
 .../Mac/Clock SignalTests/AmigaBlitterTests.mm     |  4 ++--
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/Machines/Amiga/Blitter.cpp b/Machines/Amiga/Blitter.cpp
index e15cc4289..c4cd54a23 100644
--- a/Machines/Amiga/Blitter.cpp
+++ b/Machines/Amiga/Blitter.cpp
@@ -234,9 +234,19 @@ void Blitter<record_bus>::add_modulos() {
 }
 
 template <bool record_bus>
+template <bool complete_immediately>
 bool Blitter<record_bus>::advance_dma() {
 	if(!height_) return false;
 
+	// TODO: eliminate @c complete_immediately and this workaround.
+	// See commentary in Chipset.cpp.
+	if constexpr (complete_immediately) {
+		while(get_status() & 0x4000) {
+			advance_dma<false>();
+		}
+		return true;
+	}
+
 	if(line_mode_) {
 		not_zero_flag_ = false;
 
@@ -551,3 +561,7 @@ std::vector<typename Blitter<record_bus>::Transaction> Blitter<record_bus>::get_
 
 template class Amiga::Blitter<false>;
 template class Amiga::Blitter<true>;
+template bool Amiga::Blitter<true>::advance_dma<true>();
+template bool Amiga::Blitter<true>::advance_dma<false>();
+template bool Amiga::Blitter<false>::advance_dma<true>();
+template bool Amiga::Blitter<false>::advance_dma<false>();
diff --git a/Machines/Amiga/Blitter.hpp b/Machines/Amiga/Blitter.hpp
index 720202004..256137ae6 100644
--- a/Machines/Amiga/Blitter.hpp
+++ b/Machines/Amiga/Blitter.hpp
@@ -192,7 +192,7 @@ template <bool record_bus = false> class Blitter: public DMADevice<4, 4> {
 
 		uint16_t get_status();
 
-		bool advance_dma();
+		template <bool complete_immediately> bool advance_dma();
 
 		struct Transaction {
 			enum class Type {
diff --git a/Machines/Amiga/Chipset.cpp b/Machines/Amiga/Chipset.cpp
index 3a6693543..c8accc853 100644
--- a/Machines/Amiga/Chipset.cpp
+++ b/Machines/Amiga/Chipset.cpp
@@ -654,8 +654,13 @@ template <int cycle, bool stop_if_cpu> bool Chipset::perform_cycle() {
 	}
 
 	// Give first refusal to the Blitter (if enabled), otherwise pass on to the CPU.
+	//
+	// TODO: determine why I see Blitter issues if I don't allow it to complete immediately.
+	// All tests pass without immediate completion, and immediate completion just runs the
+	// non-immediate version until the busy flag is disabled. So probably a scheduling or
+	// signalling issue out here.
 	constexpr auto BlitterEnabled = DMAFlag::AllBelow | DMAFlag::Blitter;
-	return (dma_control_ & BlitterEnabled) != BlitterEnabled || !blitter_.advance_dma();
+	return (dma_control_ & BlitterEnabled) != BlitterEnabled || !blitter_.advance_dma<true>();
 }
 
 /// Performs all slots starting with @c first_slot and ending just before @c last_slot.
diff --git a/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm b/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm
index bb744507b..9e0dfed68 100644
--- a/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm	
+++ b/OSBindings/Mac/Clock SignalTests/AmigaBlitterTests.mm	
@@ -60,7 +60,7 @@ struct Chipset {
 			// Ensure all blitting is completed between register writes; none of the tests
 			// in this test set are about illegal usage.
 			while(blitter.get_status() & 0x4000) {
-				blitter.advance_dma();
+				blitter.advance_dma<false>();
 			}
 
 			const auto transactions = blitter.get_and_reset_transactions();
@@ -190,7 +190,7 @@ struct Chipset {
 				return;
 			}
 
-			blitter.advance_dma();
+			blitter.advance_dma<false>();
 
 			const auto transactions = blitter.get_and_reset_transactions();
 			if(transactions.empty()) {