linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] HID: playstation: improvements and new device support
@ 2022-10-06  2:01 Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 1/3] HID: playstation: stop DualSense output work on remove Roderick Colenbrander
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Roderick Colenbrander @ 2022-10-06  2:01 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Roderick Colenbrander

Hi,

This patch series adds a couple of small bug fixes and improvements.

First, itt fixes a race condition on device removal in case new work got queued
during cleanup. This similar to something done before in hid-sony.

Next, the driver brings initial support for the new DualSense Edge controller.
It is a new controller with various new features (e.g. new special buttons),
but for now the device behaves like a regular DualSense.

Finally, the driver adds support for a new rumble mode, which has become
the default over time across PlayStation and non-PlayStation platforms (iOS/Windows)
as the original DualSense vibration mode felt different compared to previous
PlayStation controllers like DualShock 3 and DualShock 4. This new mode
requires an updated DualSense firmware (released in 2021) and is the default
mode used on the DualSense Edge.

Thanks,
Roderick Colenbrander
Sony Interactive Entertainment, LLC


Roderick Colenbrander (3):
  HID: playstation: stop DualSense output work on remove.
  HID: playstation: add initial DualSense Edge controller support
  HID: playstation: support updated DualSense rumble mode.

 drivers/hid/hid-ids.h         |  1 +
 drivers/hid/hid-playstation.c | 85 ++++++++++++++++++++++++++++++++---
 2 files changed, 79 insertions(+), 7 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] HID: playstation: stop DualSense output work on remove.
  2022-10-06  2:01 [PATCH 0/3] HID: playstation: improvements and new device support Roderick Colenbrander
@ 2022-10-06  2:01 ` Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 2/3] HID: playstation: add initial DualSense Edge controller support Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 3/3] HID: playstation: support updated DualSense rumble mode Roderick Colenbrander
  2 siblings, 0 replies; 6+ messages in thread
From: Roderick Colenbrander @ 2022-10-06  2:01 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, Roderick Colenbrander, stable

Ensure we don't schedule any new output work on removal and wait
for any existing work to complete. If we don't do this e.g. rumble
work can get queued during deletion and we trigger a kernel crash.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
CC: stable@vger.kernel.org
---
 drivers/hid/hid-playstation.c | 41 ++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 40050eb85c0a..d727cd2bf44e 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -46,6 +46,7 @@ struct ps_device {
 	uint32_t fw_version;
 
 	int (*parse_report)(struct ps_device *dev, struct hid_report *report, u8 *data, int size);
+	void (*remove)(struct ps_device *dev);
 };
 
 /* Calibration data for playstation motion sensors. */
@@ -174,6 +175,7 @@ struct dualsense {
 	struct led_classdev player_leds[5];
 
 	struct work_struct output_worker;
+	bool output_worker_initialized;
 	void *output_report_dmabuf;
 	uint8_t output_seq; /* Sequence number for output report. */
 };
@@ -299,6 +301,7 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = {
 	{0, 0},
 };
 
+static inline void dualsense_schedule_work(struct dualsense *ds);
 static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
 
 /*
@@ -789,6 +792,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
 	return ret;
 }
 
+
 static int dualsense_get_firmware_info(struct dualsense *ds)
 {
 	uint8_t *buf;
@@ -878,7 +882,7 @@ static int dualsense_player_led_set_brightness(struct led_classdev *led, enum le
 	ds->update_player_leds = true;
 	spin_unlock_irqrestore(&ds->base.lock, flags);
 
-	schedule_work(&ds->output_worker);
+	dualsense_schedule_work(ds);
 
 	return 0;
 }
@@ -922,6 +926,16 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
 	}
 }
 
+static inline void dualsense_schedule_work(struct dualsense *ds)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ds->base.lock, flags);
+	if (ds->output_worker_initialized)
+		schedule_work(&ds->output_worker);
+	spin_unlock_irqrestore(&ds->base.lock, flags);
+}
+
 /*
  * Helper function to send DualSense output reports. Applies a CRC at the end of a report
  * for Bluetooth reports.
@@ -1082,7 +1096,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 		spin_unlock_irqrestore(&ps_dev->lock, flags);
 
 		/* Schedule updating of microphone state at hardware level. */
-		schedule_work(&ds->output_worker);
+		dualsense_schedule_work(ds);
 	}
 	ds->last_btn_mic_state = btn_mic_state;
 
@@ -1197,10 +1211,22 @@ static int dualsense_play_effect(struct input_dev *dev, void *data, struct ff_ef
 	ds->motor_right = effect->u.rumble.weak_magnitude / 256;
 	spin_unlock_irqrestore(&ds->base.lock, flags);
 
-	schedule_work(&ds->output_worker);
+	dualsense_schedule_work(ds);
 	return 0;
 }
 
+static void dualsense_remove(struct ps_device *ps_dev)
+{
+	struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ds->base.lock, flags);
+	ds->output_worker_initialized = false;
+	spin_unlock_irqrestore(&ds->base.lock, flags);
+
+	cancel_work_sync(&ds->output_worker);
+}
+
 static int dualsense_reset_leds(struct dualsense *ds)
 {
 	struct dualsense_output_report report;
@@ -1237,7 +1263,7 @@ static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t gr
 	ds->lightbar_blue = blue;
 	spin_unlock_irqrestore(&ds->base.lock, flags);
 
-	schedule_work(&ds->output_worker);
+	dualsense_schedule_work(ds);
 }
 
 static void dualsense_set_player_leds(struct dualsense *ds)
@@ -1260,7 +1286,7 @@ static void dualsense_set_player_leds(struct dualsense *ds)
 
 	ds->update_player_leds = true;
 	ds->player_leds_state = player_ids[player_id];
-	schedule_work(&ds->output_worker);
+	dualsense_schedule_work(ds);
 }
 
 static struct ps_device *dualsense_create(struct hid_device *hdev)
@@ -1299,7 +1325,9 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 	ps_dev->battery_capacity = 100; /* initial value until parse_report. */
 	ps_dev->battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
 	ps_dev->parse_report = dualsense_parse_report;
+	ps_dev->remove = dualsense_remove;
 	INIT_WORK(&ds->output_worker, dualsense_output_worker);
+	ds->output_worker_initialized = true;
 	hid_set_drvdata(hdev, ds);
 
 	max_output_report_size = sizeof(struct dualsense_output_report_bt);
@@ -1461,6 +1489,9 @@ static void ps_remove(struct hid_device *hdev)
 	ps_devices_list_remove(dev);
 	ps_device_release_player_id(dev);
 
+	if (dev->remove)
+		dev->remove(dev);
+
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
 }
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] HID: playstation: add initial DualSense Edge controller support
  2022-10-06  2:01 [PATCH 0/3] HID: playstation: improvements and new device support Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 1/3] HID: playstation: stop DualSense output work on remove Roderick Colenbrander
@ 2022-10-06  2:01 ` Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 3/3] HID: playstation: support updated DualSense rumble mode Roderick Colenbrander
  2 siblings, 0 replies; 6+ messages in thread
From: Roderick Colenbrander @ 2022-10-06  2:01 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, Roderick Colenbrander, stable

Provide initial support for the DualSense Edge controller. The brings
support up to the level of the original DualSense, but won't yet provide
support for new features (e.g. reprogrammable buttons).

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
CC: stable@vger.kernel.org
---
 drivers/hid/hid-ids.h         | 1 +
 drivers/hid/hid-playstation.c | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f80d6193fca6..cd8087ed412c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1142,6 +1142,7 @@
 #define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2	0x09cc
 #define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE	0x0ba0
 #define USB_DEVICE_ID_SONY_PS5_CONTROLLER	0x0ce6
+#define USB_DEVICE_ID_SONY_PS5_CONTROLLER_2	0x0df2
 #define USB_DEVICE_ID_SONY_MOTION_CONTROLLER	0x03d5
 #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER	0x042f
 #define USB_DEVICE_ID_SONY_BUZZ_CONTROLLER		0x0002
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index d727cd2bf44e..396356b6760a 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1464,7 +1464,8 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_stop;
 	}
 
-	if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) {
+	if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER ||
+		hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
 		dev = dualsense_create(hdev);
 		if (IS_ERR(dev)) {
 			hid_err(hdev, "Failed to create dualsense.\n");
@@ -1499,6 +1500,8 @@ static void ps_remove(struct hid_device *hdev)
 static const struct hid_device_id ps_devices[] = {
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, ps_devices);
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] HID: playstation: support updated DualSense rumble mode.
  2022-10-06  2:01 [PATCH 0/3] HID: playstation: improvements and new device support Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 1/3] HID: playstation: stop DualSense output work on remove Roderick Colenbrander
  2022-10-06  2:01 ` [PATCH 2/3] HID: playstation: add initial DualSense Edge controller support Roderick Colenbrander
@ 2022-10-06  2:01 ` Roderick Colenbrander
  2022-10-10  9:16   ` Benjamin Tissoires
  2 siblings, 1 reply; 6+ messages in thread
From: Roderick Colenbrander @ 2022-10-06  2:01 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Roderick Colenbrander

Newer DualSense firmware supports a revised classic rumble mode,
which feels more similar to rumble as supported on previous PlayStation
controllers. It has been made the default on PlayStation and non-PlayStation
devices now (e.g. iOS and Windows). Default to this new mode when
supported.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 drivers/hid/hid-playstation.c | 39 ++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 396356b6760a..e05c61942971 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -108,6 +108,9 @@ struct ps_led_info {
 #define DS_STATUS_CHARGING		GENMASK(7, 4)
 #define DS_STATUS_CHARGING_SHIFT	4
 
+/* Feature version from DualSense Firmware Info report. */
+#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
+
 /*
  * Status of a DualSense touch point contact.
  * Contact IDs, with highest bit set are 'inactive'
@@ -126,6 +129,7 @@ struct ps_led_info {
 #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
 #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
 #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
+#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
 #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
 #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
 
@@ -143,6 +147,9 @@ struct dualsense {
 	struct input_dev *sensors;
 	struct input_dev *touchpad;
 
+	/* Update version is used as a feature/capability version. */
+	__le16 update_version;
+
 	/* Calibration data for accelerometer and gyroscope. */
 	struct ps_calibration_data accel_calib_data[3];
 	struct ps_calibration_data gyro_calib_data[3];
@@ -153,6 +160,7 @@ struct dualsense {
 	uint32_t sensor_timestamp_us;
 
 	/* Compatible rumble state */
+	bool use_vibration_v2;
 	bool update_rumble;
 	uint8_t motor_left;
 	uint8_t motor_right;
@@ -812,6 +820,14 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
 	ds->base.hw_version = get_unaligned_le32(&buf[24]);
 	ds->base.fw_version = get_unaligned_le32(&buf[28]);
 
+	/* Update version is some kind of feature version. It is distinct from
+	 * the firmware version as there can be many different variations of a
+	 * controller over time with the same physical shell, but with different
+	 * PCBs and other internal changes. The update version (internal name) is
+	 * used as a means to detect what features are available and change behavior.
+	 */
+	ds->update_version = get_unaligned_le16(&buf[44]);
+
 err_free:
 	kfree(buf);
 	return ret;
@@ -974,7 +990,10 @@ static void dualsense_output_worker(struct work_struct *work)
 	if (ds->update_rumble) {
 		/* Select classic rumble style haptics and enable it. */
 		common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT;
-		common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
+		if (ds->use_vibration_v2)
+			common->valid_flag2 |= DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2;
+		else
+			common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
 		common->motor_left = ds->motor_left;
 		common->motor_right = ds->motor_right;
 		ds->update_rumble = false;
@@ -1348,6 +1367,24 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
 		return ERR_PTR(ret);
 	}
 
+#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
+	/* Original DualSense firmware simulated classic controller rumble through
+	 * its new haptics hardware. It felt different from classic rumble users
+	 * were used to. Since then new firmwares were introduced to change behavior
+	 * and make this new 'v2' behavior default on PlayStation and other platforms.
+	 * The original DualSense requires a new enough firmware as bundled with PS5
+	 * software released in 2021. DualSense edge supports it out of the box.
+	 */
+	if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) {
+		/* Feature version 2.21 introduced new vibration method. */
+		if (ds->update_version < DS_FEATURE_VERSION(2, 21))
+			ds->use_vibration_v2 = false;
+		else
+			ds->use_vibration_v2 = true;
+	} else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
+		ds->use_vibration_v2 = true;
+	}
+
 	ret = ps_devices_list_add(ps_dev);
 	if (ret)
 		return ERR_PTR(ret);
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] HID: playstation: support updated DualSense rumble mode.
  2022-10-06  2:01 ` [PATCH 3/3] HID: playstation: support updated DualSense rumble mode Roderick Colenbrander
@ 2022-10-10  9:16   ` Benjamin Tissoires
  2022-10-10 17:25     ` Roderick Colenbrander
  0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Tissoires @ 2022-10-10  9:16 UTC (permalink / raw)
  To: Roderick Colenbrander; +Cc: Jiri Kosina, linux-input, Roderick Colenbrander

Hi Roderick,

On Thu, Oct 6, 2022 at 4:02 AM Roderick Colenbrander
<roderick@gaikai.com> wrote:
>
> Newer DualSense firmware supports a revised classic rumble mode,
> which feels more similar to rumble as supported on previous PlayStation
> controllers. It has been made the default on PlayStation and non-PlayStation
> devices now (e.g. iOS and Windows). Default to this new mode when
> supported.

I am trying to see if this patch is 6.1 or 6.2 material. So I have a
couple of questions for you:
- on the current dualsense controller, with an updated firmware, is
there any drawback in keeping the current code, or do we need to
upgrade to the new one to keep functionalities?
- on the new dualsense edge, does it support the old rumble, and so do
I need to tie the addition of the DualSense Edge to this one patch?

>
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---
>  drivers/hid/hid-playstation.c | 39 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 396356b6760a..e05c61942971 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -108,6 +108,9 @@ struct ps_led_info {
>  #define DS_STATUS_CHARGING             GENMASK(7, 4)
>  #define DS_STATUS_CHARGING_SHIFT       4
>
> +/* Feature version from DualSense Firmware Info report. */
> +#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
> +
>  /*
>   * Status of a DualSense touch point contact.
>   * Contact IDs, with highest bit set are 'inactive'
> @@ -126,6 +129,7 @@ struct ps_led_info {
>  #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
>  #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
>  #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
> +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
>  #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
>  #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
>
> @@ -143,6 +147,9 @@ struct dualsense {
>         struct input_dev *sensors;
>         struct input_dev *touchpad;
>
> +       /* Update version is used as a feature/capability version. */
> +       __le16 update_version;

Technically, if I am not wrong, the value stored here was already
processed through get_unaligned_le16(), so you have a u16, and more
likely to be consistent you should use uint16_t.

> +
>         /* Calibration data for accelerometer and gyroscope. */
>         struct ps_calibration_data accel_calib_data[3];
>         struct ps_calibration_data gyro_calib_data[3];
> @@ -153,6 +160,7 @@ struct dualsense {
>         uint32_t sensor_timestamp_us;
>
>         /* Compatible rumble state */
> +       bool use_vibration_v2;
>         bool update_rumble;
>         uint8_t motor_left;
>         uint8_t motor_right;
> @@ -812,6 +820,14 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
>         ds->base.hw_version = get_unaligned_le32(&buf[24]);
>         ds->base.fw_version = get_unaligned_le32(&buf[28]);
>
> +       /* Update version is some kind of feature version. It is distinct from
> +        * the firmware version as there can be many different variations of a
> +        * controller over time with the same physical shell, but with different
> +        * PCBs and other internal changes. The update version (internal name) is
> +        * used as a means to detect what features are available and change behavior.
> +        */
> +       ds->update_version = get_unaligned_le16(&buf[44]);
> +
>  err_free:
>         kfree(buf);
>         return ret;
> @@ -974,7 +990,10 @@ static void dualsense_output_worker(struct work_struct *work)
>         if (ds->update_rumble) {
>                 /* Select classic rumble style haptics and enable it. */
>                 common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT;
> -               common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
> +               if (ds->use_vibration_v2)
> +                       common->valid_flag2 |= DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2;
> +               else
> +                       common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
>                 common->motor_left = ds->motor_left;
>                 common->motor_right = ds->motor_right;
>                 ds->update_rumble = false;
> @@ -1348,6 +1367,24 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
>                 return ERR_PTR(ret);
>         }
>
> +#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))

Was already defined above AFAICT

> +       /* Original DualSense firmware simulated classic controller rumble through
> +        * its new haptics hardware. It felt different from classic rumble users
> +        * were used to. Since then new firmwares were introduced to change behavior
> +        * and make this new 'v2' behavior default on PlayStation and other platforms.
> +        * The original DualSense requires a new enough firmware as bundled with PS5
> +        * software released in 2021. DualSense edge supports it out of the box.
> +        */
> +       if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) {
> +               /* Feature version 2.21 introduced new vibration method. */
> +               if (ds->update_version < DS_FEATURE_VERSION(2, 21))
> +                       ds->use_vibration_v2 = false;
> +               else
> +                       ds->use_vibration_v2 = true;

if (conditional) then false else true; can easily be transformed into
ds->use_vibration_v2 = ds->update_version >= DS_FEATURE_VERSION(2, 21)
:)

> +       } else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
> +               ds->use_vibration_v2 = true;
> +       }
> +
>         ret = ps_devices_list_add(ps_dev);
>         if (ret)
>                 return ERR_PTR(ret);
> --
> 2.37.3
>

Cheers,
Benjamin


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] HID: playstation: support updated DualSense rumble mode.
  2022-10-10  9:16   ` Benjamin Tissoires
@ 2022-10-10 17:25     ` Roderick Colenbrander
  0 siblings, 0 replies; 6+ messages in thread
From: Roderick Colenbrander @ 2022-10-10 17:25 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Roderick Colenbrander, Jiri Kosina, linux-input, Roderick Colenbrander

Hi Benjamin,

On Mon, Oct 10, 2022 at 2:23 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Roderick,
>
> On Thu, Oct 6, 2022 at 4:02 AM Roderick Colenbrander
> <roderick@gaikai.com> wrote:
> >
> > Newer DualSense firmware supports a revised classic rumble mode,
> > which feels more similar to rumble as supported on previous PlayStation
> > controllers. It has been made the default on PlayStation and non-PlayStation
> > devices now (e.g. iOS and Windows). Default to this new mode when
> > supported.
>
> I am trying to see if this patch is 6.1 or 6.2 material. So I have a
> couple of questions for you:
> - on the current dualsense controller, with an updated firmware, is
> there any drawback in keeping the current code, or do we need to
> upgrade to the new one to keep functionalities?

Newer firmware supports both old and new methods. On actual PS5 the
old paths don't really get used anymore as the system defaults to the
new mode, some games may opt in to use the old method. Across
platforms we have standardized on this new method over the last year.
Windows, iOS and even SDL2 have supported it for a while now. This
would bring Linux and Android in sync.

> - on the new dualsense edge, does it support the old rumble, and so do
> I need to tie the addition of the DualSense Edge to this one patch?

There is no drawback into using the old code on DualSense Edge. The
new code is obviously desired and on iOS / PS5 would likely only ever
use the new code.

If this is too large of a change for 6.1, it is fine to delay just
this patch to 6.2. Though I can easily make an updated patch today
with the tweaks suggested below.



> >
> > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> > ---
> >  drivers/hid/hid-playstation.c | 39 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> > index 396356b6760a..e05c61942971 100644
> > --- a/drivers/hid/hid-playstation.c
> > +++ b/drivers/hid/hid-playstation.c
> > @@ -108,6 +108,9 @@ struct ps_led_info {
> >  #define DS_STATUS_CHARGING             GENMASK(7, 4)
> >  #define DS_STATUS_CHARGING_SHIFT       4
> >
> > +/* Feature version from DualSense Firmware Info report. */
> > +#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
> > +
> >  /*
> >   * Status of a DualSense touch point contact.
> >   * Contact IDs, with highest bit set are 'inactive'
> > @@ -126,6 +129,7 @@ struct ps_led_info {
> >  #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
> >  #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
> >  #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
> > +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
> >  #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
> >  #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
> >
> > @@ -143,6 +147,9 @@ struct dualsense {
> >         struct input_dev *sensors;
> >         struct input_dev *touchpad;
> >
> > +       /* Update version is used as a feature/capability version. */
> > +       __le16 update_version;
>
> Technically, if I am not wrong, the value stored here was already
> processed through get_unaligned_le16(), so you have a u16, and more
> likely to be consistent you should use uint16_t.

Yep, should change that one.

> > +
> >         /* Calibration data for accelerometer and gyroscope. */
> >         struct ps_calibration_data accel_calib_data[3];
> >         struct ps_calibration_data gyro_calib_data[3];
> > @@ -153,6 +160,7 @@ struct dualsense {
> >         uint32_t sensor_timestamp_us;
> >
> >         /* Compatible rumble state */
> > +       bool use_vibration_v2;
> >         bool update_rumble;
> >         uint8_t motor_left;
> >         uint8_t motor_right;
> > @@ -812,6 +820,14 @@ static int dualsense_get_firmware_info(struct dualsense *ds)
> >         ds->base.hw_version = get_unaligned_le32(&buf[24]);
> >         ds->base.fw_version = get_unaligned_le32(&buf[28]);
> >
> > +       /* Update version is some kind of feature version. It is distinct from
> > +        * the firmware version as there can be many different variations of a
> > +        * controller over time with the same physical shell, but with different
> > +        * PCBs and other internal changes. The update version (internal name) is
> > +        * used as a means to detect what features are available and change behavior.
> > +        */
> > +       ds->update_version = get_unaligned_le16(&buf[44]);
> > +
> >  err_free:
> >         kfree(buf);
> >         return ret;
> > @@ -974,7 +990,10 @@ static void dualsense_output_worker(struct work_struct *work)
> >         if (ds->update_rumble) {
> >                 /* Select classic rumble style haptics and enable it. */
> >                 common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT;
> > -               common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
> > +               if (ds->use_vibration_v2)
> > +                       common->valid_flag2 |= DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2;
> > +               else
> > +                       common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION;
> >                 common->motor_left = ds->motor_left;
> >                 common->motor_right = ds->motor_right;
> >                 ds->update_rumble = false;
> > @@ -1348,6 +1367,24 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
> >                 return ERR_PTR(ret);
> >         }
> >
> > +#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
>
> Was already defined above AFAICT

Weird,... I thought I had left that out. Quick to fix.

> > +       /* Original DualSense firmware simulated classic controller rumble through
> > +        * its new haptics hardware. It felt different from classic rumble users
> > +        * were used to. Since then new firmwares were introduced to change behavior
> > +        * and make this new 'v2' behavior default on PlayStation and other platforms.
> > +        * The original DualSense requires a new enough firmware as bundled with PS5
> > +        * software released in 2021. DualSense edge supports it out of the box.
> > +        */
> > +       if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) {
> > +               /* Feature version 2.21 introduced new vibration method. */
> > +               if (ds->update_version < DS_FEATURE_VERSION(2, 21))
> > +                       ds->use_vibration_v2 = false;
> > +               else
> > +                       ds->use_vibration_v2 = true;
>
> if (conditional) then false else true; can easily be transformed into
> ds->use_vibration_v2 = ds->update_version >= DS_FEATURE_VERSION(2, 21)
> :)

Yep, will change.

> > +       } else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
> > +               ds->use_vibration_v2 = true;
> > +       }
> > +
> >         ret = ps_devices_list_add(ps_dev);
> >         if (ret)
> >                 return ERR_PTR(ret);
> > --
> > 2.37.3
> >
>
> Cheers,
> Benjamin
>

Thanks,
Roderick

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-10 17:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-06  2:01 [PATCH 0/3] HID: playstation: improvements and new device support Roderick Colenbrander
2022-10-06  2:01 ` [PATCH 1/3] HID: playstation: stop DualSense output work on remove Roderick Colenbrander
2022-10-06  2:01 ` [PATCH 2/3] HID: playstation: add initial DualSense Edge controller support Roderick Colenbrander
2022-10-06  2:01 ` [PATCH 3/3] HID: playstation: support updated DualSense rumble mode Roderick Colenbrander
2022-10-10  9:16   ` Benjamin Tissoires
2022-10-10 17:25     ` Roderick Colenbrander

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).