mbox series

[v0,0/2] HID: nintendo: avoid BT rumble disconnections

Message ID 20230203215119.435091-1-djogorchock@gmail.com
Headers show
Series HID: nintendo: avoid BT rumble disconnections | expand

Message

Daniel Ogorchock Feb. 3, 2023, 9:51 p.m. UTC
The hid-nintendo driver has been plagued by an issue where rumble
command traffic to bluetooth-connected controllers can cause frequent
power downs of the controllers.

Digging into other pro controller driver implementations, I've not found
anything that hid-nintendo is doing incorrectly. Some implementations
seem to be working around the same problem (e.g. libsdl's hidapi
implementation rate limits rumble commands to avoid the problem).

hid-nintendo already rate limits rumble control packets, but that does
not solve the problem.

Using btmon output, I've fuond the the disconnections reliably occur
shortly after the controller's reporting rate become erratic. The
controller is meant to report input packets roughly every 15ms. Sending
rumble commands can sometimes result in the input packets arriving in
bursts/batches. Once the controller and/or BT stack enters this state,
even halting rumble commands will never allow the reporting rate to
recover to nominal. The controller will eventually disconnect.

This patch set strives to avoid the problematic scenario. It detects if
input reports arrive at clearly incorrect deltas. During these times,
the driver will hold off on transmitting any rumble commands to the
controller. This approach has allowed the reporting rate to reliably
recover in my testing. I've not been able to generate a controller
disconnection during hours of testing games with frequent rumble.

The behavior of this mechanism is tunable using #defines. We may need to
tweak/tune as the mitigation is used on different hardware setups.

My suspicion is that the core issue is somewhere in the bluez stack. My
next step is to investigate that lead in more detail. This patchset at
least allows for use of the controllers via bluetooth with rumble
enabled without frequently disconnecting.

Daniel J. Ogorchock (2):
  HID: nintendo: prevent rumble queue overruns
  HID: nintendo: fix rumble rate limiter

 drivers/hid/hid-nintendo.c | 95 ++++++++++++++++++++++++++++++++++----
 1 file changed, 86 insertions(+), 9 deletions(-)

Comments

Silvan Jegen March 7, 2023, 9:03 p.m. UTC | #1
Hi

"Daniel J. Ogorchock" <djogorchock@gmail.com> wrote:
> The hid-nintendo driver has been plagued by an issue where rumble
> command traffic to bluetooth-connected controllers can cause frequent
> power downs of the controllers.
> 
> Digging into other pro controller driver implementations, I've not found
> anything that hid-nintendo is doing incorrectly. Some implementations
> seem to be working around the same problem (e.g. libsdl's hidapi
> implementation rate limits rumble commands to avoid the problem).
> 
> [...]
> 
> Daniel J. Ogorchock (2):
>   HID: nintendo: prevent rumble queue overruns
>   HID: nintendo: fix rumble rate limiter
> 
>  drivers/hid/hid-nintendo.c | 95 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 86 insertions(+), 9 deletions(-)

I haven't tested this but the code of this series looks good to me.

Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
Silvan Jegen March 7, 2023, 9:06 p.m. UTC | #2
"Daniel J. Ogorchock" <djogorchock@gmail.com> wrote:
> The hid-nintendo driver has been plagued by an issue where rumble
> command traffic to bluetooth-connected controllers can cause frequent
> power downs of the controllers.
> 
> [...] 
> 
> My suspicion is that the core issue is somewhere in the bluez stack. My
> next step is to investigate that lead in more detail. This patchset at
> least allows for use of the controllers via bluetooth with rumble
> enabled without frequently disconnecting.
> 
> Daniel J. Ogorchock (2):
>   HID: nintendo: prevent rumble queue overruns
>   HID: nintendo: fix rumble rate limiter
> 
>  drivers/hid/hid-nintendo.c | 95 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 86 insertions(+), 9 deletions(-)

I haven't tested this but the code looks good to me.

Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
Jiri Kosina March 10, 2023, 2:02 p.m. UTC | #3
On Fri, 3 Feb 2023, Daniel J. Ogorchock wrote:

> The hid-nintendo driver has been plagued by an issue where rumble
> command traffic to bluetooth-connected controllers can cause frequent
> power downs of the controllers.
> 
> Digging into other pro controller driver implementations, I've not found
> anything that hid-nintendo is doing incorrectly. Some implementations
> seem to be working around the same problem (e.g. libsdl's hidapi
> implementation rate limits rumble commands to avoid the problem).
> 
> hid-nintendo already rate limits rumble control packets, but that does
> not solve the problem.
> 
> Using btmon output, I've fuond the the disconnections reliably occur
> shortly after the controller's reporting rate become erratic. The
> controller is meant to report input packets roughly every 15ms. Sending
> rumble commands can sometimes result in the input packets arriving in
> bursts/batches. Once the controller and/or BT stack enters this state,
> even halting rumble commands will never allow the reporting rate to
> recover to nominal. The controller will eventually disconnect.
> 
> This patch set strives to avoid the problematic scenario. It detects if
> input reports arrive at clearly incorrect deltas. During these times,
> the driver will hold off on transmitting any rumble commands to the
> controller. This approach has allowed the reporting rate to reliably
> recover in my testing. I've not been able to generate a controller
> disconnection during hours of testing games with frequent rumble.
> 
> The behavior of this mechanism is tunable using #defines. We may need to
> tweak/tune as the mitigation is used on different hardware setups.
> 
> My suspicion is that the core issue is somewhere in the bluez stack. My
> next step is to investigate that lead in more detail. This patchset at
> least allows for use of the controllers via bluetooth with rumble
> enabled without frequently disconnecting.
> 
> Daniel J. Ogorchock (2):
>   HID: nintendo: prevent rumble queue overruns
>   HID: nintendo: fix rumble rate limiter
> 
>  drivers/hid/hid-nintendo.c | 95 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 86 insertions(+), 9 deletions(-)

Now queued in hid.git#for-6.4/nintendo. Thanks,