Messages in this thread | | | Subject | Re: [PATCH v2] Input: Fix the HID usage of DPAD input event generation. | From | Chris Ye <> | Date | Thu, 5 Nov 2020 09:22:55 -0800 |
| |
+stable@vger.kernel.org
On 11/3/20 9:36 AM, Benjamin Tissoires wrote: > Hi Chris, > > On Mon, Nov 2, 2020 at 6:24 PM Chris Ye <linzhao.ye@gmail.com> wrote: >> Hi Benjamin, >> >> I've tried the hid-tool for testing on my linux machine and it >> works. However the issue comes from a game controller I don't posses in >> hand right now so I can't physically connect it and provide the log from >> hid-tool recording. I do have the raw HID descriptor and in Linux >> kernel the debug info is like below: >> >> # cat /d/hid/0005\:27F8\:0BBE.0001/rdesc >> 05 01 09 05 a1 01 a1 02 15 81 25 7f 05 01 09 01 a1 00 75 08 95 04 35 00 >> 46 ff 00 09 30 09 31 09 32 09 35 81 02 75 08 95 02 15 01 26 ff 00 09 39 >> 09 39 81 02 c0 05 07 19 4f 29 52 15 00 25 01 75 01 95 04 81 02 05 01 09 >> 90 09 91 09 92 09 93 75 01 95 04 81 02 75 01 95 10 05 09 19 01 29 10 81 >> 02 06 02 ff 09 01 a1 01 15 00 25 01 09 04 75 01 95 01 81 02 c0 05 0c 09 >> 01 a1 01 15 00 25 01 0a 24 02 75 01 95 01 81 06 c0 75 01 95 06 81 03 15 >> 00 25 ff 05 02 09 01 a1 00 75 08 95 02 35 00 45 ff 09 c4 09 c5 81 02 c0 >> 06 00 ff 09 80 75 08 95 08 15 00 26 ff 00 b1 02 c0 c0 > Thanks for the report descriptor. That allowed me to add the device to > the tests at https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice > > And also to realize that... "how is that thing supposed to be working????" > >> INPUT[INPUT] >> Field(0) >> Physical(GenericDesktop.Pointer) >> Application(GenericDesktop.GamePad) >> Usage(4) >> GenericDesktop.X >> GenericDesktop.Y >> GenericDesktop.Z >> GenericDesktop.Rz >> Logical Minimum(-127) >> Logical Maximum(127) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(4) >> Report Offset(0) >> Flags( Variable Absolute ) >> Field(1) >> Physical(GenericDesktop.Pointer) >> Application(GenericDesktop.GamePad) >> Usage(2) >> GenericDesktop.HatSwitch >> GenericDesktop.HatSwitch >> Logical Minimum(1) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(2) >> Report Offset(32) >> Flags( Variable Absolute ) >> Field(2) >> Application(GenericDesktop.GamePad) >> Usage(4) >> Keyboard.004f >> Keyboard.0050 >> Keyboard.0051 >> Keyboard.0052 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(4) >> Report Offset(48) >> Flags( Variable Absolute ) >> Field(3) >> Application(GenericDesktop.GamePad) >> Usage(4) >> GenericDesktop.D-PadUp >> GenericDesktop.D-PadDown >> GenericDesktop.D-PadRight >> GenericDesktop.D-PadLeft >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(4) >> Report Offset(52) >> Flags( Variable Absolute ) >> Field(4) >> Application(GenericDesktop.GamePad) >> Usage(16) >> Button.0001 >> Button.0002 >> Button.0003 >> Button.0004 >> Button.0005 >> Button.0006 >> Button.0007 >> Button.0008 >> Button.0009 >> Button.000a >> Button.000b >> Button.000c >> Button.000d >> Button.000e >> Button.000f >> Button.0010 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(16) >> Report Offset(56) >> Flags( Variable Absolute ) >> Field(5) >> Application(ff02.0001) >> Usage(1) >> ff02.0004 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(1) >> Report Offset(72) >> Flags( Variable Absolute ) >> Field(6) >> Application(Consumer.0001) >> Usage(1) >> Consumer.0224 >> Logical Minimum(0) >> Logical Maximum(1) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(1) >> Report Count(1) >> Report Offset(73) >> Flags( Variable Relative ) >> Field(7) >> Physical(Simulation.0001) >> Application(GenericDesktop.GamePad) >> Usage(2) >> Simulation.00c4 >> Simulation.00c5 >> Logical Minimum(0) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(2) >> Report Offset(80) >> Flags( Variable Absolute ) >> FEATURE[FEATURE] >> Field(0) >> Application(GenericDesktop.GamePad) >> Usage(8) >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> ff00.0080 >> Logical Minimum(0) >> Logical Maximum(255) >> Physical Minimum(0) >> Physical Maximum(255) >> Report Size(8) >> Report Count(8) >> Report Offset(0) >> Flags( Variable Absolute ) >> >> GenericDesktop.X ---> Absolute.X >> GenericDesktop.Y ---> Absolute.Y >> GenericDesktop.Z ---> Absolute.Z >> GenericDesktop.Rz ---> Absolute.Rz >> GenericDesktop.HatSwitch ---> Absolute.Hat0X >> GenericDesktop.HatSwitch ---> Absolute.Hat0Y > It took me a while to realize that you were needing > https://patchwork.kernel.org/project/linux-input/patch/20201101193452.678628-1-lzye@google.com/ > for that. > > But the weird part is that Hat switch are usually used as a single > variable, with values being mapped to Hat0X and Hat0Y. So I still > haven't manage to understand how the hid-input driver would map the > axis to a regular one between 1 and 255... > >> Keyboard.004f ---> Key.Right >> Keyboard.0050 ---> Key.Left >> Keyboard.0051 ---> Key.Down >> Keyboard.0052 ---> Key.Up >> GenericDesktop.D-PadUp ---> Absolute.Hat1X >> GenericDesktop.D-PadDown ---> Sync.Report >> GenericDesktop.D-PadRight ---> Sync.Report >> GenericDesktop.D-PadLeft ---> Sync.Report >> Button.0001 ---> Key.BtnA >> Button.0002 ---> Key.BtnB >> Button.0003 ---> Key.BtnC >> Button.0004 ---> Key.BtnX >> Button.0005 ---> Key.BtnY >> Button.0006 ---> Key.BtnZ >> Button.0007 ---> Key.BtnTL >> Button.0008 ---> Key.BtnTR >> Button.0009 ---> Key.BtnTL2 >> Button.000a ---> Key.BtnTR2 >> Button.000b ---> Key.BtnSelect >> Button.000c ---> Key.BtnStart >> Button.000d ---> Key.BtnMode >> Button.000e ---> Key.BtnThumbL >> Button.000f ---> Key.BtnThumbR >> Button.0010 ---> Key.? >> ff02.0004 ---> Key.Btn0 >> Consumer.0224 ---> Key.Back >> Simulation.00c4 ---> Absolute.Gas >> Simulation.00c5 ---> Absolute.Brake >> >> So you can see the device has D-Up, D-Down,D-Right,D-Left usages, and >> D-up is mapped to Hat1X. > OK, now I am starting to understand the problem better. > >> Also if you can send HID report from hid-tool, you will see there are >> always intail events on Hat1X/Hat1Y as the HatDir is 0, even no DPAD >> buttons pressed. When you send HID report with D-DPAD buttons with >> different state, it doesn't generate any axis events because the HatDir >> internally is still 0 regardless the report value of the 4 DPAD usages. > That's the part I am a little bit stuck. I can emulate events for X,Y, > buttons,... but I am not sure how the gamepad sends the events for the > Hat switch and the D-Pad together. > > Again, before we merge anything, I'd like to have the proper tests > written for it, on top of > https://gitlab.freedesktop.org/bentiss/hid-tools/-/commits/wip/gamevice > so we can ensure there is no regression for it, and that we will not > regress on it later on. > > Cheers, > Benjamin > >> >> Thanks! >> >> Chris >> >> >> On 11/2/20 12:16 AM, Benjamin Tissoires wrote: >>> Hi Chris, >>> >>> >>> On Sun, Nov 1, 2020 at 8:35 PM Chris Ye <lzye@google.com> wrote: >>>> Generic Desktop DPAD usage is mapped by hid-input, that only the first >>>> DPAD usage maps to usage type EV_ABS and code of an axis. If HID >>>> descriptor has DPAD UP/DOWN/LEFT/RIGHT HID usages and each of usage size >>>> is 1 bit, then only the first one will generate input event, the rest of >>>> the HID usages will be assigned to hat direction only. >>>> The hid input event should check the HID report value and generate >>>> HID event for its hat direction. >>>> >>>> Test: Connect HID device with Generic Desktop DPAD usage and press the >>>> DPAD to generate input events. >>> Thanks for the patch, but I would rather have a proper tests added to >>> https://gitlab.freedesktop.org/libevdev/hid-tools >>> >>> We already have gamepads tests, and it would be very nice to have this >>> patch reflected as a test as well. This would also allow me to better >>> understand the problem. I am not sure I follow the whole logic of this >>> patch without seeing the 2 variants of report descriptors. >>> >>> Cheers, >>> Benjamin >>> >>>> Signed-off-by: Chris Ye <lzye@google.com> >>>> --- >>>> drivers/hid/hid-input.c | 16 ++++++++++++---- >>>> 1 file changed, 12 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c >>>> index 9770db624bfa..6c1007de3409 100644 >>>> --- a/drivers/hid/hid-input.c >>>> +++ b/drivers/hid/hid-input.c >>>> @@ -1269,7 +1269,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >>>> struct input_dev *input; >>>> unsigned *quirks = &hid->quirks; >>>> >>>> - if (!usage->type) >>>> + if (!usage->type && !field->dpad) >>>> return; >>>> >>>> if (usage->type == EV_PWR) { >>>> @@ -1286,9 +1286,17 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct >>>> int hat_dir = usage->hat_dir; >>>> if (!hat_dir) >>>> hat_dir = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1; >>>> - if (hat_dir < 0 || hat_dir > 8) hat_dir = 0; >>>> - input_event(input, usage->type, usage->code , hid_hat_to_axis[hat_dir].x); >>>> - input_event(input, usage->type, usage->code + 1, hid_hat_to_axis[hat_dir].y); >>>> + if (hat_dir < 0 || hat_dir > 8 || value == 0) >>>> + hat_dir = 0; >>>> + if (field->dpad) { >>>> + input_event(input, EV_ABS, field->dpad, hid_hat_to_axis[hat_dir].x); >>>> + input_event(input, EV_ABS, field->dpad + 1, hid_hat_to_axis[hat_dir].y); >>>> + } else { >>>> + input_event(input, usage->type, usage->code, >>>> + hid_hat_to_axis[hat_dir].x); >>>> + input_event(input, usage->type, usage->code + 1, >>>> + hid_hat_to_axis[hat_dir].y); >>>> + } >>>> return; >>>> } >>>> >>>> -- >>>> 2.29.1.341.ge80a0c044ae-goog >>>>
| |