ASoC: qcom: audioreach: fix S24_LE playback and route mixer disconnects
Subsystems (5)
cover ASoC: qcom: audioreach: fix S24_LE playback and route mixer disconnects
Two AudioReach fixes found while bringing up 24-bit headphone playback on the Radxa Dragon Q6A (QCS6490, sc8280xp machine driver, WCD9385).
Patch 1: S24_LE was described to the DSP as packed 3-byte samples, so the ADSP audio_process crashed as soon as an S24_LE stream started. Using the 32-bit container size makes S24_LE work.
Patch 2: writing 0 to any "<BE> Audio Mixer <FE>" route control cleared the back end's link even if it belonged to another front end. As a result alsactl restore lost DisplayPort on every boot, and the board's UCM HDMI sequence disconnected the headphones (the PCM then stalls with hw_ptr stuck at 0).
Both were tested on 7.2.9 with the board's topology changed to allow S24_LE on MultiMedia1, and both apply to and build on asoc/for-next.
---
Marc Herrero (2):
ASoC: qcom: audioreach: use a 32-bit container for S24_LE PCM
ASoC: qcom: audioreach: only disconnect the route a mixer control owns
sound/soc/qcom/qdsp6/audioreach.c | 4 ++-- sound/soc/qcom/qdsp6/topology.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: cde5523c75f1320a55ac1d55a717ccbb566557f0
change-id: 20261111-q6apm-fixes-21db1bb2d6a1
Best regards,
Signature · 1 line
Marc Herrero <[email protected]>
PATCH 1/2 ASoC: qcom: audioreach: use a 32-bit container for S24_LE PCM
SNDRV_PCM_FORMAT_S24_LE carries 24 valid bits in a 32-bit container, but audioreach_shmem_set_media_format() and audioreach_pcm_set_media_format() set bits_per_sample = bit_width = 24 in PARAM_ID_MEDIA_FORMAT and PARAM_ID_PCM_OUTPUT_FORMAT_CFG, which tells the DSP that the samples are packed 3-byte words.
q6apm-dai advertises S24_LE, so with a topology that allows it on a playback front end the DSP walks the buffer with the wrong stride. On QCS6490 (Radxa Dragon Q6A) this crashes the ADSP audio_process:
PDM: service 'audio_process' crash: 'EX:audio_process:0x2:...' q6apm-lpass-dais ...: Failed to start APM port 113 remoteproc remoteproc0: crash detected in adsp: type fatal error
Use the container size (32) for 24-bit data and keep bit_width = 24, alignment = PCM_LSB_ALIGNED and q_factor = 23. With this change S24_LE playback works on the headphone path (codec DMA RX -> LPASS RX macro -> WCD9385), tested with the back end at S16_LE, S24_LE and S32_LE and at 48, 96 and 192 kHz.
The DSP then reads each 32-bit word as Q23, so S24_LE samples must be sign-extended into the top byte, as alsa-lib's conversions do.
--- sound/soc/qcom/qdsp6/audioreach.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/qcom/qdsp6/audioreach.c +2 -2
@@ -1216,7 +1216,7 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph, media_cfg->interleaved = module->interleave_type; media_cfg->num_channels = mcfg->num_channels; media_cfg->q_factor = mcfg->bit_width -1 ; - media_cfg->bits_per_sample = mcfg->bit_width; + media_cfg->bits_per_sample = mcfg->bit_width ==24 ?32 : mcfg->bit_width;/* Convert the physical mapping to a logical mapping of the channels */ for (i =0 , j =0 ; i <AR_PCM_MAX_NUM_CHANNEL && j < mcfg->num_channels; i++) {if (!mcfg->channel_map[i])
@@ -1308,7 +1308,7 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph, cfg->sample_rate = mcfg->sample_rate; cfg->bit_width = mcfg->bit_width; cfg->alignment =PCM_LSB_ALIGNED ; - cfg->bits_per_sample = mcfg->bit_width; + cfg->bits_per_sample = mcfg->bit_width ==24 ?32 : mcfg->bit_width; cfg->q_factor = mcfg->bit_width -1 ; cfg->endianness =PCM_LITTLE_ENDIAN ; cfg->num_channels = mcfg->num_channels;
Signature · 1 line
2.53.0
PATCH 2/2 ASoC: qcom: audioreach: only disconnect the route a mixer control owns
In reply to Marc HerreroEach AudioReach virtual mixer route control ("<BE> Audio Mixer <FE>") connects a front-end stream graph to a back-end device graph by storing the source module in the device graph's audioreach_graph_info. Writing 0 to any of these controls clears that link unconditionally, even when the device graph is connected to a different front end.
When two front ends can feed the same back end the result therefore depends on write order. On QCS6490 (Radxa Dragon Q6A):
- alsactl restore writes 'DISPLAY_PORT_RX_0 Audio Mixer MultiMedia2' = 1 and then 'DISPLAY_PORT_RX_0 Audio Mixer MultiMedia1' = 0, which leaves DisplayPort disconnected although the saved state has it connected;
- the UCM HDMI enable sequence writes
'RX_CODEC_DMA_RX_0 Audio Mixer MultiMedia2' = 0, which disconnects MultiMedia1 from the headphones. A PCM started on MultiMedia1 after that never advances (hw_ptr stays at 0).
Only drop the link when it belongs to the control being cleared, that is when audioreach_is_vmixer_connected() reports it as connected. Writing 0 to a control that is not the active route becomes a no-op, which matches what its get() callback already returns.
--- sound/soc/qcom/qdsp6/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/qcom/qdsp6/topology.c +1 -1
@@ -1300,7 +1300,7 @@ static int audioreach_put_audio_mixer(struct snd_kcontrol *kcontrol,if (ucontrol->value.integer.value[0 ]) {audioreach_connect_sub_graphs (data, scontrol, dapm_scontrol, true);snd_soc_dapm_mixer_update_power (dapm, kcontrol,1 ,NULL ); - }else { + }else if (audioreach_is_vmixer_connected (data, scontrol, dapm_scontrol)) {audioreach_connect_sub_graphs (data, scontrol, dapm_scontrol, false);snd_soc_dapm_mixer_update_power (dapm, kcontrol,0 ,NULL ); }
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH 2/2] ASoC: qcom: audioreach: only disconnect the route a mixer control owns
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [High] ASoC: qcom: audioreach: stale DAPM path in audioreach_put_audio_mixer() --
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [High] ASoC: qcom: audioreach: stale DAPM path in audioreach_put_audio_mixer()
--
commit 1fff20ab243038af732f562aca3ace62e5fdcdb9
Author: Marc Herrero <[email protected]>
ASoC: qcom: audioreach: only disconnect the route a mixer control owns
This commit checks audioreach_is_vmixer_connected() before disconnecting a virtual mixer route when 0 is written to an AudioReach mixer control.
sound/soc/qcom/qdsp6/topology.c static int audioreach_put_audio_mixer(struct snd_kcontrol *kcontrol, · line 1306
Quoted 13 lines
sound/soc/qcom/qdsp6/topology.c +1
-1@@ -1300,7 +1300,7 @@ static int audioreach_put_audio_mixer(struct snd_kcontrol *kcontrol,if (ucontrol->value.integer.value[0 ]) {audioreach_connect_sub_graphs (data, scontrol, dapm_scontrol, true);snd_soc_dapm_mixer_update_power (dapm, kcontrol,1 ,NULL ); - }else { + }else if (audioreach_is_vmixer_connected (data, scontrol, dapm_scontrol)) {audioreach_connect_sub_graphs (data, scontrol, dapm_scontrol, false);snd_soc_dapm_mixer_update_power (dapm, kcontrol,0 ,NULL ); }
[Severity: High]
Does gating snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, NULL) on audioreach_is_vmixer_connected() leave kcontrol's DAPM path stuck connected when switching front ends?
All front-end kcontrols on the same virtual mixer widget share one back-end audioreach_graph_info->src_mod_inst_id, whereas each kcontrol owns its own struct snd_soc_dapm_path in DAPM. If front end 1 is enabled (setting path_1->connect = 1) and front end 2 is then enabled before front end 1 is cleared, audioreach_connect_sub_graphs() overwrites info->src_mod_inst_id with front end 2's module instance ID. When 0 is subsequently written to front end 1's kcontrol, audioreach_is_vmixer_connected() returns false and skips snd_soc_dapm_mixer_update_power(), leaving path_1->connect set to 1 so DPCM continues to treat the back end as connected to front end 1.
Only audioreach_connect_sub_graphs(data, scontrol, dapm_scontrol, false) should be guarded by audioreach_is_vmixer_connected(), while snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, NULL) is still called in the else branch.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2