[RFC v2 0/3] ALSA controls management using index/device/sub-devices fields

[RFC v2 0/3] ALSA controls management using index/device/sub-devices fields

[Arnaud Pouliquen]

V2:

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

 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=ea9b43add)

  - ASoC: sti: use bind_pcm_ctl
  	implementation of bind_pcm_ctl for sti driver.

Arnaud Pouliquen (3):
  ASoC: core: allow DAI PCM controls bound to PCM device
  ASoC: sti: bind pcm controls to pcm device.
  ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi
    HDMI DAIs.

Regards,

Arnaud

 include/sound/soc-dai.h       |  4 ++++
 sound/soc/codecs/hdmi-codec.c |  4 ++--
 sound/soc/soc-core.c          | 37 +++++++++++++++++++++++++++++++++++++
 sound/soc/sti/sti_uniperif.c  | 33 ++++-----------------------------
 4 files changed, 47 insertions(+), 31 deletions(-)

-- 
1.9.1

[RFC v2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device

[Arnaud Pouliquen]

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    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 41 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..a494287 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1598,6 +1598,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order)
 	return 0;
 }
 
+static int soc_link_dai_pcm_controls(struct snd_soc_card *card,
+				     struct snd_soc_dai *dai,
+				     struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_kcontrol_new kctl;
+	struct snd_soc_dai_driver *drv = dai->driver;
+	int i, err;
+
+	for (i = 0; i < drv->num_pcm_controls; i++) {
+		kctl = drv->pcm_controls[i];
+		if (!rtd->dai_link->no_pcm)
+			kctl.device = rtd->pcm->device;
+		if (snd_soc_add_dai_controls(dai, &kctl, 1))
+			return err;
+	}
+
+	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 +1728,24 @@ 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 */
+			if (cpu_dai->driver->pcm_controls) {
+				ret = soc_link_dai_pcm_controls(card, cpu_dai,
+								rtd);
+				if (ret < 0)
+					return ret;
+			}
+			for (i = 0; i < rtd->num_codecs; i++) {
+				struct snd_soc_dai *dai = rtd->codec_dais[i];
+
+				if (dai->driver->pcm_controls)
+					ret = soc_link_dai_pcm_controls(card,
+									dai,
+									rtd);
+				if (ret < 0)
+					return ret;
+			}
 		} else {
 			INIT_DELAYED_WORK(&rtd->delayed_work,
 						codec2codec_close_delayed_work);
-- 
1.9.1

Re: [RFC v2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device

[Vinod Koul]

On Tue, Nov 22, 2016 at 11:53:14AM +0100, Arnaud Pouliquen wrote:

Sorry I have not looked at previous discussions, so if my questions are
repeat please do point me to relevant thread and I will read up.

> In case of several instances of the same PCM control (e.g IEC controls).

Why are we calling it PCM controls? They are just alsa kcontrols, maybe
used for doing some PCM configuration but then they are just controls.

The same problem (same control names IIUC) existis on codec and SoC's, now
that both have DSP, we can have DSP "Volume control"...

Would this solve that as well..?

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

why is that part required..? Is the problem being solved to address a
control uniquely or something else?

> 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    | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 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..a494287 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1598,6 +1598,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order)
>  	return 0;
>  }
>  
> +static int soc_link_dai_pcm_controls(struct snd_soc_card *card,
> +				     struct snd_soc_dai *dai,
> +				     struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct snd_kcontrol_new kctl;
> +	struct snd_soc_dai_driver *drv = dai->driver;
> +	int i, err;
> +
> +	for (i = 0; i < drv->num_pcm_controls; i++) {
> +		kctl = drv->pcm_controls[i];
> +		if (!rtd->dai_link->no_pcm)
> +			kctl.device = rtd->pcm->device;
> +		if (snd_soc_add_dai_controls(dai, &kctl, 1))
> +			return err;
> +	}
> +
> +	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 +1728,24 @@ 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 */
> +			if (cpu_dai->driver->pcm_controls) {
> +				ret = soc_link_dai_pcm_controls(card, cpu_dai,
> +								rtd);
> +				if (ret < 0)
> +					return ret;
> +			}
> +			for (i = 0; i < rtd->num_codecs; i++) {
> +				struct snd_soc_dai *dai = rtd->codec_dais[i];
> +
> +				if (dai->driver->pcm_controls)
> +					ret = soc_link_dai_pcm_controls(card,
> +									dai,
> +									rtd);
> +				if (ret < 0)
> +					return ret;
> +			}
>  		} else {
>  			INIT_DELAYED_WORK(&rtd->delayed_work,
>  						codec2codec_close_delayed_work);
> -- 
> 1.9.1
> 

-- 
~Vinod

Re: [RFC v2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device

[Takashi Sakamoto]

Vinod,

On Nov 24 2016 06:55, Vinod Koul wrote:
>> In case of several instances of the same PCM control (e.g IEC controls).
> 
> Why are we calling it PCM controls? They are just alsa kcontrols, maybe
> used for doing some PCM configuration but then they are just controls.

He (Arnaud) requires some control element sets which have relationships
to corresponding ALSA PCM character devices.

The control element set of 'IEC958' type is an practical example. When a
sound card has PCM components for configurable S/PDIF interface (both of
IEC 60958-3 and -4 are available), DAI drivers should produce control
element sets for user space application to configure them. His idea is
for this issue.

In this patchset, to make relationship between a control element set and
a PCM device, 'device' field of identification information of control
element set is used. This requires changes of ALSA SoC core because the
core decide the device number by itself and it's not transparent for
interface/codec drivers.

> The same problem (same control names IIUC) existis on codec and SoC's, now
> that both have DSP, we can have DSP "Volume control"...
> 
> Would this solve that as well..?

No. It's not the aim of this patchset and out of its scope.

However, in fact, we should seek good solution for the issue of
'conflict of identification information of control element set added by
ALSA SoC part automatically'.

>> 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.
> 
> why is that part required..? Is the problem being solved to address a
> control uniquely or something else?
>
>> 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    | 37 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 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..a494287 100644
>> --- a/sound/soc/soc-core.c
>> +++ b/sound/soc/soc-core.c
>> @@ -1598,6 +1598,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order)
>>  	return 0;
>>  }
>>  
>> +static int soc_link_dai_pcm_controls(struct snd_soc_card *card,
>> +				     struct snd_soc_dai *dai,
>> +				     struct snd_soc_pcm_runtime *rtd)
>> +{
>> +	struct snd_kcontrol_new kctl;
>> +	struct snd_soc_dai_driver *drv = dai->driver;
>> +	int i, err;
>> +
>> +	for (i = 0; i < drv->num_pcm_controls; i++) {
>> +		kctl = drv->pcm_controls[i];
>> +		if (!rtd->dai_link->no_pcm)
>> +			kctl.device = rtd->pcm->device;
>> +		if (snd_soc_add_dai_controls(dai, &kctl, 1))
>> +			return err;
>> +	}
>> +
>> +	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 +1728,24 @@ 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 */
>> +			if (cpu_dai->driver->pcm_controls) {
>> +				ret = soc_link_dai_pcm_controls(card, cpu_dai,
>> +								rtd);
>> +				if (ret < 0)
>> +					return ret;
>> +			}
>> +			for (i = 0; i < rtd->num_codecs; i++) {
>> +				struct snd_soc_dai *dai = rtd->codec_dais[i];
>> +
>> +				if (dai->driver->pcm_controls)
>> +					ret = soc_link_dai_pcm_controls(card,
>> +									dai,
>> +									rtd);
>> +				if (ret < 0)
>> +					return ret;
>> +			}
>>  		} else {
>>  			INIT_DELAYED_WORK(&rtd->delayed_work,
>>  						codec2codec_close_delayed_work);
>> -- 
>> 1.9.1

Regards

Takashi Sakamoto @UTC+2

Re: [RFC v2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device

[Arnaud Pouliquen]

Hello Vinod

On 11/24/2016 06:14 AM, Takashi Sakamoto wrote:

>> The same problem (same control names IIUC) existis on codec and SoC's, now
>> that both have DSP, we can have DSP "Volume control"...
>>
>> Would this solve that as well..?
> 
> No. It's not the aim of this patchset and out of its scope.
> 
> However, in fact, we should seek good solution for the issue of
> 'conflict of identification information of control element set added by
> ALSA SoC part automatically'.
>
Solution only solves issue for DAIs that are statically linked to a PCM
device on probe,
In you case, link to PCM device is done during runtime through adpcm, right?
Have you an example of a PCM control that you need to link to the PCM
character device?

>>> 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.
>>
>> why is that part required..? Is the problem being solved to address a
>> control uniquely or something else?
It treats a PCM control uniquely,
 what do you have in mind when you say "or something else"?
>>
>>> 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    | 37 +++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 41 insertions(+)

Regards,

Arnaud

[RFC v2 2/3] ASoC: sti: bind pcm controls to pcm device.

[Arnaud Pouliquen]

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

[RFC v2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi HDMI DAIs.

[Arnaud Pouliquen]

link ELD control to PCM device to support multi instances of the codec.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 sound/soc/codecs/hdmi-codec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 90b5948..a807c2f 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -339,6 +339,8 @@ static int hdmi_codec_digital_mute(struct snd_soc_dai *dai, int mute)
 		.sig_bits = 24,
 	},
 	.ops = &hdmi_dai_ops,
+	.pcm_controls = hdmi_controls,
+	.num_pcm_controls = ARRAY_SIZE(hdmi_controls),
 };
 
 static const struct snd_soc_dai_driver hdmi_spdif_dai = {
@@ -381,8 +383,6 @@ static int hdmi_of_xlate_dai_name(struct snd_soc_component *component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
 	.component_driver = {
-		.controls		= hdmi_controls,
-		.num_controls		= ARRAY_SIZE(hdmi_controls),
 		.dapm_widgets		= hdmi_widgets,
 		.num_dapm_widgets	= ARRAY_SIZE(hdmi_widgets),
 		.dapm_routes		= hdmi_routes,
-- 
1.9.1