Re: [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override
From: Srinivas Kandagatla
Date: Thu Jun 11 2026 - 05:02:05 EST
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@xxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
>