From: bsekisser <squirmyworms@embarqmail.com>
Date: Wed, 17 Sep 2014 19:31:38 +0000 (-0400)
Subject: interrupts: modify handling of interrupt state
X-Git-Tag: v1.3~61^2
X-Git-Url: https://git.htl-mechatronik.at/public/?a=commitdiff_plain;h=5e6256ae3c88c2a672d570ff1baf22d7142f5740;p=sx%2Fsimavr.git

interrupts: modify handling of interrupt state

Interrupt state edge detection and wait states combined into multi
	function variable, enabling the removal of edge detection
	code in the run loop and simplifying need to service interrupts
	by placing the burden on code directly influencing the interrupt
	handling state.

modified:   simavr/sim/sim_avr.c
	edge detection code removed from both run loops.
	raw (non-gdb) loop does precheck of interrupt state, while not
	necessary, potentially saving a few cycles.

modified:   simavr/sim/sim_avr.h
	uint8_t i_shadow changed to int8_t interrupt_state.

modified:   simavr/sim/sim_core.c
	flag changes which may impact interrupt state are routed
		through avr_sreg_set().
	multi-cycle loop simplified to check avr->interrupt_state.

modified:   simavr/sim/sim_core.h
	static inline avr_sreg_set() - handles changes to global
		interrupt state and ensures wait states if I
		flag changes from 0 -> 1.  superfluous 1 -> 1
		states are ignored, should they occur. and
		disabling interrupts clears avr->interrupt_state.
	flag changes from SET_SREG_FROM routed to avr_sreg_set();

modified:   simavr/sim/sim_interrupts.c
	avr_interrupt_reset()
		interrupt_state cleared during reset.
	avr_raise_intrrupt()
		check interrupts enabled and no pending
		interrupt_state before marking pending interrupt state.
	avr_service_interrupts()
		servicing code changed to tick pending wait state then
		mark for any pending interrupts or set to zero
		if none waiting.
		on interrupt, direct interrupt state change to
			avr_sreg_set();

modified:   simavr/sim/sim_interrupts.h
	remove pending_wait.

modified:   tests/tests.c
	interrupt edge dectection code removed from test run loop.
	no further modifications required.

	modified:   simavr/sim/sim_avr.c
	modified:   simavr/sim/sim_avr.h
	modified:   simavr/sim/sim_core.c
	modified:   simavr/sim/sim_core.h
	modified:   simavr/sim/sim_interrupts.c
	modified:   simavr/sim/sim_interrupts.h
	modified:   tests/tests.c
---

diff --git a/simavr/sim/sim_avr.c b/simavr/sim/sim_avr.c
index 73b049a..5138cd5 100644
--- a/simavr/sim/sim_avr.c
+++ b/simavr/sim/sim_avr.c
@@ -264,12 +264,6 @@ void avr_callback_run_gdb(avr_t * avr)
 #endif
 	}
 
-	// if we just re-enabled the interrupts...
-	// double buffer the I flag, to detect that edge
-	if (avr->sreg[S_I] && !avr->i_shadow)
-		avr->interrupts.pending_wait++;
-	avr->i_shadow = avr->sreg[S_I];
-
 	// run the cycle timers, get the suggested sleep time
 	// until the next timer is due
 	avr_cycle_count_t sleep = avr_cycle_timer_process(avr);
@@ -318,12 +312,6 @@ void avr_callback_run_raw(avr_t * avr)
 #endif
 	}
 
-	// if we just re-enabled the interrupts...
-	// double buffer the I flag, to detect that edge
-	if (avr->sreg[S_I] && !avr->i_shadow)
-		avr->interrupts.pending_wait++;
-	avr->i_shadow = avr->sreg[S_I];
-
 	// run the cycle timers, get the suggested sleep time
 	// until the next timer is due
 	avr_cycle_count_t sleep = avr_cycle_timer_process(avr);
@@ -344,8 +332,12 @@ void avr_callback_run_raw(avr_t * avr)
 		avr->cycle += 1 + sleep;
 	}
 	// Interrupt servicing might change the PC too, during 'sleep'
-	if (avr->state == cpu_Running || avr->state == cpu_Sleeping)
-		avr_service_interrupts(avr);
+	if (avr->state == cpu_Running || avr->state == cpu_Sleeping) {
+		/* Note: checking interrupt_state here is completely superfluous, however
+			as interrupt_state tells us all we really need to know, here
+			a simple check here may be cheaper than a call not needed. */
+		if (avr->interrupt_state) avr_service_interrupts(avr);
+	}
 }
 
 
diff --git a/simavr/sim/sim_avr.h b/simavr/sim/sim_avr.h
index 7b95bd5..aa0226b 100644
--- a/simavr/sim/sim_avr.h
+++ b/simavr/sim/sim_avr.h
@@ -210,7 +210,12 @@ typedef struct avr_t {
 	// in the opcode decoder.
 	// This array is re-synthesized back/forth when SREG changes
 	uint8_t		sreg[8];
-	uint8_t		i_shadow;	// used to detect edges on I flag
+
+	/* Interrupt state:
+		00: idle (no wait, no pending interrupts) or disabled
+		<0: wait till zero
+		>0: interrupt pending */
+	int8_t		interrupt_state;	// interrupt state
 
 	/* 
 	 * ** current PC **
diff --git a/simavr/sim/sim_core.c b/simavr/sim/sim_core.c
index c4e7b3e..0cc3000 100644
--- a/simavr/sim/sim_core.c
+++ b/simavr/sim/sim_core.c
@@ -888,7 +888,7 @@ run_one_again:
 			if ((opcode & 0xff0f) == 0x9408) {
 				get_sreg_bit(opcode);
 				STATE("%s%c\n", opcode & 0x0080 ? "cl" : "se", _sreg_bit_name[b]);
-				avr->sreg[b] = (opcode & 0x0080) == 0;
+				avr_sreg_set(avr, b, (opcode & 0x0080) == 0);
 				SREG();
 			} else switch (opcode) {
 				case 0x9588: { // SLEEP -- 1001 0101 1000 1000
@@ -942,7 +942,7 @@ run_one_again:
 					new_pc = _avr_pop_addr(avr);
 					cycle += 1 + avr->address_size;
 					if (opcode & 0x10)	// reti
-						avr->sreg[S_I] = 1;
+						avr_sreg_set(avr, S_I, 1);
 					STATE("ret%s\n", opcode & 0x10 ? "i" : "");
 					TRACE_JUMP();
 					STACK_FRAME_POP();
@@ -1404,9 +1404,9 @@ run_one_again:
 	}
 	avr->cycle += cycle;
 	
-	if( (avr->state == cpu_Running) && 
+	if ((avr->state == cpu_Running) && 
 		(avr->run_cycle_count > cycle) && 
-		!(avr->sreg[S_I] && avr_has_pending_interrupts(avr)) )
+		(avr->interrupt_state == 0))
 	{
 		avr->run_cycle_count -= cycle;
 		avr->pc = new_pc;
diff --git a/simavr/sim/sim_core.h b/simavr/sim/sim_core.h
index 15b4c3b..51c4ecd 100644
--- a/simavr/sim/sim_core.h
+++ b/simavr/sim/sim_core.h
@@ -102,12 +102,31 @@ void avr_dump_state(avr_t * avr);
 					dst |= (1 << i); \
 		}
 
+static inline void avr_sreg_set(avr_t * avr, uint8_t flag, uint8_t ival)
+{
+	/* 
+	 *	clear interrupt_state if disabling interrupts.
+	 *	set wait if enabling interrupts.
+	 *	no change if interrupt flag does not change.
+	 */
+
+	if (flag == S_I) {
+		if (ival) {
+			if (!avr->sreg[S_I])
+				avr->interrupt_state = -2;
+		} else
+			avr->interrupt_state = 0;
+	}
+
+	avr->sreg[flag] = ival;
+}
+
 /**
  * Splits the SREG value from src into the avr->sreg array.
  */
 #define SET_SREG_FROM(avr, src) { \
 			for (int i = 0; i < 8; i++) \
-				avr->sreg[i] = (src & (1 << i)) != 0; \
+				avr_sreg_set(avr, i, (src & (1 << i)) != 0); \
 		}
 
 #ifdef __cplusplus
diff --git a/simavr/sim/sim_interrupts.c b/simavr/sim/sim_interrupts.c
index 1e3866c..e08826e 100644
--- a/simavr/sim/sim_interrupts.c
+++ b/simavr/sim/sim_interrupts.c
@@ -47,7 +47,7 @@ avr_interrupt_reset(
 	printf("%s\n", __func__);
 	avr_int_table_p table = &avr->interrupts;
 	table->pending_r = table->pending_w = 0;
-	table->pending_wait = 0;
+	avr->interrupt_state = 0;
 	for (int i = 0; i < table->vector_count; i++)
 		table->vector[i]->pending = 0;
 }
@@ -127,8 +127,8 @@ avr_raise_interrupt(
 		table->pending[table->pending_w++] = vector;
 		table->pending_w = INT_FIFO_MOD(table->pending_w);
 
-		if (!table->pending_wait)
-			table->pending_wait = 1;		// latency on interrupts ??
+		if (avr->sreg[S_I] && avr->interrupt_state == 0)
+			avr->interrupt_state = 1;
 		if (avr->state == cpu_Sleeping) {
 			if (vector->trace)
 				printf("Waking CPU due to interrupt\n");
@@ -191,19 +191,18 @@ avr_service_interrupts(
 	if (!avr->sreg[S_I])
 		return;
 
-	if (!avr_has_pending_interrupts(avr))
+	if (avr->interrupt_state) {
+		if (avr->interrupt_state < 0) {
+			avr->interrupt_state++;
+			if (avr->interrupt_state == 0)
+				avr->interrupt_state = avr_has_pending_interrupts(avr);
+			return;
+		}
+	} else
 		return;
 
 	avr_int_table_p table = &avr->interrupts;
 
-	if (!table->pending_wait) {
-		table->pending_wait = 2;	// for next one...
-		return;
-	}
-	table->pending_wait--;
-	if (table->pending_wait)
-		return;
-
 	// how many are pending...
 	int cnt = table->pending_w > table->pending_r ?
 			table->pending_w - table->pending_r :
@@ -230,11 +229,12 @@ avr_service_interrupts(
 	// could also have been disabled, or cleared
 	if (!avr_regbit_get(avr, vector->enable) || !vector->pending) {
 		vector->pending = 0;
+		avr->interrupt_state = avr_has_pending_interrupts(avr);
 	} else {
 		if (vector && vector->trace)
 			printf("%s calling %d\n", __FUNCTION__, (int)vector->vector);
 		_avr_push_addr(avr, avr->pc);
-		avr->sreg[S_I] = 0;
+		avr_sreg_set(avr, S_I, 0);
 		avr->pc = vector->vector * avr->vector_size;
 
 		avr_clear_interrupt(avr, vector);
diff --git a/simavr/sim/sim_interrupts.h b/simavr/sim/sim_interrupts.h
index f86dfb1..c8c9bed 100644
--- a/simavr/sim/sim_interrupts.h
+++ b/simavr/sim/sim_interrupts.h
@@ -46,7 +46,6 @@ typedef struct avr_int_vector_t {
 typedef struct  avr_int_table_t {
 	avr_int_vector_t * vector[64];
 	uint8_t			vector_count;
-	uint8_t			pending_wait;	// number of cycles to wait for pending
 	avr_int_vector_t * pending[64]; // needs to be >= vectors and a power of two
 	uint8_t			pending_w,
 					pending_r;	// fifo cursors
diff --git a/tests/tests.c b/tests/tests.c
index 112054f..046c6e4 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -69,12 +69,6 @@ static int my_avr_run(avr_t * avr)
 	if (avr->state == cpu_Running)
 		new_pc = avr_run_one(avr);
 
-	// if we just re-enabled the interrupts...
-	// double buffer the I flag, to detect that edge
-	if (avr->sreg[S_I] && !avr->i_shadow)
-		avr->interrupts.pending_wait++;
-	avr->i_shadow = avr->sreg[S_I];
-
 	// run the cycle timers, get the suggested sleep time
 	// until the next timer is due
 	avr_cycle_count_t sleep = avr_cycle_timer_process(avr);