[alsa-devel] [PATCH] ASoC: core: allow control index different from 0
To differentiate control with same name only device field can be used. But some applications like iecset use control index to differentiate controls linked to several PCM devices or DAIs. This patch suppress index overwriting to allow to use control index field.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- sound/soc/soc-core.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..39bc1a9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2175,7 +2175,6 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template, char *name = NULL;
memcpy(&template, _template, sizeof(template)); - template.index = 0;
if (!long_name) long_name = template.name;
Hi,
On Oct 5 2016 00:36, Arnaud Pouliquen wrote:
To differentiate control with same name only device field can be used. But some applications like iecset use control index to differentiate controls linked to several PCM devices or DAIs. This patch suppress index overwriting to allow to use control index field.
An control element can be identified by two ways from user land: - A combination of name/index/interface/device/subdevice - A numerical identification number (numid)
In kernel land, some control elements can be managed as one control element set, mainly due to saving memory usage. The index represents offset of an element in the element set.
If your patch is applied, callers of snd_soc_cnew() in kernel land can assign arbitrary index number to an element set, as the offset of the first element.
According to current implementation, all of first element in element sets had index number zero; e.g. $ amixer controls ... numid=52,iface=MIXER,name='byte-element-26' numid=53,iface=MIXER,name='byte-element-26',index=1 numid=54,iface=MIXER,name='byte-element-26',index=2 numid=55,iface=MIXER,name='byte-element-26',index=3 numid=56,iface=MIXER,name='byte-element-26',index=4 ...
On the other hand, after applying your patch, some element set with index larger than zero can exist; e.g. $ amixer controls ... numid=52,iface=MIXER,name='byte-element-26',index=10 numid=53,iface=MIXER,name='byte-element-26',index=11 numid=54,iface=MIXER,name='byte-element-26',index=12 numid=55,iface=MIXER,name='byte-element-26',index=13 numid=56,iface=MIXER,name='byte-element-26',index=14 (here, I assign 10 as index to this element set) (no other elements with name of 'byte-element-26') ...
In this case, elements with index 0-9 don't exist. This case is a bit confusing to application developers, I think.
I can guess that developers for ALSA SoC part would like to avoid the missing elements with index 0-9, by force to start indexing at zero.
So, if you'd like to start elements with index non-zero, it's better to describe your case to get advantages from this patch.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
sound/soc/soc-core.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..39bc1a9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2175,7 +2175,6 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template, char *name = NULL;
memcpy(&template, _template, sizeof(template));
template.index = 0;
if (!long_name) long_name = template.name;
Regards
Takashi Sakamoto
hi
On 10/05/2016 08:59 AM, Takashi Sakamoto wrote:
Hi,
On Oct 5 2016 00:36, Arnaud Pouliquen wrote:
To differentiate control with same name only device field can be used. But some applications like iecset use control index to differentiate controls linked to several PCM devices or DAIs. This patch suppress index overwriting to allow to use control index field.
An control element can be identified by two ways from user land:
- A combination of name/index/interface/device/subdevice
- A numerical identification number (numid)
In kernel land, some control elements can be managed as one control element set, mainly due to saving memory usage. The index represents offset of an element in the element set.
If your patch is applied, callers of snd_soc_cnew() in kernel land can assign arbitrary index number to an element set, as the offset of the first element.
According to current implementation, all of first element in element sets had index number zero; e.g. $ amixer controls ... numid=52,iface=MIXER,name='byte-element-26' numid=53,iface=MIXER,name='byte-element-26',index=1 numid=54,iface=MIXER,name='byte-element-26',index=2 numid=55,iface=MIXER,name='byte-element-26',index=3 numid=56,iface=MIXER,name='byte-element-26',index=4 ...
On the other hand, after applying your patch, some element set with index larger than zero can exist; e.g. $ amixer controls ... numid=52,iface=MIXER,name='byte-element-26',index=10 numid=53,iface=MIXER,name='byte-element-26',index=11 numid=54,iface=MIXER,name='byte-element-26',index=12 numid=55,iface=MIXER,name='byte-element-26',index=13 numid=56,iface=MIXER,name='byte-element-26',index=14 (here, I assign 10 as index to this element set) (no other elements with name of 'byte-element-26') ...
In this case, elements with index 0-9 don't exist. This case is a bit confusing to application developers, I think.
I can guess that developers for ALSA SoC part would like to avoid the missing elements with index 0-9, by force to start indexing at zero.
So, if you'd like to start elements with index non-zero, it's better to describe your case to get advantages from this patch.
Thanks for the explanation. Here is my use case: I have a card with 3 output devices: hw:0,0 HDMI associated controls: numid=1,iface=PCM,name='ELD' numid=2,iface=PCM,name='IEC958 Playback Default' numid=3,iface=PCM,name='Freq.Adjustment' hw:0,1 DAC associated controls: numid=4,iface=PCM,name=' Freq Adjustment' hw:0,2 SPDIF associated controls: numid=5,iface=PCM,name='IEC958 Playback Default' numid=6,iface=PCM,name='Freq Adjustment'
My concern is to differentiate the controls associated to the outputs. Seems that it could be done using device field, but iecset is based on index (or i missed something?). Adding an option in iecset to address control by device could solve the issue... but it is to good way to handle the use case?
Complementary solution would be that control device field corresponds to PCM device, to allow to address PCM device with args (for instance AESx params for iec)
This is linked to the management of DAI PCM controls that has already been discussed in thread associated to this patch: http://www.spinics.net/lists/alsa-devel/msg47611.html. I can rework my patch (suppress iec generic control, to simplify it) but this only treat DAI controls not BE and "dai less" codecs.
Regards Arnaud
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
sound/soc/soc-core.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..39bc1a9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2175,7 +2175,6 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template, char *name = NULL;
memcpy(&template, _template, sizeof(template));
template.index = 0;
if (!long_name) long_name = template.name;
Regards
Takashi Sakamoto
On Oct 5 2016 17:41, Arnaud Pouliquen wrote:
Thanks for the explanation. Here is my use case: I have a card with 3 output devices: hw:0,0 HDMI associated controls: numid=1,iface=PCM,name='ELD' numid=2,iface=PCM,name='IEC958 Playback Default' numid=3,iface=PCM,name='Freq.Adjustment' hw:0,1 DAC associated controls: numid=4,iface=PCM,name=' Freq Adjustment' hw:0,2 SPDIF associated controls: numid=5,iface=PCM,name='IEC958 Playback Default' numid=6,iface=PCM,name='Freq Adjustment'
My concern is to differentiate the controls associated to the outputs. Seems that it could be done using device field, but iecset is based on index (or i missed something?). Adding an option in iecset to address control by device could solve the issue... but it is to good way to handle the use case?
Complementary solution would be that control device field corresponds to PCM device, to allow to address PCM device with args (for instance AESx params for iec)
This is linked to the management of DAI PCM controls that has already been discussed in thread associated to this patch: http://www.spinics.net/lists/alsa-devel/msg47611.html. I can rework my patch (suppress iec generic control, to simplify it) but this only treat DAI controls not BE and "dai less" codecs.
Hm. In configuration space (namespace) of alsa-lib, the 'hw:0,1' PCM node is a short representation of 'pcm.hw:0,1,0'; each member expresses: - pcm: interface - hw: plugin name - 0: card number - 1: device number - 0: subdevice number
Next, when identifying a control element, 'struct snd_ctl_elem_id' is used in ALSA control core. Layout of the structure is in <sound/asound.h>:
struct snd_ctl_elem_id { unsigned int numid; snd_ctl_elem_iface_t iface; unsigned int device; unsigned int subdevice; unsigned char name[44]; unsigned int index; };
Under the above design, when using a proper combination of iface/card/device/subdevice, we can represent the relationship between a control element and a PCM node.
But in a case of control elements for IEC 60958 type, we need to read this patch, commit ea9b43addc4d ("ALSA: hda - Fix broken workaround for HDMI/SPDIF conflicts"). http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=ea9b4...
In this commit for former Intel HDA driver, two issues were addressed: - multiple codecs gives functionalities of digital interface such as HDMI. - bug of mixer interface in alsa-lib
Due to the issues, 'index' field is used, instead of the design I described.
Although, the requirement for HDA codecs (verb parser) and your requirement is different. I think it better for you to follow the above design, then fix the bug of mixer interface in alsa-lib. At least, via the other ctl-related APIs such as ctl/hctl, the control element can be selected and processed in user land.
This is linked to the management of DAI PCM controls that has already been discussed in thread associated to this patch: http://www.spinics.net/lists/alsa-devel/msg47611.html. I can rework my patch (suppress iec generic control, to simplify it) but this only treat DAI controls not BE and "dai less" codecs.
I can't follow all of messages in the related threads, but:
[alsa-devel] [PATCH v3 1/4] ALSA: pcm: add IEC958 channel status control helper http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105152.html
Takashi Iwai wrote:
Hmm, this doesn't always work. It will create the substream_count ctls starting from the pcm dev# as index. What if there are 2 PCM devices where both contain 4 substreams?
struct snd_ctl_elem_id.subdevice can represent the index of PCM substream because current PCM core expresses the substreams as the subdevices. http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/...
[alsa-devel] [PATCH v3 1/4] ALSA: pcm: add IEC958 channel status control helper http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105158.html
Takashi Iwai wrote:
This pretty much depends on the hardware design. If each substream is really individual, you'd need to give the control for each substream.
I'll assist his advice for your issue.
In next time, please put URIs to your comment so that the other developers can follow related issues.
Regards
Takshi Sakamoto
On 10/06/2016 04:01 PM, Takashi Sakamoto wrote:
On Oct 5 2016 17:41, Arnaud Pouliquen wrote:
Thanks for the explanation. Here is my use case: I have a card with 3 output devices: hw:0,0 HDMI associated controls: numid=1,iface=PCM,name='ELD' numid=2,iface=PCM,name='IEC958 Playback Default' numid=3,iface=PCM,name='Freq.Adjustment' hw:0,1 DAC associated controls: numid=4,iface=PCM,name=' Freq Adjustment' hw:0,2 SPDIF associated controls: numid=5,iface=PCM,name='IEC958 Playback Default' numid=6,iface=PCM,name='Freq Adjustment'
My concern is to differentiate the controls associated to the outputs. Seems that it could be done using device field, but iecset is based on index (or i missed something?). Adding an option in iecset to address control by device could solve the issue... but it is to good way to handle the use case?
Complementary solution would be that control device field corresponds to PCM device, to allow to address PCM device with args (for instance AESx params for iec)
This is linked to the management of DAI PCM controls that has already been discussed in thread associated to this patch: http://www.spinics.net/lists/alsa-devel/msg47611.html. I can rework my patch (suppress iec generic control, to simplify it) but this only treat DAI controls not BE and "dai less" codecs.
Hm. In configuration space (namespace) of alsa-lib, the 'hw:0,1' PCM node is a short representation of 'pcm.hw:0,1,0'; each member expresses:
- pcm: interface
- hw: plugin name
- 0: card number
- 1: device number
- 0: subdevice number
Next, when identifying a control element, 'struct snd_ctl_elem_id' is used in ALSA control core. Layout of the structure is in <sound/asound.h>:
struct snd_ctl_elem_id { unsigned int numid; snd_ctl_elem_iface_t iface; unsigned int device; unsigned int subdevice; unsigned char name[44]; unsigned int index; };
Under the above design, when using a proper combination of iface/card/device/subdevice, we can represent the relationship between a control element and a PCM node.
But in a case of control elements for IEC 60958 type, we need to read this patch, commit ea9b43addc4d ("ALSA: hda - Fix broken workaround for HDMI/SPDIF conflicts"). http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=ea9b4...
In this commit for former Intel HDA driver, two issues were addressed:
- multiple codecs gives functionalities of digital interface such as HDMI.
- bug of mixer interface in alsa-lib
not aware about alsa-lib issue... Have you an URI that details the issue?
Due to the issues, 'index' field is used, instead of the design I described.
Although, the requirement for HDA codecs (verb parser) and your requirement is different. I think it better for you to follow the above design, then fix the bug of mixer interface in alsa-lib. At least, via the other ctl-related APIs such as ctl/hctl, the control element can be selected and processed in user land.
First, sorry i think my mail is not Cristal clear
Let clarify the my devs:
1) My original implementation is here : http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/soc/s... this does not work due to index that is overwritten Notice that my issue is not limited to iec958 control, but controls in generals
2) In parallel, I proposed generic code to implement: IEC control + possibility to attach DAI ctrl to PCM device:http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105502.html
Right now, I propose to focus on point 1) that integrates constraints for DAIs and forget the patch-set that includes IEC generic control (if you interested in this code, i can rework it in a second step)
STI DAI driver uses DAI control interface. Index is forced to 0 but as device is also set, no error on ctrl registration. Handle index based on HDA codec implementation means I need to bypass DAI ctrl helpers... Should be nice to have a more generic way, as several implementations can face the same issue.
Perhaps this could be handled in a generic way in control.c? Today if control is identical, snd_ctl_add returns an error. Instead an auto-incrementation mechanism could be implemented to increase index.
This is linked to the management of DAI PCM controls that has already been discussed in thread associated to this patch: http://www.spinics.net/lists/alsa-devel/msg47611.html. I can rework my patch (suppress iec generic control, to simplify it) but this only treat DAI controls not BE and "dai less" codecs.
I can't follow all of messages in the related threads, but:
[alsa-devel] [PATCH v3 1/4] ALSA: pcm: add IEC958 channel status control helper http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105152.html
Takashi Iwai wrote:
Hmm, this doesn't always work. It will create the substream_count ctls starting from the pcm dev# as index. What if there are 2 PCM devices where both contain 4 substreams?
struct snd_ctl_elem_id.subdevice can represent the index of PCM substream because current PCM core expresses the substreams as the subdevices. http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/...
[alsa-devel] [PATCH v3 1/4] ALSA: pcm: add IEC958 channel status control helper http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105158.html
Takashi Iwai wrote:
This pretty much depends on the hardware design. If each substream is really individual, you'd need to give the control for each substream.
I'll assist his advice for your issue.
For IEC generic ctrl, hope that this point has be integrated in V4: http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105505.html index is no more overwritten by snd_pcm_create_iec958_ctl and subdevice is added in function params
Thanks and Regards Arnaud
Sorry to be late. I've review your driver in for-next branch of maintainer's tree.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
Let clarify the my devs:
- My original implementation is here :
http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/soc/s... this does not work due to index that is overwritten Notice that my issue is not limited to iec958 control, but controls in generals
- In parallel, I proposed generic code to implement:
IEC control + possibility to attach DAI ctrl to PCM device:http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105502.html
Right now, I propose to focus on point 1) that integrates constraints for DAIs and forget the patch-set that includes IEC generic control (if you interested in this code, i can rework it in a second step)
The center of discussion now becomes transformed.
Anyway, at least, this patch (ASoC: core: allow control index different from 0) is exaggerated to your aim. The change influences to all of control element sets added by implementations in ALSA SoC part. (We have no guarantee that all drivers already set 0 to the field of template before calling with the template.)
And please write enough information about your aim of this patch, even if it's a part of your work for something large. It helps reviewers. Additionally, if you discuss the former patchset, please rebase them to current upstream, then post them again with enough comments, then start discussion.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
Perhaps this could be handled in a generic way in control.c? Today if control is identical, snd_ctl_add returns an error. Instead an auto-incrementation mechanism could be implemented to increase index.
Just for ALSA SoC part, something like below (not tested). This never work well because 0 is still assigned to the index field later.
But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
In a framework of ALSA SoC part, several drivers are associated to one sound card instance can add their own control element sets. There's no mechanism to prevent my concern. This idea is bad.
From 30c6abb20ca548add8cddb6e056831efde365c3e Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto o-takashi@sakamocchi.jp Date: Sun, 9 Oct 2016 11:20:08 +0900 Subject: [PATCH] hoge
--- sound/soc/soc-core.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c0bbcd9..573ca73 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2202,15 +2202,49 @@ static int snd_soc_add_controls(struct snd_card *card, struct device *dev, const struct snd_kcontrol_new *controls, int num_controls, const char *prefix, void *data) { + struct snd_kcontrol_new template; + struct snd_ctl_elem_id id; + struct snd_kcontrol *elem_set; int err, i;
for (i = 0; i < num_controls; i++) { - const struct snd_kcontrol_new *control = &controls[i]; - err = snd_ctl_add(card, snd_soc_cnew(control, data, - control->name, prefix)); + template = controls[i]; + + id.iface = template.iface; + id.device = template.device; + id.subdevice = template.subdevice; + memcpy(id.name, template.name, sizeof(id.name)); + + /* + * Seek duplicated element sets already registered in this + * sound card to use continuous number for index field. + */ + while (1) { + id.index = template.index; + + elem_set = snd_ctl_find_id(card, &id); + if (elem_set == NULL) + break; + + if (elem_set->id.index > UINT_MAX - elem_set->count) { + dev_err(dev, "ASoC: Failed to keep %s: %d\n", + template.name, err); + return -ENOSPC; + } + template.index = elem_set->count + elem_set->id.index; + } + + elem_set = snd_soc_cnew(&template, data, template.name, prefix); + if (elem_set == NULL) { + dev_err(dev, "ASoC: Failed to allocate %s: %d\n", + template.name, err); + return -ENOMEM; + } + + err = snd_ctl_add(card, elem_set); if (err < 0) { dev_err(dev, "ASoC: Failed to add %s: %d\n", - control->name, err); + template.name, err); return err; } }
On 10/09/2016 05:05 AM, Takashi Sakamoto wrote:
Sorry to be late. I've review your driver in for-next branch of maintainer's tree.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
Let clarify the my devs:
- My original implementation is here :
http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/soc/s... this does not work due to index that is overwritten Notice that my issue is not limited to iec958 control, but controls in generals
- In parallel, I proposed generic code to implement:
IEC control + possibility to attach DAI ctrl to PCM device:http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105502.html
Right now, I propose to focus on point 1) that integrates constraints for DAIs and forget the patch-set that includes IEC generic control (if you interested in this code, i can rework it in a second step)
The center of discussion now becomes transformed.
Anyway, at least, this patch (ASoC: core: allow control index different from 0) is exaggerated to your aim. The change influences to all of control element sets added by implementations in ALSA SoC part. (We have no guarantee that all drivers already set 0 to the field of template before calling with the template.)
Full in line with you. this patch is already in my trash.
And please write enough information about your aim of this patch, even if it's a part of your work for something large. It helps reviewers. Additionally, if you discuss the former patchset, please rebase them to current upstream, then post them again with enough comments, then start discussion.
Sorry if my approach to treat the topic is not good. My Aim is to find a way to support multi instances of an alsa control. As I suppose that i'm not alone to have this kind of problematic. I am trying to look for a generic way to do it.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
Perhaps this could be handled in a generic way in control.c? Today if control is identical, snd_ctl_add returns an error. Instead an auto-incrementation mechanism could be implemented to increase index.
Just for ALSA SoC part, something like below (not tested). This never work well because 0 is still assigned to the index field later.
Index is forced to 0 in snd_soc_cnew. Not updated after, please find comment in line in your patch.
But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
Today is already possible using the device or subdevice field instead of index.
In a framework of ALSA SoC part, several drivers are associated to one sound card instance can add their own control element sets. There's no mechanism to prevent my concern. This idea is bad.
Please tell me if i misunderstand. So for you, there is no real solution to do it in a generic way. Drivers has to implement it, if they want to support.
Another solution is to declare a card per instance of control. This should work for my use case and for use cases with several codecs that declare same controls. But this should not work for DPCM... The drawback for my use case, would be that i need to declare one card per PCM device.
Regards Arnaud
From 30c6abb20ca548add8cddb6e056831efde365c3e Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto o-takashi@sakamocchi.jp Date: Sun, 9 Oct 2016 11:20:08 +0900 Subject: [PATCH] hoge
sound/soc/soc-core.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c0bbcd9..573ca73 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2202,15 +2202,49 @@ static int snd_soc_add_controls(struct snd_card *card, struct device *dev, const struct snd_kcontrol_new *controls, int num_controls, const char *prefix, void *data) {
struct snd_kcontrol_new template;
struct snd_ctl_elem_id id;
struct snd_kcontrol *elem_set; int err, i;
for (i = 0; i < num_controls; i++) {
const struct snd_kcontrol_new *control = &controls[i];err = snd_ctl_add(card, snd_soc_cnew(control, data,control->name, prefix));
template = controls[i];id.iface = template.iface;id.device = template.device;id.subdevice = template.subdevice;memcpy(id.name, template.name, sizeof(id.name));/** Seek duplicated element sets already registered in this* sound card to use continuous number for index field.*/while (1) {id.index = template.index;elem_set = snd_ctl_find_id(card, &id);if (elem_set == NULL)break;if (elem_set->id.index > UINT_MAX - elem_set->count) {dev_err(dev, "ASoC: Failed to keep %s: %d\n",template.name, err);return -ENOSPC;}template.index = elem_set->count + elem_set->id.index;}elem_set = snd_soc_cnew(&template, data, template.name, prefix);
index field is forced to 0 in snd_soc_cnew so eleme_set->id.index should be overwritten to be taken into account.
if (elem_set == NULL) {dev_err(dev, "ASoC: Failed to allocate %s: %d\n",template.name, err);return -ENOMEM;}err = snd_ctl_add(card, elem_set);
control->name, err);
template.name, err); return err;
On Oct 10 2016 18:26, Arnaud Pouliquen wrote:
Sorry if my approach to treat the topic is not good. My Aim is to find a way to support multi instances of an alsa control. As I suppose that i'm not alone to have this kind of problematic. I am trying to look for a generic way to do it.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
Perhaps this could be handled in a generic way in control.c? Today if control is identical, snd_ctl_add returns an error. Instead an auto-incrementation mechanism could be implemented to increase index.
Just for ALSA SoC part, something like below (not tested). This never work well because 0 is still assigned to the index field later.
Index is forced to 0 in snd_soc_cnew. Not updated after, please find comment in line in your patch.
I should write that '0 is assigned to the index field later than calculated index number is assigned'.
But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
Today is already possible using the device or subdevice field instead of index.
Please explain about the reason or show references to the reason that usage of iface/card/device/subdevice combination is not convenient to your issue. To me, this solves the issue (second issue in your previous message) because at least, from user land, applications can identify a control element corresponding to each PCM subdevice.
In other words, in your taste, why this is not a generic way for something you face (this is not cleared yet to me).
In a framework of ALSA SoC part, several drivers are associated to one sound card instance can add their own control element sets. There's no mechanism to prevent my concern. This idea is bad.
Please tell me if i misunderstand. So for you, there is no real solution to do it in a generic way. Drivers has to implement it, if they want to support.
Over-generalizing, you did.
In my insistent, the usage of index field for this purpose is not just better. I've never said that no real solutions we have.
Another solution is to declare a card per instance of control. This should work for my use case and for use cases with several codecs that declare same controls.> But this should not work for DPCM... The drawback for my use case, would be that i need to declare one card per PCM device.
In my understanding, this idea is not good for simple-card implementation in ALSA SoC part, too. The idea needs more complicated driver to have several instances of sound card.
Regards
Takashi Sakamoto
But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
Today is already possible using the device or subdevice field instead of index.
Please explain about the reason or show references to the reason that usage of iface/card/device/subdevice combination is not convenient to your issue. To me, this solves the issue (second issue in your previous message) because at least, from user land, applications can identify a control element corresponding to each PCM subdevice.
In other words, in your taste, why this is not a generic way for something you face (this is not cleared yet to me).
iface/card/device/subdevice can be used when control is linked to a PCM device. Main difference for ASoC drivers is that controls associated to CPU_DAIs and CODEC_DAIs are not linked to PCM device. That's why i had mentioned in a previous mail a patch that proposed to link DAI to PCM (it just a first version for concept). [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper: http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105501.html
This could link control to PCM device for CPU and CODEC DAI in ASoC implementation. But issue still there for DPCM implementation where back-end is not linked to the PCM device, or for CODECS without DAI interface, as mentioned in feedbacks: http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105570.html http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105660.html
Also we can imagine 2 instances of the same codec (for instance HDMI1 and HDMI2 output for TV and AVR). They can export twice the same controls. In this case issue can also exists for none PCM controls (such as MIXER controls).
The last point is the iecset application. It uses card/index and not iface/card/device/subdevice. But this also makes sense if iec control is not linked to PCM device.
Another solution is to declare a card per instance of control. This should work for my use case and for use cases with several codecs that declare same controls.> But this should not work for DPCM... The drawback for my use case, would be that i need to declare one card per PCM device.
In my understanding, this idea is not good for simple-card implementation in ALSA SoC part, too. The idea needs more complicated driver to have several instances of sound card.
Simple card can be instantiate in DT to create several sound cards. But look like more a backup that a main solution, from my point of view.
Regards Arnaud
On Mon, Oct 10, 2016 at 11:26:00AM +0200, Arnaud Pouliquen wrote:
On 10/09/2016 05:05 AM, Takashi Sakamoto wrote:
Sorry to be late. I've review your driver in for-next branch of maintainer's tree.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote: But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
Today is already possible using the device or subdevice field instead of index.
In a framework of ALSA SoC part, several drivers are associated to one sound card instance can add their own control element sets. There's no mechanism to prevent my concern. This idea is bad.
Please tell me if i misunderstand. So for you, there is no real solution to do it in a generic way. Drivers has to implement it, if they want to support.
Another solution is to declare a card per instance of control. This should work for my use case and for use cases with several codecs that declare same controls. But this should not work for DPCM... The drawback for my use case, would be that i need to declare one card per PCM device.
Apologies if I am missing the mark here, I haven't been following this thread in great detail. But if your main concern here is multiple instances of the same CODEC creating the same controls the normal way to handle that in ASoC is using a snd_soc_codec_conf struct which lets you add a prefix to all the controls from a specific instance of a CODEC. See sound/soc/samsung/bells.c for an example using it.
Thanks, Charles
On 10/11/2016 10:30 AM, Charles Keepax wrote:
On Mon, Oct 10, 2016 at 11:26:00AM +0200, Arnaud Pouliquen wrote:
On 10/09/2016 05:05 AM, Takashi Sakamoto wrote:
Sorry to be late. I've review your driver in for-next branch of maintainer's tree.
On Oct 8 2016 01:41, Arnaud Pouliquen wrote: But I still oppose this idea. This idea allows drivers to add control element sets of different types (int/int64/bytes etc...) with the same name and different index number. This certainly brings confusions to applications.
Today is already possible using the device or subdevice field instead of index.
In a framework of ALSA SoC part, several drivers are associated to one sound card instance can add their own control element sets. There's no mechanism to prevent my concern. This idea is bad.
Please tell me if i misunderstand. So for you, there is no real solution to do it in a generic way. Drivers has to implement it, if they want to support.
Another solution is to declare a card per instance of control. This should work for my use case and for use cases with several codecs that declare same controls. But this should not work for DPCM... The drawback for my use case, would be that i need to declare one card per PCM device.
Apologies if I am missing the mark here, I haven't been following this thread in great detail. But if your main concern here is multiple instances of the same CODEC creating the same controls the normal way to handle that in ASoC is using a snd_soc_codec_conf struct which lets you add a prefix to all the controls from a specific instance of a CODEC. See sound/soc/samsung/bells.c for an example using it.
Codec multi instance is one of the use cases. CPU_DAI multi instance is another one (and my main concern). But use of a prefix_name is a good point that i have not prospected yet, thanks. Today it is not implemented neither for CPU_DAIs nor for simple card. But my main drawback of the prefix name, is that this breaks the naming of generic controls like IEC. So i can't use it.
Regards Arnaud
participants (3)
-
Arnaud Pouliquen -
Charles Keepax -
Takashi Sakamoto