ChibiOS timer fixes #16017
Merged
ChibiOS timer fixes #16017
Conversation
This file contains 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
Extract the code which effectively makes a 32-bit tick counter from a possibly 16-bit ChibiOS system timer into a separate function. Does not really change the behavior of the timer API, but makes the actions done in `timer_clear()` and `timer_read32()` more obvious.
The QMK timer API implementation for ChibiOS used a 32-bit tick counter
(obtained from the ChibiOS system timer) and then converted the value to
milliseconds to produce the timer value for QMK. However, the frequency
of the ChibiOS timer is above 1000 Hz in most cases (values of 10000 Hz
or even 100000 Hz are typically used), and therefore the 32-bit tick
counter was overflowing and wrapping around much earlier than expected
(after about 5 days for 10000 Hz, or about 12 hours for 100000 Hz).
When this wraparound happened, the QMK timer value was jumping back to
zero, which broke various code dealing with timers (e.g., deferred
executors).
Just making the tick counter 64-bit to avoid the overflow is not a good
solution, because the ChibiOS code which performs the conversion from
ticks to milliseconds may encounter overflows when handling a 64-bit
value. Adjusting just the value converted to milliseconds to account
for lost 2**32 ticks is also not possible, because 2**32 ticks may not
correspond to an integer number of milliseconds. Therefore the tick
counter overflow is handled as follows:
- A reasonably large number of ticks (the highest multiple of the
ChibiOS timer frequency that fits into uint32_t) is subtracted from
the tick counter, so that its value is again brought below 2**32.
The subtracted value is chosen so that it would correspond to an
integer number of seconds, therefore it could be converted to
milliseconds without any loss of precision.
- The equivalent number of milliseconds is then added to the converted
QMK timer value, so that the QMK timer continues to count
milliseconds as it was before the tick counter overflow.
…reliable The code which extends the 16-bit ChibiOS system timer to a 32-bit tick counter requires that it is called at least once for every overflow of the system timer (otherwise the tick counter can skip one or more overflow periods). Normally this requirement is satisfied just from various parts of QMK code reading the current timer value; however, in some rare circumstances the QMK code may be blocked waiting for some event, and when this situation is combined with having a rather high timer frequency, this may result in improper timekeeping. Enhance the timer reliability by adding a ChibiOS virtual timer which invokes a callback every half of the timer overflow period. The virtual timer callback can be invoked even when the normal QMK code is blocked; the only requirement is that the timer interrupts are enabled, and the ChibiOS kernel is not locked for an excessive time (but the timer update will eventually work correctly if the virtual timer handling is not delayed by more than a half of the timer overflow period). Keeping a virtual timer always active also works around a ChibiOS bug that can manifest with a 16-bit system timer and a relatively high timer frequency: when all active virtual timers have delays longer than the timer overflow period, the handling of virtual timers stops completely. In QMK this bug can result in a `wait_ms()` call with a delay larger than the timer overflow period just hanging indefinitely. However, when the timer update code adds a virtual timer with a shorter delay, all other virtual timers are also handled properly.
tzarc
approved these changes
Feb 4, 2022
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.
Description
Fix several problems in the QMK timer API implementation for ChibiOS:
The code uses a 32-bit tick counter internally, and that counter is overflowing and wrapping around faster than the 32-bit millisecond counter used by QMK:
CH_CFG_ST_FREQUENCYset to 10000 (typically used with the periodic tick mode, or with 16-bit hardware timers), the tick counter overflow happens after slightly less than 5 days of continuous runtime.CH_CFG_ST_FREQUENCYset to 100000 (default for most chips with 32-bit hardware timers), the tick counter overflow happens after slightly less than 5 hours.However, the existing timer code did not handle the tick counter overflow correctly — instead of continuing the millisecond counting further, the QMK timer value was reset back to 0; this broke all code that used timers in various ways (in particular, the recently added deferred executors stopped working completely, because the expected timer values were never reached).
Fixing the problem required adding some code to detect overflows of the 32-bit tick counter and handle them by adjusting two separate offsets — one for the tick counter, another for the converted millisecond value. Using just a single offset was not really possible — passing 64-bit tick values to the time conversion routines could cause overflows during conversion, and adjusting the millisecond value for 232 ticks would result in timing errors, because 232 ticks in many cases do not correspond to an integer number of milliseconds.
I tested the fix with some ridiculous values (
#define CH_CFG_ST_FREQUENCY 96000000on F411, which results in a 32-bit tick counter overflow every ≈45 seconds), and it seems to work.For the 16-bit hardware timer case the timer code relied on
timer_read32()calls being performed at least as frequently as the hardware timer overflows (otherwise some overflow periods would be skipped without advancing the timer value appropriately). However, if the QMK code is blocked waiting for something without doing any timer checks (maybe there is just some large ChibiOS-based timeout for the operation), and at the same time the timer frequency is relatively high (e.g., on some SN32 chips the minimum possible timer frequency is 187500 Hz, because the prescaler is only 8-bit), some hardware timer overflows may still be missed.To make the timekeeping with a 16-bit hardware timer more reliable, I added a ChibiOS virtual timer to the code; that virtual timer fires every half of the hardware timer overflow period, and updates the tick counter state to account for possible hardware timer overflow and wraparound. These periodic calls are usually not very frequent (every ≈3.28 seconds for 10000 Hz, or every ≈328 ms for 100000 Hz; certainly much less frequent than periodic timer interrupts) and should not result in much system load.
In addition to making the timekeeping more reliable, these periodic calls also work around the recently discovered bug in ChibiOS for the 16-bit hardware timer case: when all active virtual timers have delays longer than the hardware timer overflow period, the handling of virtual timers stops completely. In QMK this bug can result in a
wait_ms()call with a delay larger than the hardware timer overflow period just hanging indefinitely. However, when the timer update code adds a virtual timer with a shorter delay, all other virtual timers are also handled properly, even when their delay is longer.Types of Changes
Checklist