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.

Sign up for GitHub

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

Jump to bottom

Allow 30us matrix delay to be keyboard/user overridable #8216

Merged
merged 3 commits into from
Feb 21, 2020
Merged

Allow 30us matrix delay to be keyboard/user overridable #8216

merged 3 commits into from
Feb 21, 2020

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Feb 20, 2020

Description

Hitting some limits when testing the arm uart serial code, this allows the delay while waiting for IO to settle to be configurable.

Ive tested as low as 5us with success.

Types of Changes

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

Checklist

  • My code follows the code style of this project.
  • 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).

@zvecr zvecr requested a review from a team February 20, 2020 20:44
@zvecr zvecr mentioned this pull request Feb 20, 2020
Merged
13 tasks
@alex-ong
Copy link
Contributor

Nice feature, it will speed up scan rate significantly. It might be a better idea to set how many nops, rather than how many microseconds? I think looking up our set of devices (atmega32u4, cortex armv3 etc etc) to see if pin settling time is defined in microseconds vs nops might be desirable.

Finally, since i believe this figure is inherently tied to which microprocessor we are using, it could be defined by default per microprocessor and then this override could still be used (still up for discussion on whether it be defined in nops or microseconds).

@zvecr
Copy link
Member Author

zvecr commented Feb 20, 2020

I would rather avoid nops where possible, as some STM32 boards require the wait for yielding (so that the USB thread gets runtime).

An alternative implementation is in 18c948a, where it would allow the replacement of the delay logic at the expense of a function call.

@alex-ong
Copy link
Contributor

alex-ong commented Feb 20, 2020 •
edited
Loading

What about an "inline" function call? I remember that it isn't exactly guaranteed because it's just a suggestion to the compiler... Especially since two nops is significantly faster than a function call.

@zvecr
Copy link
Member Author

zvecr commented Feb 20, 2020 •
edited
Loading

inline cant be weak, so thats a non starter, it would have to be a define which you redef.

We are talking tiny amounts of time here. Given the current 30us wait, i doubt will make a measurable difference.

@alex-ong
Copy link
Contributor

alex-ong commented Feb 20, 2020 •
edited
Loading

Right, going from 30us to 1us will make the biggest difference. Qmk is always going to be battle between open apis and power users who want specific functionality at no additional cost to Rom size / speed.

I think an overrideable function is better than just using pure us and is the most flexible option.

Do you know how long a function call takes, and how long 2 nops is compared to 1us? If we know that (a function call + 2nop ) > 1us we might as well make the API only support us increments.

Edit: at 16mhz, two nops takes 1.2~ microsceconds. So seems like we can use pure us to define all waits?

@drashna drashna requested a review from a team February 21, 2020 01:49
@tzarc tzarc merged commit 7707724 into qmk:master Feb 21, 2020
@alex-ong
Copy link
Contributor

I'll have a look at speed testing this vs my scanning code, and ideally bringing back xealous-brown to a simpler default implementation :D

nesth pushed a commit to nesth/qmk_firmware that referenced this pull request Feb 21, 2020
Merge remote-tracking branch 'upstream/master'
2a8ece2 
* upstream/master: (330 commits)
  Add Danish keymap and sendstring LUT (qmk#8218)
  format code according to conventions [skip ci]
  uart.c fix from TMK (qmk#7628)
  S75 Encoder Fixes (qmk#7758)
  Add Turkish keymap aliases and sendstring LUT (qmk#7676)
  Add Arm Teensys to mcu_selection.mk (qmk#8026)
  [New keyboard]silverbullet44 (qmk#7950)
  Allow 30us matrix delay to be keyboard/user overridable  (qmk#8216)
  Merge /prime_l and /prime_l_v2 (qmk#8194)
  [Keymap] Keymap for XD75 with 7U spacebar EN-RU gamers (qmk#8184)
  Add VIA support for kbd8x mk2 (qmk#8168)
  Move Grave/Tilde and some lesser used keys on my ergo boards (qmk#8200)
  [Keyboard] KC60SE cleanup (qmk#8171)
  Made windows driver installation accept y as All to allow CI (qmk#8189)
  Change the image photo of /keyboards/reviung41/readme.md (qmk#8195)
  MxSS RGB Handler Touchup (qmk#8105)
  Centromere Configurator layout support and readme update (qmk#8190)
  dynamic keymap sanity check (qmk#8181)
  [keyboard] Austin (qmk#8176)
  Use pathlib everywhere we can (qmk#7872)
  ...
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request Feb 23, 2020
@zvecr @sowbug
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
e521aed 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
nesth pushed a commit to nesth/qmk_firmware that referenced this pull request Feb 27, 2020
Merge branch 'master' of https://github.com/nesth/qmk_firmware
70a02ed 
* 'master' of https://github.com/nesth/qmk_firmware: (331 commits)
  helix like crkbd
  Add Danish keymap and sendstring LUT (qmk#8218)
  format code according to conventions [skip ci]
  uart.c fix from TMK (qmk#7628)
  S75 Encoder Fixes (qmk#7758)
  Add Turkish keymap aliases and sendstring LUT (qmk#7676)
  Add Arm Teensys to mcu_selection.mk (qmk#8026)
  [New keyboard]silverbullet44 (qmk#7950)
  Allow 30us matrix delay to be keyboard/user overridable  (qmk#8216)
  Merge /prime_l and /prime_l_v2 (qmk#8194)
  [Keymap] Keymap for XD75 with 7U spacebar EN-RU gamers (qmk#8184)
  Add VIA support for kbd8x mk2 (qmk#8168)
  Move Grave/Tilde and some lesser used keys on my ergo boards (qmk#8200)
  [Keyboard] KC60SE cleanup (qmk#8171)
  Made windows driver installation accept y as All to allow CI (qmk#8189)
  Change the image photo of /keyboards/reviung41/readme.md (qmk#8195)
  MxSS RGB Handler Touchup (qmk#8105)
  Centromere Configurator layout support and readme update (qmk#8190)
  dynamic keymap sanity check (qmk#8181)
  [keyboard] Austin (qmk#8176)
  ...
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Mar 5, 2020
@zvecr @HokieGeek
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
02c1f74 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
@zvecr @c0psrul3
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
42f8863 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Mar 26, 2020
@zvecr @drashna
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
583bd29 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
@zvecr @kujawakk
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
0a7d386 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
@zvecr zvecr deleted the feature/matrix_delay branch April 28, 2020 01:14
jakeisnt pushed a commit to jakeisnt/qmk_firmware that referenced this pull request Aug 20, 2020
@zvecr @jakeisnt
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
b18985e 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@zvecr
Allow 30us matrix delay to be keyboard/user overridable (qmk#8216)
19e4bbf 
* Allow 30us matrix delay to be configurable via define

* Move wait logic to matrix_common

* Move wait logic to matrix_common - fix wait includes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzarc tzarc tzarc approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
core documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zvecr @alex-ong @drashna @tzarc