[1/3] ALSA: pcm: add snd_pcm_add_chmap_ctls_with_prefix

Commit Message

Srinivas Kandagatla May 5, 2023, 4:55 p.m. UTC

On Qualcomm SoC which has multiple Display Port controllers, using
snd_pcm_add_chmap_ctls() to add chmap controls from every ASoC hdmi codec
instance for a give card fails while adding second instance as the control
with same name "Playback Channel Map" already exists from the first instance.

Fix this by adding a new api wrapper to pass ASoC component name prefix to
avoid such duplicate control naming.

Signed-off-by: Srinivas Kandagatla <[e-mail]>
---
We can also make snd_pcm_add_chmap_ctls to take prefix argument and avoid adding
this new api, but that would touch lots of drivers.


 include/sound/pcm.h  |  7 +++++++
 sound/core/pcm_lib.c | 30 ++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

Comments

Takashi Iwai May 6, 2023, 5:57 a.m. UTC | #1

On Fri, 05 May 2023 18:55:12 +0200,
Srinivas Kandagatla wrote:
> 
> On Qualcomm SoC which has multiple Display Port controllers, using
> snd_pcm_add_chmap_ctls() to add chmap controls from every ASoC hdmi codec
> instance for a give card fails while adding second instance as the control
> with same name "Playback Channel Map" already exists from the first instance.
> 
> Fix this by adding a new api wrapper to pass ASoC component name prefix to
> avoid such duplicate control naming.

It implies that you have conflicting PCM objects with the same PCM
device number from the same card.  How can it be?


Takashi

Srinivas Kandagatla May 6, 2023, 7:42 a.m. UTC | #2

On 06/05/2023 06:57, Takashi Iwai wrote:
> On Fri, 05 May 2023 18:55:12 +0200,
> Srinivas Kandagatla wrote:
>>
>> On Qualcomm SoC which has multiple Display Port controllers, using
>> snd_pcm_add_chmap_ctls() to add chmap controls from every ASoC hdmi codec
>> instance for a give card fails while adding second instance as the control
>> with same name "Playback Channel Map" already exists from the first instance.
>>
>> Fix this by adding a new api wrapper to pass ASoC component name prefix to
>> avoid such duplicate control naming.
> 
> It implies that you have conflicting PCM objects with the same PCM
> device number from the same card.  How can it be?
Thanks for the inputs, this is only issue with my local changes, Sorry 
for the noise, Please ignore this series.

I had some local changes to debug an issue around pcm device numbering 
with backend dailinks from device tree and frontend dailinks from tplg.
Will start a new thread to discuss this issue.

thanks,
srini
> 
> 
> Takashi

Patch

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 27040b472a4f..39efd25b53e3 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1528,6 +1528,13 @@  int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream,
 			   unsigned long private_value,
 			   struct snd_pcm_chmap **info_ret);
 
+int snd_pcm_add_chmap_ctls_with_prefix(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,
+				       const char *prefix);
+
 /**
  * pcm_format_to_bits - Strong-typed conversion of pcm_format to bitwise
  * @pcm_format: PCM format
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 8b6aeb8a78f7..ed75dd007425 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -2483,11 +2483,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,
+int snd_pcm_add_chmap_ctls_with_prefix(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)
+			   struct snd_pcm_chmap **info_ret,
+			   const char *prefix)
 {
 	struct snd_pcm_chmap *info;
 	struct snd_kcontrol_new knew = {
@@ -2499,6 +2500,7 @@  int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream,
 		.get = pcm_chmap_ctl_get,
 		.tlv.c = pcm_chmap_ctl_tlv,
 	};
+	char *name = NULL;
 	int err;
 
 	if (WARN_ON(pcm->streams[stream].chmap_kctl))
@@ -2514,10 +2516,21 @@  int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream,
 		knew.name = "Playback Channel Map";
 	else
 		knew.name = "Capture Channel Map";
+
+	if (prefix) {
+		name = kasprintf(GFP_KERNEL, "%s %s", prefix, knew.name);
+		if (!name) {
+			kfree(info);
+			return -ENOMEM;
+		}
+		knew.name = name;
+	}
+
 	knew.device = pcm->device;
 	knew.count = pcm->streams[stream].substream_count;
 	knew.private_value = private_value;
 	info->kctl = snd_ctl_new1(&knew, info);
+	kfree(name);
 	if (!info->kctl) {
 		kfree(info);
 		return -ENOMEM;
@@ -2530,5 +2543,18 @@  int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream,
 	if (info_ret)
 		*info_ret = info;
 	return 0;
+
+}
+EXPORT_SYMBOL_GPL(snd_pcm_add_chmap_ctls_with_prefix);
+
+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_chmap_ctls_with_prefix(pcm, stream, chmap,
+						  max_channels, private_value,
+						  info_ret, NULL);
 }
 EXPORT_SYMBOL_GPL(snd_pcm_add_chmap_ctls);