Merge pull request #1748 from TomHarte/DriveSpeed

Investigate errors in drive speed.

Machines/Thomson/MO5/CD90-640.cpp

@@ -14,7 +14,94 @@ CD90_640::CD90_640() : WD::WD1770(P1770) {
 	// 325 is a peculiar RPM, but seems to match a spin-up test in the disk ROM that polls the WD1770's status register
 	// index hole bit and counts time. Furthermore there are other machines with unusual RPMs. Could definitely
 	// still just imply an issue elsewhere in the emulator though.
-	emplace_drives(2, 8000000, 325, 2);
+	emplace_drives(2, 8'000'000, 325, 2);
+
+	//
+	// On "325 RPM":
+	//
+	// Faulty reasoning was:
+	// ---------------------
+	//
+	// The DOS ROM verifies that a disk is present and spinning before proceeding.
+	// Part of that test is of spin speed.
+	//
+	// Specifically, with X = 0xa7d0 it performs:
+	//
+	//		;
+	//		; Wait for index hole set.
+	//		;
+	//		a29b	LDA     ,X			; 4 µs
+	//		a29d	ANDA    #$02		; 2 µs
+	//		a29f	BEQ     ZA29B		; 3 µs
+	//
+	//		;
+	//		; i.e. start of track falls about here
+	//		;
+	//
+	//		;
+	//		; Fixed delay...
+	//		;
+	//		a2a1	LDY     #M09C4		; 4 µs
+	//		a2a5	DEY					; 5 µs
+	//		a2a7	BNE     ZA2A5		; 3 µs	= net delay of 4 + 9C4*7 = 17,504 µs
+	//
+	//		a2a9	PSHS    CC			; 6 µs
+	//		a2ab	SEIF				; 3 µs
+	//
+	//		;
+	//		; Count time until index hole is set again
+	//		;
+	//		a2ad	LDA     ,X			; 4 µs, read WD status
+	//		a2af	INY					; 5 µs
+	//		a2b1	ANDA    #$02		; 2 µs
+	//		a2b3	BEQ     ZA2AD		; 3 µs
+	//
+	//		a2b5	PULS    CC
+	//
+	//		;
+	//		; Check whether rotation speed was within bounds
+	//		;
+	//		a2b7	CMPY    #M311B
+	//		a2bb	BMI     ZA2C7
+	//		a2bd	CMPY    #M3357
+	//		a2c1	BPL     ZA2C7
+	//
+	//		[test passed code here, test failed code from a2c7]
+	//
+	// If I've disassembled that correctly, then the process is:
+	//
+	//	(1) wait for index hole, with up to 9 µs of latency;
+	//	(2) spin in a loop for 17,504 µs;
+	//	(3) do minor stack/flag work for 9 µs;
+	//	(4) count time from here until next index hole, ending with Y = time/13;
+	//	(5) test that that is within the bounds 0x311b, 0x3357.
+	//
+	// Ignoring minor potential latencies in loop exits, that's:
+	//
+	//	(1) wait for index hole;
+	//	(2) spend 17,513 µs doing something else;
+	//	(3) get Y = [µs from there to next index hole] / 13.
+	//
+	// i.e. it's a test that the rotation it samples takes n µs, where:
+	//
+	//	14 * 0x311b < n - 17,513 < 14 * 0x3357
+	//	14 * 12,571 + 20,009 < n < 14 * 13,143 + 20,009
+	//	196,003 < n < 204,011
+	//
+	// i.e.
+	//
+	//	~331 >= RPM >= ~319 RPM.
+	//
+	// So 325 RPM is a really weird number but I can't currently say why it should be
+	// wrong rather than merely unexpected. So here it is.
+	//
+	// Correct reasoning is:
+	// ---------------------
+	//
+	//	The LEAYs that constitute INY and DEY above should be _five_ cycles long, not four.
+	//	My 6809 timing is adrift.
+	//	If loops were properly timed then 300 RPM looks appropriate.
+	//	Cf. https://forum.system-cfg.com/viewtopic.php?f=1&t=17137
 }
 
 uint8_t CD90_640::control() {

Storage/Disk/Drive.cpp

@@ -33,7 +33,7 @@ Drive::Drive(
 
 	// Get at least 64 bits of random information; rounding is likey to give this a slight bias.
 	random_source_ = 0;
-	auto half_range = (randomiser.max() - randomiser.min()) / 2;
+	const auto half_range = (randomiser.max() - randomiser.min()) / 2;
 	for(int bit = 0; bit < 64; ++bit) {
 		random_source_ <<= 1;
 		random_source_ |= ((randomiser() - randomiser.min()) >= half_range) ? 1 : 0;
@@ -233,11 +233,11 @@ void Drive::run_for(const Cycles cycles) {
 
 	if(disk_is_rotating_) {
 		if(has_disk_) {
-			Time zero(0);
+			static constexpr Time zero(0);
 
 			auto number_of_cycles = cycles.get();
 			while(number_of_cycles) {
-				auto cycles_until_next_event = get_cycles_until_next_event();
+				const auto cycles_until_next_event = get_cycles_until_next_event();
 				auto cycles_to_run_for = std::min(cycles_until_next_event, number_of_cycles);
 				if(!is_reading_ && cycles_until_bits_written_ > zero) {
 					auto write_cycles_target = cycles_until_bits_written_.get<Cycles::IntType>();
@@ -248,7 +248,7 @@ void Drive::run_for(const Cycles cycles) {
 				number_of_cycles -= cycles_to_run_for;
 				if(!is_reading_) {
 					if(cycles_until_bits_written_ > zero) {
-						Storage::Time cycles_to_run_for_time(static_cast<int>(cycles_to_run_for));
+						const Storage::Time cycles_to_run_for_time(static_cast<int>(cycles_to_run_for));
 						if(cycles_until_bits_written_ <= cycles_to_run_for_time) {
 							cycles_until_bits_written_.set_zero();
 							if(event_delegate_) {
@@ -347,7 +347,10 @@ void Drive::get_next_event(const float duration_already_passed) {
 void Drive::process_next_event() {
 	if(current_event_.type == Track::Event::IndexHole) {
 		++ready_index_count_;
-		if(ready_index_count_ == 2 && (ready_type_ == ReadyType::ShugartRDY || ready_type_ == ReadyType::ShugartModifiedRDY)) {
+		if(
+			ready_index_count_ == 2 &&
+			(ready_type_ == ReadyType::ShugartRDY || ready_type_ == ReadyType::ShugartModifiedRDY)
+		) {
 			is_ready_ = true;
 		}
 		cycles_since_index_hole_ = 0;

Storage/TimedEventLoop.cpp

@@ -14,7 +14,7 @@
 
 using namespace Storage;
 
-TimedEventLoop::TimedEventLoop(Cycles::IntType input_clock_rate) :
+TimedEventLoop::TimedEventLoop(const Cycles::IntType input_clock_rate) :
 	input_clock_rate_(input_clock_rate) {}
 
 void TimedEventLoop::add_delay(const Cycles cycles) {
@@ -73,24 +73,36 @@ void TimedEventLoop::jump_to_next_event() {
 	process_next_event();
 }
 
-void TimedEventLoop::set_next_event_time_interval(Time interval) {
+void TimedEventLoop::set_next_event_time_interval(const Time interval) {
 	set_next_event_time_interval(interval.get<float>());
 }
 
-void TimedEventLoop::set_next_event_time_interval(float interval) {
+void TimedEventLoop::set_next_event_time_interval(const float interval) {
 	// Calculate [interval]*[input clock rate] + [subcycles until this event]
-	const float float_interval = interval * float(input_clock_rate_) + subcycles_until_event_;
+	const float cycles = interval * float(input_clock_rate_) + subcycles_until_event_;
 
 	// This event will fire in the integral number of cycles from now, putting us at the remainder
 	// number of subcycles.
-	const Cycles::IntType addition = Cycles::IntType(float_interval);
+	const Cycles::IntType addition = Cycles::IntType(cycles);
 	cycles_until_event_ += addition;
-	subcycles_until_event_ = fmodf(float_interval, 1.0f);
+	subcycles_until_event_ = std::fmod(cycles, 1.0f);
+
+#ifndef NDEBUG
+	cycles_total_ += addition;
+#endif
 
 	assert(cycles_until_event_ >= 0);
 	assert(subcycles_until_event_ >= 0.0f);
 }
 
+#ifndef NDEBUG
+float TimedEventLoop::get_reset_cycles_total() {
+	const auto result = cycles_total_;
+	cycles_total_ = 0;
+	return result;
+}
+#endif
+
 Time TimedEventLoop::get_time_into_next_event() {
 	// TODO: calculate, presumably as [length of interval] - ([cycles left] + [subcycles left])
 	Time zero;

Storage/TimedEventLoop.hpp

@@ -111,12 +111,17 @@ protected:
 	*/
 	Time get_time_into_next_event();
 
+#ifndef NDEBUG
+	float get_reset_cycles_total();
+#endif
+
 private:
 	Cycles::IntType input_clock_rate_ = 0;
 	Cycles::IntType cycles_until_event_ = 0;
 	float subcycles_until_event_ = 0.0f;
 #ifndef NDEBUG
 	int event_count_ = 0;
+	float cycles_total_ = 0.0f;
 #endif
 };