diff mbox series

[v1] hid-playstation: DS4: Update rumble and lightbar together

Message ID 20240616163055.75174-1-max@enpas.org (mailing list archive)
State New
Delegated to: Jiri Kosina
Headers show
Series [v1] hid-playstation: DS4: Update rumble and lightbar together | expand

Commit Message

Max Staudt June 16, 2024, 4:30 p.m. UTC
Some 3rd party gamepads expect updates to rumble and lightbar together,
and setting one may cancel the other.

This patch maximises compatibility by always sending rumble and lightbar
updates whenever updates are sent to the gamepad: valid_flag0 is always
>= 0x03.

Further background reading:

- Apparently the PS4 always sends rumble and lightbar updates together:

  https://eleccelerator.com/wiki/index.php?title=DualShock_4#0x11_2

- 3rd party gamepads may not implement lightbar_blink, and may simply
  ignore updates with 0x07 set, according to:

  https://github.com/Ryochan7/DS4Windows/pull/1839

This patch leaves the lightbar blink feature as-is: Likely, most users
are unaware that it exists, hence it's unlikely that a packet with
0x07 set will even be sent in practice. Let's leave the function there,
so users of first-party gamepads can continue to use it.
---
 drivers/hid/hid-playstation.c | 55 ++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 24 deletions(-)

Comments

Roderick Colenbrander June 17, 2024, 11:01 p.m. UTC | #1
Hi Max,

Thanks for your patch. I'm a little hesitant to make changes here. The
console itself does the updates similar to this, though there are
different userspace kind of ways there to send things (so some paths
may kind of do what you saw). These flags are often used also to
enable other features in the output report (e.g. audio settings or
many others).

This driver is used in a lot of devices. Different vendors ranging
e.g. car manufacturers, phones, TVs,.. have their own unit /
regression tests suites to test integration. Where they send FF rumble
for example and check the bits in the output reports

We went through a lot of pain with many of such vendors (and are still
going through some). I would rather not shake things up for such a
narrow use cases for non-official devices.

Thanks,
Roderick

On Sun, Jun 16, 2024 at 9:40 AM Max Staudt <max@enpas.org> wrote:
>
> Some 3rd party gamepads expect updates to rumble and lightbar together,
> and setting one may cancel the other.
>
> This patch maximises compatibility by always sending rumble and lightbar
> updates whenever updates are sent to the gamepad: valid_flag0 is always
> >= 0x03.
>
> Further background reading:
>
> - Apparently the PS4 always sends rumble and lightbar updates together:
>
>   https://eleccelerator.com/wiki/index.php?title=DualShock_4#0x11_2
>
> - 3rd party gamepads may not implement lightbar_blink, and may simply
>   ignore updates with 0x07 set, according to:
>
>   https://github.com/Ryochan7/DS4Windows/pull/1839
>
> This patch leaves the lightbar blink feature as-is: Likely, most users
> are unaware that it exists, hence it's unlikely that a packet with
> 0x07 set will even be sent in practice. Let's leave the function there,
> so users of first-party gamepads can continue to use it.
> ---
>  drivers/hid/hid-playstation.c | 55 ++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index e7c309cfe3a0..986a4ca8b664 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -387,12 +387,10 @@ struct dualshock4 {
>         bool update_bt_poll_interval;
>         uint8_t bt_poll_interval;
>
> -       bool update_rumble;
>         uint8_t motor_left;
>         uint8_t motor_right;
>
>         /* Lightbar leds */
> -       bool update_lightbar;
>         bool update_lightbar_blink;
>         bool lightbar_enabled; /* For use by global LED control. */
>         uint8_t lightbar_red;
> @@ -2092,8 +2090,6 @@ static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
>                 }
>         }
>
> -       ds4->update_lightbar = true;
> -
>         spin_unlock_irqrestore(&ds4->base.lock, flags);
>
>         dualshock4_schedule_work(ds4);
> @@ -2143,26 +2139,39 @@ static void dualshock4_output_worker(struct work_struct *work)
>
>         spin_lock_irqsave(&ds4->base.lock, flags);
>
> -       if (ds4->update_rumble) {
> -               /* Select classic rumble style haptics and enable it. */
> -               common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_MOTOR;
> -               common->motor_left = ds4->motor_left;
> -               common->motor_right = ds4->motor_right;
> -               ds4->update_rumble = false;
> -       }
> +       /*
> +        * PS4 seems to send 0xf3 updates by default, according to a HCI trace:
> +        *   https://eleccelerator.com/wiki/index.php?title=DualShock_4#0x11_2
> +        *
> +        * This seems to be a very compatible value with third-party pads:
> +        *   https://github.com/Ryochan7/DS4Windows/pull/1839
> +        *
> +        * However, hid-playstation in v6.10 does not set the upper nibble,
> +        * and neither does hid-sony in v6.2, or BlueRetro. We should stick
> +        * to that for now, to minimise the chance of unexpected changes.
> +        *
> +        * So let's always update rumble and lightbar at the same time, with
> +        * the upper nibble cleared, resulting in valid_flag0 == 0x03.
> +        * Hopefully this will maximise compatibility with third-party pads.
> +        */
> +       common->valid_flag0 = DS4_OUTPUT_VALID_FLAG0_MOTOR |
> +                             DS4_OUTPUT_VALID_FLAG0_LED;
>
> -       if (ds4->update_lightbar) {
> -               common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED;
> -               /* Comptabile behavior with hid-sony, which used a dummy global LED to
> -                * allow enabling/disabling the lightbar. The global LED maps to
> -                * lightbar_enabled.
> -                */
> -               common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0;
> -               common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0;
> -               common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0;
> -               ds4->update_lightbar = false;
> -       }
> +       common->motor_left = ds4->motor_left;
> +       common->motor_right = ds4->motor_right;
> +
> +       /* Compatible behavior with hid-sony, which used a dummy global LED to
> +        * allow enabling/disabling the lightbar. The global LED maps to
> +        * lightbar_enabled.
> +        */
> +       common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0;
> +       common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0;
> +       common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0;
>
> +       /*
> +        * Output reports updating lightbar_blink will simply be ignored
> +        * by incompatible controllers, since valid_flag0 == 0x07.
> +        */
>         if (ds4->update_lightbar_blink) {
>                 common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED_BLINK;
>                 common->lightbar_blink_on = ds4->lightbar_blink_on;
> @@ -2459,7 +2468,6 @@ static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
>                 return 0;
>
>         spin_lock_irqsave(&ds4->base.lock, flags);
> -       ds4->update_rumble = true;
>         ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
>         ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
>         spin_unlock_irqrestore(&ds4->base.lock, flags);
> @@ -2520,7 +2528,6 @@ static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
>         ds4->lightbar_green = player_colors[player_id][1];
>         ds4->lightbar_blue = player_colors[player_id][2];
>
> -       ds4->update_lightbar = true;
>         dualshock4_schedule_work(ds4);
>  }
>
> --
> 2.39.2
>
>
Max Staudt June 20, 2024, 7:26 p.m. UTC | #2
Hi Roderick,

So as far as I understand, my suggested driver behaviour is sound, because both the console's own behaviour as well as other drivers show that flags == 0x03 is working perfectly fine with original controllers. Is my understanding correct?

In fact, hid-sony used to send these updates at the same time (it had flags == 0x07), so for some 3rd party controllers, the move to hid-playstation has already been a regression in the FF/lightbar department.

Do you see a way to move forward with this patch and get it merged, even if it is with some delay? Is there something that I can improve?


As for downstream users' regression tests, this argument confuses me. Could you please give me a bit of help here?

My understanding, so far, is as follows:

Tests checking the FF bit should not fail if, say, the lightbar bit is also set. If they fail, then that means that the test is too sensitive. After all, the patch does not change anything from userspace's point of view, nor from the actual human user's point of view. The DualShock 4 behaves all the same, and it's just the wire protocol that changes.

So if a downstream user wishes to do a full end-to-end integration test, technically they would need to connect a real DualShock 4 and test that. But I can see that this is not easily automatable ;) so they may test at the HID level instead. The result is that, depending on how they structure their tests, they might no longer be testing end-to-end, but rather testing an implementation and its quirks. This bears the risk that the test will fail because of a legitimate change in the driver, or elsewhere in the kernel.

I suppose this is what you want to avoid, but... isn't avoiding such changes the reason why LTS kernels exist?

And there is only one LTS kernel with this driver, v6.6, released 8 months ago. How did it become necessary to ossify the driver's wire behaviour in this time frame?

Hence I'm confused why changing the wire protocol upstream, without breaking any functionality on the original controller or backporting to LTS kernels, creates problems. Either the tests are correctly written in a way to not be affected by this change, mimicking the original gamepad which is unaffected. Or the tests are protected from breaking by using LTS kernels. In the latter case, the tests will break on a kernel version bump - legitimately so, and fixing them should be easy.


Am I missing something?


Do you see a way to get this patch in?

Would it help you to have some time for warning your downstream contacts to stabilise their tests, and I could re-send this patch in 6 months from now?


Hopeful greetings,
Max
Roderick Colenbrander July 8, 2024, 4:07 p.m. UTC | #3
Hi Max,

See my comments inline.

Thanks,
Roderick

On Thu, Jun 20, 2024 at 12:26 PM Max Staudt <max@enpas.org> wrote:
>
> Hi Roderick,
>
> So as far as I understand, my suggested driver behaviour is sound, because both the console's own behaviour as well as other drivers show that flags == 0x03 is working perfectly fine with original controllers. Is my understanding correct?
>

The console behavior (I checked the code) does use the flags as well
like I do. The architecture there between usermode/kernel is a bit
different, so in some cases flags do get set when not needed.

Various devices tried to capture bit patterns and see what kind of
worked even though not really right. (Officially licensed controllers
are a different story they use different hid reports.) We didn't know
other devices did this wrong.

> In fact, hid-sony used to send these updates at the same time (it had flags == 0x07), so for some 3rd party controllers, the move to hid-playstation has already been a regression in the FF/lightbar department.
>
Kind of a regression, but not entirely. Many devices didn't work prior
in hid-sony, because sensor coefficients were 0 or they didn't handle
some reports (e.g. the one to get mac address, which was changed to a
more compatible one in hid-playstation).

> Do you see a way to move forward with this patch and get it merged, even if it is with some delay? Is there something that I can improve?
>
>
> As for downstream users' regression tests, this argument confuses me. Could you please give me a bit of help here?
>
> My understanding, so far, is as follows:
>
> Tests checking the FF bit should not fail if, say, the lightbar bit is also set. If they fail, then that means that the test is too sensitive. After all, the patch does not change anything from userspace's point of view, nor from the actual human user's point of view. The DualShock 4 behaves all the same, and it's just the wire protocol that changes.
>
> So if a downstream user wishes to do a full end-to-end integration test, technically they would need to connect a real DualShock 4 and test that. But I can see that this is not easily automatable ;) so they may test at the HID level instead. The result is that, depending on how they structure their tests, they might no longer be testing end-to-end, but rather testing an implementation and its quirks. This bears the risk that the test will fail because of a legitimate change in the driver, or elsewhere in the kernel.

Correct the validation tests are all uhid based, which is the best
which can be done. There is the hid-tools one, but the one which we
help out with, but the key one is the Android ones. We have so many
problems with these. Mostly because of vendors not enabling e.g. FF
support or LED support other things. The kernel landscape for Android
was very fragmented and has been getting better in the last few years
(binary kernels instead of vendors hacking up their own kernel builds
based on e.g. a fork from Qualcomm forked from Android kernel forked
from upstream).

> I suppose this is what you want to avoid, but... isn't avoiding such changes the reason why LTS kernels exist?
>
> And there is only one LTS kernel with this driver, v6.6, released 8 months ago. How did it become necessary to ossify the driver's wire behaviour in this time frame?

The main new Android kernel (public knowledge) is now 6.6 and many new
devices due later this year/early next year will use it.  The eco
system is a lot wider now and the drivers are used a lot on non-mobile
devices (cars, televisions, chromecast,..). Occassionally driver
patches are also backported from upstream to older Android kernels
(patches have to be merged upstream first).

> Hence I'm confused why changing the wire protocol upstream, without breaking any functionality on the original controller or backporting to LTS kernels, creates problems. Either the tests are correctly written in a way to not be affected by this change, mimicking the original gamepad which is unaffected. Or the tests are protected from breaking by using LTS kernels. In the latter case, the tests will break on a kernel version bump - legitimately so, and fixing them should be easy.
>
>
> Am I missing something?
>
>
> Do you see a way to get this patch in?
>
> Would it help you to have some time for warning your downstream contacts to stabilise their tests, and I could re-send this patch in 6 months from now?

Not that I wouldn't want these kind of patches, but I have to weigh
both sides. The pain on addressing things downstream and in Android
conformance tests is quite painful. We would also have both code paths
used in the wild forever, because existing 6.6 devices wouldn't change
behavior. (The official Android tests are kind of kernel version
agnostic as they work across multiple kernel and Android versions.

>
> Hopeful greetings,
> Max
>
Max Staudt July 10, 2024, 3:35 p.m. UTC | #4
Hi Roderick,


On 7/9/24 01:07, Roderick Colenbrander wrote:
> The console behavior (I checked the code) does use the flags as well 
> like I do. The architecture there between usermode/kernel is a bit 
> different, so in some cases flags do get set when not needed.

Thank you so, so much for double checking this. It's always great to 
have someone who can speak authoritatively on such matters and eliminate 
the guesswork.


> Various devices tried to capture bit patterns and see what kind of 
> worked even though not really right. (Officially licensed
> controllers are a different story they use different hid reports.) We
> didn't know other devices did this wrong.

Licensed controllers... That will be my next patch set, apologies in 
advance :)

They need quite a few quirks, too... And as it turns out, my previous 
patches have laid a lot of ground work for them :)


> Correct the validation tests are all uhid based, which is the best 
> which can be done.

Please correct me if I'm getting the wrong idea here, but what I read 
between the lines and from your email address is that this is something 
in Sony's interest.

So an idea comes to mind: Maybe somewhere inside Sony, there exists 
something like a DS4 simulator at the HID level, which could serve as a 
foundation for improving the tests? That would get the tests much closer 
to the gold standard, which is using a real controller.

If not, then maybe there is protocol documentation that could help test 
writers in creating more precise tests?


> There is the hid-tools one, but the one which we help out with, but
> the key one is the Android ones. We have so many problems with these.
> Mostly because of vendors not enabling e.g. FF support or LED support
> other things.

Hm, but downstream users misconfiguring kernels is not our fault, is it? 
In that case, the tests actually do their work correctly if they show 
that something is amiss.


> The main new Android kernel (public knowledge) is now 6.6 and many
> new devices due later this year/early next year will use it.  The
> eco system is a lot wider now and the drivers are used a lot on
> non-mobile devices (cars, televisions, chromecast,..). Occassionally
> driver patches are also backported from upstream to older Android
> kernels (patches have to be merged upstream first).

I see. But still, that is just typical downstream risk of building on 
behaviour that the kernel does not provide guarantees for. I know 
first-hand that backporting is a lot of work and easy to get wrong, but 
this is the first time that I hear that as a reason to stop improving 
the mainline kernel. Hence my confusion here.


> Not that I wouldn't want these kind of patches, but I have to weigh 
> both sides.

Thanks for your understanding, and hence my offer to help if I somehow 
can...


> The pain on addressing things downstream and in Android conformance
> tests is quite painful.

Hm, I can somewhat imagine this. I've heard that Android conformance is 
quite strict.

Given Sony's supposed interest (see above), I guess it would be a 
worthwhile investment to make the tests more robust? We could just hold 
off on this patch for a while until downstream has better tests... What 
would be a timeline for this to trickle downstream?


> We would also have both code paths used in the wild forever, because
> existing 6.6 devices wouldn't change behavior.

Well, that's kind of the point of LTS releases, if I'm not mistaken...


> (The official Android tests are kind of kernel version agnostic as
> they work across multiple kernel and Android versions.

Hm, sounds to me like the Android test framework is broken if it cannot 
be kernel-specific in such cases. What's required in order to improve this?



Max
Roderick Colenbrander July 17, 2024, 12:26 a.m. UTC | #5
Hi Max,

On Wed, Jul 10, 2024 at 8:35 AM Max Staudt <max@enpas.org> wrote:
>
> Hi Roderick,
>
>
> On 7/9/24 01:07, Roderick Colenbrander wrote:
> > The console behavior (I checked the code) does use the flags as well
> > like I do. The architecture there between usermode/kernel is a bit
> > different, so in some cases flags do get set when not needed.
>
> Thank you so, so much for double checking this. It's always great to
> have someone who can speak authoritatively on such matters and eliminate
> the guesswork.
>
>
> > Various devices tried to capture bit patterns and see what kind of
> > worked even though not really right. (Officially licensed
> > controllers are a different story they use different hid reports.) We
> > didn't know other devices did this wrong.
>
> Licensed controllers... That will be my next patch set, apologies in
> advance :)

Oh I see. Hm, we have been doing a lot of work in this space for both
PS4 and PS5 controllers. We just didn't submit it yet and need to
clean up the code a bit as we have some internal demand for this as
well. We need to get our hands on some more of these controllers,
validate before we were about to share the work. There are some
details to these controllers (on the console it is kind of a separate
driver even).

>
> They need quite a few quirks, too... And as it turns out, my previous
> patches have laid a lot of ground work for them :)
>
>
> > Correct the validation tests are all uhid based, which is the best
> > which can be done.
>
> Please correct me if I'm getting the wrong idea here, but what I read
> between the lines and from your email address is that this is something
> in Sony's interest.
>
> So an idea comes to mind: Maybe somewhere inside Sony, there exists
> something like a DS4 simulator at the HID level, which could serve as a
> foundation for improving the tests? That would get the tests much closer
> to the gold standard, which is using a real controller.
>
> If not, then maybe there is protocol documentation that could help test
> writers in creating more precise tests?

Unfortunately none of the documentation for our controllers is public.
Just read between the lines in the code, which we cover with some
clues here and there :)

>
> > There is the hid-tools one, but the one which we help out with, but
> > the key one is the Android ones. We have so many problems with these.
> > Mostly because of vendors not enabling e.g. FF support or LED support
> > other things.
>
> Hm, but downstream users misconfiguring kernels is not our fault, is it?
> In that case, the tests actually do their work correctly if they show
> that something is amiss.
>
>
> > The main new Android kernel (public knowledge) is now 6.6 and many
> > new devices due later this year/early next year will use it.  The
> > eco system is a lot wider now and the drivers are used a lot on
> > non-mobile devices (cars, televisions, chromecast,..). Occassionally
> > driver patches are also backported from upstream to older Android
> > kernels (patches have to be merged upstream first).
>
> I see. But still, that is just typical downstream risk of building on
> behaviour that the kernel does not provide guarantees for. I know
> first-hand that backporting is a lot of work and easy to get wrong, but
> this is the first time that I hear that as a reason to stop improving
> the mainline kernel. Hence my confusion here.
>
>
> > Not that I wouldn't want these kind of patches, but I have to weigh
> > both sides.
>
> Thanks for your understanding, and hence my offer to help if I somehow
> can...
>
>
> > The pain on addressing things downstream and in Android conformance
> > tests is quite painful.
>
> Hm, I can somewhat imagine this. I've heard that Android conformance is
> quite strict.
>
> Given Sony's supposed interest (see above), I guess it would be a
> worthwhile investment to make the tests more robust? We could just hold
> off on this patch for a while until downstream has better tests... What
> would be a timeline for this to trickle downstream?
>
>
> > We would also have both code paths used in the wild forever, because
> > existing 6.6 devices wouldn't change behavior.
>
> Well, that's kind of the point of LTS releases, if I'm not mistaken...
>
>
> > (The official Android tests are kind of kernel version agnostic as
> > they work across multiple kernel and Android versions.
>
> Hm, sounds to me like the Android test framework is broken if it cannot
> be kernel-specific in such cases. What's required in order to improve this?

It is a bit of a long process we work on with Google. Some initial
fixing of some of the bugs will be for this year to make sure the 6.6
tests pass properly. But any work to maybe handle multiple behaviors,
that's quite tricky and would take quite a bit of time to be honest.
Considering how widely Android and all these devices are used, I'm
hesitant to make changes to not cause regressions. Even just a simple
one can take forever to trickle down.

>
>
> Max
>
>

Roderick
Max Staudt July 21, 2024, 2:33 p.m. UTC | #6
On 7/17/24 09:26, Roderick Colenbrander wrote:
>> If not, then maybe there is protocol documentation that could help 
>> test writers in creating more precise tests?
> 
> Unfortunately none of the documentation for our controllers is 
> public. Just read between the lines in the code, which we cover with 
> some clues here and there :)

I've seen the clues and they are certainly an improvement of
hid-playstation over hid-sony that I am grateful for :)

As for the documentation, I didn't mean that you should open it to the
wide public (although that would be fabulous!). Rather, I suggested
providing what's needed to the people writing the downstream tests that
we've been going on about for a while now.

The utility of tests is... rather limited, if they don't verify an
actual specification, but just a specific code's behaviour. After all,
these tests would fail even when talking to a real PlayStation 4.


>>> (The official Android tests are kind of kernel version agnostic 
>>> as they work across multiple kernel and Android versions.
>> 
>> Hm, sounds to me like the Android test framework is broken if it 
>> cannot be kernel-specific in such cases. What's required in order 
>> to improve this?
> 
> It is a bit of a long process we work on with Google. Some initial 
> fixing of some of the bugs will be for this year to make sure the 6.6
> tests pass properly. But any work to maybe handle multiple behaviors,
> that's quite tricky and would take quite a bit of time to be honest.

Sure, time is not the issue, as long as there is a clear light of
correct software at the end of the tunnel :)

Are these tests open source?

Can kernel patchers run the tests easily (without installing a full
Android SDK) and see which tests their patches would break?


> Considering how widely Android and all these devices are used, I'm 
> hesitant to make changes to not cause regressions. Even just a simple
> one can take forever to trickle down.

Apologies, I am still confused.

With v6.2, hid-playstation gained DS4 support. With v6.3, hid-sony lost
DS4 support.

DS4 support was removed from hid-sony just one minor release after it
was included in hid-playstation. I didn't see any hesitation there,
despite the wire protocol seeing significant changes compared to
hid-sony, which had been deployed for almost a decade.

Given how quickly the DS4 code was removed from hid-sony, why are fixes 
suddenly difficult, less than two years into the hid-playstation era, 
after around 8 years of DS4 support in hid-sony?


The change from hid-sony to hid-playstation caused a regression on a 
real-world device, on Android phones:

I have an 8BitDo controller that quirkily emulates a DS4, and which
worked fine with my old Android phone, with a kernel from the hid-sony
days. With v6.3, this controller broke, and this went into LTS v6.6.

Does this kind of regression, which has clearly spread across the 
Android ecosystem with v6.6, deserve to be fixed?


As a generalised question, how about controllers that work on Android 
phones with kernel v6.1 (hid-sony), but not with v6.6 (hid-playstation), 
because of protocol changes that don't affect first-party controllers. 
Do we accept upstream regressions on actual, physical devices for the 
sake of passing downstream tests?



Max
diff mbox series

Patch

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index e7c309cfe3a0..986a4ca8b664 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -387,12 +387,10 @@  struct dualshock4 {
 	bool update_bt_poll_interval;
 	uint8_t bt_poll_interval;
 
-	bool update_rumble;
 	uint8_t motor_left;
 	uint8_t motor_right;
 
 	/* Lightbar leds */
-	bool update_lightbar;
 	bool update_lightbar_blink;
 	bool lightbar_enabled; /* For use by global LED control. */
 	uint8_t lightbar_red;
@@ -2092,8 +2090,6 @@  static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brig
 		}
 	}
 
-	ds4->update_lightbar = true;
-
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
 
 	dualshock4_schedule_work(ds4);
@@ -2143,26 +2139,39 @@  static void dualshock4_output_worker(struct work_struct *work)
 
 	spin_lock_irqsave(&ds4->base.lock, flags);
 
-	if (ds4->update_rumble) {
-		/* Select classic rumble style haptics and enable it. */
-		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_MOTOR;
-		common->motor_left = ds4->motor_left;
-		common->motor_right = ds4->motor_right;
-		ds4->update_rumble = false;
-	}
+	/*
+	 * PS4 seems to send 0xf3 updates by default, according to a HCI trace:
+	 *   https://eleccelerator.com/wiki/index.php?title=DualShock_4#0x11_2
+	 *
+	 * This seems to be a very compatible value with third-party pads:
+	 *   https://github.com/Ryochan7/DS4Windows/pull/1839
+	 *
+	 * However, hid-playstation in v6.10 does not set the upper nibble,
+	 * and neither does hid-sony in v6.2, or BlueRetro. We should stick
+	 * to that for now, to minimise the chance of unexpected changes.
+	 *
+	 * So let's always update rumble and lightbar at the same time, with
+	 * the upper nibble cleared, resulting in valid_flag0 == 0x03.
+	 * Hopefully this will maximise compatibility with third-party pads.
+	 */
+	common->valid_flag0 = DS4_OUTPUT_VALID_FLAG0_MOTOR |
+			      DS4_OUTPUT_VALID_FLAG0_LED;
 
-	if (ds4->update_lightbar) {
-		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED;
-		/* Comptabile behavior with hid-sony, which used a dummy global LED to
-		 * allow enabling/disabling the lightbar. The global LED maps to
-		 * lightbar_enabled.
-		 */
-		common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0;
-		common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0;
-		common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0;
-		ds4->update_lightbar = false;
-	}
+	common->motor_left = ds4->motor_left;
+	common->motor_right = ds4->motor_right;
+
+	/* Compatible behavior with hid-sony, which used a dummy global LED to
+	 * allow enabling/disabling the lightbar. The global LED maps to
+	 * lightbar_enabled.
+	 */
+	common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0;
+	common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0;
+	common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0;
 
+	/*
+	 * Output reports updating lightbar_blink will simply be ignored
+	 * by incompatible controllers, since valid_flag0 == 0x07.
+	 */
 	if (ds4->update_lightbar_blink) {
 		common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED_BLINK;
 		common->lightbar_blink_on = ds4->lightbar_blink_on;
@@ -2459,7 +2468,6 @@  static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_e
 		return 0;
 
 	spin_lock_irqsave(&ds4->base.lock, flags);
-	ds4->update_rumble = true;
 	ds4->motor_left = effect->u.rumble.strong_magnitude / 256;
 	ds4->motor_right = effect->u.rumble.weak_magnitude / 256;
 	spin_unlock_irqrestore(&ds4->base.lock, flags);
@@ -2520,7 +2528,6 @@  static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4)
 	ds4->lightbar_green = player_colors[player_id][1];
 	ds4->lightbar_blue = player_colors[player_id][2];
 
-	ds4->update_lightbar = true;
 	dualshock4_schedule_work(ds4);
 }