Re: [PATCH] ASoC: qcom: x1e80100: Allow userspace WSA RX channel map override

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

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>
>