Fix BLTouch PWM reliability in HAL/STM32 (#18702)

Marlin/src/HAL/STM32/HAL.cpp

@@ -79,7 +79,7 @@ void HAL_init() {
     while (!LL_PWR_IsActiveFlag_BRR());   // Wait until backup regulator is initialized
   #endif
 
-  SetSoftwareSerialTimerInterruptPriority();
+  SetTimerInterruptPriorities();
 
   TERN_(EMERGENCY_PARSER, USB_Hook_init());
 }

Marlin/src/HAL/STM32/Servo.cpp

@@ -33,6 +33,18 @@ static libServo *servos[NUM_SERVOS] = {0};
 constexpr millis_t servoDelay[] = SERVO_DELAY;
 static_assert(COUNT(servoDelay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");
 
+// Initialize to the default timer priority. This will be overridden by a call from timers.cpp.
+// This allows all timer interrupt priorities to be managed from a single location in the HAL.
+static uint32_t servo_interrupt_priority = NVIC_EncodePriority(NVIC_GetPriorityGrouping(), TIM_IRQ_PRIO, TIM_IRQ_SUBPRIO);
+
+// This must be called after the STM32 Servo class has intialized the timer.
+// It may only be needed after the first call to attach(), but it is possible
+// that is is necessary after every detach() call. To be safe this is currently
+// called after every call to attach().
+static void fixServoTimerInterruptPriority() {
+  NVIC_SetPriority(getTimerUpIrq(TIMER_SERVO), servo_interrupt_priority);
+}
+
 libServo::libServo()
 : delay(servoDelay[servoCount]),
   was_attached_before_pause(false),
@@ -44,13 +56,17 @@ libServo::libServo()
 int8_t libServo::attach(const int pin) {
   if (servoCount >= MAX_SERVOS) return -1;
   if (pin > 0) servo_pin = pin;
-  return stm32_servo.attach(servo_pin);
+  auto result = stm32_servo.attach(servo_pin);
+  fixServoTimerInterruptPriority();
+  return result;
 }
 
 int8_t libServo::attach(const int pin, const int min, const int max) {
   if (servoCount >= MAX_SERVOS) return -1;
   if (pin > 0) servo_pin = pin;
-  return stm32_servo.attach(servo_pin, min, max);
+  auto result = stm32_servo.attach(servo_pin, min, max);
+  fixServoTimerInterruptPriority();
+  return result;
 }
 
 void libServo::move(const int value) {
@@ -86,5 +102,9 @@ void libServo::resume_all_servos() {
     if (servo) servo->resume();
 }
 
+void libServo::setInterruptPriority(uint32_t preemptPriority, uint32_t subPriority) {
+  servo_interrupt_priority = NVIC_EncodePriority(NVIC_GetPriorityGrouping(), preemptPriority, subPriority);
+}
+
 #endif // HAS_SERVOS
 #endif // ARDUINO_ARCH_STM32 && !STM32GENERIC

Marlin/src/HAL/STM32/Servo.h

@@ -41,6 +41,7 @@ class libServo {
 
     static void pause_all_servos();
     static void resume_all_servos();
+    static void setInterruptPriority(uint32_t preemptPriority, uint32_t subPriority);
 
   private:
     Servo stm32_servo;

Marlin/src/HAL/STM32/timers.cpp

@@ -29,27 +29,42 @@
 
 #define NUM_HARDWARE_TIMERS 2
 
+// Default timer priorities. Override by specifying alternate priorities in the board pins file.
+// The TONE timer is not present here, as it currently cannot be set programmatically. It is set
+// by defining TIM_IRQ_PRIO in the variant.h or platformio.ini file, which adjusts the default
+// priority for STM32 HardwareTimer objects.
+#define SWSERIAL_TIMER_IRQ_PRIO_DEFAULT  1 // Requires tight bit timing to communicate reliably with TMC drivers
+#define SERVO_TIMER_IRQ_PRIO_DEFAULT     1 // Requires tight PWM timing to control a BLTouch reliably
+#define STEP_TIMER_IRQ_PRIO_DEFAULT      2
+#define TEMP_TIMER_IRQ_PRIO_DEFAULT     14 // Low priority avoids interference with other hardware and timers
+
 #ifndef STEP_TIMER_IRQ_PRIO
-  #define STEP_TIMER_IRQ_PRIO 2
+  #define STEP_TIMER_IRQ_PRIO STEP_TIMER_IRQ_PRIO_DEFAULT
 #endif
 #ifndef TEMP_TIMER_IRQ_PRIO
-  #define TEMP_TIMER_IRQ_PRIO 14   // 14 = after hardware ISRs
+  #define TEMP_TIMER_IRQ_PRIO TEMP_TIMER_IRQ_PRIO_DEFAULT
 #endif
-
+#if HAS_TMC_SW_SERIAL
+  #include <SoftwareSerial.h>
+  #ifndef SWSERIAL_TIMER_IRQ_PRIO
+    #define SWSERIAL_TIMER_IRQ_PRIO SWSERIAL_TIMER_IRQ_PRIO_DEFAULT
+  #endif
+#endif
+#if HAS_SERVOS
+  #include "Servo.h"
+  #ifndef SERVO_TIMER_IRQ_PRIO
+    #define SERVO_TIMER_IRQ_PRIO SERVO_TIMER_IRQ_PRIO_DEFAULT
+  #endif
+#endif
+#if ENABLED(SPEAKER)
   // Ensure the default timer priority is somewhere between the STEP and TEMP priorities.
   // The STM32 framework defaults to interrupt 14 for all timers. This should be increased so that
-// timing-sensitive operations such as speaker output are note impacted by the long-running
+  // timing-sensitive operations such as speaker output are not impacted by the long-running
   // temperature ISR. This must be defined in the platformio.ini file or the board's variant.h,
   // so that it will be consumed by framework code.
   #if !(TIM_IRQ_PRIO > STEP_TIMER_IRQ_PRIO && TIM_IRQ_PRIO < TEMP_TIMER_IRQ_PRIO)
     #error "Default timer interrupt priority is unspecified or set to a value which may degrade performance."
   #endif
-
-#if HAS_TMC_SW_SERIAL
-  #include <SoftwareSerial.h>
-  #ifndef SWSERIAL_TIMER_IRQ_PRIO
-    #define SWSERIAL_TIMER_IRQ_PRIO 1
-  #endif
 #endif
 
 #ifdef STM32F0xx
@@ -189,8 +204,9 @@ TIM_TypeDef * HAL_timer_device(const uint8_t timer_num) {
   return nullptr;
 }
 
-void SetSoftwareSerialTimerInterruptPriority() {
+void SetTimerInterruptPriorities() {
   TERN_(HAS_TMC_SW_SERIAL, SoftwareSerial::setInterruptPriority(SWSERIAL_TIMER_IRQ_PRIO, 0));
+  TERN_(HAS_SERVOS, libServo::setInterruptPriority(SERVO_TIMER_IRQ_PRIO, 0));
 }
 
 #endif // ARDUINO_ARCH_STM32 && !STM32GENERIC

Marlin/src/HAL/STM32/timers.h

@@ -86,8 +86,9 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num);
 void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
+// Configure timer priorities for peripherals such as Software Serial or Servos.
 // Exposed here to allow all timer priority information to reside in timers.cpp
-void SetSoftwareSerialTimerInterruptPriority();
+void SetTimerInterruptPriorities();
 
 //TIM_TypeDef* HAL_timer_device(const uint8_t timer_num); no need to be public for now. not public = not used externally