Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible duplicate controller detection with evdev and rawhid #4707

Open
kakra opened this issue Mar 30, 2021 · 16 comments
Open

Possible duplicate controller detection with evdev and rawhid #4707

kakra opened this issue Mar 30, 2021 · 16 comments

Comments

@kakra
Copy link
Contributor

kakra commented Mar 30, 2021

Tested with Proton Experimental on 2021-03-29

Taken from the original report for Cyberpunk:

@ivyl

Creating HID device for xbox controllers on Linux side may indeed be the problem here. Winebus.sys is responsible for controllers. bus_udev.c picks up everything (-ish) with rawhid that we can access. bus_sdl.c uses SDL to pick up everything else and creates a faux HID/xinput devices on the Wine/Windows side.

Actually, ALL Bluetooth-connected Xbox controllers are HID devices. But what's new in xpadneo is that there's now a udev rule in master which allows access to the rawhid device for the current seat user. So if winebus.sys picks up /dev/hidraw*, and SDL picks up /dev/input/event*, we are seeing two devices which are actually the same hardware.

If button mapping for the rawhid device is wrong, we need to fix this: The winebus.sys driver seems to make wrong assumptions about what the HID descriptor tells it: xpadneo shows 12 buttons in the HID descriptor (10 default buttons + Guide + Share). But it seems to expect a sparse map of 15 bits for 11 or 12 buttons. It's difficult to make such assumptions because we have at least two Xbox Controller models with conflicting bit positions for a button. Could you point me to the code that interprets the hidraw reports for Xbox controllers? I'd like to look at whether we could fix that properly at the xpadneo level, or if we need to improve the wine code. Wine seems to be the only user of rawhid so far, it seems. So it's a good opportunity to get that right now.

This worked as xbox controllers are not HID devices and xpad wasn't trying to provide that faux HID. Looks like that is changing with xpadneo (is this going to be upstreamed?) and the assumptions may need to get revised.

This is a udev thing, not a xpadneo thing. Actually, different input devices allow seat user access to their rawhid device. (atar-axis/xpadneo@eee6d5b)

Duplicate devices would need to be filtered by looking at the udev parent device path: If it is the same, you are looking at the same hardware to rawhid and evdev.

Are you using the current master of xpadneo or do you have some custom patches applied?

I'm using my patch queue in atar-axis/xpadneo@master...kakra:queue/for-0.10 - but for the hidraw case it's identical with current master. Since I'm maintaining and programming that driver, I'm using master + some patches in testing (queue).

There's currently no plan to upstream the driver in its current form but we may try to port fixes over to the kernel. There's a sister project xone (from the xow author) which makes similar efforts but that's not a HID device. It instead works on creating a GIP bus in the kernel for GIP devices (controllers, headsets etc).

Originally posted by @kakra in #4450 (comment)

@kakra kakra mentioned this issue Mar 30, 2021
2 tasks
@kakra
Copy link
Contributor Author

kakra commented Mar 30, 2021

Summary of original discussion

kakra commented:

Anyone else seeing weird controller mappings with 1.2, like the left bumper also switching weapons (LB emits Y) or the start button throwing a grenade (but also opening the menu)? I've already disabled Steam Input, and other games don't show this problem. (Proton Experimental)

ivyl commented:

The game uses xinput for controller handling. I haven't noticed the behavior you describe. I've tested with Xbox Series X controller (1914 with updated firmware) over USB and X360 Wireless on the newest experimental.

The only thing that comes to my mind is SDL, which is used for non-HID controllers, would have incorrect mapping for your hardware, but then you would see that with other games as well...

Can you reproduce this reliably? What controllers are you using? Which driver and version/kernel? On what hardware? Can you run some xinput test program through Proton to see if inputs are reported correctly?

kakra commented:

Yes, I can reproduce reliably. Other games (also xinput games) behave just fine. Before the Cyberpunk 1.2 patch, it was fine, too. So something changed in the game. I'm using the Xbox One S or Xbox Elite 2 controller (but only one is connected at the same time) over Bluetooth with xpadneo (that's the driver I'm developing and maintaining), my driver dev-version supports hidraw access for users now, so SDL_JOYSTICK_HIDAPI may come into play here?

But well, other games are not affected (all tested with latest Proton Experimental). The controller is a HID device through xpadneo, it exposes are firmware version which is not part of the SDL controller db, so SDL falls back to a standard mapping which xpadneo fits perfectly.

I think it may actually boil down to rawhid, as the button bits in the HID reports are sent as in the original Windows connection mode of the controller (10 buttons) instead of in the Linux mode (15 or 16 buttons with sparse bitmap).

From that perspective it looks like Cyberpunk may actually see two controllers, one with correct mappings, and one with broken mappings, because when I press LB, I get both LB and Y in the game (it switches weapon and activates quick scan). The axes seem okay, only buttons are affected.

Linux HID mapping of the controller is buttons A,B,C,X,Y,Z,LB,RB,... (where it actually has no C or Z button of course) while Windows mapping is A,B,X,Y,LB,RB in the button bitmap (which xpadneo uses) - so LB would really take position of Y. But then again: Why does the game see both the correct and incorrect mapping then?

ivyl commented:

Yes, I can reproduce reliably. Other games (also xinput games) behave just fine. Before the Cyberpunk 1.2 patch, it was fine, too. So something changed in the game. I'm using the Xbox One S or Xbox Elite 2 controller (but only one is connected at the same time) over Bluetooth with xpadneo (that's the driver I'm developing and maintaining), my driver dev-version supports hidraw access for users now, so SDL_JOYSTICK_HIDAPI may come into play here?

But well, other games are not affected (all tested with latest Proton Experimental). The controller is a HID device through xpadneo, it exposes are firmware version which is not part of the SDL controller db, so SDL falls back to a standard mapping which xpadneo fits perfectly.

If having two xinput controllers is indeed the culprit, the games that work may just pick up and use the first one in order of enumeration, while CP2077 may just take all inputs from all controllers?

Creating HID device for xbox controllers on Linux side may indeed be the problem here. Winebus.sys is responsible for controllers. bus_udev.c picks up everything (-ish) with rawhid that we can access. bus_sdl.c uses SDL to pick up everything else and creates a faux HID/xinput devices on the Wine/Windows side.

This worked as xbox controllers are not HID devices and xpad wasn't trying to provide that faux HID. Looks like that is changing with xpadneo (is this going to be upstreamed?) and the assumptions may need to get revised.

Are you using the current master of xpadneo or do you have some custom patches applied?

Continued in initial post of this issue.

@ivyl

Logs with WINEDEBUG=+xinput,+plugplay,+dinput,+hid,+rawinput should tell us more on how the devices are discovered and handled by the game on your setup.

Will do so...

@ivyl
Copy link
Collaborator

ivyl commented Mar 30, 2021

Actually, ALL Bluetooth-connected Xbox controllers are HID devices.

Oh, thanks for clarification! I've been mostly testing things using the wired mode and I've missed that.

But what's new in xpadneo is that there's now a udev rule in master which allows access to the rawhid device for the current seat user. So if winebus.sys picks up /dev/hidraw*, and SDL picks up /dev/input/event*, we are seeing two devices which are actually the same hardware.

That explains why you are the only one seeing this behavior. I have xpadneo 0.9 installed and it doesn't come with those rules yet. Steam ships it's own set of udev rules that do similar thing for devices we need to handle through hidraw, e.g. modern Sony controllers. This was the baseline for development so far.

As of why the two devices are being picked up, that's something that needs a bit more investigation. Theoretically we don't open SDL devices if hidraw is used. But the check may not be robust enough. And even if we would pick up a single device both with bus_sdl.c and bus_udev.c only the SDL one should result in creation of an xinput device (more on that below). As far as I know Cyberpunk 2077 supports only xinput devices, so that needs more debugging. Maybe SDL is opening both hidraw* and event* for some reason?

Anyway, I should be able to get a repro now. Thanks!

If button mapping for the rawhid device is wrong, we need to fix this: The winebus.sys driver seems to make wrong assumptions about what the HID descriptor tells it: xpadneo shows 12 buttons in the HID descriptor (10 default buttons + Guide + Share). But it seems to expect a sparse map of 15 bits for 11 or 12 buttons. It's difficult to make such assumptions because we have at least two Xbox Controller models with conflicting bit positions for a button. Could you point me to the code that interprets the hidraw reports for Xbox controllers? I'd like to look at whether we could fix that properly at the xpadneo level, or if we need to improve the wine code. Wine seems to be the only user of rawhid so far, it seems. So it's a good opportunity to get that right now.

Sadly it's not that simple. I'll explain how things work on Windows and how we map that onto Proton to be sure we are on the same page.

On Windows (wired case, I have to educate myself on Bluetooth):

  • the controller uses its own protocol (XUSB/GIP)
  • you can access the controller through XInputGetState() and friends
  • there also seems to be a HID Minidriver that provides the HID interface. HID is most likely for DirectInput consumption, but there's also a fair share of games that use xinput controllers using RawInput.

WRT Bluetooth - I wonder if the the HID device is exposed as is or is there a driver that applies some quirks on top of it. Also are XInput*() calls implemented on top of that?

Extra notes on Windows APIs:

  • Those faux HID devices for wired controllers doesn't seem to have a bit for the guide button. I need to check Bluetooth.
  • On Windows you don't work with HID report descriptors. There doesn't seem to be even an interface to get the raw value. You still get reports, but you parse them using helper functions and something called preparsed data.
  • You also get a separate "HID device" for each top-level collection in the descriptor (currently unsupported in Wine/Proton, but it's very rare and I've never seen it in a controller).

With Proton:

  • If we have permissions to open the hidraw device (70-steam-input.rules) we just do that, and that's it. The device should not be considered xinput.
  • If we don't have hidraw access then we try to open the device using SDL. SDL is handy as it comes with a lot of mappings for controllers and normalizes inputs for us.
  • We then create a fake HID device just like the Windows' Minidriver. We build a report descriptor for Wine's HID parser that gives us the exact same report format and behavior with the helper functions as we see on Windows, including lack of guide button and coupled triggers.
  • We create a secondary HID device under slightly different GUID (xinput_hack) for the consumption of xinput*.dll. It has the triggers decoupled and supports the guide button.

That's a bit of an oversimplification, but it mostly works, making a lot of "generic" non-xinput controllers behave as xinput.

By having a quick look - we cannot just use the raw HID device we get from xpadneo. We would need to parse the HID, build xinput AND the special weird HID representation on top of that.

This is a udev thing, not a xpadneo thing. Actually, different input devices allow seat user access to their rawhid device. (atar-axis/xpadneo@eee6d5b)

Shipping those udev rules is a xpadneo thing though, most people don't have permissions for the rawhid devices not listed in 70-steam-input.rules.

Duplicate devices would need to be filtered by looking at the udev parent device path: If it is the same, you are looking at the same hardware to rawhid and evdev.

Yes, I think I need to make duplicates detection more robust.

There's currently no plan to upstream the driver in its current form but we may try to port fixes over to the kernel. There's a sister project xone (from the xow author) which makes similar efforts but that's not a HID device. It instead works on creating a GIP bus in the kernel for GIP devices (controllers, headsets etc).

Oh. If hidraw won't become "the default" Linux behavior for xbox controllers I don't think working on hidraw->xinput in Proton would be justified. Linux Input -> SDL -> xinput / X-HID is easier to maintain, covers current drivers, and makes adding new mappings a breeze.

I think that making sure we don't open xbox controllers using hidraw* (either throug bus_udev.c or through SDL) is a better option for now.

@kakra
Copy link
Contributor Author

kakra commented Mar 30, 2021

Ok, let me first generate a log so we can see at what problem we are actually looking...
steam-1091500.log

Generated with WINEDEBUG=+xinput,+plugplay,+dinput,+hid,+rawinput PROTON_LOG=1 gamemode %command% (where gamemode is a small wrapper which sets up some tuning options for me, including raising the priority of wineserver).

Tests done: Went to the settings menu, pressed "Select" and "Start" to find that it switches the settings pages as "LB" and "RB" would do. Then pressed LB to see that it switched one page to the left but asked for reset to defaults which is usually bound to the Y button. So it actually seems to see two controllers.

Here's the dmesg log from the same session so you can properly map hidraw device numbers from the log:

[194264.195440] xpadneo 0005:045E:02FD.001A: report descriptor size: 335 bytes
[194264.195442] xpadneo 0005:045E:02FD.001A: fixing up report descriptor size
[194264.195443] xpadneo 0005:045E:02FD.001A: fixing up Rx axis
[194264.195443] xpadneo 0005:045E:02FD.001A: fixing up Ry axis
[194264.195444] xpadneo 0005:045E:02FD.001A: fixing up Z axis
[194264.195444] xpadneo 0005:045E:02FD.001A: fixing up Rz axis
[194264.195445] xpadneo 0005:045E:02FD.001A: fixing up button mapping
[194264.195566] xpadneo 0005:045E:02FD.001A: battery detected
[194264.195568] xpadneo 0005:045E:02FD.001A: gamepad detected
[194264.195569] xpadneo 0005:045E:02FD.001A: pretending XB1S Windows wireless mode (changed PID from 0x02FD to 0x02E0)
[194264.195569] xpadneo 0005:045E:02FD.001A: disabling dead zones
[194264.195570] xpadneo 0005:045E:02FD.001A: enabling compliance with Linux Gamepad Specification
[194264.195607] input: Xbox Wireless Controller as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2.7/2-1.2.7.2/2-1.2.7.2:1.0/bluetooth/hci0/hci0:75/0005:045E:02FD.001A/input/input37
[194264.195687] xpadneo 0005:045E:02FD.001A: consumer controls detected
[194264.195714] input: Xbox Wireless Controller Consumer Control as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2.7/2-1.2.7.2/2-1.2.7.2:1.0/bluetooth/hci0/hci0:75/0005:045E:02FD.001A/input/input38
[194264.195774] xpadneo 0005:045E:02FD.001A: input,hidraw15: BLUETOOTH HID v9.03 Gamepad [Xbox Wireless Controller] on 00:1a:7d:da:71:15
[194264.195777] xpadneo 0005:045E:02FD.001A: controller quirks: 0x00000010
[194264.195778] xpadneo xpadneo_welcome_rumble start
[194265.189207] xpadneo xpadneo_welcome_rumble took 994ms
[194265.189212] xpadneo 0005:045E:02FD.001A: Xbox Wireless Controller [c8:3f:26:a9:2d:6c] connected
[194267.687561] xpadneo 0005:045E:02FD.001A: battery registered

Now about your reply:

Actually, ALL Bluetooth-connected Xbox controllers are HID devices.

Oh, thanks for clarification! I've been mostly testing things using the wired mode and I've missed that.

Yeah but it's a bit weird. When connected to Bluetooth, the controller firmware seems to detect whether it's connected to a Linux box (or rather it seems to assume Android where the strange HID report makes sense) or to a Windows box. Depending on detection, it exposes different HID descriptors and sends a different HID report format (matching the descriptor). Early firmware versions of the Xbox One S controller could be tricked into assuming the wrong mode on connection, this was "fixed" by later versions and later models. When connected via Bluetooth to Windows, the controller is also a HID device and shows up on the Bluetooth bus with a HID profile. I don't know how the Windows Xbox gamepad driver comes into play here, and if it maps that back to the xinput driver model. But the controller shows the same rumble bugs in Windows as in Linux when in Bluetooth HID mode: Sending rumble commands faster than 10ms intervals crashes the controller firmware. The Windows driver seems to take care of that and sends rumble no faster than each 10ms. So xpadneo does the same.

In USB mode (no matter if that is via the Xbox Wifi dongle, or a physical USB wire), the controller speaks the GIP protocol which is (according to xow/xone) byte-identical for dongle and physical USB. The reports itself are quite similar in format to the HID protocol it uses for Bluetooth, the biggest difference is different report op-codes, and GIP has some more control over the device (we could change LED intensity, force a shutdown etc, which is not possible with HID). When in Linux HID mode, the report format is different in the button bitmap compared to GIP. GIP mode has the same 10ms bug, at least in wireless mode, it doesn't seem to apply in wired mode, as least I'm not aware of any reports on that, OTOH, I never cared about USB wired mode.

xpadneo was mostly made in the early stages to detect and support both HID connection modes (Linux and Windows), nowadays and since I've taken over maintainership, we also found similar behavior changes in other models, not always reproducibly, we've found some protocol bugs where the firmware would send strangely duplicated bytes in one report (fixed in later firmware updates), and we've found that some models have conflicting bit positions for some buttons (there's a bit which is the share button on the latest model, while it is the back button in previous models). Since I've worked on rawhid support, my goal was to expose a consistent report format for all models, so xpadneo rewrites HID reports before it passed it to the /dev/hidraw* device. This simplified a lot of handling in the event handler of Linux, as we simply fix the reports before Linux interprets them. To stay consistent, we pretend to be the original Xbox One S model in Windows HID mode with firmware version that's unknown to SDL, so it won't pick up one of the mapping fixups from its database.

But what's new in xpadneo is that there's now a udev rule in master...

That explains why you are the only one seeing this behavior. I have xpadneo 0.9 installed and it doesn't come with those rules yet. Steam ships its own set of udev rules that do similar thing for devices we need to handle through hidraw, e.g. modern Sony controllers. This was the baseline for development so far.

I'm willing to adjust the xpadneo hidraw format to whatever Proton suits best: Proton seems to be the only user so far, so it makes sense. Actually, xpadneo is meant to sent hidraw reports in a format that's mostly 1:1 bit compatible with GIP (except the first op-code byte), so it could be passed down to the Proton xinput handler without complex adjustments (I think this is why the HID format is identical to GIP when paired to a Windows box, it made driver development easier for Microsoft). I'm not sure if this makes sense from a latency point of view. Rumble on the reverse path could be used in the same way by just replacing the op-code. Of course, Proton would have to obey the 10ms rule which is also implemented in the Windows driver. I don't think that rumble is implemented yet on that code path as I've had users who reported that rumble stopped working. OTOH, those users reported getting it working again by using SDL_JOYSTICK_HIDAPI=0 - which doesn't seem to make sense now that you've explained how it works internally.

As of why the two devices are being picked up, that's something that needs a bit more investigation. Theoretically we don't open SDL devices if hidraw is used. But the check may not be robust enough. And even if we would pick up a single device both with bus_sdl.c and bus_udev.c only the SDL one should result in creation of an xinput device (more on that below). As far as I know Cyberpunk 2077 supports only xinput devices, so that needs more debugging. Maybe SDL is opening both hidraw* and event* for some reason?

Well, there's "HIDAPI" now in SDL which would do that, but last time I looked at their code, they didn't use that for Xbox controllers, just a few selected Nintendo controllers (or rather for devices not covered by the controller DB with some Nintendo-/8BitDo-specific quirk fixups). It also doesn't have the 10ms throttle in the rumble code. I would've submitted a patch for the 10ms throttle if the SDL code would've used hidraw for Xbox controllers by then. I don't know of the current state, tho. The SDL code is somewhat messy in some locations: It's not so easy understanding and seeing the bigger picture of the code.

Anyway, I should be able to get a repro now. Thanks!

Maybe my log clears up some things and we can settle on a fix. If the hidraw mode is something that can lower latency overhead, I'd like to get that properly fixed and support more controllers. Actually, there's a plan to support more controllers with the xpadneo driver once it's mostly feature complete (I'm also going to add USB mode, and maybe even dongle mode, so basically the GIP protocol).

If button mapping for the rawhid device is wrong, we need to fix this: The winebus.sys driver seems to make wrong assumptions about what the HID descriptor tells it: xpadneo shows 12 buttons in the HID descriptor (10 default buttons + Guide + Share). But it seems to expect a sparse map of 15 bits for 11 or 12 buttons. It's difficult to make such assumptions because we have at least two Xbox Controller models with conflicting bit positions for a button. Could you point me to the code that interprets the hidraw reports for Xbox controllers? I'd like to look at whether we could fix that properly at the xpadneo level, or if we need to improve the wine code. Wine seems to be the only user of rawhid so far, it seems. So it's a good opportunity to get that right now.

Sadly it's not that simple. I'll explain how things work on Windows and how we map that onto Proton to be sure we are on the same page.

On Windows (wired case, I have to educate myself on Bluetooth):

  • the controller uses its own protocol (XUSB/GIP)
  • you can access the controller through XInputGetState() and friends
  • there also seems to be a HID Minidriver that provides the HID interface. HID is most likely for DirectInput consumption, but there's also a fair share of games that use xinput controllers using RawInput.

WRT Bluetooth - I wonder if the the HID device is exposed as is or is there a driver that applies some quirks on top of it. Also are XInput*() calls implemented on top of that?

I have no idea, I'm mostly clueless about Windows APIs. I used the Xinput docs from Microsoft to set some protocol stuff up and make consistent HID reports, so mostly making our HID reports matching with xinput as best as we can, which matched what games expected before SDL gained support for the controllers. So xpadneo was the first driver to actually support the controller with a consistent mapping across all major APIs (jsdev, evdev, SDL, wine-dinput, wine-xinput). Since SDL gained support, we had to jump some hoops and loops to work around SDL double-fixing a mapping that xpadneo already fixed. It is a valid discussion whether this should be fixed in kernel space or user space, and I believe most kernel devs would argue for the latter case, that's why we probably never upstream xpadneo. So I instead opted in for adding additional useful features to the driver, and adding support for Xbox Elite 2 profile programming (coming soon to xpadneo, I hope). We are going to work together with the xow/xone programmer, feel free to join the Discord server.

Extra notes on Windows APIs:

  • Those faux HID devices for wired controllers doesn't seem to have a bit for the guide button. I need to check Bluetooth.

Yes, that's true. The Guide button is a system button. In the Bluetooth HID reports we MAY sometimes get the bit for the Guide button in the buttons bitmap but usually it's sent in the consumer control HID page. This makes me think that the Guide button isn't actually meant to be readable by games. xpadneo is currently sending it as a keyboard event in the consumer control sub-device. Apparently, the Linux Steam client doesn't interpret it, and thus xpadneo cannot launch big picture mode (or the overlay) from the controller: The Steam client should look for the KEY_MODE key event: Even without xpadneo, the Xbox Elite 2 controller has the Guide button in the consumer control page so it won't show up in the button that SDL sees (except in some corner cases depending on connection mode quirks and firmware version).

You may want to look into the docs folder of the xpadneo project: There are decoded HID descriptors and some notes about quirky behavior. If you use git blame on these docs, you may easily find the driver code in the same or nearby commit.

  • On Windows you don't work with HID report descriptors. There doesn't seem to be even an interface to get the raw value. You still get reports, but you parse them using helper functions and something called preparsed data.

Linux may expose something similar on the sysfs debug directory: There's a file per device with decoded descriptors. It may be useful if you're working on that part. But I always used an online decoder for that (link is in the xpadneo docs).

  • You also get a separate "HID device" for each top-level collection in the descriptor (currently unsupported in Wine/Proton, but it's very rare and I've never seen it in a controller).

The Xbox Elite 2 controller has separate pages (and has the Guide button in the consumer page), and the kernel will present them as separate devices below the parent device in sysfs and udev. Running udevadm monitor -p and udevadm test ...sysfspath... are quite interesting tools to see how this works. In Linux of course... I actually don't have a Windows box (however, I installed a Windows 10 VM lately to support my reverse engineering attempts to get Xbox Elite 2 programmable profiles into xpadneo).

With Proton:

  • If we have permissions to open the hidraw device (70-steam-input.rules) we just do that, and that's it. The device should not be considered xinput.

But it could be if the PID/VID/version match specific data, in the xpadneo case we patch the version to bypass SDL fixups. The hidraw reports could be passed with only a few fixups (depending on the version field) to the xinput bus in wine.

  • If we don't have hidraw access then we try to open the device using SDL. SDL is handy as it comes with a lot of mappings for controllers and normalizes inputs for us.

Yeah but in Linux that is quite cumbersome as it could just read the proper mappings on the evdev IOCTLs instead without using all those mappings. I believe this is mostly owed to SDL under Windows then as you wrote it doesn't have some easy notation of HID descriptors. Actually, the SDL HIDAPI patches seem to be actually about this.

  • We then create a fake HID device just like the Windows' Minidriver. We build a report descriptor for Wine's HID parser that gives us the exact same report format and behavior with the helper functions as we see on Windows, including lack of guide button and coupled triggers.

I haven't looked at the link yet but I think I remember which code you're talking about. I was using that to aid me in xpadneo to construct a proper report format. The Guide button is not part of this as seemingly intended by Microsoft: xinput has 6 axis and 10 buttons - that's it. The xow driver code (which uses wireless GIP) sees the same phenomenon: The state report doesn't include the Guide button, it comes in an extra report - perfectly mirroring what we see in Bluetooth HID mode. Earlier models and firmware versions still put the Guide button in the state report bitmap, later versions no longer do that according to the descriptor (but the bit can still be seen in the state reports, too).

  • We create a secondary HID device under slightly different GUID (xinput_hack) for the consumption of xinput*.dll. It has the triggers decoupled and supports the guide button.

What do you mean with coupled/decoupled? The triggers should be on separate axes. However, xpadneo has an extra axis which combines both triggers. This was asked for by users using simulators to get a proper rudder axis which cannot otherwise be assigned (as most simulators cannot combine two 0..100% into one -100..100% axis. If you're looking at this strange extra axis using the xpadneo driver, please ignore it in Proton. Limiting to 6 axes should be enough, I took care to order it behind the other axes in evdev (so it's ABS_MISC instead of the more proper ABS_RUDDER).

That's a bit of an oversimplification, but it mostly works, making a lot of "generic" non-xinput controllers behave as xinput.

It's not very complicated to understand since I pulled my hair off over the different quirks of the controller in xpadneo, actually, I understood a lot of the code in Proton after that. ;-) And TBH, I'm not doing things much differently (tho, it's much easier for me in the driver): I just swap bits around in the reports to make them look the same across all models so it'll work the same for the general case (which is the event loop calling back into xpadneo).

By having a quick look - we cannot just use the raw HID device we get from xpadneo. We would need to parse the HID, build xinput AND the special weird HID representation on top of that.

xpadneo is built to have a consistent HID state report (op-code 0x01) across all models. So IF you'd want to do a special case for xpadneo, I'll give you a guarantee that the report format will be the same for all supported models, and if it isn't for one future model, I will make it consistent. I'll still have to make some investigations to support the Xbox Elite 2 paddles properly in the reports - that's not finalized yet. I'm not even sure how xinput handles that with its 10-buttons-only state format. Maybe I need to look at the reports that xow receives from the GIP dongle.

This is a udev thing, not a xpadneo thing. Actually, different input devices allow seat user access to their rawhid device. (atar-axis/xpadneo@eee6d5b)

Shipping those udev rules is a xpadneo thing though, most people don't have permissions for the rawhid devices not listed in 70-steam-input.rules.

Yes, but it looks like more and more input devices get a seat user tag in the rule files nowadays, e.g. my gaming mouse has rawhid permissions for the seat user, there's even a GUI application to program LEDs and macros for it: https://github.com/libratbag/piper. I have something similar in mind to support the Xbox Elite 2 controller with a GUI application.

Duplicate devices would need to be filtered by looking at the udev parent device path: If it is the same, you are looking at the same hardware to rawhid and evdev.

Yes, I think I need to make duplicates detection more robust.

Looking at udevadm monitor -p while connecting the controller in Bluetooth mode should give some hints. But I really don't know the udev API and what you can get from it, you may need to look at the sysfs interface to sort that out.

There's currently no plan to upstream the driver in its current form but we may try to port fixes over to the kernel. There's a sister project xone (from the xow author) which makes similar efforts but that's not a HID device. It instead works on creating a GIP bus in the kernel for GIP devices (controllers, headsets etc).

Oh. If hidraw won't become "the default" Linux behavior for xbox controllers I don't think working on hidraw->xinput in Proton would be justified. Linux Input -> SDL -> xinput / X-HID is easier to maintain, covers current drivers, and makes adding new mappings a breeze.

I think the kernel prefers that user space uses the evdev interface as it does all the heavy lifting of parsing the reports and return proper symbolic and consistent keycodes to user space. You can bypass that with hidraw access.

Using hidraw can be beneficial to support devices that have no working HID driver in Linux (in which case it falls back to the hid-generic driver which may map things wrongly). So having proper hidraw support can decouple Proton from depending on latest kernel versions for new yet unsupported devices. Also, it may reduce latency as the input event code in the kernel seems rather complex with its report framing etc. For the Xbox One S controller, I see the event callback called up to 100 times per HID report frame before the report is even passed down to the evdev interface while it would have appeared immediately on the hidraw device. xpadneo has an anti-spam algorithm which reduces the pressure on the Linux input system by filtering duplicate HID reports out which we are seeing on some models, effectively reducing the callbacks to like 16 rounds per frame.

I think that making sure we don't open xbox controllers using hidraw* (either throug bus_udev.c or through SDL) is a better option for now.

Probably, yes. However, I'm willing to adjust the xpadneo HID reports you get from hidraw to a format that suits Proton best. We can use a xpadneo-specific firmware version number in the device GUID as seen by SDL to tell it apart from non-xpadneo-driven controllers.

Additional note about hidraw permissions: I think it may become common that gaming devices gain hidraw permissions for the seat user. That will probably not happen for keyboards as this could open a security hole: In the end you're typing passwords on it. But for purely gaming devices it should be okay. systemd does that by adding and removing filesystem ACLs to the device nodes, this may need proper error handling in Proton as systemd may temporarily revoke ACLs if you switch to another console (and thus remove an active seat). It should thus treat intermediate permission problems as non-fatal, the permissions will return when the console is switched back.

Notes about HID patching: Be aware that xpadneo patches HID descriptors and HID reports, so i'ts not a reliable source of the format for that particular model that you could apply to the general HID kernel driver (hid-generic) or the Microsoft specific quirks kernel driver (hid-microsoft). New Xbox controller models may first show up on hid-generic until the kernel is patched to move the device over to hid-microsoft. xpadneo uses udev rules and modalias to move supported devices over to hid-xpadneo (some users use blacklisting to prevent hid-microsoft from loading), finally resulting in HID descriptor and HID report patching with no chance for user space of seeing the original descriptor or report.

@kakra
Copy link
Contributor Author

kakra commented Mar 31, 2021

Looking at this Proton log line:

0080:trace:plugplay:try_add_device Found sdl game controller 0x80000000 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 1)

the Proton code seems to take PID and VID from the wrong device: 045e:02fd comes from the parent device but xpadneo patched it to be 045e:02e0 - which is the mapping we actually expose. See here:

[194264.195569] xpadneo 0005:045E:02FD.001A: pretending XB1S Windows wireless mode (changed PID from 0x02FD to 0x02E0)

While the device node is still under 0005 (Bluetooth) 045e:02fd, the input device is patched to show 0x02E0 - which is what SDL sees for my controller:

# ~/tmp/bin/gamepad-tool
[LOG] SDL2 Gamepad Tool v1.1.2 by General Arcade (compiled with SDL version 2.0.4, DLL version 2.0.12)
[LOG] Website: http://generealarcade.com/gamepadtool/
[LOG] Searching gamepads...
[LOG] Found 3 gamepad(s):
[LOG] "Thrustmaster TWCS Throttle", 030000004f04000087b6000011010000
[LOG] "Thrustmaster T.16000M", 030000004f0400000ab1000000010000
[LOG] "Xbox One Wireless Controller", 050000005e040000e002000003090000 (mapping available)
##########                                            ^^^^^^^^ <- it's 02e0 here
Request worker start in Thread  0x7f6f0241b880
[LOG] Environment variable "SDL_GAMECONTROLLERCONFIG" is not defined
Starting worker process in Thread  0x7f6ef0ff9640
[LOG] Checking if new mappings available from github: https://github.com/gabomdq/SDL_GameControllerDB
Request worker aborting in Thread  0x7f6f0241b880
Aborting worker process in Thread  0x7f6ef0ff9640
Worker process finished in Thread  0x7f6ef0ff9640
Deleting thread and worker in Thread  0x7f6f0241b880

Will that (a) explain why the controller is detected twice or (b) why there is a partially wrong mapping because the Windows SDL layer (if the game uses SDL) see the unpatched PID?

If you look at the sysfs paths:

[194264.195568] xpadneo 0005:045E:02FD.001A: gamepad detected
[194264.195569] xpadneo 0005:045E:02FD.001A: pretending XB1S Windows wireless mode (changed PID from 0x02FD to 0x02E0)
[194264.195607] input: Xbox Wireless Controller as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2.7/2-1.2.7.2/2-1.2.7.2:1.0/bluetooth/hci0/hci0:75/0005:045E:02FD.001A/input/input37
[194264.195687] xpadneo 0005:045E:02FD.001A: consumer controls detected
[194264.195714] input: Xbox Wireless Controller Consumer Control as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2.7/2-1.2.7.2/2-1.2.7.2:1.0/bluetooth/hci0/hci0:75/0005:045E:02FD.001A/input/input38

.../hci0:75/0005:045E:02FD.001A/input should have 02fd, but .../hci0:75/0005:045E:02FD.001A/input/input37 should have 02e0 which is the actual gamepad input:

# input > grep '^' input37/id/* (gamepad page)
input37/id/bustype:0005
input37/id/product:02e0
input37/id/vendor:045e
input39/id/version:0903
# input > grep '^' input38/id/* (consumer control page)
input38/id/bustype:0005
input38/id/product:02fd
input38/id/vendor:045e
input38/id/version:0903

BTW:

007c:trace:plugplay:try_add_device hidraw "/dev/hidraw15": ignoring xinput device 045e/02fd

which somewhat destroys our first assumption. Still the game sees the device twice.

Another theory: Cyberpunk uses xinput and SDL (now) - but Proton re-emits the device as 02fd which is a broken mapping for xpadneo.

Worth a note: The hidraw device will still be presented under the unpatched PID, xpadneo cannot change that, so if your matching code between rawhid and evdev is based on the VID/PID, the matcher would break if you used the correct PID for the actual gamepad device.

It could also explain why games that only support the 02e0 PID are not seeing the controller (some research showed this is probably the case for Horizon Zero Dawn and Farcry Primal). There's an open xpadneo PR related to this: atar-axis/xpadneo#283. It reportedly fixes detection of the controller in a Linux-native game (I think Divinity Original Sin, seems to look for PID 028e only, this is like looking for the string "Windows 9" to assume it's 95 or 98 and considered too old, and thus MS had to skip Windows 9 and made it Windows 10, sigh lazy programmers...).

@kakra
Copy link
Contributor Author

kakra commented Mar 31, 2021

Just FTR, even when connected via GIP protocol, the controller seems to have a HID descriptor, so it may actually be a HID device in that mode but Windows may just hide the fact: https://github.com/medusalix/xone_preview/commit/5cc7e8b69eef316deabf807fbabfa854704313b8. OTOH, it may just be used for the chatpad.

@ivyl
Copy link
Collaborator

ivyl commented Apr 2, 2021

Hey, sorry for the delay I was a bit busy with other Proton work.

Two Controllers

007c:trace:plugplay:try_add_device hidraw "/dev/hidraw15": ignoring xinput device 045e/02fd

which somewhat destroys our first assumption. Still the game sees the device twice.

Yep, I've missed this bit when reading through the code. Luckily the logs you've shared shed some light on the situation.

$ % grep 'Found sdl' steam-1091500.log | grep 'xinput_hack: 0'
0080:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 0)
0080:trace:plugplay:try_add_device Found sdl game controller 0x3 (vid 045e, pid 02e0, version 2307, serial L"050000005e040000e002000003090000", xinput_hack: 0)

Looks like the controller is picked up twice by the SDL itself, under both product IDs.

In SDL the deduplication happens on VID/PID level in src/joystick/linux/SDL_sysjoystick.c:IsJoystick(). It calls HIDAPI_IsDevicePresent() to check if HIDAPI already took care of the device.

Since xpadneo is applying the PID hack only to evdev I think it's justified for SDL to consider the hidraw and evdev as different devices.

As of bad mapping with HIDAPI instance - looks like SDL has mappings for Xbox One and Series controllers for a few months already and indeed with xpadneo the mapping result in a wonky experience, e.g. the mentioned LB registering as Y press.

You've mentioned:

Since SDL gained support, we had to jump some hoops and loops to work around SDL double-fixing a mapping that xpadneo already fixed. It is a valid discussion whether this should be fixed in kernel space or user space, and I believe most kernel devs would argue for the latter case, that's why we probably never upstream xpadneo.

I think it accidentally summarizes the problem really well. SDL has to work with whatever is the "default" behavior. If xpadneo would end up upstream while fixing all the quirks in the driver, it would be SDL's responsiblity to work with that.

Since xpadneo exists in its own downstream niche we end up in this cycle of caving in to whatever SDL does, while SDL is not particularly incentivized to make things works with xpadneo.

I think we should check with the SDL developers if this assessments holds true.

I am still not sure what to think about this and what degree of support should be provided by Proton though.

Other Stuff

(I think we can move most of this to xpadneo)

I wasn't aware the xbox controller world is that wild... That was very insightful. Thanks for writing all those things down!

Now to selectively address some of the points:

Since I've worked on rawhid support, my goal was to expose a consistent report format for all models, so xpadneo rewrites HID reports before it passed it to the /dev/hidraw* device. This simplified a lot of handling in the event handler of Linux, as we simply fix the reports before Linux interprets them. To stay consistent, we pretend to be the original Xbox One S model in Windows HID mode with firmware version that's unknown to SDL, so it won't pick up one of the mapping fixups from its database.

This approach sounds just like what hid-sony is doing. I don't know how that compares to the Windows side of things though. As of SDL, they have controller mappings and quirks per OS, so the Windows ones don't have to apply on Linux.

The Xbox Elite 2 controller has separate pages (and has the Guide button in the consumer page), and the kernel will present them as separate devices below the parent device in sysfs and udev. Running udevadm monitor -p and udevadm test ...sysfspath... are quite interesting tools to see how this works. In Linux of course... I actually don't have a Windows box (however, I installed a Windows 10 VM lately to support my reverse engineering attempts to get Xbox Elite 2 programmable profiles into xpadneo).

This would be interesting to see on Windows. All the xbox controllers I've tested so far had only one HID top level collections and were missing guide button completely. The only way to query the button state is with xinput dlls using the @100 export aka XInputGetStateEx(). I wouldn't be too surprised if this is discarded/normalized by the HID minidriver.

What do you mean with coupled/decoupled? The triggers should be on separate axes. However, xpadneo has an extra axis which combines both triggers.

This is roughly what I get through HID on Windows:

https://github.com/ValveSoftware/wine/blob/a6e37ccbca630b259a446c697c297e68745c538f/dlls/winebus.sys/bus_sdl.c#L306

Maybe that hold true only for the original X360? I have to check with the Series X and check what we get through HID. I've added a note to my TODO.

It could also explain why games that only support the 02e0 PID are not seeing the controller (some research showed this is probably the case for Horizon Zero Dawn and Farcry Primal). There's an open xpadneo PR related to this: atar-axis/xpadneo#283. It reportedly fixes detection of the controller in a Linux-native game (I think Divinity Original Sin, seems to look for PID 028e only, this is like looking for the string "Windows 9" to assume it's 95 or 98 and considered too old, and thus MS had to skip Windows 9 and made it Windows 10, sigh lazy programmers...).

Far Cry Primal should get fixes in Proton soon. Those are already upstream:
f69c8f018188 ("ntoskrnl.exe: Use correct name format when sending WM_DEVICECHANGE.")
0a82d891fc0b ("dinput: Implement device creation using product GUID.")

I am yet to look at HZD.

We are going to work together with the xow/xone programmer, feel free to join the Discord server.

I was unable to find it. Feel free to mail me an invite. Those long form letters are getting a bit out of hand already :-)

@kakra
Copy link
Contributor Author

kakra commented Apr 2, 2021

Two Controllers

007c:trace:plugplay:try_add_device hidraw "/dev/hidraw15": ignoring xinput device 045e/02fd
which somewhat destroys our first assumption. Still the game sees the device twice.

Yep, I've missed this bit when reading through the code. Luckily the logs you've shared shed some light on the situation.

$ % grep 'Found sdl' steam-1091500.log | grep 'xinput_hack: 0'
0080:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 0)
0080:trace:plugplay:try_add_device Found sdl game controller 0x3 (vid 045e, pid 02e0, version 2307, serial L"050000005e040000e002000003090000", xinput_hack: 0)

Looks like the controller is picked up twice by the SDL itself, under both product IDs.

This look like we are on the right track. Thanks for the help finding it. It gives me something to work with.

In SDL the deduplication happens on VID/PID level in src/joystick/linux/SDL_sysjoystick.c:IsJoystick(). It calls HIDAPI_IsDevicePresent() to check if HIDAPI already took care of the device.

Well, isn't SDL_JOYSTICK_HIDAPI=0 supposed to disable hidraw access? At least, this setting doesn't fix Cyberpunk for me. Or this variable isn't passed through Proton's bwrap down into the container maybe?

Since xpadneo is applying the PID hack only to evdev I think it's justified for SDL to consider the hidraw and evdev as different devices.

I'll look into a way to hack the PID for the parent device, too. I'm not sure if this is possible/allowed. The problem is that the parent device has the hidraw device but the actual event interfaces live in children devices.

As of bad mapping with HIDAPI instance - looks like SDL has mappings for Xbox One and Series controllers for a few months already and indeed with xpadneo the mapping result in a wonky experience, e.g. the mentioned LB registering as Y press.

Yep, it has this since a while. Back then I've implemented a hack to work around SDL detecting the controller by changing the version field. But now it also looks at the hidraw device and sees the original non-hacked PID/VID/version.

Since modern SDL is widely deployed now, and since SDL can overlay an old linked SDL version with a newer one, we can probably relax some of the SDL work-arounds in the driver. But I want to stay away from offering sparse button bitmaps because this blows joydev up with too many buttons (because it can only count not skip bits). I wish an input driver could communicate to good old joydev which buttons actually exist - and in which order. This could fix sooo many things. OTOH, MS is at fault here because they offer dead button bits in their descriptor to work around Android having no proper input driver.

I think it accidentally summarizes the problem really well. SDL has to work with whatever is the "default" behavior. If xpadneo would end up upstream while fixing all the quirks in the driver, it would be SDL's responsiblity to work with that.

And then we have the crazy Chrome Gamepad API implementation which reads the mappings from evdev but reads the values from joydev - which are completely separate as one could remap joydev devices and evdev won't reflect that. Why don't they just read the values from evdev if they get the mappings from there anyways? They even have complicated code to map evdev devices to joydev devices, and that way they also get joydev deadzones which isn't particularly helpful for precise input control on modern gamepads.

Since xpadneo exists in its own downstream niche we end up in this cycle of caving in to whatever SDL does, while SDL is not particularly incentivized to make things works with xpadneo.

I don't expect that, thus I've gone with the hacky approach. Looks like I have to improve that. You've given me the right clues.

I am still not sure what to think about this and what degree of support should be provided by Proton though.

If I can work around the hidraw mappings in SDL, I don't think we need any special support in Proton. It should just work then. Let's not think about another option until then.

Other Stuff

(I think we can move most of this to xpadneo)

Yep. The Discord channel may be a nice place, or we open or join one of the issues in xpadneo.

Since I've worked on rawhid support, my goal was to expose a consistent report format for all models, so xpadneo rewrites HID reports before it passed it to the /dev/hidraw* device...

This approach sounds just like what hid-sony is doing. I don't know how that compares to the Windows side of things though. As of SDL, they have controller mappings and quirks per OS, so the Windows ones don't have to apply on Linux.

Interesting, I'll have a look.

This would be interesting to see on Windows. All the xbox controllers I've tested so far had only one HID top level collections and were missing guide button completely. The only way to query the button state is with xinput dlls using the @100 export aka XInputGetStateEx(). I wouldn't be too surprised if this is discarded/normalized by the HID minidriver.

The decoded Linux mode descriptor is here: https://github.com/atar-axis/xpadneo/blob/master/docs/descriptors/xbe2_linux.md

It has multiple collections. It may work differently when paired to Windows, or Windows hides the fact, IDK. It may also be different for different firmware versions.

What do you mean with coupled/decoupled? The triggers should be on separate axes. However, xpadneo has an extra axis which combines both triggers.

This is roughly what I get through HID on Windows:

https://github.com/ValveSoftware/wine/blob/a6e37ccbca630b259a446c697c297e68745c538f/dlls/winebus.sys/bus_sdl.c#L306

So it's the other way around and you have to combine both triggers into one value? Like into lower byte and higher byte of a 16 bit value? This explains why the original Xbox360 controller only seems to have a 0..255 range per each trigger.

Maybe that hold true only for the original X360? I have to check with the Series X and check what we get through HID. I've added a note to my TODO.

The HID descriptor and GIP reports have full range triggers on separate 16 bit values in the reports, tho only 10 bits are used (range 0..1023). The Xbox Elite 2 controller has a hardware range limiter where we could physically set the triggers to full range, half range, and digital (which physically limits how far you can press a trigger). The firmware still maps that to full 10 bit range on output. But the current physical range can be read from the reports. It's documented in the xpadneo docs if you'd like to make any use of it.

That you're seeing signed 16 bit ranges is actually a result of how evdev axis are handled by joydev/SDL: They rescales them to -32768..32767, no matter how fine or coarse the hardware resolution is. Modern Xbox controllers have full 16 bits per axis, and 10 bits per trigger, and you can actually get each of those values, it not upscaled. They seem to have a pretty good denoise implementation in the hardware so the values don't jump wildly around.

I think only the X360 has the 8+8 bit per trigger as a single 16 bit value.

Far Cry Primal should get fixes in Proton soon. Those are already upstream:

Cool! Looking forward!

I am yet to look at HZD.

That would be great.

We are going to work together with the xow/xone programmer, feel free to join the Discord server.

I was unable to find it. Feel free to mail me an invite. Those long form letters are getting a bit out of hand already :-)

Oh, it's here: https://discord.gg/j8reCvPanF (or you click on the badges in the project READMEs).

So next stop is getting my PID hack into the parent device, and if that works, I can finally continue Cyberpunk. If Proton is stable enough for that, you may not see me for a while... hehe ;-)

@kakra
Copy link
Contributor Author

kakra commented Apr 2, 2021

So next stop is getting my PID hack into the parent device, and if that works, I can finally continue Cyberpunk. If Proton is stable enough for that, you may not see me for a while... hehe ;-)

... and after a hard reboot...

Either I did something really dumb, or hacking this data crashes the kernel very hard. Locking up an input driver is very nasty: The system won't shut down and IO eventually freezes due to blocked kernel workers. :-(

@kakra
Copy link
Contributor Author

kakra commented Apr 2, 2021

Next try, seems to work now:

# grep 'Found sdl' ~/steam-1091500.log | grep 'xinput_hack: 0'
009c:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02e0, version 0, serial L"030000005e040000e002000000006800", xinput_hack: 0)

except that Cyberpunk now has ONLY the wrong mapping. But I should be able to fix it by finding a compatible controllerdb entry.

But I wonder about the serial parameter here: Why does it have bus number 3 while in the kernel it's on bus number 5? The SDL gamepad tool also shows bus 5 correctly. And what about 0x00680000? This seems to be a fake version number that's not coming from xpadneo. It should actually be 0x00000903. Is this so your own wine HID driver properly exposes the device to wine user space because you're creating your own HID descriptor in wine?

@kakra
Copy link
Contributor Author

kakra commented Apr 2, 2021

Ok, actually, the xpadneo mapping is in the community controllerdb (and upstream, it seems):

050000005e040000e002000003090000,Xbox One Wireless Controller,\
a:b0,b:b1,back:b6,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b10,leftshoulder:b4,leftstick:b8,\
lefttrigger:a2,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b9,righttrigger:a5,rightx:a3,righty:a4,start:b7,\
x:b2,y:b3,platform:Linux,

It maps exactly to what xpadneo exposes - both on evdev and on hidraw.

Here's the new log, Proton Experimental 6.3, xpadneo fixed to patch the PID of the parent device:
steam-1091500.log

All games I tested show a wrong mapping now.

kakra added a commit to atar-axis/xpadneo that referenced this issue Apr 3, 2021
This prevents SDL from presenting our controller device twice when
HIDAPI is used. This is especially visible in Proton after the Xinput
implementation enumerated all the devices:
```
# grep 'Found sdl' steam-1091500.log | grep 'xinput_hack: 0'
0080:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 0)
0080:trace:plugplay:try_add_device Found sdl game controller 0x3 (vid 045e, pid 02e0, version 2307, serial L"050000005e040000e002000003090000", xinput_hack: 0)
```

Many games only support one controller anyways, so the issue is not
visible for most games but Cyberpunk 2077 since patch 1.2 seems to
support multiple controllers at the same time.

See-also: ValveSoftware/Proton#4707
Maybe-affects: #180
Signed-off-by: Kai Krakow <[email protected]>
kakra added a commit to atar-axis/xpadneo that referenced this issue Apr 3, 2021
Proton seems to have a special handling for SDL HID devices which, when
HIDAPI is used, re-exports the controller under a crafted SDL GUID.
This messes up the mappings in all Proton games tested so far. Only
recent Proton versions may be affected.

So let's temporarily revoke user access from hidraw devices until the
issue is resolved.

Thanks to https://github.com/ivyl from CodeWeavers for helping in
uncovering this issue.

See-also: ValveSoftware/Proton#4707
Signed-off-by: Kai Krakow <[email protected]>
@kakra
Copy link
Contributor Author

kakra commented Apr 3, 2021

@ivyl Disabling hidraw user access until the issue is resolved works around the mapping problem successfully, I've linked the two commits related to this here.

kakra added a commit to kakra/xpadneo that referenced this issue Apr 3, 2021
This prevents SDL from presenting our controller device twice when
HIDAPI is used. This is especially visible in Proton after the Xinput
implementation enumerated all the devices:
```
# grep 'Found sdl' steam-1091500.log | grep 'xinput_hack: 0'
0080:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 0)
0080:trace:plugplay:try_add_device Found sdl game controller 0x3 (vid 045e, pid 02e0, version 2307, serial L"050000005e040000e002000003090000", xinput_hack: 0)
```

Many games only support one controller anyways, so the issue is not
visible for most games but Cyberpunk 2077 since patch 1.2 seems to
support multiple controllers at the same time.

See-also: ValveSoftware/Proton#4707
Maybe-affects: atar-axis#180
Signed-off-by: Kai Krakow <[email protected]>
kakra added a commit to atar-axis/xpadneo that referenced this issue Apr 3, 2021
This prevents SDL from presenting our controller device twice when
HIDAPI is used. This is especially visible in Proton after the Xinput
implementation enumerated all the devices:
```
# grep 'Found sdl' steam-1091500.log | grep 'xinput_hack: 0'
0080:trace:plugplay:try_add_device Found sdl game controller 0x0 (vid 045e, pid 02fd, version 0, serial L"030000005e040000fd02000000006800", xinput_hack: 0)
0080:trace:plugplay:try_add_device Found sdl game controller 0x3 (vid 045e, pid 02e0, version 2307, serial L"050000005e040000e002000003090000", xinput_hack: 0)
```

Many games only support one controller anyways, so the issue is not
visible for most games but Cyberpunk 2077 since patch 1.2 seems to
support multiple controllers at the same time.

See-also: ValveSoftware/Proton#4707
Maybe-affects: #180
Signed-off-by: Kai Krakow <[email protected]>
@ivyl
Copy link
Collaborator

ivyl commented Apr 5, 2021

Well, isn't SDL_JOYSTICK_HIDAPI=0 supposed to disable hidraw access? At least, this setting doesn't fix Cyberpunk for me. Or this variable isn't passed through Proton's bwrap down into the container maybe?

[SDL on Linux] It is and it should. It works with a simple SDL test prog. I've added a note to my TODO to check this with Proton & xinput.

And then we have the crazy Chrome Gamepad API implementation which reads the mappings from evdev but reads the values from joydev - which are completely separate as one could remap joydev devices and evdev won't reflect that. Why don't they just read the values from evdev if they get the mappings from there anyways? They even have complicated code to map evdev devices to joydev devices, and that way they also get joydev deadzones which isn't particularly helpful for precise input control on modern gamepads.

This sounds bad, but I think it's similar situation like with SDL. They have to work with whatever is shipping in Linux and is popular enough. If xpadneo would be upstream that would be enough pressure on Chrome devs to make sure it all works on their side.

IMO the best way is to do "the right thing" upstream and let the user space deal with that. Ofc things have to make sense from user space point of view, and helping those developers is important.

So it's the other way around and you have to combine both triggers into one value? Like into lower byte and higher byte of a 16 bit value? This explains why the original Xbox360 controller only seems to have a 0..255 range per each trigger.

[Windows] It's even weirder. The formula is 0x8000 + left_trigger_value - right_trigger_value. That's why proton has so called xinput_hack which creates another HID-like device (different GUID, we +1 it to make a distinction and not expose it through the usual APIs) that allows us to untangle the triggers (left_trigger_value << 8 | right_trigger_value) and adds one extra bit for the guide button. It's used internally by xinput.dll.

If you want to learn more on how things are represented on Windows SDL is a good source. Since version 2.0.14 it uses hybrid RawInput/xinput/Windows.Gaming.Input solution (see SDL_rawinputjoystick.c) that treats HID as the primary source of inputs in order to support more than 4 xinput controllers. It also tries to guess-map xinput representation to get independent trigger values and guide button support whenever possible.

The HID representation seems to be stable across controller revisions (RAWINPUT_HandleStatePacket()).

Oh, it's here: https://discord.gg/j8reCvPanF (or you click on the badges in the project READMEs).

In my defense the badge doesn't mention Discord by name and is not particularly ctrl+f friendly :-P

Cyberpunk now has ONLY the wrong mapping. But I should be able to fix it by finding a compatible controllerdb entry.

It maps exactly to what xpadneo exposes - both on evdev and on hidraw.

All games I tested show a wrong mapping now.

[SDL on Linux] If you have both HID and evdev available it will use the HID, so that means there's still something wrong with the mapping for that. It may be just that it's not there yet or overridden.

Is it part of the released SDL? You can check what's in the currently released Steam Runtime or try to override the mappings with SDL_GAMECONTROLLERCONFIG. Steam seems to be setting that variable as well, so you may want to check the environment for the game process (cat /proc/$PID/environ | tr '\0' '\n').

Gamepad-tool may come in handy for testing the mappings.

@kakra
Copy link
Contributor Author

kakra commented Apr 5, 2021

It's even weirder. The formula is 0x8000 + left_trigger_value - right_trigger_value.

This is insane: It makes both triggers just a single axis centered around 32768. There's no way to get independent triggers back from that. Who invented that? ;-)

This is somewhat (or rather exactly) like we do in xpadneo to combine both triggers to form a singular 7th axis for rudder control in simulators. But we keep the original trigger axes within the first 6 axes.

If you want to learn more on that SDL is a good source. Since version 2.0.14 it uses hybrid RawInput/xinput/Windows.Gaming.Input solution (see SDL_rawinputjoystick.c) that treats HID as the primary source of inputs in order to support more than 4 xinput controllers. It also tries to guess-map xinput representation to get independent trigger values and guide button support whenever possible.

That's the Windows side of things. I think we should first look into if the Linux side of SDL gets things right, and mappings look correct for me when using gamepad-tool. OTOH, it's not using libsdl2 from the Steam runtime so the test results may be bogus for Proton. Or maybe I need to update my system-libsdl2 to SDL 2.0.14?

The HID representation seems to be stable across controller revisions (RAWINPUT_HandleStatePacket()).

Yes, that enum for the buttons matches exactly what we have in our /dev/hidrawXX reports, no sparse bitmaps, just 10 bits. SDL in Linux seems to just work and uses SDL buttons b1 to b10 to map those bits to SDL buttons 1:1, at least gamepad-tool works correctly, no customer controllerdb needed.

Oh, it's here: https://discord.gg/j8reCvPanF (or you click on the badges in the project READMEs).

In my defense the badge doesn't mention Discord by name and is not particularly ctrl+f friendly :-P

Okay I should try to make that more discoverable then. :-)

If you have both HID and evdev available it will use the HID, so that means there's still something wrong with the mapping for that. It may be just that it's not there yet or overridden.

If we talk "it will use HID" we are still talking about the Linux-side of SDL? Because Windows raw HID and Linux hidraw device seem to be a somewhat different concept: In Linux it's just a character device where you can read the stream of HID reports coming from the driver (which may have been patched or filtered by the driver as we do in xpadneo, e.g. we filter out battery reports, and adjust the bitmaps to match our descriptor). In Windows, this seem to be a set of API call where you operate on handles and callbacks. And then there the GIP protocol with Xinput and its state packets: Actually, those are not much different from our HID reports: Last time I checked, the struct is just a 1:1 representation of the HID packet of the Xbox Bluetooth controller mode, prepended by a HID report ID which is 0x01. I'm not sure what the caller of RAWINPUT_HandleStatePacket() is but it looks like it first passes the data pointer through the SDL HID handler:

    if (SDL_HidP_GetData(HidP_Input, ctx->data, &data_length, ctx->preparsed_data, (PCHAR)data, size) != HIDP_STATUS_SUCCESS) {
        return;
    }

So, anything interesting is happening there, the rest of the function just looks at the button indices to create a joystick state for SDL from it, and this function is actually a procedure address from some HID DLL. So the code that does anything is probably in wine?

    SDL_HidP_GetData = (HidP_GetData_t)GetProcAddress(s_pHIDDLL, "HidP_GetData");

So we should probably look at how wine creates the HID reports it reads from the Linux SDL API?

Is it part of the released [Steam Runtime] SDL?

It has 2.0.14, the controllerdb is converted into a header file and compiled into the code, so there's no way to look at it in a binary package. But 2.0.12 has GUID 050000005e040000e002000003090000 on my system with proper mappings, so 2.0.14 should have it, too. But 2.0.14 may handle hidraw differently, so I probably need to update and check if the mappings are still correct when I enable hidraw user access in xpadneo. If yes, then we should start looking at the next layer, and that's probably wine crafting synthetic HID reports from the Linux-side SDL events - I mean before we are even in Windows user space?

or try to override the mappings with SDL_GAMECONTROLLERCONFIG.

On the Linux side, mappings are correct in SDL, even with hidraw, at least with version 2.0.12. Let me check 2.0.14 first. An override should not be needed, and I didn't define one. There are actually more places where you can add mappings (you could place a controllerdb in your $HOME/.{config,local}/ somewhere, I believe, but I don't have that).

Steam seems to be setting that variable as well, so you may want to check the environment for the game process (cat /proc/$PID/environ | tr '\0' '\n').

Yes, it does. It stores a mapping in config.vdf which is passes to games via environment then. I cleared those config settings. Early, before we had Steam Input or xpadneo, I used that to properly map my controllers, but that was of limited success because some games didn't use SDL, so I started to work on xpadneo to fix that at one level lower.

I think we should look at if Proton passes that correctly into the bwrap container, then look at if wineserver sees that. Does wine pass Linux environment variables over into the Windows process environment? I mean, would I see that in cmd when I run set? I'm not sure what's correct but there may be conflicts if it is passed into the Windows process environment but outside-SDL and wine mangle the report format into one common abstraction. OTOH, not passing it into the Windows process environment may have unexpected results for some users. Probably the sane way would be to mangle all SDL controllers into a consistent report format as seen by Windows processes, and then just not pass the variable at all - or craft a new variable that matches the exact format produced by the wine input handler.

Gamepad-tool may come in handy for testing the mappings.

Yes, I'm using that. It's easy, simple, fast, and tells me if it found mappings or didn't. I just wish it would also say where it found a mapping instead of just saying "mapping found" - but that's probably a limitation of SDL itself: It doesn't track the source of mappings when it loads them into its configuration state.

BTW: Proton seems to craft gamepads from joysticks somehow, at least I am able to control my AC:Odyssey character with my joystick. But the thing is: While gamepad-tool sees the joystick, too, none of the inputs match any of the Xbox inputs shows in the schematic. But Proton manages to do that. This leads to situations where my character would move in circles until I touch my joystick once (which initializes the calibration properly and makes all axes centered). Maybe Proton should not treat joysticks as xinput devices. Per Linux kernel definition, you can tell gamepads and joysticks apart by looking if evdev has a button BTN_GAMEPAD or BTN_JOYSTICK. Not sure if SDL does or has something similar as Proton probably wants to use SDL only and not bother with evdev. But TBH, I haven't tested this issue for a while now. Maybe it's no longer an issue but back then I simply blacklisted my joysticks for SDL unless running specific games:

[ "${PROTON_USE_JOYSTICK}" -eq "1" ] || export SDL_GAMECONTROLLER_IGNORE_DEVICES=0x044F/0xB10A,0x044F/0xB687

which allows me to run games with PROTON_USE_JOYSTICK=1 if I really use a joystick game (Elite Dangerous for that matter).

@kakra
Copy link
Contributor Author

kakra commented Apr 5, 2021

BTW, I'm logging the environment each time when starting a proton game through my launcher wrapper (my-wrapper %command%):

declare -x SDL_AUDIO_CHANNELS="6"
declare -x SDL_AUDIO_FREQUENCY="48000"
declare -x SDL_GAMECONTROLLERCONFIG="03000000de280000ff11000001000000,Steam Virtual Gamepad,a:b0,b:b1,back:b6,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b8,leftshoulder:b4,leftstick:b9,lefttrigger:a2,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b10,righttrigger:a5,rightx:a3,righty:a4,start:b7,x:b2,y:b3,
declare -x SDL_GAMECONTROLLER_ALLOW_STEAM_VIRTUAL_GAMEPAD="1"
declare -x SDL_GAMECONTROLLER_IGNORE_DEVICES="0x044F/0xB10A,0x044F/0xB687"
declare -x SDL_JOYSTICK_DEVICE="/dev/input/event8:/dev/input/event9:/dev/input/event30:/dev/input/js0:/dev/input/js1:/dev/input/js2"
declare -x SDL_JOYSTICK_HIDAPI_STEAMXBOX="0"
declare -x SDL_VIDEO_X11_DGAMOUSE="0"

What is SDL_GAMECONTROLLER_ALLOW_STEAM_VIRTUAL_GAMEPAD?

    if (SDL_GetHintBoolean("SDL_GAMECONTROLLER_ALLOW_STEAM_VIRTUAL_GAMEPAD", SDL_FALSE)) {
        /* We shouldn't ignore Steam's virtual gamepad since it's using the hints to filter out the real controllers so it can remap input for the virtual controller */
        SDL_bool bSteamVirtualGamepad = SDL_FALSE;
#if defined(__LINUX__)
        bSteamVirtualGamepad = (vendor == 0x28DE && product == 0x11FF);
#elif defined(__MACOSX__)
        bSteamVirtualGamepad = (vendor == 0x045E && product == 0x028E && version == 1);
#elif defined(__WIN32__)
        /* We can't tell on Windows, but Steam will block others in input hooks */
        bSteamVirtualGamepad = SDL_TRUE;
#endif
        if (bSteamVirtualGamepad) {
            return SDL_FALSE;
        }
    }

I think that's a Steam Input thing and this code should do nothing when I disabled Steam Input?

@kakra
Copy link
Contributor Author

kakra commented Apr 5, 2021

Okay, updated to SDL 2.0.14 and added full permissions to the hidraw device:

# sudo chmod a+rw /dev/hidraw18
# gamepad-tool
[LOG] SDL2 Gamepad Tool v1.1.2 by General Arcade (compiled with SDL version 2.0.4, DLL version 2.0.14)
[LOG] Website: http://generealarcade.com/gamepadtool/

[LOG] Searching gamepads...
[LOG] Found 3 gamepad(s):
[LOG] "Xbox One S Controller", 030000005e040000e002000000006800 (mapping available)
[LOG] "Thrustmaster TWCS Throttle", 030000004f04000087b6000011010000
[LOG] "Thrustmaster T.16000M", 030000004f0400000ab1000000010000
Request worker start in Thread  0x7f58f8476dc0
[LOG] Environment variable "SDL_GAMECONTROLLERCONFIG" is not defined
Starting worker process in Thread  0x7f58d2ffd640
[LOG] Checking if new mappings available from github: https://github.com/gabomdq/SDL_GameControllerDB
Request worker aborting in Thread  0x7f58f8476dc0
Aborting worker process in Thread  0x7f58d2ffd640
Worker process finished in Thread  0x7f58d2ffd640
Deleting thread and worker in Thread  0x7f58f8476dc0

The mappings are broken now. The GUID "030000005e040000e002000000006800" seems to be crafted by SDL, and it has a wrong mapping.

Revoking permissions from hidraw, and all is fine again (and the Xbox controller won't be the first but the last device detected then).

@kakra
Copy link
Contributor Author

kakra commented Apr 5, 2021

So I'll dig my nose into the SDL source to find where they get the broken mapping from because 045E:02E0 doesn't have the mapping they seem to assume (it's in their own controllerdb that this has a non-sparse 1:1 button bitmap).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants