improve ergodox ez performance

With these changes, the ergodox ez goes from 315 scans per second
when no keys are pressed (~3.17ms/scan) to 447 (~2.24ms/scan).

The changes to the pin read are just condensing the logic, and
replacing a lot of conditional operations with a single bitwise
inversion.

The change to row scanning is more significant, and merits
explanation. In general, you can only scan one row of a keyboard
at a time, because if you scan two rows, you no longer know
which row is pulling a given column down. But in the Ergodox
design, this isn't the case; the left hand is controlled by an
I2C-based GPIO expander, and the columns and rows are *completely
separate* electrically from the columns and rows on the right-hand
side.

So simply reading rows in parallel offers two significant
improvements. One is that we no longer need the 30us delay after
each right-hand row, because we're spending more than 30us
communicating with the left hand over i2c. Another is that we're
no longer wastefully sending i2c messages to the left hand
to unselect rows when no rows had actually been selected in the
first place. These delays were, between them, coming out to
nearly 30% of the time spent in each scan.

Signed-off-by: seebs <seebs@seebs.net>

2 files changed, 34 insertions, 20 deletions

diff --git a/keyboards/ergodox_ez/config.h b/keyboards/ergodox_ez/config.h
index bf3ebceb11..cbe615c09b 100644
--- a/keyboards/ergodox_ez/config.h
+++ b/keyboards/ergodox_ez/config.h
@@ -31,6 +31,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 /* key matrix size */
 #define MATRIX_ROWS 14
+#define MATRIX_ROWS_PER_SIDE (MATRIX_ROWS / 2)
 #define MATRIX_COLS 6
 
 #define MOUSEKEY_INTERVAL       20
@@ -80,7 +81,16 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #define RGBW 1
 
-/* Set 0 if debouncing isn't needed */
+/* "debounce" is measured in keyboard scans. Some users reported
+ * needing values as high as 15, which was at the time around 50ms.
+ * If you don't define it here, the matrix code will default to
+ * 5, which is now closer to 10ms, but still plenty according to
+ * manufacturer specs.
+ *
+ * Default is quite high, because of reports with some production
+ * runs seeming to need it. This may change when configuration for
+ * this is more directly exposed.
+ */
 #define DEBOUNCE    15
 
 #define PREVENT_STUCK_MODIFIERS
diff --git a/keyboards/ergodox_ez/matrix.c b/keyboards/ergodox_ez/matrix.c
index 70efb43d8e..f0550e9b88 100644
--- a/keyboards/ergodox_ez/matrix.c
+++ b/keyboards/ergodox_ez/matrix.c
@@ -47,7 +47,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
  * Now it's only 317 scans/second, or about 3.15 msec/scan.
  * According to Cherry specs, debouncing time is 5 msec.
  *
- * And so, there is no sense to have DEBOUNCE higher than 2.
+ * However, some switches seem to have higher debouncing requirements, or
+ * something else might be wrong. (Also, the scan speed has improved since
+ * that comment was written.)
  */
 
 #ifndef DEBOUNCE
@@ -203,16 +205,23 @@ uint8_t matrix_scan(void)
 #endif
 
 #ifdef LEFT_LEDS
-     mcp23018_status = ergodox_left_leds_update();
+    mcp23018_status = ergodox_left_leds_update();
 #endif // LEFT_LEDS
-    for (uint8_t i = 0; i < MATRIX_ROWS; i++) {
+    for (uint8_t i = 0; i < MATRIX_ROWS_PER_SIDE; i++) {
         select_row(i);
-        wait_us(30);  // without this wait read unstable value.
+        // and select on left hand
+        select_row(i + MATRIX_ROWS_PER_SIDE);
+        // we don't need a 30us delay anymore, because selecting a
+        // left-hand row requires more than 30us for i2c.
         matrix_row_t mask = debounce_mask(i);
         matrix_row_t cols = (read_cols(i) & mask) | (matrix[i] & ~mask);
         debounce_report(cols ^ matrix[i], i);
         matrix[i] = cols;
-
+        // grab cols from right hand
+        mask = debounce_mask(i + MATRIX_ROWS_PER_SIDE);
+        cols = (read_cols(i + MATRIX_ROWS_PER_SIDE) & mask) | (matrix[i + MATRIX_ROWS_PER_SIDE] & ~mask);
+        debounce_report(cols ^ matrix[i + MATRIX_ROWS_PER_SIDE], i + MATRIX_ROWS_PER_SIDE);
+        matrix[i + MATRIX_ROWS_PER_SIDE] = cols;
         unselect_rows();
     }
 
@@ -295,14 +304,13 @@ static matrix_row_t read_cols(uint8_t row)
             return data;
         }
     } else {
-        // read from teensy
-        return
-            (PINF&(1<<0) ? 0 : (1<<0)) |
-            (PINF&(1<<1) ? 0 : (1<<1)) |
-            (PINF&(1<<4) ? 0 : (1<<2)) |
-            (PINF&(1<<5) ? 0 : (1<<3)) |
-            (PINF&(1<<6) ? 0 : (1<<4)) |
-            (PINF&(1<<7) ? 0 : (1<<5)) ;
+        /* read from teensy
+	 * bitmask is 0b11110011, but we want those all
+	 * in the lower six bits.
+	 * we'll return 1s for the top two, but that's harmless.
+	 */
+
+        return ~((PINF & 0x03) | ((PINF & 0xF0) >> 2));
     }
 }
 
@@ -325,9 +333,7 @@ static void unselect_rows(void)
         // set all rows hi-Z : 1
         mcp23018_status = i2c_start(I2C_ADDR_WRITE);    if (mcp23018_status) goto out;
         mcp23018_status = i2c_write(GPIOA);             if (mcp23018_status) goto out;
-        mcp23018_status = i2c_write( 0xFF
-                              & ~(0<<7)
-                          );                            if (mcp23018_status) goto out;
+        mcp23018_status = i2c_write(0xFF);              if (mcp23018_status) goto out;
     out:
         i2c_stop();
     }
@@ -353,9 +359,7 @@ static void select_row(uint8_t row)
             // set other rows hi-Z : 1
             mcp23018_status = i2c_start(I2C_ADDR_WRITE);        if (mcp23018_status) goto out;
             mcp23018_status = i2c_write(GPIOA);                 if (mcp23018_status) goto out;
-            mcp23018_status = i2c_write( 0xFF & ~(1<<row)
-                                  & ~(0<<7)
-                              );                                if (mcp23018_status) goto out;
+            mcp23018_status = i2c_write(0xFF & ~(1<<row));      if (mcp23018_status) goto out;
         out:
             i2c_stop();
         }