Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Use polled waiting on ChibiOS platforms that support it #17607

Merged
merged 3 commits into from
Jul 11, 2022

Conversation

KarlK90
Copy link
Member

@KarlK90 KarlK90 commented Jul 9, 2022

Description

Due to context switching overhead waiting a very short amount of time on a sleeping thread is often not accurate and in fact not usable for timing critical usage i.e. in a driver. Thus we use polled waiting for ranges in the us range on platforms that support it instead. The fallback is the thread sleeping mechanism.

This includes:

  • ARM platforms with CYCCNT register (ARMv7, ARMv8) this is incremented at CPU clock frequency
  • GD32VF103 RISC-V port with CSR_MCYCLE register this is incremented at CPU clock frequency
  • RP2040 ARMv6 port which uses the integrated timer peripheral which is incremented with a fixed 1MHz frequency

Comparison busy waiting vs sleeping

Waiting values are 0, 1, 5, 10, 25, 50, 100, 150, 200, 500 and 1000 microseconds with 10us spacing. Pink is busy waiting, green is sleeping by yielding the thread.

STM32F411

stm32f411

busy waiting

  • minimal busy waiting: 1us
  • minimal sleeping time: 111us

RP2040

rp2040

  • minimal busy waiting: 1us
  • minimal sleeping time: 28us

GD32VF103

gd32vf103

  • minimal busy waiting: 1us
  • minimal sleeping time: 189us

Matrix scanning frequency on DZZY keypad with 5x6 matrix:

  • busy waiting: 4.6k (575%!)
  • thread sleeping: 800

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

  • Overhead in pointing device drivers and matrix scanning

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@KarlK90 KarlK90 force-pushed the feature/busy-wait-us branch from 56a4f8f to a34bb8a Compare July 9, 2022 12:42
@KarlK90 KarlK90 requested a review from a team July 9, 2022 12:58
@KarlK90 KarlK90 mentioned this pull request Jul 10, 2022
14 tasks
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

platforms/chibios/drivers/serial.c Outdated Show resolved Hide resolved
@KarlK90 KarlK90 force-pushed the feature/busy-wait-us branch from a34bb8a to 4c6f706 Compare July 11, 2022 08:57
@drashna drashna requested a review from a team July 11, 2022 11:05
@KarlK90 KarlK90 force-pushed the feature/busy-wait-us branch from 4c6f706 to 868d80c Compare July 11, 2022 11:29
@KarlK90 KarlK90 force-pushed the feature/busy-wait-us branch from 868d80c to e215c93 Compare July 11, 2022 12:07
@github-actions github-actions bot added cli qmk cli command python labels Jul 11, 2022
KarlK90 added 3 commits July 11, 2022 14:09
Due to context switching overhead waiting a very short amount of time on
a sleeping thread is often not accurate and in fact not usable for timing
critical usage i.e. in a driver. Thus we use polled waiting for ranges
in the us range on platforms that support it instead. The fallback is
the thread sleeping mechanism.

This includes:

* ARM platforms with CYCCNT register (ARMv7, ARMv8) this is
  incremented at CPU clock frequency
* GD32VF103 RISC-V port with CSR_MCYCLE register this is incremented at
  CPU clock frequency
* RP2040 ARMv6 port which uses the integrated timer peripheral which is
  incremented with a fixed 1MHz frequency
...as it is powered by busy waiting now.
@KarlK90 KarlK90 force-pushed the feature/busy-wait-us branch from e215c93 to 101cff2 Compare July 11, 2022 12:09
@github-actions github-actions bot removed python cli qmk cli command labels Jul 11, 2022
@KarlK90 KarlK90 merged commit 3f5dc47 into qmk:develop Jul 11, 2022
mtei added a commit to mtei/qmk_firmware that referenced this pull request Nov 10, 2022
Since qmk's standard wait_us() is now accurate due to the changes in qmk#17607, the workaround is no longer necessary.
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
…7607)

* Use polled waiting on platforms that support it

Due to context switching overhead waiting a very short amount of time on
a sleeping thread is often not accurate and in fact not usable for timing
critical usage i.e. in a driver. Thus we use polled waiting for ranges
in the us range on platforms that support it instead. The fallback is
the thread sleeping mechanism.

This includes:

* ARM platforms with CYCCNT register (ARMv7, ARMv8) this is
  incremented at CPU clock frequency
* GD32VF103 RISC-V port with CSR_MCYCLE register this is incremented at
  CPU clock frequency
* RP2040 ARMv6 port which uses the integrated timer peripheral which is
  incremented with a fixed 1MHz frequency

* Use wait_us() instead of chSysPolledDelayX

...as it is powered by busy waiting now.

* Add chibios waiting methods test bench
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants