[alsa-devel] [PATCH v4 0/2] ALSA controls management using index/device/sub-devices fields
Previous discussions summary: - Topic of the patchset: Patch concerns ASoc DAI drivers. Aim is to be able to instantiate PCM control using device field according to the associated PCM char device. For this, a relationship between DAIs PCM control and PCM char device needs to be establish. => Proposal is to add field in DAI driver struct to declare PCM controls that need to be linked to PCM character device on DAI link probing.
- Limitation of the patchset: This patchset seems only a first step as 2 concerns are been highlighted: - Patch is limited to "static" DAI-link. This not responds to backend (no_pcm dai links) and some none-DAI codecs, that could register PCm controls. In theses cases it is not possible to establish relationship during probe. => No solution proposed for time beeing.
- This patchset does not fix conflict of 2 "identical" PCM controls declared by 2 drivers. For instance, a conflict can exist between a PCM control created in DAI driver and the same control declared in a codec driver. As index field is forced to 0, prefix should be used in codec. => To discuss solution in a separate thread as issue already present without this patchset several approaches seem possible like using index field ( control auto indexation in ASoC) or prefix in Naming. V4: - [PATCH v4 1/2] ASoC: core: allow DAI PCM controls bound to PCM device Fix and rework error management in soc_link_dai_pcm_controls function. - [PATCH v4 2/2] ASoC: sti: bind pcm controls to pcm device. No change since V2.
V3: - [PATCH v3 1/2] ASoC: core: allow DAI PCM controls bound to PCM device Code optimization based on Takashi Sakamoto suggestion - [RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi patch suppressed as it was send as example in previous version. Patch is valid but can have impact on existing drivers as control device field value is updated - [PATCH v3 2/2] ASoC: sti: bind pcm controls to pcm device. No change vs V2
V2: http://www.spinics.net/lists/alsa-devel/msg57045.html
Aim of this version is to continue discussion on DAI PCM control focused on ASoC drivers. In this V2 implementation in Soc-core is simplified to limit impact on existing code.
Update of the RFC V1 based on discussions: - [RFC 4/4] iecset: allow to select control with device and sub-device numbers no more part of the RFC V2, will be discussed in a separate thread - [RFC 2/4] ALSA: control: increment index field for duplicated control. no more part of the RFC V2, no more need as RFC subject is PCM controls
- [RFC V2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device Patch reworked from V1 to simplify implementation - Binding is not done for Dai links tagged with no_pcm (DPCM). - no more possibility to add the controls after the DAI link probing.
- [RFC V2 2/3] ASoC: sti: bind PCM controls to PCM device. - [RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi Example of implementation in STI DAI driver and HDMI-codec drivers
V1: http://www.spinics.net/lists/alsa-devel/msg56479.html
1) Alsa-utils patch
- iecset: allow to select control with device and sub-device numbers This patch allows to access to 2 iec controls differentiated by device/sub-devices numbers => For me, this patch is mandatory to be able to address the ASoC IEC controls, in case of no fix is implemented to allows index field update in ASoC.
2) Alsa driver patches - ASoC: core: allow PCM control binding to PCM device Add relationship between DAIs PCM controls and PCM device.
- ALSA: control: increment index field for duplicated control. Generic implementation of the patch proposed in HDA driver (http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=ea9b4...)
- ASoC: sti: use bind_pcm_ctl implementation of bind_pcm_ctl for sti driver.
Arnaud Pouliquen (2): ASoC: core: allow DAI PCM controls bound to PCM device ASoC: sti: bind pcm controls to pcm device.
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ sound/soc/sti/sti_uniperif.c | 33 ++++----------------------------- 3 files changed, 43 insertions(+), 29 deletions(-)
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 200e1f0..3ff1a86 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order; + + /* Optional PCM controls to bind to PCM device on DAIs link*/ + const struct snd_kcontrol_new *pcm_controls; + int num_pcm_controls; };
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index aaab26a..7b79a31 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1598,6 +1598,32 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais, + struct snd_soc_pcm_runtime *rtd) +{ + struct snd_kcontrol_new kctl; + int i, j, ret; + + for (i = 0; i < num_dais; ++i) { + for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) { + kctl = dais[i]->driver->pcm_controls[j]; + if (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM || + rtd->dai_link->no_pcm) { + dev_err(dais[i]->dev, + "Failed to bind control: %s\n", + kctl.name); + return -EINVAL; + } + kctl.device = rtd->pcm->device; + ret = snd_soc_add_dai_controls(dais[i], &kctl, 1); + if (ret < 0) + return ret; + } + } + + return 0; +} + static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; } + + /* Bind DAIs pcm controls to the PCM device */ + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); + if (ret < 0) + return ret; + ret = soc_link_dai_pcm_controls(rtd->codec_dais, + rtd->num_codecs, rtd); + if (ret < 0) + return ret; } else { INIT_DELAYED_WORK(&rtd->delayed_work, codec2codec_close_delayed_work);
Hi,
On Nov 30 2016 19:24, Arnaud Pouliquen wrote:
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 200e1f0..3ff1a86 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order;
- /* Optional PCM controls to bind to PCM device on DAIs link*/
In this patchset, you newly request developers to pay enough attention to 'iface' field of identification information for the control element set. This is logically correct, however such kind of special requirement should be described in code comment for the other developers to start using this feature easily.
- const struct snd_kcontrol_new *pcm_controls;
- int num_pcm_controls;
};
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index aaab26a..7b79a31 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1598,6 +1598,32 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, ret;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM ||rtd->dai_link->no_pcm) {dev_err(dais[i]->dev,"Failed to bind control: %s\n",kctl.name);
It's better to output name of the DAI driver, to make it easy to identify which driver causes this issue, as well.
return -EINVAL;}kctl.device = rtd->pcm->device;ret = snd_soc_add_dai_controls(dais[i], &kctl, 1);if (ret < 0)return ret;}- }
- return 0;
+}
static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; }
/* Bind DAIs pcm controls to the PCM device */ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd);if (ret < 0)return ret;ret = soc_link_dai_pcm_controls(rtd->codec_dais,rtd->num_codecs, rtd);if (ret < 0)return ret;
Regards
Takashi Sakamoto
Hi,
Takashi (Sakamoto): Thanks for the remarks, i will integrate them in V5
Liam, Marc, Takashi (Iwai), I was wondering if you have any comment and/or feedback about this patch-set. That's would help to determine if i have to continue to evolve the patch-set in this way, or if you see bocking point in the concept.
Regards,
Arnaud
On 12/05/2016 12:48 PM, Takashi Sakamoto wrote:
Hi,
On Nov 30 2016 19:24, Arnaud Pouliquen wrote:
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 200e1f0..3ff1a86 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order;
- /* Optional PCM controls to bind to PCM device on DAIs link*/
In this patchset, you newly request developers to pay enough attention to 'iface' field of identification information for the control element set. This is logically correct, however such kind of special requirement should be described in code comment for the other developers to start using this feature easily.
- const struct snd_kcontrol_new *pcm_controls;
- int num_pcm_controls;
};
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index aaab26a..7b79a31 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1598,6 +1598,32 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, ret;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM ||rtd->dai_link->no_pcm) {dev_err(dais[i]->dev,"Failed to bind control: %s\n",kctl.name);It's better to output name of the DAI driver, to make it easy to identify which driver causes this issue, as well.
return -EINVAL;}kctl.device = rtd->pcm->device;ret = snd_soc_add_dai_controls(dais[i], &kctl, 1);if (ret < 0)return ret;}- }
- return 0;
+}
static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; }
/* Bind DAIs pcm controls to the PCM device */ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd);if (ret < 0)return ret;ret = soc_link_dai_pcm_controls(rtd->codec_dais,rtd->num_codecs, rtd);if (ret < 0)return ret;Regards
Takashi Sakamoto
On Mon, 05 Dec 2016 14:51:23 +0100, Arnaud Pouliquen wrote:
Hi,
Takashi (Sakamoto): Thanks for the remarks, i will integrate them in V5
Liam, Marc, Takashi (Iwai), I was wondering if you have any comment and/or feedback about this patch-set. That's would help to determine if i have to continue to evolve the patch-set in this way, or if you see bocking point in the concept.
I see no big issue there as the changes look fairly straightforward.
thanks,
Takashi
Regards,
Arnaud
On 12/05/2016 12:48 PM, Takashi Sakamoto wrote:
Hi,
On Nov 30 2016 19:24, Arnaud Pouliquen wrote:
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 200e1f0..3ff1a86 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order;
- /* Optional PCM controls to bind to PCM device on DAIs link*/
In this patchset, you newly request developers to pay enough attention to 'iface' field of identification information for the control element set. This is logically correct, however such kind of special requirement should be described in code comment for the other developers to start using this feature easily.
- const struct snd_kcontrol_new *pcm_controls;
- int num_pcm_controls;
};
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index aaab26a..7b79a31 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1598,6 +1598,32 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, ret;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM ||rtd->dai_link->no_pcm) {dev_err(dais[i]->dev,"Failed to bind control: %s\n",kctl.name);It's better to output name of the DAI driver, to make it easy to identify which driver causes this issue, as well.
return -EINVAL;}kctl.device = rtd->pcm->device;ret = snd_soc_add_dai_controls(dais[i], &kctl, 1);if (ret < 0)return ret;}- }
- return 0;
+}
static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; }
/* Bind DAIs pcm controls to the PCM device */ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd);if (ret < 0)return ret;ret = soc_link_dai_pcm_controls(rtd->codec_dais,rtd->num_codecs, rtd);if (ret < 0)return ret;Regards
Takashi Sakamoto
Move PCM control list in DAI driver struct, to bind PCM control to PCM device created during DAI linking.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- sound/soc/sti/sti_uniperif.c | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/sound/soc/sti/sti_uniperif.c b/sound/soc/sti/sti_uniperif.c index 98eb205..cf42982 100644 --- a/sound/soc/sti/sti_uniperif.c +++ b/sound/soc/sti/sti_uniperif.c @@ -248,34 +248,6 @@ int sti_uniperiph_get_tdm_word_pos(struct uniperif *uni, }
/* - * sti_uniperiph_dai_create_ctrl - * This function is used to create Ctrl associated to DAI but also pcm device. - * Request is done by front end to associate ctrl with pcm device id - */ -static int sti_uniperiph_dai_create_ctrl(struct snd_soc_dai *dai) -{ - struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai); - struct uniperif *uni = priv->dai_data.uni; - struct snd_kcontrol_new *ctrl; - int i; - - if (!uni->num_ctrls) - return 0; - - for (i = 0; i < uni->num_ctrls; i++) { - /* - * Several Control can have same name. Controls are indexed on - * Uniperipheral instance ID - */ - ctrl = &uni->snd_ctrls[i]; - ctrl->index = uni->id; - ctrl->device = uni->id; - } - - return snd_soc_add_dai_controls(dai, uni->snd_ctrls, uni->num_ctrls); -} - -/* * DAI */ int sti_uniperiph_dai_hw_params(struct snd_pcm_substream *substream, @@ -365,7 +337,7 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai) dai_data->dma_data.addr = dai_data->uni->fifo_phys_address; dai_data->dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- return sti_uniperiph_dai_create_ctrl(dai); + return 0; }
static const struct snd_soc_dai_driver sti_uniperiph_dai_template = { @@ -487,6 +459,9 @@ static int sti_uniperiph_probe(struct platform_device *pdev)
ret = sti_uniperiph_cpu_dai_of(node, priv);
+ priv->dai->pcm_controls = priv->dai_data.uni->snd_ctrls; + priv->dai->num_pcm_controls = priv->dai_data.uni->num_ctrls; + dev_set_drvdata(&pdev->dev, priv);
ret = devm_snd_soc_register_component(&pdev->dev,
participants (3)
-
Arnaud Pouliquen -
Takashi Iwai -
Takashi Sakamoto