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

Update to xealousbrown. #8215

Merged
merged 6 commits into from
Apr 21, 2020
Merged

Update to xealousbrown. #8215

merged 6 commits into from
Apr 21, 2020

Conversation

alex-ong
Copy link
Contributor

@alex-ong alex-ong commented Feb 20, 2020

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

Updated XealousBrown, various performance improvements.

Description

  • Decreased USB polling latency via setting rate from 10ms to 1ms
  • Finally incorporated BOOTMAGIC so I can flash without opening the keyboard up.
  • Used Eager_PK rather than sym_g which has 5ms lead lag time.
  • Used CUSTOM_MATRIX = lite to increase polling rate using optimizations
  • Changed pin polling code to pre-compiled scan for entire column.

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).

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!
@zvecr zvecr added the keyboard label Feb 20, 2020
@zvecr zvecr requested a review from a team February 20, 2020 20:45
@zvecr
Copy link
Member

zvecr commented Feb 20, 2020

While I like to see people using custom matrix lite, you might get the bulk of the performance gains from another solution.

Something like #8216, and setting the new define to 1. While this wont be the same as two noops, it will cut out a significant portion of the idle time without having to jump to custom code.

@alex-ong
Copy link
Contributor Author

While I like to see people using custom matrix lite, you might get the bulk of the performance gains from another solution.

Something like #8216, and setting the new define to 1. While this wont be the same as two noops, it will cut out a significant portion of the idle time without having to jump to custom code.

Thanks for the feedback! There are actually two parts to this; adding the nops "uncaps" the speed, allowing it to reach 10khz+. However the slow part is still the scanning.

The majority of the speedup is actually from get_cols(). I actually tested this over a year ago. The QMK way, though very modular, spends a lot of time setting rows and polling each column individually. It does quite a bit of lookup to achieve it. The TMK way you hardcode some arbitrary bitbash mask and it does it very quickly.

I haven't tested it thoroughly on qmk recently, since I am just applying all my old hacks together. The net increase is from something like 1900 scanrate to 8000 scanrate.

It's nice that QMK has moved a lot of the hacks i made into core code (for example, setting usb polling rate to 1ms, CUSTOM_MATRIX = lite, Debouncing are all core features now)

@zvecr
Copy link
Member

zvecr commented Feb 20, 2020

My experience is that the code within get_cols wouldn't give you much more than a 10% increase. And if you were to produce something like a flame chart, most of the time is spent in the waits.

The net increase is good, but my preference is using core code where possible. Any chance you could you provide the perf difference for just the get_cols part of the change?

@alex-ong
Copy link
Contributor Author

Any easy way to generate such charts? I haven't tested all these speedups part by part recently so Ill try and document the speedups I did, I was going to write a blog post at some point anyway

@drashna drashna requested a review from a team February 21, 2020 01:54
@alex-ong
Copy link
Contributor Author

So my optimizations are three things:

  1. pin-settle time from wait_us(30) to wait_us(1) or nops or nothing at all
  2. select_row() with a bit-bashy implementation
  3. read_cols() with a bit-bashy implementation
  4. unselect_rows() with a bit-bashy implementation.

First, overall performance compared to qmk matrix.c:

pin-settle matrix_scan scan_rate Notes
wait_us(30) qmk_default 1900 standard qmk implementation.
wait_us(1) qmk_default 2635 Changed wait_us by changing quantum/matrix.c
nop; nop; qmk_default 2590 Replaced wait_us in quantum/matrix.c
(no settle) qmk_default 2670 Deleted wait_us in quantum/matrix.c
wait_us(30) bit-bash 3662 custom_matrix = lite
wait_us(1) bit-bash 7290
nop; nop; bit-bash 8200
(no settle) bit-bash 8250

bit-bash means 2,3,4 use the "optimized" version.

Next table shows the performance difference of each part inside the custom_matrix implementation:

pin_settle select_row read_cols unselect_rows scan_rate
nop; nop; fast fast fast 8250
nop; nop; "qmk" fast "qmk" 5200
nop; nop; "qmk" "qmk" "qmk" 2548

"qmk" represents an almost copy paste from qmk's matrix.c back into custom_matrix_lite, with slight modifications. you can see that the last option (2548) roughly matches qmk's real matrix implementation (2590).

But wait! There's more!
Changing a few compiler options also helps.

  1. in TMK's rules.mk, you can enable Optimization level for speed vs size
  2. you can enable LINK_TIME_OPTIMIZATION_ENABLE = yes for smaller/slightly faster binaries
TMK's OPT LINK_TIME_OPTIMIZATION_ENABLE scan_rate binary size
3 null (off) 12036 21814
2 null (off) 11963 19488
1 null (off) 11964 20744
0 (off) null (off) no_compile -
s (default) null (off) 8212 17860
3 yes 13674 20634
2 yes 13655 17324
1 yes 9142 18952
s (default) yes 8564 15926

Anyway its good that I sat down and did the research.

@drashna drashna requested a review from a team March 4, 2020 10:05
@stale
Copy link

stale bot commented Apr 18, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

keyboards/handwired/xealousbrown/rules.mk Outdated Show resolved Hide resolved
keyboards/handwired/xealousbrown/rules.mk Outdated Show resolved Hide resolved
@stale stale bot removed the awaiting changes label Apr 20, 2020
@alex-ong
Copy link
Contributor Author

Thanks for looking at it!

@noroadsleft noroadsleft merged commit 837ffd0 into qmk:master Apr 21, 2020
@noroadsleft
Copy link
Member

Thanks!

As an aside, this board is a standard ANSI 60%, correct?

@alex-ong
Copy link
Contributor Author

Yep, there's something about storing my bindings in user's/key-maps right?

kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
@noroadsleft
Copy link
Member

There is, but that wasn't what prompted the question.

I maintain the default keymaps over on QMK Configurator, and every time I pass by this one in my updates there, I'm reminded that your QMK Configurator implementation isn't correct – a user would have to assign the keys like this to get the expected key assignments.

I'd like to fix it, but the correct way to do that per QMK's conventions requires that I edit your keymap.c and xealousbrown.h files, which I didn't want to do without permission.

As far as storing your bindings in users/, that's up to you, and whether that's worth your time really depends on how many QMK keyboards you own and how you use them. I currently only own one assembled QMK keyboard (have three more in various states of non-completion), so I don't use userspace.

@alex-ong
Copy link
Contributor Author

@noroadsleft Thanks for that.

Feel free to correct it for me (aren't all devs lazy?); otherwise, teach a man to fish... is there a link to qmk conventions so i can correct it myself?

@alex-ong
Copy link
Contributor Author

My guess is i have to change my keymap to this:
image

And then correct my pin mappings to suit?

@noroadsleft
Copy link
Member

@alex-ong You posted while I was writing a response, but you've got it there. You just need that change to the keymap.c, and then to rearrange

#define LAYOUT( \
K00, K01, K02, K03, K04, K05, K06, K07, K08, K09, K0A, K0B, K0C, \
K10, K11, K12, K13, K14, K15, K16, K17, K18, K19, K1A, K1B, K1C, \
K20, K21, K22, K23, K24, K25, K26, K27, K28, K29, K2A, K2B, K2C, \
K30, K31, K32, K33, K34, K35, K36, K37, K38, K39, K3A, K3B, K3C, \
K40, K41, K42, K45, K46, K47, K48, K49, K4C \
) { \

in the same way, so the keycodes that move are still assigned to the same arguments in the layout macro (Backspace as K4C, Backslash as K3C, etc.).

@alex-ong
Copy link
Contributor Author

Alright i'll give it a shot tomorrow night, using GH60 layout as a guide to make sure those arrows in that diagram are right and i'm not missing any KC_NO's. Then will PR for someone to check and tell me i did it wrong lmao.

rockwarrior added a commit to rockwarrior/qmk_firmware that referenced this pull request Apr 23, 2020
* upstream: (1037 commits)
  [Keyboard] Add wasdat code controller (qmk#8858)
  fix sample code indent in feature_encoders.md (qmk#8883)
  [Keymap] Clean up my ergo keymaps and userspace (qmk#8857)
  idb 60 Bugfixes / Preparations for Open Source Hardware (qmk#8866)
  Rebound: add rev2 and thus rev1 as well (qmk#8630)
  Update vitamins included default keymap, enable NKRO, rev2 rgbsplit (qmk#8871)
  Update to xealousbrown. (qmk#8215)
  [Keyboard] DMQ Design SPIN (qmk#8820)
  Wheatfield Blocked65: Update RGBLED num (qmk#8725)
  Add VIA support to ID80 (qmk#8791)
  CFTKB Mysterium & Discipad VIA support (qmk#8794)
  Clean up ATSAM ifdefs (qmk#8808)
  [Docs] Japanese translation of docs/feature_encoders.md (qmk#8843)
  Add naked60 layout, clean up my userspace files and rules.mk. (qmk#8848)
  Fixing DecadePad Numlock LED Bug (qmk#8831)
  [Docs] Japanese translation of docs/feature_command.md (qmk#8672)
  Add support for YMD75 rev 2 (qmk#8853)
  Remove no-longer-necessary LTO checks from keyboards' config.h files (qmk#8773)
  Fix ta-65 tsangan layouts (qmk#8855)
  Fix Plain60 layout (qmk#8854)
  ...
violet-fish pushed a commit to violet-fish/qmk_firmware that referenced this pull request May 3, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
bitherder pushed a commit to bitherder/qmk_firmware that referenced this pull request May 15, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request May 24, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
sjmacneil pushed a commit to sjmacneil/qmk_firmware that referenced this pull request Feb 19, 2021
* Update to xealousbrown.

5-13ms Latency decrease, 4x scan rate improvement.
(CUSTOM_MATRIX = lite) is a really great feature!

* Updated Readme.md, added an extra speedhack.

* More optimizations

* Update keyboards/handwired/xealousbrown/rules.mk

* Update keyboards/handwired/xealousbrown/rules.mk
@alex-ong alex-ong deleted the xealousbrown branch August 3, 2021 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants