fast exti interrupt handoff (#3497)

* fast exti

* test code snuck in

* full interrupt disable :(

* do it the old fashioned way

* enable interrupt

* consume stored timestamp

* dead

* h7 maybe

* guard maybe

* non-stm32

* exti 16 wrong on f4/f7

* CORTEX_MAXIMUM_PRIORITY

* safer but uglier

* s

* chibios

* no const

* initializers

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/ChibiOS

@@ -1 +1 @@
-Subproject commit 8bae576fdde4e2220bfdc577ca71920bcf1043b2
+Subproject commit e19494fd2323c21337fc0fc8377d967175d36898

firmware/controllers/sensors/frequency_sensor.cpp

@@ -9,9 +9,8 @@
 #include "digital_input_exti.h"
 
 // Callback adapter since we can't pass a member function to a callback
-static void freqSensorExtiCallback(void* arg) {
+static void freqSensorExtiCallback(void* arg, efitick_t nowNt) {
-	auto inst = reinterpret_cast<FrequencySensor*>(arg);
+	reinterpret_cast<FrequencySensor*>(arg)->onEdge(nowNt);
-	inst->onEdge(getTimeNowNt());
 }
 
 void FrequencySensor::init(brain_pin_e pin) {

firmware/hw_layer/digital_input/digital_input_exti.cpp

@@ -18,16 +18,25 @@
  * because pin '0' would be used on two different ports
  */
 
-static ioportmask_t ext_used = 0;
+#ifdef STM32_I2C_I2C1_IRQ_PRIORITY
-static const char *EXT_USED[16];
-
 void efiExtiInit() {
-	memset(EXT_USED, 0, sizeof(EXT_USED));
+	nvicEnableVector(I2C1_EV_IRQn, STM32_I2C_I2C1_IRQ_PRIORITY);
 }
 
-// EXT is not able to give you the front direction but you could read the pin in the callback.
+struct ExtiChannel
-void efiExtiEnablePin(const char *msg, brain_pin_e brainPin, uint32_t mode, palcallback_t cb, void *cb_data) {
+{
+	ExtiCallback Callback = nullptr;
+	void* CallbackData;
 
+	efitick_t Timestamp = 0;
+
+	const char* Name = nullptr;
+};
+
+static ExtiChannel channels[16];
+
+// EXT is not able to give you the front direction but you could read the pin in the callback.
+void efiExtiEnablePin(const char *msg, brain_pin_e brainPin, uint32_t mode, ExtiCallback cb, void *cb_data) {
 	/* paranoid check, in case of GPIO_UNASSIGNED getHwPort will return NULL
 	 * and we will fail on next check */
 	if (!isBrainPinValid(brainPin)) {
@@ -47,23 +56,24 @@ void efiExtiEnablePin(const char *msg, brain_pin_e brainPin, uint32_t mode, palc
 
 	int index = getHwPin(msg, brainPin);
 
+	auto& channel = channels[index];
+
 	/* is this index already used? */
-	if (ext_used & PAL_PORT_BIT(index)) {
+	if (channel.Callback) {
 		firmwareError(CUSTOM_ERR_PIN_ALREADY_USED_2, "%s: pin %s/index %d: exti index already used by %s",
 		msg,
 		hwPortname(brainPin),
 		index,
-		EXT_USED[index]);
+		channel.Name);
 		return;
 	}
 
 	ioline_t line = PAL_LINE(port, index);
 	palEnableLineEvent(line, mode);
-	palSetLineCallback(line, cb, cb_data);
 
-	/* mark used */
+	channel.Name = msg;
-	ext_used |= PAL_PORT_BIT(index);
+	channel.Callback = cb;
-	EXT_USED[index] = msg;
+	channel.CallbackData = cb_data;
 }
 
 void efiExtiDisablePin(brain_pin_e brainPin)
@@ -80,8 +90,10 @@ void efiExtiDisablePin(brain_pin_e brainPin)
 
 	int index = getHwPin("exti", brainPin);
 
+	auto& channel = channels[index];
+
 	/* is this index was used? */
-	if (!(ext_used & PAL_PORT_BIT(index))) {
+	if (!channel.Callback) {
 		return;
 	}
 
@@ -89,8 +101,9 @@ void efiExtiDisablePin(brain_pin_e brainPin)
 	palDisableLineEvent(line);
 
 	/* mark unused */
-	ext_used &= ~PAL_PORT_BIT(index);
+	channel.Name = nullptr;
-	EXT_USED[index] = nullptr;
+	channel.Callback = nullptr;
+	channel.CallbackData = nullptr;
 }
 
 digital_input_s* startDigitalCaptureExti(const char *msg, brain_pin_e brainPin) {
@@ -103,9 +116,96 @@ digital_input_s* startDigitalCapture(const char *msg, brain_pin_e brainPin) {
 }
 #endif // EFI_ICU_INPUTS
 
+static inline void triggerInterrupt() {
+	// Manually fire the I2C1_EV interrupt, it will be queued after this interrupt returns
+	NVIC->STIR = I2C1_EV_IRQn;
+}
+
+CH_IRQ_HANDLER(STM32_I2C1_EVENT_HANDLER) {
+	OSAL_IRQ_PROLOGUE();
+
+	for (size_t i = 0; i < 16; i++) {
+		auto& channel = channels[i];
+
+		// get the timestamp out under lock
+		// todo: lock freeeeee!
+		__disable_irq();
+		auto timestamp = channel.Timestamp;
+		channel.Timestamp = 0;
+		__enable_irq();
+
+		if (timestamp != 0) {
+			channel.Callback(channel.CallbackData, timestamp);
+		}
+	}
+
+	OSAL_IRQ_EPILOGUE();
+}
+
+void handleExtiIsr(uint8_t index) {
+	// No need to lock anything, we're already the highest-pri interrupt!
+
+	uint32_t pr;
+	extiGetAndClearGroup1(1U << index, pr);
+
+	if (pr & (1 << index)) {
+		auto& timestamp = channels[index].Timestamp;
+
+		if (timestamp == 0) {
+			timestamp = getTimeNowNt();
+		}
+
+		triggerInterrupt();
+	}
+}
+
+CH_FAST_IRQ_HANDLER(Vector58) {
+	handleExtiIsr(0);
+}
+
+CH_FAST_IRQ_HANDLER(Vector5C) {
+	handleExtiIsr(1);
+}
+
+CH_FAST_IRQ_HANDLER(Vector60) {
+	handleExtiIsr(2);
+}
+
+CH_FAST_IRQ_HANDLER(Vector64) {
+	handleExtiIsr(3);
+}
+
+CH_FAST_IRQ_HANDLER(Vector68) {
+	handleExtiIsr(4);
+}
+
+CH_FAST_IRQ_HANDLER(Vector9C) {
+	handleExtiIsr(5);
+	handleExtiIsr(6);
+	handleExtiIsr(7);
+	handleExtiIsr(8);
+	handleExtiIsr(9);
+}
+
+CH_FAST_IRQ_HANDLER(VectorE0) {
+	handleExtiIsr(10);
+	handleExtiIsr(11);
+	handleExtiIsr(12);
+	handleExtiIsr(13);
+	handleExtiIsr(14);
+	handleExtiIsr(15);
+}
+
+#else // not STM32
+
+// TODO: non-stm32 exti
+void efiExtiInit() {
+	firmwareError(OBD_PCM_Processor_Fault, "exti not supported");
+}
+
+void efiExtiEnablePin(const char *, brain_pin_e, uint32_t, ExtiCallback, void *) { }
+void efiExtiDisablePin(brain_pin_e) { }
+
+#endif
+
 #endif /* HAL_USE_PAL && EFI_PROD_CODE */
-
-
-
-
-

firmware/hw_layer/digital_input/digital_input_exti.h

@@ -10,7 +10,10 @@
 #include "digital_input.h"
 
 #if HAL_USE_PAL
+
+using ExtiCallback = void(*)(void*, efitick_t);
+
 void efiExtiInit();
-void efiExtiEnablePin(const char *msg, brain_pin_e pin, uint32_t mode, palcallback_t cb, void *cb_data);
+void efiExtiEnablePin(const char *msg, brain_pin_e pin, uint32_t mode, ExtiCallback cb, void *cb_data);
 void efiExtiDisablePin(brain_pin_e brainPin);
 #endif /* HAL_USE_PAL */

firmware/hw_layer/digital_input/trigger/trigger_input_exti.cpp

@@ -24,9 +24,8 @@
 static ioline_t shaftLines[TRIGGER_SUPPORTED_CHANNELS];
 static ioline_t camLines[CAM_INPUTS_COUNT];
 
-static void shaft_callback(void *arg) {
+static void shaft_callback(void *arg, efitick_t stamp) {
 	// do the time sensitive things as early as possible!
-	efitick_t stamp = getTimeNowNt();
 	TRIGGER_BAIL_IF_DISABLED
 //#if HW_CHECK_MODE
 //	TRIGGER_BAIL_IF_SELF_STIM
@@ -42,8 +41,7 @@ static void shaft_callback(void *arg) {
 	hwHandleShaftSignal(index, rise, stamp);
 }
 
-static void cam_callback(void *arg) {
+static void cam_callback(void *arg, efitick_t stamp) {
-	efitick_t stamp = getTimeNowNt();
 	TRIGGER_BAIL_IF_DISABLED
 //#if HW_CHECK_MODE
 //	TRIGGER_BAIL_IF_SELF_STIM

firmware/hw_layer/ports/stm32/mcuconf_common_f4_f7.h

@@ -89,7 +89,19 @@
 /*
  * EXT driver system settings.
  */
-#define STM32_IRQ_EXTI_PRIORITY             ICU_PRIORITY
+
+#define STM32_DISABLE_EXTI0_HANDLER
+#define STM32_DISABLE_EXTI1_HANDLER
+#define STM32_DISABLE_EXTI2_HANDLER
+#define STM32_DISABLE_EXTI3_HANDLER
+#define STM32_DISABLE_EXTI4_HANDLER
+#define STM32_DISABLE_EXTI5_9_HANDLER
+#define STM32_DISABLE_EXTI10_15_HANDLER
+
+// we hijack this interrupt handler as the EXTI chained handler, see digital_input_exti.cpp
+#define STM32_I2C_I2C1_IRQ_PRIORITY         ICU_PRIORITY
+
+#define STM32_IRQ_EXTI_PRIORITY             CORTEX_MAXIMUM_PRIORITY
 #define STM32_IRQ_EXTI0_PRIORITY            STM32_IRQ_EXTI_PRIORITY
 #define STM32_IRQ_EXTI1_PRIORITY            STM32_IRQ_EXTI_PRIORITY
 #define STM32_IRQ_EXTI2_PRIORITY            STM32_IRQ_EXTI_PRIORITY
@@ -97,14 +109,14 @@
 #define STM32_IRQ_EXTI4_PRIORITY            STM32_IRQ_EXTI_PRIORITY
 #define STM32_IRQ_EXTI5_9_PRIORITY          STM32_IRQ_EXTI_PRIORITY
 #define STM32_IRQ_EXTI10_15_PRIORITY        STM32_IRQ_EXTI_PRIORITY
-#define STM32_IRQ_EXTI16_PRIORITY           STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI16_PRIORITY           15
 #define STM32_IRQ_EXTI17_PRIORITY           15  /* why? */
-#define STM32_IRQ_EXTI18_PRIORITY           STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI18_PRIORITY           15
-#define STM32_IRQ_EXTI19_PRIORITY           STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI19_PRIORITY           15
-#define STM32_IRQ_EXTI20_PRIORITY           STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI20_PRIORITY           15
 #define STM32_IRQ_EXTI21_PRIORITY           15  /* why? */
 #define STM32_IRQ_EXTI22_PRIORITY           15  /* why? */
-#define STM32_IRQ_EXTI23_PRIORITY           STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI23_PRIORITY           15
 
 /*
  * GPT driver system settings.
@@ -138,7 +150,6 @@
 #define STM32_I2C_I2C3_TX_DMA_STREAM        STM32_DMA_STREAM_ID(1, 4)
 #define STM32_I2C_I2C4_RX_DMA_STREAM        STM32_DMA_STREAM_ID(1, 2)
 #define STM32_I2C_I2C4_TX_DMA_STREAM        STM32_DMA_STREAM_ID(1, 5)
-#define STM32_I2C_I2C1_IRQ_PRIORITY         5
 #define STM32_I2C_I2C2_IRQ_PRIORITY         5
 #define STM32_I2C_I2C3_IRQ_PRIORITY         5
 #define STM32_I2C_I2C4_IRQ_PRIORITY         5

firmware/hw_layer/ports/stm32/stm32h7/cfg/mcuconf.h

@@ -175,13 +175,27 @@
 /*
  * IRQ system settings.
  */
-#define STM32_IRQ_EXTI0_PRIORITY            6
+
-#define STM32_IRQ_EXTI1_PRIORITY            6
+#define STM32_DISABLE_EXTI0_HANDLER
-#define STM32_IRQ_EXTI2_PRIORITY            6
+#define STM32_DISABLE_EXTI1_HANDLER
-#define STM32_IRQ_EXTI3_PRIORITY            6
+#define STM32_DISABLE_EXTI2_HANDLER
-#define STM32_IRQ_EXTI4_PRIORITY            6
+#define STM32_DISABLE_EXTI3_HANDLER
-#define STM32_IRQ_EXTI5_9_PRIORITY          6
+#define STM32_DISABLE_EXTI4_HANDLER
-#define STM32_IRQ_EXTI10_15_PRIORITY        6
+#define STM32_DISABLE_EXTI5_9_HANDLER
+#define STM32_DISABLE_EXTI10_15_HANDLER
+
+// we hijack this interrupt handler as the EXTI chained handler, see digital_input_exti.cpp
+#define STM32_I2C_I2C1_IRQ_PRIORITY         6
+
+#define STM32_IRQ_EXTI_PRIORITY             CORTEX_MAXIMUM_PRIORITY
+#define STM32_IRQ_EXTI0_PRIORITY            STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI1_PRIORITY            STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI2_PRIORITY            STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI3_PRIORITY            STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI4_PRIORITY            STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI5_9_PRIORITY          STM32_IRQ_EXTI_PRIORITY
+#define STM32_IRQ_EXTI10_15_PRIORITY        STM32_IRQ_EXTI_PRIORITY
+
 #define STM32_IRQ_EXTI16_PRIORITY           6
 #define STM32_IRQ_EXTI17_PRIORITY           6
 #define STM32_IRQ_EXTI18_PRIORITY           6
@@ -293,7 +307,7 @@
 #define STM32_I2C_I2C3_TX_DMA_STREAM        STM32_DMA_STREAM_ID_ANY
 #define STM32_I2C_I2C4_RX_BDMA_STREAM       STM32_BDMA_STREAM_ID_ANY
 #define STM32_I2C_I2C4_TX_BDMA_STREAM       STM32_BDMA_STREAM_ID_ANY
-#define STM32_I2C_I2C1_IRQ_PRIORITY         5
+// STM32_I2C_I2C1_IRQ_PRIORITY is defined above, reused for fast EXTI handoff
 #define STM32_I2C_I2C2_IRQ_PRIORITY         5
 #define STM32_I2C_I2C3_IRQ_PRIORITY         5
 #define STM32_I2C_I2C4_IRQ_PRIORITY         5

firmware/hw_layer/trigger_input_adc.cpp

@@ -115,13 +115,12 @@ static void onTriggerChanged(efitick_t stamp, bool isPrimary, bool isRising) {
 }
 
 
-static void shaft_callback(void *arg) {
+static void shaft_callback(void *arg, efitick_t stamp) {
 	if (curAdcMode != TRIGGER_EXTI) {
 		return;
 	}
 	
 	// do the time sensitive things as early as possible!
-	efitick_t stamp = getTimeNowNt();
 	ioline_t pal_line = (ioline_t)arg;
 	bool rise = (palReadLine(pal_line) == PAL_HIGH);
 
@@ -148,7 +147,7 @@ static void shaft_callback(void *arg) {
 	prevStamp = stamp;
 }
 
-static void cam_callback(void *) {
+static void cam_callback(void *, efitick_t) {
 }
 
 // todo: add cam support?