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

Kernel - Mainline #44

Closed
atar-axis opened this issue Sep 25, 2018 · 52 comments
Closed

Kernel - Mainline #44

atar-axis opened this issue Sep 25, 2018 · 52 comments

Comments

@atar-axis
Copy link
Owner

atar-axis commented Sep 25, 2018

I would really appreciate if somebody, who is experienced there, would help me to push this driver into the kernel.

@kakra

@atar-axis atar-axis changed the title Kernel Mainline Kernel - Mainline Sep 25, 2018
@atar-axis atar-axis added the help wanted Extra attention is needed label Sep 26, 2018
@kakra
Copy link
Collaborator

kakra commented Sep 26, 2018

Not really experienced with the process itself but I could start with a code and style review, and research the rest of the requirements.

@atar-axis
Copy link
Owner Author

Hooray - I would really appreciate that! I think the driver is stable enough now.

@atar-axis
Copy link
Owner Author

atar-axis commented Sep 26, 2018

https://lore.kernel.org/patchwork/patch/973394/#1175786

@cgutman
Copy link

cgutman commented Dec 17, 2018

Did you ever submit your Bluetooth patch upstream? I'm interested in helping with that.

@atar-axis
Copy link
Owner Author

Hey @cgutman, unfortunately I haven't since patches to the bluetooth part of linux seem to require some additional verfication process.

@cgutman
Copy link

cgutman commented Dec 17, 2018

I ordered the PTS dongle, so I can do the verification once I receive it.

@atar-axis
Copy link
Owner Author

Wonderful :) Tell me when it arrived.

@Hedronmx
Copy link

Is this on kernel already? Saw an article about kernel 4.20 and shows that xbox one s controller rumble is supported... I am a bit confused.

@atar-axis
Copy link
Owner Author

it isn't, but yes valve added basic rumble support in 4.20 but none of the other things like battery indication, mapping correction and so on. :)

@ah-
Copy link

ah- commented Feb 27, 2019

Hey, are you still interested in getting your improvements upstreamed?

It looks like there are a whole bunch of things this driver does better than current upstream, like advanced rumble, battery reports, report fixups etc.

And it'd be really nice to have that work just out of the box on Linux rather than having to install a dkms driver manually.

I suspect the best way of getting these changes accepted is to do it really incrementally and add to the existing hid-microsoft driver a feature at a time.

A good starting point might be the battery reporting?
So take the existing battery parsing functionality, extract just the minimally necessary functions and port them over to https://github.com/torvalds/linux/blob/master/drivers/hid/hid-microsoft.c

https://github.com/torvalds/linux/blob/master/drivers/hid/hid-sony.c looks helpful as a reference for how to wire it all up.

That should result in a small patch that we could send upstream and hopefully merged :)

@julien-f
Copy link

It would be so nice to see this upstreamed 🙂

If this is a lot of work, maybe you could do bounty for this or do a crowdfounding campain like @flibitijibibo did for adding Nintendo's USB GameCube controller support in SDL2.

I'm sure you could get some traction on this.

@atar-axis
Copy link
Owner Author

The point is, that I am not too eager to stuff the whole driver into hid-microsoft, this is simply not the right (or at least a not clever) way to go in my eyes.

The reson is that this driver is still in progress, Elite support and other devices and interfaces will follow - all this will more and more blow up hid-microsoft.

It wouldn't be fun to maintain this thing then.

If someone with decisional power would tell me that we can leave hid-xpadneo in one peace, I woud immediately do it.

@mateusfccp
Copy link

@atar-axis did you give up on this?

@atar-axis
Copy link
Owner Author

Not really, but there is still so much to do before it is worth the efforts. But you are right, let me reopen it ;)

@atar-axis atar-axis reopened this May 15, 2019
@mateusfccp
Copy link

I don't know the process, but it doesn't seem to be very straightforward. I'm sorry I can't really help, as I don't really know anything about kernels, drivers, or mainlining process.

Anyway, so that everyone can see, what does it need to be done that is blocking this issue?

@ernestask
Copy link

There is the staging tree, you know.

@luser
Copy link

luser commented Apr 16, 2020

Hey! I wrote the Xbox One wired controller support in the xpad driver a few years ago. If there's anything I could do to help you here I'd be happy to try! Getting patches into the Linux kernel is a huge headache. I wound up exchanging emails with someone at Valve who helped get my patch landed.

@kakra
Copy link
Collaborator

kakra commented May 10, 2020

I think it may be worth porting single features that this has over upstream hid-microsoft.c directly to that latter driver. One I can think of would be to support trigger rumble upstream so we could remove that from this driver and use upstream instead. That way, trigger rumble could become actually supported in games. And kernel upstream surely has better ideas how to integrate it into the FF API - misusing direction doesn't feel like the proper ultimate way to do it.

@luser Do you know of any discussion enhancing the FF API?

@mercuriete
Copy link

I agree with @kakra
It would be nice to upstream small features like....

select and guide button not working on elite 2 controller.
or battery monitoring feature.

I think the select and guide button are not reported properly using the hid-generic so we need at least a few quirks to have a good out the box experience.

btw thank for this great project :)

@kakra
Copy link
Collaborator

kakra commented May 30, 2020

The XBE2 controller still has issues according to reports in this project. So we'd need to fix those first before the feature could be considered for porting to upstream.

@mateusfccp
Copy link

I don't know... Simply mainlining battery or things like these... In my experience (Xbox One S controller, last revision), there are cases that the controller will simply not work at all with xpad. For example, to use it with Kodi I had to compile Kodi with xpadneo, because with the default xpad driver only the select button would work, and incorrectly mapped.

@hadess
Copy link

hadess commented Jan 26, 2021

The pre-requisite Bluetooth patch has landed in bluetooth-next, and work on upstream can proceed.

I think the best course of action would be to clean up the upstream to be able to bring in (stable) functionality that's available here piecemeal, and once that's done, rebase the driver in this repo to some sort of a staging ground for the upstream driver.

The first step would be to split up the xbox controller support from the hid-microsoft.c source into a separate driver. The new PS5 controller support is also using a separate file for that, rather than adding more into hid-sony.c. Then each new functionality or device ID can be a separate patch.

@atar-axis you have my email address, feel free to CC: me on patches you want to send upstream, or branches you'd like to me to inspect on github, I'm eager to have all this work reach upstream so anyone can use those devices. I also have a bunch of XBox One and Series X controllers here for testing should that be needed.

@kakra
Copy link
Collaborator

kakra commented Jan 26, 2021

@hadess Cool, sounds like a plan then.

@mercuriete
Copy link

Sorry for the noise.
I have an elite 2 controller and I am a Gentoo user and I know how to compile the kernel.

I can test patches if you need testers.

I would like to see this driver in mainline.

Thanks for your work.

@kakra
Copy link
Collaborator

kakra commented Jan 26, 2021

@mercuriete For Gentoo, download https://github.com/kakra/linux/pull/11.patch and drop the file into /etc/portage/patches/sys-kernel/gentoo-sources/9300_xpadneo.patch (simply create missing directories, or if you're using a different kernel, use an appropriate named directory). Then re-emerge gentoo-sources. If you're later seeing problems during merge, you may need to re-download the file as I will update the PR regularly to match 5.10 LTS (you can subscribe the PR). Run make menuconfig to select xpadneo either built-in or as a module.

With this patch in place, you can remove the ERTM fix, udev rules (mostly) and modalias hints.

This is actually how I am using this in Gentoo so I don't have to re-install xpadneo after each kernel update.

BTW: This is for kernel 5.10 which may still be keyworded in Gentoo. I'm pretty sure you know how to properly get that kernel version for Gentoo.

@mercuriete
Copy link

mercuriete commented Jan 27, 2021

@kakra
TLDR;
WORKS_FOR_ME

I will update this comment when I finish more tests

  1. the patch was copied today 2021-01-27
  2. the patch was applied on top of sys-kernel/gentoo-sources-5.10.10
  3. my controller is a Elite Wireless Controller Series 2

tested on:

uname -a
Linux desktop 5.10.10-gentoo #1 SMP PREEMPT Wed Jan 27 10:58:11 WET 2021 x86_64 AMD Ryzen 7 2700X Eight-Core Processor AuthenticAMD GNU/Linux

my kernel config is:

cat .config | grep XPADNEO
CONFIG_HID_XPADNEO=y

bluetooth dongle (my dongle is reported to be buggy but it works for me):

lsusb
Bus 001 Device 005: ID 0a12:0001 Cambridge Silicon Radio, Ltd Bluetooth Dongle (HCI mode)

features that works:

  • Pairing without disabling ERTM
  • Buttons and axis
  • Vibration
  • Battery reporting

features that doesn't seems to work:

weird behaviours:

  • when you open on KDE joystik KCM (systemsettings) there are a huge input lag like seconds of delay when using the axis. But the input lag is perfect on games. It seems this bug is known and not a problem from this driver.

PS: @kakra thank you very much for explaining how to patch the kernel on gentoo. You are a very kind person. I hope you succeed making this driver mainline.

Updated: Battery works, Joystik KCM is known to be buggy.

@kakra
Copy link
Collaborator

kakra commented Jan 27, 2021

  • when you open on KDE joystik KCM (systemsettings) there are a huge input lag like seconds of delay when using the axis. But the input lag is perfect on games.

Yeah, I'm seeing this for joysticks, too. jstest (which is the same API that KDE is using) is fine, tho. So that's probably a KDE thing...

Battery reporting in KDE should work just fine. But the battery won't initialize until the controller sends the first report: We cannot query it. It works for me:

image

@kakra
Copy link
Collaborator

kakra commented Jan 27, 2021

TLDR;
WORKS_FOR_ME

I will update this comment when I finish more tests

BTW: There are a few other patches in that project which you might be interested in, they are applied the same. There's a Steam patch (futex2 and fsync_futex support for Proton games), and there's the CK patches which improve gaming performance. You may want to try them. Just append .patch to the URL of the PRs you are interested in to download them:

https://github.com/kakra/linux/pulls

@luser
Copy link

luser commented Jan 27, 2021

@luser Do you know of any discussion enhancing the FF API?

I missed your question—no, I'm not aware of any discussion in that area. I don't see any changes in Valve's SteamOS kernel tree that haven't been upstreamed (AFAICT) either.

@kakra
Copy link
Collaborator

kakra commented Jan 27, 2021

There's discussion about adding PS5 Dual Shock rumble APIs to the kernel which would also support direct control of the trigger rumble. But expect some years to pass until that settles.

@kakra
Copy link
Collaborator

kakra commented Jan 29, 2021

@hadess BTW: Turning on ERTM again and using the L2CAP patch instead seems to fix the remaining rumble related crash but I have to test this a little more...

@kakra
Copy link
Collaborator

kakra commented Jan 29, 2021

Turning on ERTM break the XBE2 controller, however: If I send input data constantly, it eventually stops sending more data to the Bluetooth stack until I stop generating input data. It looks like it needs some sort of ACK, maybe? @hadess Do you know more about the Bluetooth protocol and what may be missing?

@kakra
Copy link
Collaborator

kakra commented Jan 29, 2021

It really looks like it needs some ACK: After such an incident, the controller will spam the Bluetooth stack with data even when sitting idle - it seems to expect some sort of ACK that data was received.

@kakra
Copy link
Collaborator

kakra commented Jan 29, 2021

This may explain why some people see extreme lag: As soon as I move a stick, the controller stops sending data - probably because some internal buffer of unACKed packets is full. Any new values seem to lag behind now.

@kakra
Copy link
Collaborator

kakra commented Jan 29, 2021

If I send input data constantly, it eventually stops sending more data to the Bluetooth stack until I stop generating input data

Some more updates: This behavior is gone when the controller is properly authenticated (aka there's a link key in the Bluetooth info file for the device). I didn't manage to pair the controller correctly with a link key through bluetoothctl but it worked just fine using the KDE Bluetooth applet.

@X6205
Copy link

X6205 commented Aug 26, 2021

This driver is excellent. New XBOX series controller is working perfectly via bluetooth with this driver. I don't even need to apply aditional tweaks on ubuntu, like disable etrm. It should be merged asap into kernel and replace current, obsolete driver.

@kakra
Copy link
Collaborator

kakra commented Sep 2, 2021

The ERTM thing is nothing xpadneo does, it has been fixed in current kernels, it's a fix in the bluetooth stack

@mercuriete
Copy link

hi @kakra I have been using your patchs since long time ago.
Now I am trying to move to the new LTS kernel (5.15) and it doesn't apply correctly.

Can you rebase your patches to the new linux 5.15 LTS kernel?
Thank you very much.

patch: https://github.com/kakra/linux/pull/11.patch

@kakra
Copy link
Collaborator

kakra commented Nov 7, 2021

5.15 is a stable kernel but it's not promoted as an LTS kernel yet. There's still a chance that 5.16 will become the next LTS kernel because usually the kernel that goes stable before new years eve will become LTS. And even when, I usually wait for 2 or 3 point releases until I'm running that new kernel on my system.

So, until then, I'll probably rebase my patches only after 2022-01-01. Are there any reasons why you cannot stick to 5.10 LTS?

BTW: This is off-topic here because it's about the Steam performance patches. Also, kernel 5.15 probably pulls in a new version of the futex_waitv calls, so you could probably just drop all those futex/futex2 patches to make it work. I haven't looked at it in detail, and I won't rebase those patches unless I'm running that kernel myself - but I don't run non-LTS kernels. That said, I've created a discussion board over at https://github.com/kakra/linux/discussions

Sorry for the confusion, I just realized you're referencing the xpadneo patches - not the Steam patches (I'm not sure what happened on my side). Still, I think the discussion should be continued there...

@Apacelus
Copy link

Just wondering, do you plan to upstream the driver to the kernel? The newest xbox controllers dont work properly ootb on 6.0.12.

@kakra
Copy link
Collaborator

kakra commented Feb 21, 2023

This is difficult as user-space like SDL and Steam Input rely on the "broken" behavior of the default input drivers (actually, it's not really broken, it's rather more or less unfiltered raw HID reports). Thus, "fixing" that introduces input bugs in said software and we are already jumping loops and hoops to work around that.

Fixing this will essentially mean that we more or less identical to upstream kernel drivers so I probably try to upstream fixes instead, like properly throttling rumble commands to fix firmware crashes. Xpadneo will more likely remain a driver that adds additional features at the driver level which you would otherwise use Steam Input for.

That said, there are a few considerations in #286 and maybe some advanced features for XBE2 controllers could be ported to the kernel (like RGB control for the LED, profile programming) but I didn't reverse engineer that yet.

So in the long run, I'd like to port fixes and native hardware features over to the kernel drivers but xpadneo itself won't become a kernel driver (it breaks some rules about what kernel drivers should do), and some fixes (like rumble throttling) should even be placed outside of the input drivers but rather in ff_memless.

There's a report about the latest firmware update being problematic, not sure if your post is about that: #407

@KAMiKAZOW
Copy link

This issue is open since 5 years and in all that time nobody noticed that you need to move to GPLv2 first?

GPLv3 is incompatible with the kernel license which is v2-only. You need to relicense your code under GPLv2-only or GPLv2-or-later first.

Luckily contributions to documentation and CI scripts are not part of the core code, so there's fewer people to ask but maybe extending https://github.com/medusalix/xone instead may be less work and that is GPLv2 already.

@kakra
Copy link
Collaborator

kakra commented Aug 17, 2023

xone is a completely different driver, they have nothing in common. You cannot just replace one with the other, they are using different protocols.

Also, there's no need to put xpadneo in the kernel - the kernel already supports the devices just fine. But the kernel could need a patch in ff_memless and that would benefit all drivers: hid-microsoft, xpadneo, and xone. Since this code is not patching ff_memless, a patch would be completely new code and just based on ideas I actually developed myself here.

OTOH, xone seems to be more or less dead currently, there's no ongoing development.

That said, xpadneo doesn't provide any functions currently which the kernel is missing - and the additional QoL features it provides would never be accepted into the kernel. Thus, I'm first heading xpadneo into a slightly different direction, maybe even making it obsolete as a kernel driver and just providing a user-space daemon.

Without our special features, xpadneo just provides the firmware rumble crash fixup which should rather go directly into ff_memless. It's not specific to xpadneo: xone implements it, too. SDL implements it. Probably other do.

@kakra
Copy link
Collaborator

kakra commented Aug 17, 2023

Closing as no longer planned. See also: #289

@TCROC
Copy link

TCROC commented Aug 29, 2023

xone is a completely different driver, they have nothing in common. You cannot just replace one with the other, they are using different protocols.

Also, there's no need to put xpadneo in the kernel - the kernel already supports the devices just fine. But the kernel could need a patch in ff_memless and that would benefit all drivers: hid-microsoft, xpadneo, and xone. Since this code is not patching ff_memless, a patch would be completely new code and just based on ideas I actually developed myself here.

OTOH, xone seems to be more or less dead currently, there's no ongoing development.

That said, xpadneo doesn't provide any functions currently which the kernel is missing - and the additional QoL features it provides would never be accepted into the kernel. Thus, I'm first heading xpadneo into a slightly different direction, maybe even making it obsolete as a kernel driver and just providing a user-space daemon.

Without our special features, xpadneo just provides the firmware rumble crash fixup which should rather go directly into ff_memless. It's not specific to xpadneo: xone implements it, too. SDL implements it. Probably other do.

I'm currently using Pop OS kernel 6.4.6-76060406-generic. The default driver included in the kernel did not work for me with by XBox Series X controller. The right joystick and right trigger were flipped with each other which broke any usefullness of the game pad. this repo worked correctly right out of the box. So I think it still potentially has value being mainstreamed into the kernel.

NOTE: I just downloaded this and am new here. I may have misunderstood. Just saw this issue title and got excited :)

@kakra
Copy link
Collaborator

kakra commented Aug 30, 2023

The right joystick and right trigger were flipped with each other which broke any usefullness of the game pad. this repo worked correctly right out of the box

This behavior should be fixed with latest SDL versions. If this affects Steam, turn off Steam Input. However, even Steam Input should work properly once the new SDL landed.

The way xpadneo works around this problem is faking a different device ID which matches the Steam virtual controller in wine, and we adjust our input reports to those expectations - and thus it maps properly. Such a behavior is unlikely to be accepted into the kernel. User-space must be fixed at this point. SDL has lately added features to properly detect the mapping layouts of all xpad-like HID drivers.

@kakra kakra closed this as not planned Won't fix, can't repro, duplicate, stale Aug 30, 2023
@TCROC
Copy link

TCROC commented Aug 30, 2023

The right joystick and right trigger were flipped with each other which broke any usefullness of the game pad. this repo worked correctly right out of the box

This behavior should be fixed with latest SDL versions. If this affects Steam, turn off Steam Input. However, even Steam Input should work properly once the new SDL landed.

The way xpadneo works around this problem is faking a different device ID which matches the Steam virtual controller in wine, and we adjust our input reports to those expectations - and thus it maps properly. Such a behavior is unlikely to be accepted into the kernel. User-space must be fixed at this point. SDL has lately added features to properly detect the mapping layouts of all xpad-like HID drivers.

When are these sdl versions landing in the kernel? Or that may be the wrong question:

When will it be fixed to work out of the box?

@kakra
Copy link
Collaborator

kakra commented Aug 30, 2023

SDL is user-space, not kernel. You get an updated SDL version from your distribution. SDL (Simple DirectMedia Layer) is somewhat related to what DirectX (as a package of components for audio/video/graphics/input) is to Windows, while the kernel is what drivers are to Windows.

Official releases are here: https://github.com/libsdl-org/SDL/releases

But your distribution provides the installation packages. Version 2.28.0 or higher should have the fixed behavior.

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

No branches or pull requests