From 06982df5ea44131c7ec9d108da31ad6e4ead13da Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 15:07:42 -0800 Subject: [PATCH 01/11] Hint to GCC that a few functions are critical path Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index 2feaa996..f71c2fc5 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -59,7 +59,7 @@ void ATMegaKeyboard::attachToHost() { UDCON &= ~_BV(DETACH); } -void ATMegaKeyboard::readMatrix(void) { +void __attribute__((optimize(3))) ATMegaKeyboard::readMatrix(void) { for (uint8_t current_row = 0; current_row < KeyboardHardware.matrix_rows; current_row++) { uint16_t mask, cols; @@ -68,6 +68,7 @@ void ATMegaKeyboard::readMatrix(void) { mask = KeyboardHardware.debounceMaskForRow(current_row); OUTPUT_TOGGLE(KeyboardHardware.matrix_row_pins[current_row]); + asm volatile("nop"); cols = (KeyboardHardware.readCols() & mask) | (KeyboardHardware.keyState_[current_row] & ~mask); OUTPUT_TOGGLE(KeyboardHardware.matrix_row_pins[current_row]); KeyboardHardware.debounceRow(cols ^ KeyboardHardware.keyState_[current_row], current_row); @@ -95,7 +96,7 @@ bool ATMegaKeyboard::isKeyswitchPressed(uint8_t keyIndex) { keyIndex % KeyboardHardware.matrix_columns); } -void ATMegaKeyboard::actOnMatrixScan() { +void __attribute__((optimize(3))) ATMegaKeyboard::actOnMatrixScan() { for (byte row = 0; row < KeyboardHardware.matrix_rows; row++) { for (byte col = 0; col < KeyboardHardware.matrix_columns; col++) { uint8_t keyState = (bitRead(KeyboardHardware.previousKeyState_[row], col) << 0) | From f78bd53faf5dc7aa67527dd32ce79205124f6429 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 15:08:08 -0800 Subject: [PATCH 02/11] Remove handcoded optimization we no longer need Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/technomancy/Atreus.cpp | 16 ---------------- src/kaleidoscope/hardware/technomancy/Atreus.h | 1 - 2 files changed, 17 deletions(-) diff --git a/src/kaleidoscope/hardware/technomancy/Atreus.cpp b/src/kaleidoscope/hardware/technomancy/Atreus.cpp index 1cc441dd..e41e3677 100644 --- a/src/kaleidoscope/hardware/technomancy/Atreus.cpp +++ b/src/kaleidoscope/hardware/technomancy/Atreus.cpp @@ -70,22 +70,6 @@ void Atreus::resetDevice() { asm volatile("jmp 0x7E00"); } -uint16_t Atreus::readCols() { - uint16_t results = 0; - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[0]) << 0); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[1]) << 1); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[2]) << 2); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[3]) << 3); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[4]) << 4); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[5]) << 5); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[6]) << 6); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[7]) << 7); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[8]) << 8); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[9]) << 9); - results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[10]) << 10); - - return results; -} } } diff --git a/src/kaleidoscope/hardware/technomancy/Atreus.h b/src/kaleidoscope/hardware/technomancy/Atreus.h index 1ac4c393..d4f0821b 100644 --- a/src/kaleidoscope/hardware/technomancy/Atreus.h +++ b/src/kaleidoscope/hardware/technomancy/Atreus.h @@ -52,7 +52,6 @@ class Atreus: public kaleidoscope::hardware::ATMegaKeyboard { void resetDevice(); protected: - uint16_t readCols(); }; #define KEYMAP( \ From 854fba3ff78f214cd79639f66c1c0df2b55a69f9 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 15:56:59 -0800 Subject: [PATCH 03/11] Revert "Remove unneeded includes" This reverts commit 91b7c0c92378825a45790307af3c9e92ae07cc6f. Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/olkb/Planck.cpp | 1 + src/kaleidoscope/hardware/technomancy/Atreus.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/kaleidoscope/hardware/olkb/Planck.cpp b/src/kaleidoscope/hardware/olkb/Planck.cpp index 2a70ba99..5597171f 100644 --- a/src/kaleidoscope/hardware/olkb/Planck.cpp +++ b/src/kaleidoscope/hardware/olkb/Planck.cpp @@ -18,6 +18,7 @@ #ifdef ARDUINO_AVR_PLANCK #include +#include namespace kaleidoscope { namespace hardware { diff --git a/src/kaleidoscope/hardware/technomancy/Atreus.cpp b/src/kaleidoscope/hardware/technomancy/Atreus.cpp index e41e3677..61a7c375 100644 --- a/src/kaleidoscope/hardware/technomancy/Atreus.cpp +++ b/src/kaleidoscope/hardware/technomancy/Atreus.cpp @@ -27,6 +27,7 @@ #ifdef ARDUINO_AVR_ATREUS #include +#include namespace kaleidoscope { namespace hardware { From 3e9552cf44e2321376d6e555dd42d7f266e1a24c Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 16:55:54 -0800 Subject: [PATCH 04/11] Switch our debouncing to only run 3 times per 5 ms, which is closer to what we 'really' want per switch manufacturers Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index f71c2fc5..1a41678c 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -40,11 +40,11 @@ void ATMegaKeyboard::setup(void) { OUTPUT_HIGH(KeyboardHardware.matrix_row_pins[i]); } - /* Set up Timer1 for 500usec */ + /* Set up Timer1 for 1700usec */ TCCR1B = _BV(WGM13); TCCR1A = 0; - const uint32_t cycles = (F_CPU / 2000000) * 500; + const uint32_t cycles = (F_CPU / 2000000) * 1700; ICR1 = cycles; TCCR1B = _BV(WGM13) | _BV(CS10); From 7f6c26d6583c2a432201177f5f8b4dc96077eb50 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 16:57:20 -0800 Subject: [PATCH 05/11] Report the current keyboard state to Kaleidoscope on every loop not just when the timer triggers Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index 1a41678c..807f3682 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -110,12 +110,12 @@ void __attribute__((optimize(3))) ATMegaKeyboard::actOnMatrixScan() { } void ATMegaKeyboard::scanMatrix() { - if (!do_scan_) - return; - - do_scan_ = false; - - KeyboardHardware.readMatrix(); + if (do_scan_) { + do_scan_ = false; + // We only want to update our matrix if the timer has expired. + KeyboardHardware.readMatrix(); + } + // We ALWAYS want to tell Kaleidoscope about the state of the matrix KeyboardHardware.actOnMatrixScan(); } From d193d7bba0eea1b527e8aba0edd26bed9896e3b6 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 17:03:30 -0800 Subject: [PATCH 06/11] Reset our "last state" when updating Kaleidoscope, rather than when reading. (This eliminates a "chatter" causing bug Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index 807f3682..5fead89c 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -63,8 +63,6 @@ void __attribute__((optimize(3))) ATMegaKeyboard::readMatrix(void) { for (uint8_t current_row = 0; current_row < KeyboardHardware.matrix_rows; current_row++) { uint16_t mask, cols; - KeyboardHardware.previousKeyState_[current_row] = KeyboardHardware.keyState_[current_row]; - mask = KeyboardHardware.debounceMaskForRow(current_row); OUTPUT_TOGGLE(KeyboardHardware.matrix_row_pins[current_row]); From fa94cb6e20603c8ebfd677b1909a4791593abfd1 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 20 Nov 2018 19:59:22 -0800 Subject: [PATCH 07/11] Correct where we need to pause before reading Otherwise, for example, Pin F0 may not be read on the Planck Signed-off-by: Jesse Vincent --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index 5fead89c..230eb3de 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -66,7 +66,6 @@ void __attribute__((optimize(3))) ATMegaKeyboard::readMatrix(void) { mask = KeyboardHardware.debounceMaskForRow(current_row); OUTPUT_TOGGLE(KeyboardHardware.matrix_row_pins[current_row]); - asm volatile("nop"); cols = (KeyboardHardware.readCols() & mask) | (KeyboardHardware.keyState_[current_row] & ~mask); OUTPUT_TOGGLE(KeyboardHardware.matrix_row_pins[current_row]); KeyboardHardware.debounceRow(cols ^ KeyboardHardware.keyState_[current_row], current_row); @@ -141,6 +140,7 @@ bool ATMegaKeyboard::isKeyMasked(byte row, byte col) { uint16_t ATMegaKeyboard::readCols() { uint16_t results = 0x00 ; for (uint8_t i = 0; i < KeyboardHardware.matrix_columns; i++) { + asm("NOP"); // We need to pause a beat before reading or we may read before the pin is hot results |= (!READ_PIN(KeyboardHardware.matrix_col_pins[i]) << i); } return results; From 6abc406172a1e08d575557f43e23212013d174dd Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 21 Nov 2018 07:09:07 +0100 Subject: [PATCH 08/11] hardware/ErgoDox: Switch debouncing to ~3 times / 5ms Based on the change made to `ATMegaKeyboard`, apply the same treatment to the `ErgoDoxScanner` too, and run debouncing only 3 times per 5ms, which is closer to what switch manufacturers recommend. Signed-off-by: Gergely Nagy --- src/kaleidoscope/hardware/ez/ErgoDox.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kaleidoscope/hardware/ez/ErgoDox.cpp b/src/kaleidoscope/hardware/ez/ErgoDox.cpp index 5b183c3c..ce12d28e 100644 --- a/src/kaleidoscope/hardware/ez/ErgoDox.cpp +++ b/src/kaleidoscope/hardware/ez/ErgoDox.cpp @@ -67,11 +67,11 @@ void ErgoDox::setup(void) { setStatusLEDBrightness(2, 15); setStatusLEDBrightness(3, 15); - /* Set up Timer1 for 500usec */ + /* Set up Timer1 for 1700usec */ TCCR1B = _BV(WGM13); TCCR1A = 0; - const uint32_t cycles = (F_CPU / 2000000) * 500; + const uint32_t cycles = (F_CPU / 2000000) * 1700; ICR1 = cycles; TCCR1B = _BV(WGM13) | _BV(CS10); From 3202bc417e403e90b58aeeb42d2ba73f6552c5fc Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 21 Nov 2018 07:11:05 +0100 Subject: [PATCH 09/11] hardware/ErgoDox: Reset our latest state when updating Kaleidoscope Set the previous key state when reporting the state to Kaleidoscope, instead of on every read. This eliminates a possible chatter bug. Idea taken from a similar change made to `ATMegaKeyboard`. Signed-off-by: Gergely Nagy --- src/kaleidoscope/hardware/ez/ErgoDox.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kaleidoscope/hardware/ez/ErgoDox.cpp b/src/kaleidoscope/hardware/ez/ErgoDox.cpp index ce12d28e..92ee98b9 100644 --- a/src/kaleidoscope/hardware/ez/ErgoDox.cpp +++ b/src/kaleidoscope/hardware/ez/ErgoDox.cpp @@ -85,7 +85,6 @@ ISR(TIMER1_OVF_vect) { void ErgoDox::readMatrixRow(uint8_t row) { uint8_t mask, cols; - previousKeyState_[row] = keyState_[row]; mask = debounceMaskForRow(row); cols = (scanner_.readCols(row) & mask) | (keyState_[row] & ~mask); debounceRow(cols ^ keyState_[row], row); @@ -116,6 +115,7 @@ void ErgoDox::actOnMatrixScan() { if (keyState) handleKeyswitchEvent(Key_NoKey, row, col, keyState); } + previousKeyState_[row] = keyState_[row]; } } From e63341babb3ee30fdd4ea056fa87eb58b9d7fb31 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 21 Nov 2018 07:12:40 +0100 Subject: [PATCH 10/11] hardware/ErgoDox: Report the current keyboard state every cycle Instead of only reporting the state when the timer triggers, report it every loop, even if it is unchanged. This is based on a similar change to `ATMegaKeyboard`. Signed-off-by: Gergely Nagy --- src/kaleidoscope/hardware/ez/ErgoDox.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/kaleidoscope/hardware/ez/ErgoDox.cpp b/src/kaleidoscope/hardware/ez/ErgoDox.cpp index 92ee98b9..088be547 100644 --- a/src/kaleidoscope/hardware/ez/ErgoDox.cpp +++ b/src/kaleidoscope/hardware/ez/ErgoDox.cpp @@ -120,10 +120,13 @@ void ErgoDox::actOnMatrixScan() { } void ErgoDox::scanMatrix() { - if (!do_scan_) - return; + if (do_scan_) { + do_scan_ = false; + // We only want to update our matrix if the timer has expired. + readMatrix(); + } - readMatrix(); + // We ALWAYS want to tell Kaleidoscope about the state of the matrix actOnMatrixScan(); } From b33f4b5fc4ca7a6e363cec50af5e35631d97ca7f Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 21 Nov 2018 07:14:24 +0100 Subject: [PATCH 11/11] hardware/ErgoDox: Hint at GCC that some functions are critical path Based on a similar change to `ATMegaKeyboard`. Signed-off-by: Gergely Nagy --- src/kaleidoscope/hardware/ez/ErgoDox.cpp | 6 +++--- src/kaleidoscope/hardware/ez/ErgoDox/ErgoDoxScanner.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/kaleidoscope/hardware/ez/ErgoDox.cpp b/src/kaleidoscope/hardware/ez/ErgoDox.cpp index 088be547..7150ef29 100644 --- a/src/kaleidoscope/hardware/ez/ErgoDox.cpp +++ b/src/kaleidoscope/hardware/ez/ErgoDox.cpp @@ -82,7 +82,7 @@ ISR(TIMER1_OVF_vect) { do_scan_ = true; } -void ErgoDox::readMatrixRow(uint8_t row) { +void __attribute__((optimize(3))) ErgoDox::readMatrixRow(uint8_t row) { uint8_t mask, cols; mask = debounceMaskForRow(row); @@ -91,7 +91,7 @@ void ErgoDox::readMatrixRow(uint8_t row) { keyState_[row] = cols; } -void ErgoDox::readMatrix() { +void __attribute__((optimize(3))) ErgoDox::readMatrix() { do_scan_ = false; scanner_.reattachExpanderOnError(); @@ -107,7 +107,7 @@ void ErgoDox::readMatrix() { } } -void ErgoDox::actOnMatrixScan() { +void __attribute__((optimize(3))) ErgoDox::actOnMatrixScan() { for (byte row = 0; row < ROWS; row++) { for (byte col = 0; col < COLS; col++) { uint8_t keyState = (bitRead(previousKeyState_[row], col) << 0) | diff --git a/src/kaleidoscope/hardware/ez/ErgoDox/ErgoDoxScanner.cpp b/src/kaleidoscope/hardware/ez/ErgoDox/ErgoDoxScanner.cpp index aed1c342..c2042c8f 100644 --- a/src/kaleidoscope/hardware/ez/ErgoDox/ErgoDoxScanner.cpp +++ b/src/kaleidoscope/hardware/ez/ErgoDox/ErgoDoxScanner.cpp @@ -99,7 +99,7 @@ ErgoDoxScanner::begin() { initCols(); } -void +void __attribute__((optimize(3))) ErgoDoxScanner::selectRow(int row) { if (row < 7) { if (!expander_error_) { @@ -149,7 +149,7 @@ out: } } -void +void __attribute__((optimize(3))) ErgoDoxScanner::unselectRows() { DDRB &= ~(1 << 0 | 1 << 1 | 1 << 2 | 1 << 3); PORTB &= ~(1 << 0 | 1 << 1 | 1 << 2 | 1 << 3); @@ -159,7 +159,7 @@ ErgoDoxScanner::unselectRows() { PORTC &= ~(1 << 6); } -uint8_t +uint8_t __attribute__((optimize(3))) ErgoDoxScanner::readCols(int row) { if (row < 7) { if (expander_error_) {