Micro-controller timing tuning and fixes #7128
Merged
+5
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KevinOConnor
force-pushed
the
work-tuning-20251124
branch
2 times, most recently
from
7e4adf8 to
26577ff
Compare
Collaborator
|
Thanks, Kevin. I will test these patches on my printer. But I agree that broader testing is necessary. |
Collaborator
|
MCU changes look simpler than I expected. So, after the week, I can only say there are no issues with it on my setup. Thanks. |
|
Working fine here |
The TIMER_IDLE_REPEAT_TICKS was intended to reduce the number of cases where timers would defer to tasks when tasks are mostly idle. However, with commit ea546c7 this became less important because timers now only defer to tasks if tasks are consistently busy for two consecutive calls to sched_check_set_tasks_busy(). The TIMER_IDLE_REPEAT_TICKS mechanism could result in extended task delays if timers do become busy. Timers can become busy in normal operation if timers are scheduled to run more than 500,000 times a second (every 2us or faster). This can occur on stepper movement when using high microstep settings. If timers become busy, it could take up to 1ms for tasks to next be given an opportunity to run (two calls to sched_check_set_tasks_busy() at 500us per call). This wouldn't typically be an issue if tasks are also busy, but in some loads tasks may need to run periodically in such a way that the task status alternates between idle and busy. In this case, the TIMER_IDLE_REPEAT_TICKS mechanism could effectively limit the number of task wakeups to only ~1000 per second. The "USB to canbus bridge" code uses tasks to transfer data to/from USB and canbus. If timers become busy, the limiting of task wakeups could lead to a situation where the effective bandwidth becomes severely constrained. In particular, this can be an issue on USB implementations that do not support "double buffering" (such as the stm32 usbotg code). There are reports of "Timer too close" errors on "USB to canbus bridge" mode as a result of this issue. Fix by removing the TIMER_IDLE_REPEAT_TICKS check. Check for busy tasks every TIMER_REPEAT_TICKS instead (100us). Signed-off-by: Kevin O'Connor <[email protected]>
Change TIMER_MIN_TRY_TICKS from 2us to 90 instructions. On newer chips 2us is a large amount of time - for example on the 520Mhz stm32h723 it would be 1040 instructions. Using a large time can result in "busy waiting" in the irq handler when the cpu may be better spent running tasks. The armcm_timer.c code is used on most ARM cortex-M chips and on all of these chips the SysTick timer should be tied directly to the instruction counter. This change should be safe because it should not take more than 90 instructions to reschedule the timer on any of these chips. Also, all of these chips should be able to exit the irq handler and reenter it in less than 90 instructions allowing more time for tasks to run if the next timer is more than 90 timer ticks in the future. Signed-off-by: Kevin O'Connor <[email protected]>
The MIN_REQTIME_DELTA parameter controls when the host will flush incomplete message blocks to the mcu. If the message had a target time less than 250ms it would result in a flush even if a message block was not completely full. In the situation where the host generates lots of queue_step commands to the point that it fills the mcu move_queue, then it would be possible for individual queue_step commands to become eligible for transmit only microseconds apart. It could also lead to a situation where the target time was less than 250ms in the future. The result could lead to many small message blocks as each became flushed individually. Tune the MIN_REQTIME_DELTA to 100ms to reduce the chance of this. Signed-off-by: Kevin O'Connor <[email protected]>
KevinOConnor
force-pushed
the
work-tuning-20251124
branch
from
26577ff to
2e58023
Compare
Collaborator
Author
|
Okay, thanks. I went ahead and merged this PR. -Kevin |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some "timer too close" errors when using "usb to canbus" mode have been recently tracked down to poor interaction between the USB controller and the low-level timer scheduling system. This PR introduces three changes to the scheduling and transmission system to avoid these interactions.
The individual commit messages have more details on the changes, but in brief the three changes are:
In addition the above, it has also been suggested that we could further improve the system with two additional changes: increasing the maximum move_queue size from 1024 entries; and implementing a manual USB "double buffering" transmit system on stm32 chips using the usbotg driver (stm32f4, f7, and h7 chips). However, those changes will be something to look at in a future PR.
Although these changes are likely to improve the timing on a relatively small number of setups they do change the generic code. Thus, there is a raised potential for impact to other more common configurations. Testing feedback is appreciated.
Also mentioned briefly at: https://klipper.discourse.group/t/timer-too-close-after-canbus-conversion/25174/18
@dmbutyugin , @nefelim4ag - FYI.
-Kevin