* [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override @ 2026-06-10 11:16 Abel Vesa 2026-06-10 16:00 ` Mark Brown 2026-06-11 8:58 ` Srinivas Kandagatla 0 siblings, 2 replies; 4+ messages in thread From: Abel Vesa @ 2026-06-10 11:16 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Neil Armstrong, linux-sound, linux-arm-msm, linux-kernel, Abel Vesa The WSA RX slot mapping is currently computed entirely from the channel count in .prepare() and pushed unconditionally via snd_soc_dai_set_channel_map(). Userspace has no way to express a different channel map, so any map defined in UCM is ignored. Add a writable "Playback Channel Map" control on the WSA RX PCM via snd_pcm_add_chmap_ctls() and make it writable. The map selected by userspace (SNDRV_CHMAP_*) is stored per AFE port and, at .prepare() time, translated into the q6 PCM_CHANNEL_* slots and programmed into the DSP. When userspace has not provided a full map, the previous default mapping is used, so existing behaviour is preserved. Assisted-by: Codex:GPT-5.5 Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com> --- sound/soc/qcom/x1e80100.c | 152 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 2 deletions(-) diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c index c81df41ace88..63cd00620ade 100644 --- a/sound/soc/qcom/x1e80100.c +++ b/sound/soc/qcom/x1e80100.c @@ -15,14 +15,105 @@ #include "qdsp6/q6dsp-common.h" #include "sdw.h" +#define X1E80100_WSA_MAX_CHANNELS 4 + struct x1e80100_snd_data { bool stream_prepared[AFE_PORT_MAX]; struct snd_soc_card *card; struct snd_soc_jack jack; struct snd_soc_jack dp_jack[8]; bool jack_setup; + unsigned int user_chmap[AFE_PORT_MAX][4]; + bool chmap_added[AFE_PORT_MAX]; }; +static const struct snd_pcm_chmap_elem x1e80100_wsa_chmaps[] = { + { .channels = 1, .map = { SNDRV_CHMAP_FC } }, + { .channels = 2, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR } }, + { .channels = 3, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, + SNDRV_CHMAP_FC } }, + { .channels = 4, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, + SNDRV_CHMAP_RL, SNDRV_CHMAP_RR } }, + { } +}; + +static int x1e80100_chmap_to_q6(unsigned int pos) +{ + switch (pos) { + case SNDRV_CHMAP_FL: return PCM_CHANNEL_FL; + case SNDRV_CHMAP_FR: return PCM_CHANNEL_FR; + case SNDRV_CHMAP_MONO: + case SNDRV_CHMAP_FC: return PCM_CHANNEL_FC; + case SNDRV_CHMAP_RL: return PCM_CHANNEL_LB; + case SNDRV_CHMAP_RR: return PCM_CHANNEL_RB; + case SNDRV_CHMAP_SL: return PCM_CHANNEL_LS; + case SNDRV_CHMAP_SR: return PCM_CHANNEL_RS; + case SNDRV_CHMAP_LFE: return PCM_CHANNEL_LFE; + case SNDRV_CHMAP_RC: return PCM_CHANNEL_CS; + default: return -EINVAL; + } +} + +static int x1e80100_chmap_ctl_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); + unsigned int *map = data->user_chmap[cpu_dai->id]; + int i; + + for (i = 0; i < info->max_channels; i++) + ucontrol->value.integer.value[i] = map[i]; + + return 0; +} + +static int x1e80100_chmap_ctl_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); + unsigned int *map = data->user_chmap[cpu_dai->id]; + int i; + + for (i = 0; i < info->max_channels; i++) { + unsigned int pos = ucontrol->value.integer.value[i]; + + /* Validate every requested non-unset position up front. */ + if (pos && x1e80100_chmap_to_q6(pos) < 0) + return -EINVAL; + + map[i] = pos; + } + + return 0; +} + +static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_pcm_chmap *info; + unsigned int i; + int ret; + + ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, + x1e80100_wsa_chmaps, + X1E80100_WSA_MAX_CHANNELS, 0, &info); + if (ret < 0) + return ret; + + /* Make the query-only chmap control writable from userspace. */ + for (i = 0; i < info->kctl->count; i++) + info->kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_WRITE; + info->kctl->get = x1e80100_chmap_ctl_get; + info->kctl->put = x1e80100_chmap_ctl_put; + + return 0; +} + static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) { struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); @@ -34,6 +125,9 @@ static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) switch (cpu_dai->id) { case WSA_CODEC_DMA_RX_0: case WSA_CODEC_DMA_RX_1: + if (!rtd->pcm) + return 0; + /* * Set limit of -3 dB on Digital Volume and 0 dB on PA Volume * to reduce the risk of speaker damage until we have active @@ -92,6 +186,7 @@ static int x1e80100_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, return 0; } +/* Default WSA RX slot mapping when userspace has not provided a channel map. */ static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) { switch (num) { @@ -120,6 +215,58 @@ static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) return 0; } +static int x1e80100_snd_build_rx_slot(struct x1e80100_snd_data *data, + unsigned int dai_id, unsigned int channels, + unsigned int *rx_slot) +{ + unsigned int *user = data->user_chmap[dai_id]; + unsigned int i, set = 0; + + for (i = 0; i < channels && i < ARRAY_SIZE(data->user_chmap[0]); i++) + if (user[i]) + set++; + + if (set != channels) + return x1e80100_snd_hw_map_channels(rx_slot, channels); + + for (i = 0; i < channels; i++) { + int q6 = x1e80100_chmap_to_q6(user[i]); + + if (q6 < 0) + return q6; + rx_slot[i] = q6; + } + + return 0; +} + +static int x1e80100_snd_startup(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); + int ret; + + ret = qcom_snd_sdw_startup(substream); + if (ret) + return ret; + + switch (cpu_dai->id) { + case WSA_CODEC_DMA_RX_0: + case WSA_CODEC_DMA_RX_1: + if (!data->chmap_added[cpu_dai->id]) { + ret = x1e80100_add_wsa_chmap_ctls(rtd); + if (ret) + return ret; + + data->chmap_added[cpu_dai->id] = true; + } + break; + } + + return 0; +} + static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); @@ -132,7 +279,8 @@ static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) switch (cpu_dai->id) { case WSA_CODEC_DMA_RX_0: case WSA_CODEC_DMA_RX_1: - ret = x1e80100_snd_hw_map_channels(rx_slot, channels); + ret = x1e80100_snd_build_rx_slot(data, cpu_dai->id, channels, + rx_slot); if (ret) return ret; @@ -158,7 +306,7 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream) } static const struct snd_soc_ops x1e80100_be_ops = { - .startup = qcom_snd_sdw_startup, + .startup = x1e80100_snd_startup, .shutdown = qcom_snd_sdw_shutdown, .hw_free = x1e80100_snd_hw_free, .prepare = x1e80100_snd_prepare, --- base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72 change-id: 20260609-sound-qcom-x1e80100-allow-ucm-channel-map-2302f2d552d5 Best regards, -- Abel Vesa <abel.vesa@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override 2026-06-10 11:16 [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override Abel Vesa @ 2026-06-10 16:00 ` Mark Brown 2026-06-11 8:58 ` Srinivas Kandagatla 1 sibling, 0 replies; 4+ messages in thread From: Mark Brown @ 2026-06-10 16:00 UTC (permalink / raw) To: Abel Vesa Cc: Srinivas Kandagatla, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Neil Armstrong, linux-sound, linux-arm-msm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2166 bytes --] On Wed, Jun 10, 2026 at 02:16:04PM +0300, Abel Vesa wrote: > The WSA RX slot mapping is currently computed entirely from the channel > count in .prepare() and pushed unconditionally via > snd_soc_dai_set_channel_map(). Userspace has no way to express a different > channel map, so any map defined in UCM is ignored. > Add a writable "Playback Channel Map" control on the WSA RX PCM via > snd_pcm_add_chmap_ctls() and make it writable. The map selected by > userspace (SNDRV_CHMAP_*) is stored per AFE port and, at .prepare() > time, translated into the q6 PCM_CHANNEL_* slots and programmed into the > DSP. > When userspace has not provided a full map, the previous default > mapping is used, so existing behaviour is preserved. > +static int x1e80100_chmap_ctl_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); > + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; > + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); > + unsigned int *map = data->user_chmap[cpu_dai->id]; > + int i; > + > + for (i = 0; i < info->max_channels; i++) { > + unsigned int pos = ucontrol->value.integer.value[i]; > + > + /* Validate every requested non-unset position up front. */ > + if (pos && x1e80100_chmap_to_q6(pos) < 0) > + return -EINVAL; > + > + map[i] = pos; > + } > + > + return 0; This should return 1 if the value changed. > @@ -34,6 +125,9 @@ static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) > switch (cpu_dai->id) { > case WSA_CODEC_DMA_RX_0: > case WSA_CODEC_DMA_RX_1: > + if (!rtd->pcm) > + return 0; > + > /* > * Set limit of -3 dB on Digital Volume and 0 dB on PA Volume > * to reduce the risk of speaker damage until we have active This hunk seems unrelated, I can't see anything about it in the commit log either? I'm a bit confused about what this aims to achieve, I've not checked which it is but either we should always have created the PCMs here or we will never have created the PCMs here, it's weird that this might vary. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override 2026-06-10 11:16 [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override Abel Vesa 2026-06-10 16:00 ` Mark Brown @ 2026-06-11 8:58 ` Srinivas Kandagatla 2026-06-11 9:00 ` Srinivas Kandagatla 1 sibling, 1 reply; 4+ messages in thread From: Srinivas Kandagatla @ 2026-06-11 8:58 UTC (permalink / raw) To: Abel Vesa, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Neil Armstrong, linux-sound, linux-arm-msm, linux-kernel Thanks Abel, for the patch. On 6/10/26 12:16 PM, Abel Vesa wrote: > The WSA RX slot mapping is currently computed entirely from the channel > count in .prepare() and pushed unconditionally via > snd_soc_dai_set_channel_map(). Userspace has no way to express a different > channel map, so any map defined in UCM is ignored. > > Add a writable "Playback Channel Map" control on the WSA RX PCM via > snd_pcm_add_chmap_ctls() and make it writable. The map selected by > userspace (SNDRV_CHMAP_*) is stored per AFE port and, at .prepare() > time, translated into the q6 PCM_CHANNEL_* slots and programmed into the > DSP. > > When userspace has not provided a full map, the previous default > mapping is used, so existing behaviour is preserved. > > Assisted-by: Codex:GPT-5.5 > Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com> > --- > sound/soc/qcom/x1e80100.c | 152 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 150 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c > index c81df41ace88..63cd00620ade 100644 > --- a/sound/soc/qcom/x1e80100.c > +++ b/sound/soc/qcom/x1e80100.c > @@ -15,14 +15,105 @@ > #include "qdsp6/q6dsp-common.h" > #include "sdw.h" > > +#define X1E80100_WSA_MAX_CHANNELS 4 this is not the case anymore, we have 6 channels on Glymur. > + > struct x1e80100_snd_data { > bool stream_prepared[AFE_PORT_MAX]; > struct snd_soc_card *card; > struct snd_soc_jack jack; > struct snd_soc_jack dp_jack[8]; > bool jack_setup; > + unsigned int user_chmap[AFE_PORT_MAX][4]; > + bool chmap_added[AFE_PORT_MAX]; > }; > > +static const struct snd_pcm_chmap_elem x1e80100_wsa_chmaps[] = { can you not use snd_pcm_std_chmaps instead ? > + { .channels = 1, .map = { SNDRV_CHMAP_FC } }, > + { .channels = 2, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR } }, > + { .channels = 3, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, > + SNDRV_CHMAP_FC } }, > + { .channels = 4, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, > + SNDRV_CHMAP_RL, SNDRV_CHMAP_RR } }, > + { } > +}; > + > +static int x1e80100_chmap_to_q6(unsigned int pos) > +{> + switch (pos) { > + case SNDRV_CHMAP_FL: return PCM_CHANNEL_FL; > + case SNDRV_CHMAP_FR: return PCM_CHANNEL_FR; > + case SNDRV_CHMAP_MONO: > + case SNDRV_CHMAP_FC: return PCM_CHANNEL_FC; > + case SNDRV_CHMAP_RL: return PCM_CHANNEL_LB; > + case SNDRV_CHMAP_RR: return PCM_CHANNEL_RB; > + case SNDRV_CHMAP_SL: return PCM_CHANNEL_LS; > + case SNDRV_CHMAP_SR: return PCM_CHANNEL_RS; > + case SNDRV_CHMAP_LFE: return PCM_CHANNEL_LFE; > + case SNDRV_CHMAP_RC: return PCM_CHANNEL_CS; > + default: return -EINVAL; > + } > +} > + > +static int x1e80100_chmap_ctl_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); > + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; > + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); > + unsigned int *map = data->user_chmap[cpu_dai->id]; > + int i; > + > + for (i = 0; i < info->max_channels; i++) > + ucontrol->value.integer.value[i] = map[i]; > + > + return 0; > +} > + > +static int x1e80100_chmap_ctl_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); > + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; > + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); > + unsigned int *map = data->user_chmap[cpu_dai->id]; > + int i; > + > + for (i = 0; i < info->max_channels; i++) { > + unsigned int pos = ucontrol->value.integer.value[i]; > + > + /* Validate every requested non-unset position up front. */ > + if (pos && x1e80100_chmap_to_q6(pos) < 0) > + return -EINVAL; > + > + map[i] = pos; > + } > + > + return 0; > +} > + > +static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd) > +{ > + struct snd_pcm_chmap *info; > + unsigned int i; > + int ret; > + > + ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, > + x1e80100_wsa_chmaps, > + X1E80100_WSA_MAX_CHANNELS, 0, &info); This will give mixer control as "Playback Channel Map", can we prefix this with WSA, as this can be cryptic for user side to interpret Playback Channel Map as a WSA Channel map and also if we add more channel maps for example from hdmi codec we will endup with something like on my t14s: numid=136,iface=PCM,name='Playback Channel Map',device=16 numid=141,iface=PCM,name='Playback Channel Map',device=17 numid=146,iface=PCM,name='Playback Channel Map',device=18 numid=152,iface=PCM,name='Playback Channel Map',device=19 This is really confusing, with below patch you should get: numid=136,iface=PCM,name='Playback Channel Map',device=16 numid=141,iface=PCM,name='Playback Channel Map',device=17 numid=146,iface=PCM,name='Playback Channel Map',device=18 numid=152,iface=PCM,name='WSA Playback Channel Map',device=21 Clearly giving users more clear picture. Please use this patch and let me know I have tested this on t14s along with your patch and making some modifications to it. -------------------------------------->cut<---------------------------- Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> Date: Thu Jun 11 08:12:17 2026 +0100 ALSA: pcm: Add snd_pcm_add_named_chmap_ctls() for custom control names Add snd_pcm_add_named_chmap_ctls() to allowing allowing drivers to register channel map controls with names other than the default "Playback/Capture Channel Map". Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 76fc33dce537..2a51f7c02dc6 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1501,6 +1501,12 @@ extern const struct snd_pcm_chmap_elem snd_pcm_alt_chmaps[]; #define SND_PCM_CHMAP_MASK_246 (SND_PCM_CHMAP_MASK_24 | (1U << 6)) #define SND_PCM_CHMAP_MASK_2468 (SND_PCM_CHMAP_MASK_246 | (1U << 8)) +int snd_pcm_add_named_chmap_ctls(struct snd_pcm *pcm, int stream, + const char *name, + const struct snd_pcm_chmap_elem *chmap, + int max_channels, + unsigned long private_value, + struct snd_pcm_chmap **info_ret); int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, const struct snd_pcm_chmap_elem *chmap, int max_channels, diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index fe597f7d522d..7dae270eaedc 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2572,9 +2572,10 @@ static void pcm_chmap_ctl_private_free(struct snd_kcontrol *kcontrol) } /** - * snd_pcm_add_chmap_ctls - create channel-mapping control elements + * snd_pcm_add_named_chmap_ctls - create channel-mapping control elements with a custom name * @pcm: the assigned PCM instance * @stream: stream direction + * @name: the control element name * @chmap: channel map elements (for query) * @max_channels: the max number of channels for the stream * @private_value: the value passed to each kcontrol's private_value field @@ -2583,11 +2584,12 @@ static void pcm_chmap_ctl_private_free(struct snd_kcontrol *kcontrol) * Create channel-mapping control elements assigned to the given PCM stream(s). * Return: Zero if successful, or a negative error value. */ -int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, - const struct snd_pcm_chmap_elem *chmap, - int max_channels, - unsigned long private_value, - struct snd_pcm_chmap **info_ret) +int snd_pcm_add_named_chmap_ctls(struct snd_pcm *pcm, int stream, + const char *name, + const struct snd_pcm_chmap_elem *chmap, + int max_channels, + unsigned long private_value, + struct snd_pcm_chmap **info_ret) { struct snd_pcm_chmap *info; struct snd_kcontrol_new knew = { @@ -2611,10 +2613,7 @@ int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, info->stream = stream; info->chmap = chmap; info->max_channels = max_channels; - if (stream == SNDRV_PCM_STREAM_PLAYBACK) - knew.name = "Playback Channel Map"; - else - knew.name = "Capture Channel Map"; + knew.name = name; knew.device = pcm->device; knew.count = pcm->streams[stream].substream_count; knew.private_value = private_value; @@ -2632,4 +2631,29 @@ int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, *info_ret = info; return 0; } +EXPORT_SYMBOL_GPL(snd_pcm_add_named_chmap_ctls); + +/** + * snd_pcm_add_chmap_ctls - create channel-mapping control elements + * @pcm: the assigned PCM instance + * @stream: stream direction + * @chmap: channel map elements (for query) + * @max_channels: the max number of channels for the stream + * @private_value: the value passed to each kcontrol's private_value field + * @info_ret: store struct snd_pcm_chmap instance if non-NULL + * + * Create channel-mapping control elements assigned to the given PCM stream(s). + * Return: Zero if successful, or a negative error value. + */ +int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, + const struct snd_pcm_chmap_elem *chmap, + int max_channels, + unsigned long private_value, + struct snd_pcm_chmap **info_ret) +{ + return snd_pcm_add_named_chmap_ctls(pcm, stream, + stream == SNDRV_PCM_STREAM_PLAYBACK ? "Playback Channel Map" : + "Capture Channel Map", chmap, max_channels, + private_value, info_ret); +} EXPORT_SYMBOL_GPL(snd_pcm_add_chmap_ctls); -------------------------------------->cut<---------------------------- changes to your patch: > + if (ret < 0) > + return ret; > + > + /* Make the query-only chmap control writable from userspace. */ > + for (i = 0; i < info->kctl->count; i++) > + info->kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_WRITE; > + info->kctl->get = x1e80100_chmap_ctl_get; > + info->kctl->put = x1e80100_chmap_ctl_put; > + > + return 0; > +} > + > static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) > { > struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); > @@ -34,6 +125,9 @@ static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) > switch (cpu_dai->id) { > case WSA_CODEC_DMA_RX_0: > case WSA_CODEC_DMA_RX_1: > + if (!rtd->pcm) > + return 0; > + > /* > * Set limit of -3 dB on Digital Volume and 0 dB on PA Volume > * to reduce the risk of speaker damage until we have active > @@ -92,6 +186,7 @@ static int x1e80100_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, > return 0; > } > > +/* Default WSA RX slot mapping when userspace has not provided a channel map. */ > static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) > { > switch (num) { > @@ -120,6 +215,58 @@ static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) > return 0; > } > > +static int x1e80100_snd_build_rx_slot(struct x1e80100_snd_data *data, > + unsigned int dai_id, unsigned int channels, > + unsigned int *rx_slot) > +{ > + unsigned int *user = data->user_chmap[dai_id]; > + unsigned int i, set = 0; > + > + for (i = 0; i < channels && i < ARRAY_SIZE(data->user_chmap[0]); i++) > + if (user[i]) > + set++; > + > + if (set != channels) > + return x1e80100_snd_hw_map_channels(rx_slot, channels); > + > + for (i = 0; i < channels; i++) { > + int q6 = x1e80100_chmap_to_q6(user[i]); > + > + if (q6 < 0) > + return q6; > + rx_slot[i] = q6; > + } > + > + return 0; > +} > + > +static int x1e80100_snd_startup(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); > + int ret; > + > + ret = qcom_snd_sdw_startup(substream); > + if (ret) > + return ret; > + > + switch (cpu_dai->id) { > + case WSA_CODEC_DMA_RX_0: > + case WSA_CODEC_DMA_RX_1: > + if (!data->chmap_added[cpu_dai->id]) { > + ret = x1e80100_add_wsa_chmap_ctls(rtd); This is will lockup, because startup is called under a dapm_mutex and function calls to snd_ctl_add(), which acquires card->controls_rwsem (write). This creates an AB-BA deadlock with DAPM: Thread A -- PCM open: pcm->open_mutex -> dpcm_mutex -> controls_rwsem (write, via snd_ctl_add) Thread B -- DAPM mixer write: controls_rwsem (write, via snd_ctl_elem_write) -> dpcm_mutex (via dapm_power_widgets) So better move this to to late_probe: Here is what i changed in your patch: -------------------->cut<---------------------------- diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c index 63cd00620ade..8c066f1fd2b7 100644 --- a/sound/soc/qcom/x1e80100.c +++ b/sound/soc/qcom/x1e80100.c @@ -24,7 +24,6 @@ struct x1e80100_snd_data { struct snd_soc_jack dp_jack[8]; bool jack_setup; unsigned int user_chmap[AFE_PORT_MAX][4]; - bool chmap_added[AFE_PORT_MAX]; }; static const struct snd_pcm_chmap_elem x1e80100_wsa_chmaps[] = { @@ -93,14 +92,14 @@ static int x1e80100_chmap_ctl_put(struct snd_kcontrol *kcontrol, return 0; } -static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd) +static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd, const char *name) { struct snd_pcm_chmap *info; unsigned int i; int ret; - ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, - x1e80100_wsa_chmaps, + ret = snd_pcm_add_named_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, + name, x1e80100_wsa_chmaps, X1E80100_WSA_MAX_CHANNELS, 0, &info); if (ret < 0) return ret; @@ -242,29 +241,7 @@ static int x1e80100_snd_build_rx_slot(struct x1e80100_snd_data *data, static int x1e80100_snd_startup(struct snd_pcm_substream *substream) { - struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); - struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); - struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); - int ret; - - ret = qcom_snd_sdw_startup(substream); - if (ret) - return ret; - - switch (cpu_dai->id) { - case WSA_CODEC_DMA_RX_0: - case WSA_CODEC_DMA_RX_1: - if (!data->chmap_added[cpu_dai->id]) { - ret = x1e80100_add_wsa_chmap_ctls(rtd); - if (ret) - return ret; - - data->chmap_added[cpu_dai->id] = true; - } - break; - } - - return 0; + return qcom_snd_sdw_startup(substream); } static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) @@ -312,6 +289,29 @@ static const struct snd_soc_ops x1e80100_be_ops = { .prepare = x1e80100_snd_prepare, }; +static int x1e80100_late_probe(struct snd_soc_card *card) +{ + struct snd_soc_pcm_runtime *rtd; + int ret; + + for_each_card_rtds(card, rtd) { + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); + + switch (cpu_dai->id) { + case WSA_CODEC_DMA_RX_0: + case WSA_CODEC_DMA_RX_1: + if (!rtd->pcm) + break; + ret = x1e80100_add_wsa_chmap_ctls(rtd, "WSA Playback Channel Map"); + if (ret) + return ret; + break; + } + } + + return 0; +} + static void x1e80100_add_be_ops(struct snd_soc_card *card) { struct snd_soc_dai_link *link; @@ -351,6 +351,7 @@ static int x1e80100_platform_probe(struct platform_device *pdev) return ret; card->driver_name = of_device_get_match_data(dev); + card->late_probe = x1e80100_late_probe; x1e80100_add_be_ops(card); return devm_snd_soc_register_card(dev, card); -------------------->cut<---------------------------- > + if (ret) > + return ret; > + > + data->chmap_added[cpu_dai->id] = true; > + } > + break; > + } > + > + return 0; > +} > + > static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) > { > struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > @@ -132,7 +279,8 @@ static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) > switch (cpu_dai->id) { > case WSA_CODEC_DMA_RX_0: > case WSA_CODEC_DMA_RX_1: > - ret = x1e80100_snd_hw_map_channels(rx_slot, channels); > + ret = x1e80100_snd_build_rx_slot(data, cpu_dai->id, channels, > + rx_slot); > if (ret) > return ret; > > @@ -158,7 +306,7 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream) > } > > static const struct snd_soc_ops x1e80100_be_ops = { > - .startup = qcom_snd_sdw_startup, > + .startup = x1e80100_snd_startup, > .shutdown = qcom_snd_sdw_shutdown, > .hw_free = x1e80100_snd_hw_free, > .prepare = x1e80100_snd_prepare, > > --- > base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72 > change-id: 20260609-sound-qcom-x1e80100-allow-ucm-channel-map-2302f2d552d5 > > Best regards, > -- > Abel Vesa <abel.vesa@oss.qualcomm.com> > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override 2026-06-11 8:58 ` Srinivas Kandagatla @ 2026-06-11 9:00 ` Srinivas Kandagatla 0 siblings, 0 replies; 4+ messages in thread From: Srinivas Kandagatla @ 2026-06-11 9:00 UTC (permalink / raw) To: Srinivas Kandagatla, Abel Vesa, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Neil Armstrong, linux-sound, linux-arm-msm, linux-kernel On 6/11/26 9:58 AM, Srinivas Kandagatla wrote: > Thanks Abel, for the patch. > > On 6/10/26 12:16 PM, Abel Vesa wrote: >> The WSA RX slot mapping is currently computed entirely from the channel >> count in .prepare() and pushed unconditionally via >> snd_soc_dai_set_channel_map(). Userspace has no way to express a different >> channel map, so any map defined in UCM is ignored. >> >> Add a writable "Playback Channel Map" control on the WSA RX PCM via >> snd_pcm_add_chmap_ctls() and make it writable. The map selected by >> userspace (SNDRV_CHMAP_*) is stored per AFE port and, at .prepare() >> time, translated into the q6 PCM_CHANNEL_* slots and programmed into the >> DSP. >> >> When userspace has not provided a full map, the previous default >> mapping is used, so existing behaviour is preserved. >> Also if possible can you move some common code to common.c so that other machine drivers can find useful. --srini >> Assisted-by: Codex:GPT-5.5 >> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com> >> --- >> sound/soc/qcom/x1e80100.c | 152 +++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 150 insertions(+), 2 deletions(-) >> >> diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c >> index c81df41ace88..63cd00620ade 100644 >> --- a/sound/soc/qcom/x1e80100.c >> +++ b/sound/soc/qcom/x1e80100.c >> @@ -15,14 +15,105 @@ >> #include "qdsp6/q6dsp-common.h" >> #include "sdw.h" >> >> +#define X1E80100_WSA_MAX_CHANNELS 4 > > this is not the case anymore, we have 6 channels on Glymur. > >> + >> struct x1e80100_snd_data { >> bool stream_prepared[AFE_PORT_MAX]; >> struct snd_soc_card *card; >> struct snd_soc_jack jack; >> struct snd_soc_jack dp_jack[8]; >> bool jack_setup; >> + unsigned int user_chmap[AFE_PORT_MAX][4]; >> + bool chmap_added[AFE_PORT_MAX]; >> }; >> >> +static const struct snd_pcm_chmap_elem x1e80100_wsa_chmaps[] = { > > can you not use snd_pcm_std_chmaps instead ? > >> + { .channels = 1, .map = { SNDRV_CHMAP_FC } }, >> + { .channels = 2, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR } }, >> + { .channels = 3, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, >> + SNDRV_CHMAP_FC } }, >> + { .channels = 4, .map = { SNDRV_CHMAP_FL, SNDRV_CHMAP_FR, >> + SNDRV_CHMAP_RL, SNDRV_CHMAP_RR } }, >> + { } >> +}; >> + >> +static int x1e80100_chmap_to_q6(unsigned int pos) >> +{> + switch (pos) { >> + case SNDRV_CHMAP_FL: return PCM_CHANNEL_FL; >> + case SNDRV_CHMAP_FR: return PCM_CHANNEL_FR; >> + case SNDRV_CHMAP_MONO: >> + case SNDRV_CHMAP_FC: return PCM_CHANNEL_FC; >> + case SNDRV_CHMAP_RL: return PCM_CHANNEL_LB; >> + case SNDRV_CHMAP_RR: return PCM_CHANNEL_RB; >> + case SNDRV_CHMAP_SL: return PCM_CHANNEL_LS; >> + case SNDRV_CHMAP_SR: return PCM_CHANNEL_RS; >> + case SNDRV_CHMAP_LFE: return PCM_CHANNEL_LFE; >> + case SNDRV_CHMAP_RC: return PCM_CHANNEL_CS; >> + default: return -EINVAL; >> + } >> +} >> + >> +static int x1e80100_chmap_ctl_get(struct snd_kcontrol *kcontrol, >> + struct snd_ctl_elem_value *ucontrol) >> +{ >> + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); >> + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; >> + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); >> + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); >> + unsigned int *map = data->user_chmap[cpu_dai->id]; >> + int i; >> + >> + for (i = 0; i < info->max_channels; i++) >> + ucontrol->value.integer.value[i] = map[i]; >> + >> + return 0; >> +} >> + >> +static int x1e80100_chmap_ctl_put(struct snd_kcontrol *kcontrol, >> + struct snd_ctl_elem_value *ucontrol) >> +{ >> + struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); >> + struct snd_soc_pcm_runtime *rtd = info->pcm->private_data; >> + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); >> + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); >> + unsigned int *map = data->user_chmap[cpu_dai->id]; >> + int i; >> + >> + for (i = 0; i < info->max_channels; i++) { >> + unsigned int pos = ucontrol->value.integer.value[i]; >> + >> + /* Validate every requested non-unset position up front. */ >> + if (pos && x1e80100_chmap_to_q6(pos) < 0) >> + return -EINVAL; >> + >> + map[i] = pos; >> + } >> + >> + return 0; >> +} >> + >> +static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd) >> +{ >> + struct snd_pcm_chmap *info; >> + unsigned int i; >> + int ret; >> + >> + ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, >> + x1e80100_wsa_chmaps, >> + X1E80100_WSA_MAX_CHANNELS, 0, &info); > > This will give mixer control as "Playback Channel Map", can we prefix > this with WSA, as this can be cryptic for user side to interpret > Playback Channel Map as a WSA Channel map and also if we add more > channel maps for example from hdmi codec we will endup with something > like on my t14s: > numid=136,iface=PCM,name='Playback Channel Map',device=16 > numid=141,iface=PCM,name='Playback Channel Map',device=17 > numid=146,iface=PCM,name='Playback Channel Map',device=18 > numid=152,iface=PCM,name='Playback Channel Map',device=19 > > This is really confusing, with below patch > you should get: > numid=136,iface=PCM,name='Playback Channel Map',device=16 > numid=141,iface=PCM,name='Playback Channel Map',device=17 > numid=146,iface=PCM,name='Playback Channel Map',device=18 > numid=152,iface=PCM,name='WSA Playback Channel Map',device=21 > > Clearly giving users more clear picture. > Please use this patch and let me know I have tested this on t14s along > with your patch and making some modifications to it. > > > -------------------------------------->cut<---------------------------- > Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com> > Date: Thu Jun 11 08:12:17 2026 +0100 > > ALSA: pcm: Add snd_pcm_add_named_chmap_ctls() for custom control names > > Add snd_pcm_add_named_chmap_ctls() to allowing allowing drivers to > register channel map controls with names other than the default > "Playback/Capture Channel Map". > > Signed-off-by: Srinivas Kandagatla > <srinivas.kandagatla@oss.qualcomm.com> > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h > index 76fc33dce537..2a51f7c02dc6 100644 > --- a/include/sound/pcm.h > +++ b/include/sound/pcm.h > @@ -1501,6 +1501,12 @@ extern const struct snd_pcm_chmap_elem > snd_pcm_alt_chmaps[]; > #define SND_PCM_CHMAP_MASK_246 (SND_PCM_CHMAP_MASK_24 | (1U << 6)) > #define SND_PCM_CHMAP_MASK_2468 (SND_PCM_CHMAP_MASK_246 | (1U << 8)) > > +int snd_pcm_add_named_chmap_ctls(struct snd_pcm *pcm, int stream, > + const char *name, > + const struct snd_pcm_chmap_elem *chmap, > + int max_channels, > + unsigned long private_value, > + struct snd_pcm_chmap **info_ret); > int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, > const struct snd_pcm_chmap_elem *chmap, > int max_channels, > diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c > index fe597f7d522d..7dae270eaedc 100644 > --- a/sound/core/pcm_lib.c > +++ b/sound/core/pcm_lib.c > @@ -2572,9 +2572,10 @@ static void pcm_chmap_ctl_private_free(struct > snd_kcontrol *kcontrol) > } > > /** > - * snd_pcm_add_chmap_ctls - create channel-mapping control elements > + * snd_pcm_add_named_chmap_ctls - create channel-mapping control > elements with a custom name > * @pcm: the assigned PCM instance > * @stream: stream direction > + * @name: the control element name > * @chmap: channel map elements (for query) > * @max_channels: the max number of channels for the stream > * @private_value: the value passed to each kcontrol's private_value field > @@ -2583,11 +2584,12 @@ static void pcm_chmap_ctl_private_free(struct > snd_kcontrol *kcontrol) > * Create channel-mapping control elements assigned to the given PCM > stream(s). > * Return: Zero if successful, or a negative error value. > */ > -int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, > - const struct snd_pcm_chmap_elem *chmap, > - int max_channels, > - unsigned long private_value, > - struct snd_pcm_chmap **info_ret) > +int snd_pcm_add_named_chmap_ctls(struct snd_pcm *pcm, int stream, > + const char *name, > + const struct snd_pcm_chmap_elem *chmap, > + int max_channels, > + unsigned long private_value, > + struct snd_pcm_chmap **info_ret) > { > struct snd_pcm_chmap *info; > struct snd_kcontrol_new knew = { > @@ -2611,10 +2613,7 @@ int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, > int stream, > info->stream = stream; > info->chmap = chmap; > info->max_channels = max_channels; > - if (stream == SNDRV_PCM_STREAM_PLAYBACK) > - knew.name = "Playback Channel Map"; > - else > - knew.name = "Capture Channel Map"; > + knew.name = name; > knew.device = pcm->device; > knew.count = pcm->streams[stream].substream_count; > knew.private_value = private_value; > @@ -2632,4 +2631,29 @@ int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, > int stream, > *info_ret = info; > return 0; > } > +EXPORT_SYMBOL_GPL(snd_pcm_add_named_chmap_ctls); > + > +/** > + * snd_pcm_add_chmap_ctls - create channel-mapping control elements > + * @pcm: the assigned PCM instance > + * @stream: stream direction > + * @chmap: channel map elements (for query) > + * @max_channels: the max number of channels for the stream > + * @private_value: the value passed to each kcontrol's private_value field > + * @info_ret: store struct snd_pcm_chmap instance if non-NULL > + * > + * Create channel-mapping control elements assigned to the given PCM > stream(s). > + * Return: Zero if successful, or a negative error value. > + */ > +int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream, > + const struct snd_pcm_chmap_elem *chmap, > + int max_channels, > + unsigned long private_value, > + struct snd_pcm_chmap **info_ret) > +{ > + return snd_pcm_add_named_chmap_ctls(pcm, stream, > + stream == SNDRV_PCM_STREAM_PLAYBACK ? "Playback > Channel Map" : > + "Capture Channel Map", chmap, > max_channels, > + private_value, info_ret); > +} > EXPORT_SYMBOL_GPL(snd_pcm_add_chmap_ctls); > > -------------------------------------->cut<---------------------------- > > changes to your patch: > > >> + if (ret < 0) >> + return ret; >> + >> + /* Make the query-only chmap control writable from userspace. */ >> + for (i = 0; i < info->kctl->count; i++) >> + info->kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_WRITE; >> + info->kctl->get = x1e80100_chmap_ctl_get; >> + info->kctl->put = x1e80100_chmap_ctl_put; >> + >> + return 0; >> +} >> + >> static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) >> { >> struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); >> @@ -34,6 +125,9 @@ static int x1e80100_snd_init(struct snd_soc_pcm_runtime *rtd) >> switch (cpu_dai->id) { >> case WSA_CODEC_DMA_RX_0: >> case WSA_CODEC_DMA_RX_1: >> + if (!rtd->pcm) >> + return 0; >> + >> /* >> * Set limit of -3 dB on Digital Volume and 0 dB on PA Volume >> * to reduce the risk of speaker damage until we have active >> @@ -92,6 +186,7 @@ static int x1e80100_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, >> return 0; >> } >> >> +/* Default WSA RX slot mapping when userspace has not provided a channel map. */ >> static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) >> { >> switch (num) { >> @@ -120,6 +215,58 @@ static int x1e80100_snd_hw_map_channels(unsigned int *ch_map, int num) >> return 0; >> } >> >> +static int x1e80100_snd_build_rx_slot(struct x1e80100_snd_data *data, >> + unsigned int dai_id, unsigned int channels, >> + unsigned int *rx_slot) >> +{ >> + unsigned int *user = data->user_chmap[dai_id]; >> + unsigned int i, set = 0; >> + >> + for (i = 0; i < channels && i < ARRAY_SIZE(data->user_chmap[0]); i++) >> + if (user[i]) >> + set++; >> + >> + if (set != channels) >> + return x1e80100_snd_hw_map_channels(rx_slot, channels); >> + >> + for (i = 0; i < channels; i++) { >> + int q6 = x1e80100_chmap_to_q6(user[i]); >> + >> + if (q6 < 0) >> + return q6; >> + rx_slot[i] = q6; >> + } >> + >> + return 0; >> +} >> + >> +static int x1e80100_snd_startup(struct snd_pcm_substream *substream) >> +{ >> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); >> + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); >> + struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); >> + int ret; >> + >> + ret = qcom_snd_sdw_startup(substream); >> + if (ret) >> + return ret; >> + >> + switch (cpu_dai->id) { >> + case WSA_CODEC_DMA_RX_0: >> + case WSA_CODEC_DMA_RX_1: >> + if (!data->chmap_added[cpu_dai->id]) { >> + ret = x1e80100_add_wsa_chmap_ctls(rtd); > > This is will lockup, because startup is called under a dapm_mutex and > function calls to snd_ctl_add(), which acquires card->controls_rwsem > (write). > > This creates an AB-BA deadlock with DAPM: > > Thread A -- PCM open: > pcm->open_mutex -> dpcm_mutex -> controls_rwsem (write, via snd_ctl_add) > > Thread B -- DAPM mixer write: > controls_rwsem (write, via snd_ctl_elem_write) -> dpcm_mutex (via > dapm_power_widgets) > > > So better move this to to late_probe: > > Here is what i changed in your patch: > > -------------------->cut<---------------------------- > > diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c > index 63cd00620ade..8c066f1fd2b7 100644 > --- a/sound/soc/qcom/x1e80100.c > +++ b/sound/soc/qcom/x1e80100.c > @@ -24,7 +24,6 @@ struct x1e80100_snd_data { > struct snd_soc_jack dp_jack[8]; > bool jack_setup; > unsigned int user_chmap[AFE_PORT_MAX][4]; > - bool chmap_added[AFE_PORT_MAX]; > }; > > static const struct snd_pcm_chmap_elem x1e80100_wsa_chmaps[] = { > @@ -93,14 +92,14 @@ static int x1e80100_chmap_ctl_put(struct > snd_kcontrol *kcontrol, > return 0; > } > > -static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd) > +static int x1e80100_add_wsa_chmap_ctls(struct snd_soc_pcm_runtime *rtd, > const char *name) > { > struct snd_pcm_chmap *info; > unsigned int i; > int ret; > > - ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK, > - x1e80100_wsa_chmaps, > + ret = snd_pcm_add_named_chmap_ctls(rtd->pcm, > SNDRV_PCM_STREAM_PLAYBACK, > + name, x1e80100_wsa_chmaps, > X1E80100_WSA_MAX_CHANNELS, 0, &info); > if (ret < 0) > return ret; > @@ -242,29 +241,7 @@ static int x1e80100_snd_build_rx_slot(struct > x1e80100_snd_data *data, > > static int x1e80100_snd_startup(struct snd_pcm_substream *substream) > { > - struct snd_soc_pcm_runtime *rtd = > snd_soc_substream_to_rtd(substream); > - struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > - struct x1e80100_snd_data *data = > snd_soc_card_get_drvdata(rtd->card); > - int ret; > - > - ret = qcom_snd_sdw_startup(substream); > - if (ret) > - return ret; > - > - switch (cpu_dai->id) { > - case WSA_CODEC_DMA_RX_0: > - case WSA_CODEC_DMA_RX_1: > - if (!data->chmap_added[cpu_dai->id]) { > - ret = x1e80100_add_wsa_chmap_ctls(rtd); > - if (ret) > - return ret; > - > - data->chmap_added[cpu_dai->id] = true; > - } > - break; > - } > - > - return 0; > + return qcom_snd_sdw_startup(substream); > } > > static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) > @@ -312,6 +289,29 @@ static const struct snd_soc_ops x1e80100_be_ops = { > .prepare = x1e80100_snd_prepare, > }; > > +static int x1e80100_late_probe(struct snd_soc_card *card) > +{ > + struct snd_soc_pcm_runtime *rtd; > + int ret; > + > + for_each_card_rtds(card, rtd) { > + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); > + > + switch (cpu_dai->id) { > + case WSA_CODEC_DMA_RX_0: > + case WSA_CODEC_DMA_RX_1: > + if (!rtd->pcm) > + break; > + ret = x1e80100_add_wsa_chmap_ctls(rtd, "WSA > Playback Channel Map"); > + if (ret) > + return ret; > + break; > + } > + } > + > + return 0; > +} > + > static void x1e80100_add_be_ops(struct snd_soc_card *card) > { > struct snd_soc_dai_link *link; > @@ -351,6 +351,7 @@ static int x1e80100_platform_probe(struct > platform_device *pdev) > return ret; > > card->driver_name = of_device_get_match_data(dev); > + card->late_probe = x1e80100_late_probe; > x1e80100_add_be_ops(card); > > return devm_snd_soc_register_card(dev, card); > > -------------------->cut<---------------------------- > > >> + if (ret) >> + return ret; >> + >> + data->chmap_added[cpu_dai->id] = true; >> + } >> + break; >> + } >> + >> + return 0; >> +} >> + >> static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) >> { >> struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); >> @@ -132,7 +279,8 @@ static int x1e80100_snd_prepare(struct snd_pcm_substream *substream) >> switch (cpu_dai->id) { >> case WSA_CODEC_DMA_RX_0: >> case WSA_CODEC_DMA_RX_1: >> - ret = x1e80100_snd_hw_map_channels(rx_slot, channels); >> + ret = x1e80100_snd_build_rx_slot(data, cpu_dai->id, channels, >> + rx_slot); >> if (ret) >> return ret; >> >> @@ -158,7 +306,7 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream) >> } >> >> static const struct snd_soc_ops x1e80100_be_ops = { >> - .startup = qcom_snd_sdw_startup, >> + .startup = x1e80100_snd_startup, >> .shutdown = qcom_snd_sdw_shutdown, >> .hw_free = x1e80100_snd_hw_free, >> .prepare = x1e80100_snd_prepare, >> >> --- >> base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72 >> change-id: 20260609-sound-qcom-x1e80100-allow-ucm-channel-map-2302f2d552d5 >> >> Best regards, >> -- >> Abel Vesa <abel.vesa@oss.qualcomm.com> >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-11 9:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-06-10 11:16 [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override Abel Vesa 2026-06-10 16:00 ` Mark Brown 2026-06-11 8:58 ` Srinivas Kandagatla 2026-06-11 9:00 ` Srinivas Kandagatla
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®