Keyboards with multiple bitmask (NKRO) style key reports don't work correctly

[valpackett]

Admittedly I am doing somewhat cursed things with keyboard firmware :D Here it is:

0x05, 0x01,        // Usage Page (Generic Desktop Ctrls)
0x09, 0x06,        // Usage (Keyboard)
0xA1, 0x01,        // Collection (Application)
0x85, 0x05,        //   Report ID (5)
0x05, 0x07,        //   Usage Page (Kbrd/Keypad)
0x19, 0xE0,        //   Usage Minimum (0xE0)
0x29, 0xE7,        //   Usage Maximum (0xE7)
0x15, 0x00,        //   Logical Minimum (0)
0x25, 0x01,        //   Logical Maximum (1)
0x95, 0x08,        //   Report Count (8)
0x75, 0x01,        //   Report Size (1)
0x81, 0x02,        //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x05, 0x07,        //   Usage Page (Kbrd/Keypad)
0x19, 0x00,        //   Usage Minimum (0x00)
0x29, 0x2F,        //   Usage Maximum (0x2F)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0x01, 0x00,  //   Logical Maximum (1)
0x95, 0x30,        //   Report Count (48)
0x75, 0x01,        //   Report Size (1)
0x81, 0x02,        //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0xC0,              // End Collection
0xA1, 0x01,        // Collection (Application)
0x85, 0x06,        //   Report ID (6)
0x05, 0x07,        //   Usage Page (Kbrd/Keypad)
0x19, 0x30,        //   Usage Minimum (0x30)
0x29, 0x67,        //   Usage Maximum (0x67)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0x01, 0x00,  //   Logical Maximum (1)
0x95, 0x38,        //   Report Count (56)
0x75, 0x01,        //   Report Size (1)
0x81, 0x02,        //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0xC0,              // End Collection
0xA1, 0x01,        // Collection (Application)
0x85, 0x07,        //   Report ID (7)
0x05, 0x07,        //   Usage Page (Kbrd/Keypad)
0x19, 0x68,        //   Usage Minimum (0x68)
0x29, 0x77,        //   Usage Maximum (0x77)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0x01, 0x00,  //   Logical Maximum (1)
0x95, 0x18,        //   Report Count (24)
0x75, 0x01,        //   Report Size (1)
0x81, 0x02,        //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0xC0,              // End Collection

Basically this is splitting the key bits

 * byte |0       |1       |2       |3       |4       |5       |6       |7        ... |15
 * -----+--------+--------+--------+--------+--------+--------+--------+--------     +--------
 * desc |mods    |bits[0] |bits[1] |bits[2] |bits[3] |bits[4] |bits[5] |bits[6]  ... |bits[14]

across three reports of max length 8, due to microcontroller USB stack limitations

 * byte |0       |1       |2       |3       |4       |5       |6       |7       
 * -----+--------+--------+--------+--------+--------+--------+--------+--------
 * desc |RID 5   |mods    |bits[0] |bits[1] |bits[2] |bits[3] |bits[4] |bits[5] 

 * byte |0       |1       |2       |3       |4       |5       |6       |7       
 * -----+--------+--------+--------+--------+--------+--------+--------+--------
 * desc |RID 6   |bits[6] |bits[7] |bits[8] |bits[9] |bits[10]|bits[11]|bits[12]

 * byte |0       |1       |2       
 * -----+--------+--------+--------
 * desc |RID 7   |bits[13]|bits[14]

For each key state change the keyboard fires off all three reports in sequence.

This works fine on Linux, keys from the different report IDs actually end up on different corresponding evdevs.

With ukbd, all keys are recognized, but only keys from the last report have working repeat. Seems like each report resets the whole bitmask in the driver, making it forget the keys from the previous reports in the batch.

With hkbd, only keys from the first report are recognized at all.

[wulf7]

The report descriptor is broken. It does not define usage number for 2-nd and 3-rd collections.

[wulf7]

Why does this report descriptor have 3 collections instead of single one? Just defining of 3 reports should be enough.

[valpackett]

hm, usage number is not global for all subsequent collections in the descriptor? okay..

With either a single collection or usage set before each top level collection, hkbd now only has the same issue as ukbd, key repeat only working for the last report.

[wulf7]

hm, usage number is not global for all subsequent collections in the descriptor? okay..

IIRC, Upper 16 bits of usage or UsagePage is global for all HID items including collection, input an so on. Lower 16bit or just Usage is local for each item.

With either a single collection ... , hkbd now only has the same issue as ukbd

It is expected. hkbd is almost untouched copy of ukbd, so it shares all issues with latter on single collection report descriptors.

With ... usage set before each top level collection, hkbd now only has the same issue as ukbd,

You should get 3 evdevs with different keys repeated on different evdevs or not repeating at all, depending on incoming report sequence.

It also worth a try to test hskbd. It was not pushed to 13 and still left only in iichid

[wulf7]

You can test current iichid or port commit aaf8987 to FreeBSD 13/14

[valpackett]

hmm looks like that failed to fix the problem, actually made it worse (now a single report is broken too).

I don't think there should be a condition, but instead only the part of sc_ndata that matches the report descriptor should be cleaned

[wulf7]

instead only the part of sc_ndata that matches the report descriptor should be cleaned

Done. Please test again.

[wulf7]

Maybe, bit clearing should be done in separate pass to properly handle a cases when several physical keys reports the same logical value.

[valpackett]

Okay, take 2 works fine 👍

[wulf7]

bit clearing should be done in separate pass to properly handle a cases when several physical keys reports the same logical value.

Done in take 3. Could you test it?

It should fix autorepeats for keyboards which use boot-alike protocol and coexist with other device like "consumer page" or mouse living on other ReportID also. Such a devices are exotic in USB world but are very common in bluetooth one.

[valpackett]

nah in take 3 repeat is broken again. Possibly because you forgot bit_set(sc->sc_ndata0, key); in the if (hid_get_data(buf, len, &sc->sc_loc_key[i])) branch, retesting with that now…

UPD: yes, that.

--- i/sys/dev/hid/hkbd.c
+++ w/sys/dev/hid/hkbd.c
@@ -731,6 +731,7 @@ hkbd_intr_callback(void *context, void *data, hid_size_t len)
 				continue;
 			/* set key in bitmap */
 			bit_set(sc->sc_ndata, key);
+			bit_set(sc->sc_ndata0, key);
 		}
 	}
 #ifdef HID_DEBUG

[wulf7]

Possibly because you forgot bit_set(sc->sc_ndata0, key); in the if (hid_get_data(buf, len, &sc->sc_loc_key[i])) branch

bit_set(sc->sc_ndata0, key); should not be here. Inserting it wont help. sc_ndata0 and sc_odata0 are not applicable to your device at all. They are used only on 6RKO/bootprotocol keyboards. You can comment out all string containing sc_Xdata0.

I tried to reproduce your case with periodic injecting of packets with different ReportID but it did not break repeats for me.

Strange thing.

[wulf7]

Could you share console output of your case after sysctl hw.hid.hkbd.debug=16 ?

[valpackett]

Oops, sorry, it actually works fine without that line.

*facepalm* What was happening was ukbd taking over again >_< hw.usb.usbhid.enable is not good enough, devd + devmatch can still match ukbd when unloading hkbd. I'll just completely remove ukbd from my build, but idk what's a good upstream solution.

Also, debug required https://reviews.freebsd.org/D28995

[valpackett]

It's been a few months :) maybe it's time to commit these hkbd updates to base?

[wulf7]

Ok. I have started reviewing process: https://reviews.freebsd.org/D31469