From 9a49ce2fc4d4944dc84c33778339306b092816c8 Mon Sep 17 00:00:00 2001 From: joverbee Date: Mon, 9 Jan 2017 20:53:53 +0100 Subject: [PATCH 1/6] Improve interrupt handling response time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the way ISRs are stored. Store now in a list on a ‘first come first served basis’, the first interrupt that gets attached will be served first. This improves the speed of the ISR calling from about 6us to 2us for a single interrupt on extint14 See #200 --- cores/arduino/WInterrupts.c | 125 ++++++++++++++++++++++++------------ 1 file changed, 85 insertions(+), 40 deletions(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index 5a2d8b9ed..220e0bfde 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -21,12 +21,17 @@ #include -static voidFuncPtr callbacksInt[EXTERNAL_NUM_INTERRUPTS]; +static voidFuncPtr ISRcallback[EXTERNAL_NUM_INTERRUPTS]; +static EExt_Interrupts ISRlist[EXTERNAL_NUM_INTERRUPTS]; +static uint32_t nints; // Stores total number of attached interrupts + /* Configure I/O interrupt sources */ static void __initialize() { - memset(callbacksInt, 0, sizeof(callbacksInt)); + memset(ISRlist, 0, sizeof(ISRlist)); + memset(ISRcallback, 0, sizeof(ISRcallback)); + nints = 0; NVIC_DisableIRQ(EIC_IRQn); NVIC_ClearPendingIRQ(EIC_IRQn); @@ -76,42 +81,65 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) // Assign pin to EIC pinPeripheral(pin, PIO_EXTINT); - // Assign callback to interrupt - callbacksInt[in] = callback; + // Only store when there is really an ISR to call. + // This allow for calling attachInterrupt(pin, NULL, mode), we set up all needed register + // but won't service the interrupt, this way we also don't need to check it inside the ISR. + if (callback) + { + // Store interrupts to service in order of when they were attached + // to allow for first come first serve handler + uint32_t current=0; + + // Check if we already have this interrupt + int id = -1; + for (uint32_t i=0; i EXTERNAL_INT_7) { - config = 1; - } else { - config = 0; - } + if (id == -1) { + // Need to make a new entry + current = nints; + nints++; + } else { + // We already have an entry for this pin + current = id; + } + ISRlist[current] = in; // List with nr of interrupt in order of when they were attached + ISRcallback[current] = callback; // List of callback adresses + + // Look for right CONFIG register to be addressed + if (in > EXTERNAL_INT_7) { + config = 1; + } else { + config = 0; + } - // Configure the interrupt mode - pos = (in - (8 * config)) << 2; - EIC->CONFIG[config].reg &=~ (EIC_CONFIG_SENSE0_Msk << pos);//reset sense mode, important when changing trigger mode during runtime - switch (mode) - { - case LOW: - EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_LOW_Val << pos; - break; + // Configure the interrupt mode + pos = (in - (8 * config)) << 2; + EIC->CONFIG[config].reg &=~ (EIC_CONFIG_SENSE0_Msk << pos); // Reset sense mode, important when changing trigger mode during runtime + switch (mode) + { + case LOW: + EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_LOW_Val << pos; + break; - case HIGH: - EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_HIGH_Val << pos; - break; + case HIGH: + EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_HIGH_Val << pos; + break; - case CHANGE: - EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_BOTH_Val << pos; - break; + case CHANGE: + EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_BOTH_Val << pos; + break; - case FALLING: - EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_FALL_Val << pos; - break; + case FALLING: + EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_FALL_Val << pos; + break; - case RISING: - EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_RISE_Val << pos; - break; + case RISING: + EIC->CONFIG[config].reg |= EIC_CONFIG_SENSE0_RISE_Val << pos; + break; + } } - // Enable the interrupt EIC->INTENSET.reg = EIC_INTENSET_EXTINT(1 << in); } @@ -133,6 +161,23 @@ void detachInterrupt(uint32_t pin) // Disable wakeup capability on pin during sleep EIC->WAKEUP.reg &= ~(1 << in); + + // Remove callback from the ISR list + int id = -1; + for (uint32_t i=0; iINTFLAG.reg & (1 << i)) != 0) + if ((EIC->INTFLAG.reg & 1<INTFLAG.reg = 1 << i; + EIC->INTFLAG.reg = 1< Date: Fri, 8 Sep 2017 11:35:02 +0200 Subject: [PATCH 2/6] interrupts: remove unneeded cleanup --- cores/arduino/WInterrupts.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index 220e0bfde..c130dacf7 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -174,9 +174,6 @@ void detachInterrupt(uint32_t pin) ISRlist[i] = ISRlist[i+1]; ISRcallback[i] = ISRcallback[i+1]; } - // And remove the top item - ISRlist[nints]=0; - ISRcallback[nints]=NULL; nints--; } From 02d8bf82a20ffc706f123571e7b69c85f25e9368 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 8 Sep 2017 15:30:44 +0200 Subject: [PATCH 3/6] Fixed allocation/deallocation of interrupts subroutines --- cores/arduino/WInterrupts.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index c130dacf7..192cfaced 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -91,18 +91,14 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) uint32_t current=0; // Check if we already have this interrupt - int id = -1; - for (uint32_t i=0; iWAKEUP.reg &= ~(1 << in); // Remove callback from the ISR list - int id = -1; - for (uint32_t i=0; i Date: Fri, 8 Sep 2017 16:05:40 +0200 Subject: [PATCH 4/6] Some code makeup --- cores/arduino/WInterrupts.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index 192cfaced..017ae5cf3 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -21,15 +21,15 @@ #include -static voidFuncPtr ISRcallback[EXTERNAL_NUM_INTERRUPTS]; +static voidFuncPtr ISRcallback[EXTERNAL_NUM_INTERRUPTS]; static EExt_Interrupts ISRlist[EXTERNAL_NUM_INTERRUPTS]; -static uint32_t nints; // Stores total number of attached interrupts +static uint32_t nints; // Stores total number of attached interrupts /* Configure I/O interrupt sources */ static void __initialize() { - memset(ISRlist, 0, sizeof(ISRlist)); + memset(ISRlist, 0, sizeof(ISRlist)); memset(ISRcallback, 0, sizeof(ISRcallback)); nints = 0; @@ -88,7 +88,7 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) { // Store interrupts to service in order of when they were attached // to allow for first come first serve handler - uint32_t current=0; + uint32_t current = 0; // Check if we already have this interrupt for (current=0; current Date: Fri, 8 Sep 2017 16:55:33 +0200 Subject: [PATCH 5/6] Slightly clearer formula for interrupt config register position --- cores/arduino/WInterrupts.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index 017ae5cf3..fad33ba8f 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -106,12 +106,13 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) // Look for right CONFIG register to be addressed if (in > EXTERNAL_INT_7) { config = 1; + pos = (in - 8) << 2; } else { config = 0; + pos = in << 2; } // Configure the interrupt mode - pos = (in - (8 * config)) << 2; EIC->CONFIG[config].reg &=~ (EIC_CONFIG_SENSE0_Msk << pos); // Reset sense mode, important when changing trigger mode during runtime switch (mode) { From 812d3e90cc7f873b73a419a658c5f2d453c5c689 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 8 Sep 2017 16:56:20 +0200 Subject: [PATCH 6/6] Put interrupt mask instead of interrupt num in ISR callbacks list This should save some cycles inside ISR Handler. --- cores/arduino/WInterrupts.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/cores/arduino/WInterrupts.c b/cores/arduino/WInterrupts.c index fad33ba8f..c78ddf62b 100644 --- a/cores/arduino/WInterrupts.c +++ b/cores/arduino/WInterrupts.c @@ -21,9 +21,9 @@ #include -static voidFuncPtr ISRcallback[EXTERNAL_NUM_INTERRUPTS]; -static EExt_Interrupts ISRlist[EXTERNAL_NUM_INTERRUPTS]; -static uint32_t nints; // Stores total number of attached interrupts +static voidFuncPtr ISRcallback[EXTERNAL_NUM_INTERRUPTS]; +static uint32_t ISRlist[EXTERNAL_NUM_INTERRUPTS]; +static uint32_t nints; // Stores total number of attached interrupts /* Configure I/O interrupt sources */ @@ -76,7 +76,8 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) } // Enable wakeup capability on pin in case being used during sleep - EIC->WAKEUP.reg |= (1 << in); + uint32_t inMask = 1 << in; + EIC->WAKEUP.reg |= inMask; // Assign pin to EIC pinPeripheral(pin, PIO_EXTINT); @@ -92,7 +93,7 @@ void attachInterrupt(uint32_t pin, voidFuncPtr callback, uint32_t mode) // Check if we already have this interrupt for (current=0; currentINTENSET.reg = EIC_INTENSET_EXTINT(1 << in); + EIC->INTENSET.reg = EIC_INTENSET_EXTINT(inMask); } /* @@ -154,15 +155,16 @@ void detachInterrupt(uint32_t pin) if (in == NOT_AN_INTERRUPT || in == EXTERNAL_INT_NMI) return; - EIC->INTENCLR.reg = EIC_INTENCLR_EXTINT(1 << in); + uint32_t inMask = 1 << in; + EIC->INTENCLR.reg = EIC_INTENCLR_EXTINT(inMask); // Disable wakeup capability on pin during sleep - EIC->WAKEUP.reg &= ~(1 << in); + EIC->WAKEUP.reg &= ~inMask; // Remove callback from the ISR list uint32_t current; for (current=0; currentINTFLAG.reg & 1<INTFLAG.reg & ISRlist[i]) != 0) { // Call the callback function ISRcallback[i](); // Clear the interrupt - EIC->INTFLAG.reg = 1<INTFLAG.reg = ISRlist[i]; } } }