Re: [PATCH v2] Input: Fix the HID usage of DPAD input event generation.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+stable@xxxxxxxxxxxxxxx

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@xxxxxxxxx> 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@xxxxxxxxxx/
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@xxxxxxxxxx> 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@xxxxxxxxxx>
---
   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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux