| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEc3jaAm4gws8drVKvQGYHoAT4fb5i7sSPJ0LFiSGHaVAzKZnA@mail.gmail.com>
Date: Mon, 9 Jun 2025 22:04:52 -0700
From: Roderick Colenbrander <thunderbird2k@...il.com>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
Cc: Roderick Colenbrander <roderick.colenbrander@...y.com>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>, Henrik Rydberg <rydberg@...math.org>, kernel@...labora.com,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/11] HID: playstation: Support DualSense audio jack
hotplug detection
Hi Christian,
Thanks for sharing I need to have a closer look to see what we do on
the console as well (we want to keep behavior where possible in sync
also due to official PlayStation use cases on embedded devices).
At least use this logic only for USB, we should not register these
nodes or apply such settings for Bluetooth. Just setting these on
bluetooth without audio I could see affecting battery life a bit. But
in addition, I honestly don't really see a path to supporting
Bluetooth audio for the DualSense. It would add confusion for users.
The reason, I don't think there is a good way for adding audio is that
the DualSense is very, very complex. In Bluetooth mode we have our own
ways of doing audio (non-standard), which is already tricky. Above all
else the audio is compressed and not PCM. The only way to do it is
probably in userspace in something PulseAudio like. So before rushing
adding this also for Bluetooth, I want to limit this to USB.
I'm sharing some other feedback in line. It is small stuff, but in
particular I like to keep datasheets and code in sync, so of it is
around naming. It allows us internally for easier review as well.
There are some areas I need to study some more and get internal
documentation, so I don't have thorough feedback on the audio code
itself yet.
Thanks,
Roderick
On Mon, May 26, 2025 at 6:04 AM Cristian Ciocaltea
<cristian.ciocaltea@...labora.com> wrote:
>
> The default audio output path on DualSense controller hardware is set to
> headphones, regardless of whether they are actually inserted or not.
>
> Detect when the plugged state of the 3.5mm audio jack changes and toggle
> audio output between headphones and internal speaker, as required. The
> latter is achieved by essentially routing the right channel of the audio
> source to the mono speaker.
>
> Additionally, adjust the speaker volume since its default level is too
> low and, therefore, cannot generate any audible sound.
>
> It's worth noting the audio functionality is currently not supported for
> Bluetooth, hence it's limited to USB connectivity.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> ---
> drivers/hid/hid-playstation.c | 86 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index b7777d3230b2fe277a4a2217ef6b1eff9cfad182..ed8e67c5bf5d3a9e9721da31a6bd84f0b6802b14 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -117,6 +117,12 @@ struct ps_led_info {
> #define DS_STATUS_BATTERY_CHARGING GENMASK(7, 4)
> #define DS_STATUS_BATTERY_CHARGING_SHIFT 4
>
> +/* Plugged status field of DualSense input report. */
> +#define DS_STATUS_PLUGGED_HEADPHONES BIT(0)
> +#define DS_STATUS_PLUGGED_MIC BIT(1)
> +#define DS_STATUS_PLUGGED_JACK (DS_STATUS_PLUGGED_HEADPHONES | \
> + DS_STATUS_PLUGGED_MIC)
Like I mentioned in patch 10, there are 3 status bytes. So let's call
this DS_STATUS1_. The other 5 bits in this one are unrelated to audio.
Official datasheets call these 'HP_DETECT', 'MIC_DETECT' and
'MIC_MUTE'. I guess we should use those names.
> +
> /* Feature version from DualSense Firmware Info report. */
> #define DS_FEATURE_VERSION_MINOR GENMASK(7, 0)
> #define DS_FEATURE_VERSION_MAJOR GENMASK(15, 8)
> @@ -144,13 +150,18 @@ struct ps_led_info {
> /* Flags for DualSense output report. */
> #define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
> #define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
> +#define DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE BIT(5)
Just for completeness also add BIT(6) which is MIC_VOLUME_ENABLE
> +#define DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE BIT(7)
> #define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
> #define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
> #define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
> #define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
> #define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
> +#define DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE BIT(7)
> #define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
> #define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
> +#define DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL GENMASK(5, 4)
> +#define DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN GENMASK(2, 0)
> #define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
> #define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
>
> @@ -193,6 +204,11 @@ struct dualsense {
> u8 lightbar_green;
> u8 lightbar_blue;
>
> + /* Audio Jack plugged state */
> + u8 plugged_state;
> + u8 prev_plugged_state;
> + bool prev_plugged_state_valid;
> +
> /* Microphone */
> bool update_mic_mute;
> bool mic_muted;
> @@ -237,7 +253,8 @@ struct dualsense_input_report {
>
> u8 reserved3[12];
> u8 status_battery;
> - u8 reserved4[10];
> + u8 status_plugged;
> + u8 reserved4[9];
> } __packed;
> /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
> static_assert(sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - 1);
> @@ -252,11 +269,15 @@ struct dualsense_output_report_common {
> u8 motor_left;
>
> /* Audio controls */
> - u8 reserved[4];
> + u8 headphone_audio_volume; /* 0-0x7f */
Just call this headphone_volume
> + u8 speaker_audio_volume; /* 0-0xff */
Just call this speaker_volume
> + u8 internal_microphone_volume; /* 0-0x40 */
We call this one mic_volume
> + u8 audio_flags;
We call this one audio_control.
> u8 mute_button_led;
>
> u8 power_save_control;
> - u8 reserved2[28];
> + u8 reserved2[27];
> + u8 audio_flags2;
This one we call audio_control2
>
> /* LEDs and lightbar */
> u8 valid_flag2;
> @@ -1304,6 +1325,46 @@ static void dualsense_output_worker(struct work_struct *work)
> ds->update_player_leds = false;
> }
>
> + if (ds->plugged_state != ds->prev_plugged_state) {
> + u8 val = ds->plugged_state & DS_STATUS_PLUGGED_HEADPHONES;
> +
> + if (val != (ds->prev_plugged_state & DS_STATUS_PLUGGED_HEADPHONES)) {
> + common->valid_flag0 = DS_OUTPUT_VALID_FLAG0_AUDIO_CONTROL_ENABLE;
> + /*
> + * _--------> Output path setup in audio_flag0
> + * / _------> Headphone (HP) Left channel sink
> + * | / _----> Headphone (HP) Right channel sink
> + * | | / _--> Internal Speaker (SP) sink
> + * | | | /
> + * | | | | L/R - Left/Right channel source
> + * 0 L-R X X - Unrouted (muted) channel source
> + * 1 L-L X
> + * 2 L-L R
> + * 3 X-X R
> + */
> + if (val) {
> + /* Mute SP and route L+R channels to HP */
> + common->audio_flags = 0;
> + } else {
> + /* Mute HP and route R channel to SP */
> + common->audio_flags =
> + FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS_OUTPUT_PATH_SEL, 0x3);
> + /*
> + * Set SP hardware volume to 100%.
> + * Note the accepted range seems to be [0x3d..0x64]
> + */
> + common->valid_flag0 |= DS_OUTPUT_VALID_FLAG0_SPEAKER_VOLUME_ENABLE;
> + common->speaker_audio_volume = 0x64;
> + /* Set SP preamp gain to ~30% */
> + common->valid_flag1 = DS_OUTPUT_VALID_FLAG1_AUDIO_CONTROL2_ENABLE;
> + common->audio_flags2 =
> + FIELD_PREP(DS_OUTPUT_AUDIO_FLAGS2_SP_PREAMP_GAIN, 0x2);
> + }
> + }
> +
> + ds->prev_plugged_state = ds->plugged_state;
> + }
> +
> if (ds->update_mic_mute) {
> common->valid_flag1 |= DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE;
> common->mute_button_led = ds->mic_muted;
> @@ -1407,6 +1468,25 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
> }
> ds->last_btn_mic_state = btn_mic_state;
>
> + /* Parse HP/MIC plugged state data */
> + value = ds_report->status_plugged & DS_STATUS_PLUGGED_JACK;
> + if (!ds->prev_plugged_state_valid) {
> + /* Initial handling of the plugged state report */
> + scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> + ds->plugged_state = (~value) & DS_STATUS_PLUGGED_JACK;
> + ds->prev_plugged_state_valid = true;
> + }
> + }
> + if (value != ds->plugged_state) {
> + scoped_guard(spinlock_irqsave, &ps_dev->lock) {
> + ds->prev_plugged_state = ds->plugged_state;
> + ds->plugged_state = value;
> + }
> +
> + /* Schedule audio routing towards active endpoint. */
> + dualsense_schedule_work(ds);
> + }
> +
> /* Parse and calibrate gyroscope data. */
> for (i = 0; i < ARRAY_SIZE(ds_report->gyro); i++) {
> int raw_data = (short)le16_to_cpu(ds_report->gyro[i]);
>
> --
> 2.49.0
>
>
Powered by blists - more mailing lists