[BUG] `SysTick_Callback` interferes with timing critical code - causes speed jitter - LPC176x

[mh-dm]

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

SysTick_Callback() should not be used as doing so causes timing jitter for all interrupts. SysTick_Callback() is an api from the pio-framework-arduino-lpc176x.

Here's the relevant section from pio-framework-arduino-lpc176x/cores/arduino/main.cpp:

 __attribute__ ((weak)) void SysTick_Callback() {}
// [..]
volatile uint64_t _millis;
// [..]
    NVIC_SetPriority(SysTick_IRQn, NVIC_EncodePriority(0, 0, 0));
// [..]
  void SysTick_Handler(void) {
    ++_millis;
    SysTick_Callback();
  }

0 is the highest priority level which means SysTick_Handler is going to run regardless of anyone. This would be absolutely fine if it was just incrementing _millis. But it's not.

SysTick_Callback() running as part of SysTick_IRQn and at the highest possible priority puts it ahead of every other interrupt: stepper isr, uart, serial, pwm, watchdog, you name it. And it runs every millisecond.

The main problematic SysTick_Callback() is from Marlin/src/HAL/LPC1768/HAL.cpp:

void SysTick_Callback() { disk_timerproc(); }

Where disk_timerproc() is actually from framework-arduino-lpc176x/system/CMSIS/lib/chanfs/mmc_ssp.c.

Issue

A logic analyzer capture of step/dir pins best shows the issue: (Note in this capture the stepper isr was at the priority 0)

You can see speed jitter equivalent to 6000mm/s^2 or more throughout, even at a somewhat leisurely 100mm/s movement speed. In my testing, I've disabled all features I could think of incl. lcd and watchdog and increased the stepper isr priority to highest (0), but still jitter.

Root cause

To confirm that it's SysTick_Callback() I've commented it out completely, including definition and use. This is the result: (Note in this capture the stepper isr was at the priority 0)

The slightly strange part is that without the disk_timerproc(); call, reading from/writing to SD card still seems to work. So It's definitely not crucial. Is it even needed? Every ms? / When is it needed?

One more

Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp (unused in my build) also defines a SysTick_Callback().

pio-framework-arduino-lpc176x

I've also opened pio-framework-arduino-lpc176x/issues/49.

Impact

It makes ADAPTIVE_STEP_SMOOTHING seem lees effective. ADAPTIVE_STEP_SMOOTHING reduces a larger source of speed jitter, namely bresenham aliasing for the minor axes. It may interfere with communication like serial and spi, depending on timing sensitivity or implementation.

Bug Timeline

Old issue, maybe 2019. Git blame points to SysTick_Callback added in 60cb36bef3644640f2eb1c9d2b30189e41e81ef2 "(grafted)"

Expected behavior

No significant isr jitter.

Actual behavior

Interrupt jitter.

Steps to Reproduce

Run this simple/X-axis only gcode and look at the step/dir pins with a logic analyzer (at >=12Mhz sample rate).

M92 X80 Y80 ; 80 steps per mm
G92 X0 Y0
G4 P550 ; configurable delay
M205 X10 Y10
M204 S5000
G0 F7500 X60
G0 F5400 X120
G0 F5700 X180
G0 F6000 X240
G0 F6000 X0

Version of Marlin Firmware

Last sync: 06762db050399137796bbd192b94ed9da4a77e11 [cron] Bump distribution date (2024-05-18)

Printer model

Customized Ender-3

Electronics

SKR v1.4 Turbo but all LPC176x are affected

LCD/Controller

No response

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

SD Card (headless)

Don't forget to include

• [ ] A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

(In lieu of configuration zip file I've root caused to specific lines of code.)

[InsanityAutomation]

I believe @p3p was on vacation so we likely wont hear something until next week.

[github-actions[bot]]

Greetings from the Marlin AutoBot! This issue has had no activity for the last 90 days. Do you still see this issue with the latest bugfix-2.1.x code? Please add a reply within 14 days or this issue will be automatically closed. To keep a confirmed issue open we can also add a "Bug: Confirmed" tag.

Disclaimer: This is an open community project with lots of activity and limited resources. The main project contributors will do a bug sweep ahead of the next release, but any skilled member of the community may jump in at any time to fix this issue. That can take a while depending on our busy lives so please be patient, and take advantage of other resources such as the MarlinFirmware Discord to help solve the issue.

[ellensp]

This needs looking into

[thisiskeithb]

This needs looking into

We're waiting on https://github.com/MarlinFirmware/Marlin/pull/27131 to get merged.