Takes another stab at MFP interrupt management.

Components/68901/MFP68901.cpp

@@ -8,6 +8,8 @@
 
 #include "MFP68901.hpp"
 
+#include <cstring>
+
 #define LOG_PREFIX "[MFP] "
 //#define NDEBUG
 #include "../../Outputs/Log.hpp"
@@ -87,27 +89,31 @@ void MFP68901::write(int address, uint8_t value) {
 		case 0x03:
 			LOG("Write: interrupt enable A " << PADHEX(2) << int(value));
 			interrupt_enable_ = (interrupt_enable_ & 0x00ff) | (value << 8);
-			update_interrupts();
 		break;
 		case 0x04:
 			LOG("Write: interrupt enable B " << PADHEX(2) << int(value));
 			interrupt_enable_ = (interrupt_enable_ & 0xff00) | value;
-			update_interrupts();
 		break;
 		case 0x05:
 			LOG("Write: interrupt pending A " << PADHEX(2) << int(value));
-			interrupt_pending_ = (interrupt_pending_ & 0x00ff) | (value << 8);
+			interrupt_pending_ &= 0x00ff | (value << 8);
-			interrupt_in_service_ &= 0x00ff | (value << 8);
 			update_interrupts();
 		break;
 		case 0x06:
 			LOG("Write: interrupt pending B " << PADHEX(2) << int(value));
-			interrupt_pending_ = (interrupt_pending_ & 0xff00) | value;
+			interrupt_pending_ &= 0xff00 | value;
+			update_interrupts();
+		break;
+		case 0x07:
+			LOG("Write: interrupt in-service A (no-op?) " << PADHEX(2) << int(value));
+			interrupt_in_service_ &= 0x00ff | (value << 8);
+			update_interrupts();
+		break;
+		case 0x08:
+			LOG("Write: interrupt in-service B (no-op?) " << PADHEX(2) << int(value));
 			interrupt_in_service_ &= 0xff00 | value;
 			update_interrupts();
 		break;
-		case 0x07:		LOG("Write: interrupt in-service A (no-op?) " << PADHEX(2) << int(value));	break;
-		case 0x08:		LOG("Write: interrupt in-service B (no-op?) " << PADHEX(2) << int(value));	break;
 		case 0x09:
 			LOG("Write: interrupt mask A " << PADHEX(2) << int(value));
 			interrupt_mask_ = (interrupt_mask_ & 0x00ff) | (value << 8);
@@ -121,6 +127,13 @@ void MFP68901::write(int address, uint8_t value) {
 		case 0x0b:
 			LOG("Write: vector " << PADHEX(2) << int(value));
 			interrupt_vector_ = value;
+
+			// If automatic end-of-interrupt mode has now been enabled, clear
+			// the in-process mask and re-evaluate.
+			if(!(interrupt_vector_ & 0x08)) {
+				interrupt_in_service_ = 0;
+				update_interrupts();
+			}
 		break;
 		case 0x0c:
 		case 0x0d: {
@@ -266,20 +279,34 @@ void MFP68901::reevaluate_gpip_interrupts() {
 // MARK: - Interrupts
 
 void MFP68901::begin_interrupts(int interrupt) {
-	interrupt_in_service_ |= interrupt;
+	interrupt_pending_ |= interrupt;
 	update_interrupts();
 }
 
 void MFP68901::end_interrupts(int interrupt) {
-	interrupt_in_service_ &= ~interrupt;
+	interrupt_pending_ &= ~interrupt;
 	update_interrupts();
 }
 
 void MFP68901::update_interrupts() {
-	const bool old_interrupt_line = interrupt_line_;
+	const auto old_interrupt_line = interrupt_line_;
-	interrupt_pending_ = interrupt_in_service_ & interrupt_enable_;
+	const auto permitted_interrupts = interrupt_pending_ & interrupt_mask_;
-	interrupt_line_ = interrupt_pending_ & interrupt_mask_;
 
+	if(!permitted_interrupts) {
+		interrupt_line_ = false;
+	} else {
+		if(interrupt_vector_ & 0x8) {
+			// Software interrupt mode: permit only if no higher interrupts
+			// are currently in service.
+			const int highest_bit = 1 << (fls(permitted_interrupts) - 1);
+			interrupt_line_ = !(interrupt_in_service_ & ~(highest_bit + highest_bit - 1));
+		} else {
+			// Auto-interrupt mode; just signal.
+			interrupt_line_ = true;
+		}
+	}
+
+	// Update the delegate if necessary.
 	if(interrupt_delegate_ && interrupt_line_ != old_interrupt_line) {
 		interrupt_delegate_->mfp68901_did_change_interrupt_status(this);
 	}
@@ -290,14 +317,20 @@ bool MFP68901::get_interrupt_line() {
 }
 
 uint8_t MFP68901::acknowledge_interrupt() {
-	uint8_t selected_interrupt = 15;
+	const int selected = fls(interrupt_pending_ & interrupt_mask_) - 1;
-	uint16_t interrupt_mask = 0x8000;
+	const int mask = 1 << selected;
-	while(!(interrupt_pending_ & interrupt_mask) && interrupt_mask) {
+
-		interrupt_mask >>= 1;
+	// Clear the pending bit regardless.
-		--selected_interrupt;
+	interrupt_pending_ &= ~mask;
+
+	// If this is software interrupt mode, set the in-service bit.
+	if(interrupt_vector_ & 0x8) {
+		interrupt_in_service_ |= mask;
 	}
-	end_interrupts(interrupt_mask);
+
-	return (interrupt_vector_ & 0xf0) | selected_interrupt;
+	update_interrupts();
+
+	return (interrupt_vector_ & 0xf0) | uint8_t(selected);
 }
 
 void MFP68901::set_interrupt_delegate(InterruptDelegate *delegate) {

Components/68901/MFP68901.hpp

@@ -76,24 +76,34 @@ class MFP68901 {
 
 		InterruptDelegate *interrupt_delegate_ = nullptr;
 
-		// Ad hoc documentation: there seems to be a four-stage process here.
+		// Ad hoc documentation:
-		// This is my current understanding:
 		//
-		// Interrupt in-service refers to whether the signal that would cause an
+		// An interrupt becomes pending if it is enabled at the time it occurs.
-		// interrupt is active.
 		//
-		// If the interrupt is in-service and enabled, it will be listed as pending.
+		// If a pending interrupt is enabled in the interrupt mask, a processor
+		// interrupt is generated. Otherwise no processor interrupt is generated.
 		//
-		// If a pending interrupt is enabled in the interrupt mask, it will generate
+		// (Disabling a bit in the enabled mask also instantaneously clears anything
-		// a processor interrupt.
+		// in the pending mask.)
+		//
+		// The user can write to the pending interrupt register; a write
+		// masks whatever is there — so you can disable bits but you cannot set them.
+		//
+		// If the vector register's 'S' bit is set then software end-of-interrupt mode applies:
+		// Acknowledgement of an interrupt clears that interrupt's pending bit, but also sets
+		// its in-service bit. That bit will remain set until the user writes a zero to its position.
+		// If any bits are set in the in-service register, then they will prevent lower-priority
+		// interrupts from being signalled to the CPU. Further interrupts of the same or a higher
+		// priority may occur.
+		//
+		// If the vector register's 'S' bit is clear then automatic end-of-interrupt mode applies:
+		// Acknowledgement of an interrupt will automatically clear the corresponding
+		// pending bit.
 		//
-		// So, the designers seem to have wanted to allow for polling and interrupts,
-		// and then also decided to have some interrupts be able to be completely
-		// disabled, so that don't even show up for polling.
-		int interrupt_in_service_ = 0;
 		int interrupt_enable_ = 0;
 		int interrupt_pending_ = 0;
 		int interrupt_mask_ = 0;
+		int interrupt_in_service_ = 0;
 		bool interrupt_line_ = false;
 		uint8_t interrupt_vector_ = 0;