* [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
@ 2016-03-08 12:53 ` Arnaud Pouliquen
2016-03-10 5:06 ` Vinod Koul
2016-03-08 12:53 ` [PATCH v4 2/6] ASoC: sti: use " Arnaud Pouliquen
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:53 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
Add helper function to register DAI controls that need to be
linked to pcm device.
A list is handled in case controls are created before dai_link probe
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
include/sound/soc-dai.h | 1 +
include/sound/soc.h | 2 +
sound/soc/soc-core.c | 164 +++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 146 insertions(+), 21 deletions(-)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 964b7de..6e0fcb0 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -292,6 +292,7 @@ struct snd_soc_dai {
unsigned int rx_mask;
struct list_head list;
+ struct list_head list_pcm_ctl;
};
static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai *dai,
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 02b4a21..044adcf 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -598,6 +598,8 @@ int snd_soc_add_card_controls(struct snd_soc_card *soc_card,
const struct snd_kcontrol_new *controls, int num_controls);
int snd_soc_add_dai_controls(struct snd_soc_dai *dai,
const struct snd_kcontrol_new *controls, int num_controls);
+int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
+ const struct snd_kcontrol_new *controls, int num_controls);
int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo);
int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 790ee2b..95aae5e 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1582,11 +1582,63 @@ static int soc_link_dai_widgets(struct snd_soc_card *card,
return 0;
}
+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)
+{
+ 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));
+ if (err < 0) {
+ dev_err(dev, "ASoC: Failed to add %s: %d\n",
+ control->name, err);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+struct snd_soc_dai_pcm_ctls {
+ struct snd_kcontrol_new *controls;
+ int num_controls;
+ struct list_head list;
+};
+
+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_soc_dai_pcm_ctls *ctls, *_ctls;
+ struct snd_kcontrol_new *kctl;
+ int i, ret;
+
+ list_for_each_entry_safe(ctls, _ctls, &dai->list_pcm_ctl, list) {
+ kctl = ctls->controls;
+ for (i = 0; i < ctls->num_controls; i++)
+ kctl[i].device = rtd->pcm->device;
+
+ ret = snd_soc_add_controls(card->snd_card, dai->dev, kctl,
+ ctls->num_controls, NULL, dai);
+ kfree(kctl);
+ list_del(&ctls->list);
+ kfree(ctls);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
static int soc_probe_link_dais(struct snd_soc_card *card,
struct snd_soc_pcm_runtime *rtd, int order)
{
struct snd_soc_dai_link *dai_link = rtd->dai_link;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *dai, *cpu_dai = rtd->cpu_dai;
int i, ret;
dev_dbg(card->dev, "ASoC: probe %s dai link %d late %d\n",
@@ -1651,6 +1703,35 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
dai_link->stream_name, ret);
return ret;
}
+
+ /* link CPU DAI pcm controls to pcm device */
+ if (!list_empty(&cpu_dai->list_pcm_ctl))
+ ret = soc_link_dai_pcm_controls(card, cpu_dai,
+ rtd);
+ if (ret < 0) {
+ dev_err(card->dev,
+ "ASoC: Can't link %s control to %s :%d\n",
+ cpu_dai->name, dai_link->stream_name,
+ ret);
+ return ret;
+ }
+
+ /* link CODEC DAI pcm control to pcm device */
+ for (i = 0; i < rtd->num_codecs; i++) {
+ dai = rtd->codec_dais[i];
+ if (!list_empty(&dai->list_pcm_ctl))
+ ret = soc_link_dai_pcm_controls(card,
+ dai, rtd);
+ if (ret < 0)
+ break;
+ }
+ if (ret < 0) {
+ dev_err(card->dev,
+ "ASoC: Can't link %s control to %s :%d\n",
+ dai->name, dai_link->stream_name, ret);
+ return ret;
+ }
+
} else {
INIT_DELAYED_WORK(&rtd->delayed_work,
codec2codec_close_delayed_work);
@@ -2187,26 +2268,6 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
}
EXPORT_SYMBOL_GPL(snd_soc_cnew);
-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)
-{
- 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));
- if (err < 0) {
- dev_err(dev, "ASoC: Failed to add %s: %d\n",
- control->name, err);
- return err;
- }
- }
-
- return 0;
-}
-
struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
const char *name)
{
@@ -2320,6 +2381,65 @@ int snd_soc_add_dai_controls(struct snd_soc_dai *dai,
EXPORT_SYMBOL_GPL(snd_soc_add_dai_controls);
/**
+ * snd_soc_add_dai_pcm_controls - add an array of pcm controls to a DAI.
+ * Convenience function to add a list of DAI controls linked to the PCM device.
+ *
+ * @dai: DAI to add controls to
+ * @controls: array of controls to add
+ * @num_controls: number of elements in the array
+ *
+ * Return 0 for success, else error.
+ */
+int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
+ const struct snd_kcontrol_new *controls,
+ int num_controls)
+{
+ struct snd_soc_card *card = dai->component->card;
+ struct snd_soc_pcm_runtime *rtd;
+ struct snd_soc_dai_pcm_ctls *ctls_list;
+ struct snd_kcontrol_new *kctl;
+ int i, dai_found = 0;
+
+ for (i = 0; i < num_controls; i++) {
+ if (controls[i].iface != SNDRV_CTL_ELEM_IFACE_PCM) {
+ dev_err(dai->dev, "%s: not a pcm device control !!!\n",
+ controls[i].name);
+ return -EINVAL;
+ }
+ }
+
+ kctl = kcalloc(num_controls, sizeof(*kctl), GFP_KERNEL);
+ memcpy(kctl, controls, sizeof(*kctl) * num_controls);
+
+ if (dai->probed) {
+ /* pcm device exists. Control can be linked to it */
+ list_for_each_entry(rtd, &card->rtd_list, list) {
+ if (dai == rtd->cpu_dai) {
+ dai_found = 1;
+ break;
+ }
+ }
+ if (!dai_found)
+ return -EINVAL;
+
+ for (i = 0; i < num_controls; i++)
+ kctl[i].device = rtd->pcm->device;
+ snd_soc_add_controls(card->snd_card, dai->dev, kctl,
+ num_controls, NULL, dai);
+ kfree(kctl);
+ } else {
+ /* pcm device does not exists. Postpone to dai link creation */
+ ctls_list = kzalloc(sizeof(*ctls_list), GFP_KERNEL);
+ ctls_list->controls = kctl;
+ ctls_list->num_controls = num_controls;
+ list_add(&ctls_list->list, &dai->list_pcm_ctl);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_add_dai_pcm_controls);
+
+/**
* snd_soc_dai_set_sysclk - configure DAI system or master clock.
* @dai: DAI
* @clk_id: DAI specific clock ID
@@ -2795,6 +2915,8 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
if (!dai->driver->ops)
dai->driver->ops = &null_dai_ops;
+ INIT_LIST_HEAD(&dai->list_pcm_ctl);
+
list_add(&dai->list, &component->dai_list);
component->num_dai++;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-08 12:53 ` [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper Arnaud Pouliquen
@ 2016-03-10 5:06 ` Vinod Koul
2016-03-10 9:08 ` Arnaud Pouliquen
0 siblings, 1 reply; 12+ messages in thread
From: Vinod Koul @ 2016-03-10 5:06 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Jean-Francois Moine, alsa-devel, Lars-Peter Clausen,
Russell King - ARM Linux, David Airlie, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
On Tue, Mar 08, 2016 at 01:53:56PM +0100, Arnaud Pouliquen wrote:
> Add helper function to register DAI controls that need to be
> linked to pcm device.
> A list is handled in case controls are created before dai_link probe
Overall this patch looks good to us. But on first read it is not very clear
how PCM and DAIs are inter related and why you need to do this. Since we are
having similar issues we were able to quickly understand this, the
suggestion would be to elborate a bit more in changelog.
Second, why do we need a new API for this. Why not use existing asoc
concepts and add one more field in dai_driver for dai_controls.
Core can automagically create those controls and link to PCM.
Lastly, this doesn't help our usecase of DPCM where the HDMI codec is
connected to a BE, so that rtd cannot be used and we need to link to FE, so
not sure how we can solve that...
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> ---
> include/sound/soc-dai.h | 1 +
> include/sound/soc.h | 2 +
> sound/soc/soc-core.c | 164 +++++++++++++++++++++++++++++++++++++++++-------
> 3 files changed, 146 insertions(+), 21 deletions(-)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 964b7de..6e0fcb0 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -292,6 +292,7 @@ struct snd_soc_dai {
> unsigned int rx_mask;
>
> struct list_head list;
> + struct list_head list_pcm_ctl;
> };
>
> static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai *dai,
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 02b4a21..044adcf 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -598,6 +598,8 @@ int snd_soc_add_card_controls(struct snd_soc_card *soc_card,
> const struct snd_kcontrol_new *controls, int num_controls);
> int snd_soc_add_dai_controls(struct snd_soc_dai *dai,
> const struct snd_kcontrol_new *controls, int num_controls);
> +int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
> + const struct snd_kcontrol_new *controls, int num_controls);
> int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
> struct snd_ctl_elem_info *uinfo);
> int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 790ee2b..95aae5e 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1582,11 +1582,63 @@ static int soc_link_dai_widgets(struct snd_soc_card *card,
> return 0;
> }
>
> +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)
> +{
> + 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));
> + if (err < 0) {
> + dev_err(dev, "ASoC: Failed to add %s: %d\n",
> + control->name, err);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +struct snd_soc_dai_pcm_ctls {
> + struct snd_kcontrol_new *controls;
> + int num_controls;
> + struct list_head list;
> +};
> +
> +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_soc_dai_pcm_ctls *ctls, *_ctls;
> + struct snd_kcontrol_new *kctl;
> + int i, ret;
> +
> + list_for_each_entry_safe(ctls, _ctls, &dai->list_pcm_ctl, list) {
> + kctl = ctls->controls;
> + for (i = 0; i < ctls->num_controls; i++)
> + kctl[i].device = rtd->pcm->device;
> +
> + ret = snd_soc_add_controls(card->snd_card, dai->dev, kctl,
> + ctls->num_controls, NULL, dai);
> + kfree(kctl);
> + list_del(&ctls->list);
> + kfree(ctls);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int soc_probe_link_dais(struct snd_soc_card *card,
> struct snd_soc_pcm_runtime *rtd, int order)
> {
> struct snd_soc_dai_link *dai_link = rtd->dai_link;
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *dai, *cpu_dai = rtd->cpu_dai;
> int i, ret;
>
> dev_dbg(card->dev, "ASoC: probe %s dai link %d late %d\n",
> @@ -1651,6 +1703,35 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
> dai_link->stream_name, ret);
> return ret;
> }
> +
> + /* link CPU DAI pcm controls to pcm device */
> + if (!list_empty(&cpu_dai->list_pcm_ctl))
> + ret = soc_link_dai_pcm_controls(card, cpu_dai,
> + rtd);
> + if (ret < 0) {
> + dev_err(card->dev,
> + "ASoC: Can't link %s control to %s :%d\n",
> + cpu_dai->name, dai_link->stream_name,
> + ret);
> + return ret;
> + }
> +
> + /* link CODEC DAI pcm control to pcm device */
> + for (i = 0; i < rtd->num_codecs; i++) {
> + dai = rtd->codec_dais[i];
> + if (!list_empty(&dai->list_pcm_ctl))
> + ret = soc_link_dai_pcm_controls(card,
> + dai, rtd);
> + if (ret < 0)
> + break;
> + }
> + if (ret < 0) {
> + dev_err(card->dev,
> + "ASoC: Can't link %s control to %s :%d\n",
> + dai->name, dai_link->stream_name, ret);
> + return ret;
> + }
> +
> } else {
> INIT_DELAYED_WORK(&rtd->delayed_work,
> codec2codec_close_delayed_work);
> @@ -2187,26 +2268,6 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
> }
> EXPORT_SYMBOL_GPL(snd_soc_cnew);
>
> -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)
> -{
> - 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));
> - if (err < 0) {
> - dev_err(dev, "ASoC: Failed to add %s: %d\n",
> - control->name, err);
> - return err;
> - }
> - }
> -
> - return 0;
> -}
> -
> struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
> const char *name)
> {
> @@ -2320,6 +2381,65 @@ int snd_soc_add_dai_controls(struct snd_soc_dai *dai,
> EXPORT_SYMBOL_GPL(snd_soc_add_dai_controls);
>
> /**
> + * snd_soc_add_dai_pcm_controls - add an array of pcm controls to a DAI.
> + * Convenience function to add a list of DAI controls linked to the PCM device.
> + *
> + * @dai: DAI to add controls to
> + * @controls: array of controls to add
> + * @num_controls: number of elements in the array
> + *
> + * Return 0 for success, else error.
> + */
> +int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
> + const struct snd_kcontrol_new *controls,
> + int num_controls)
> +{
> + struct snd_soc_card *card = dai->component->card;
> + struct snd_soc_pcm_runtime *rtd;
> + struct snd_soc_dai_pcm_ctls *ctls_list;
> + struct snd_kcontrol_new *kctl;
> + int i, dai_found = 0;
> +
> + for (i = 0; i < num_controls; i++) {
> + if (controls[i].iface != SNDRV_CTL_ELEM_IFACE_PCM) {
> + dev_err(dai->dev, "%s: not a pcm device control !!!\n",
> + controls[i].name);
> + return -EINVAL;
> + }
> + }
> +
> + kctl = kcalloc(num_controls, sizeof(*kctl), GFP_KERNEL);
> + memcpy(kctl, controls, sizeof(*kctl) * num_controls);
> +
> + if (dai->probed) {
> + /* pcm device exists. Control can be linked to it */
> + list_for_each_entry(rtd, &card->rtd_list, list) {
> + if (dai == rtd->cpu_dai) {
> + dai_found = 1;
> + break;
> + }
> + }
> + if (!dai_found)
> + return -EINVAL;
> +
> + for (i = 0; i < num_controls; i++)
> + kctl[i].device = rtd->pcm->device;
> + snd_soc_add_controls(card->snd_card, dai->dev, kctl,
> + num_controls, NULL, dai);
> + kfree(kctl);
> + } else {
> + /* pcm device does not exists. Postpone to dai link creation */
> + ctls_list = kzalloc(sizeof(*ctls_list), GFP_KERNEL);
> + ctls_list->controls = kctl;
> + ctls_list->num_controls = num_controls;
> + list_add(&ctls_list->list, &dai->list_pcm_ctl);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_add_dai_pcm_controls);
> +
> +/**
> * snd_soc_dai_set_sysclk - configure DAI system or master clock.
> * @dai: DAI
> * @clk_id: DAI specific clock ID
> @@ -2795,6 +2915,8 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component,
> if (!dai->driver->ops)
> dai->driver->ops = &null_dai_ops;
>
> + INIT_LIST_HEAD(&dai->list_pcm_ctl);
> +
> list_add(&dai->list, &component->dai_list);
> component->num_dai++;
>
> --
> 1.9.1
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-10 5:06 ` Vinod Koul
@ 2016-03-10 9:08 ` Arnaud Pouliquen
2016-03-10 12:58 ` Subhransu S. Prusty
0 siblings, 1 reply; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-10 9:08 UTC (permalink / raw)
To: Vinod Koul
Cc: Jean-Francois Moine, alsa-devel@alsa-project.org,
Lars-Peter Clausen, Russell King - ARM Linux, David Airlie,
Liam Girdwood, Jyri Sarha, Takashi Iwai, Mark Brown,
Philipp Zabel, Moise GERGAUD
On 03/10/2016 06:06 AM, Vinod Koul wrote:
> On Tue, Mar 08, 2016 at 01:53:56PM +0100, Arnaud Pouliquen wrote:
>> Add helper function to register DAI controls that need to be
>> linked to pcm device.
>> A list is handled in case controls are created before dai_link probe
>
> Overall this patch looks good to us. But on first read it is not very clear
> how PCM and DAIs are inter related and why you need to do this. Since we are
> having similar issues we were able to quickly understand this, the
> suggestion would be to elborate a bit more in changelog.
Right, i will provide more details in commit message.
>
> Second, why do we need a new API for this. Why not use existing asoc
> concepts and add one more field in dai_driver for dai_controls.
> Core can automagically create those controls and link to PCM.
Yes this was my first approach. Finally, i created a separate API, to be
able to support iec generic control in DAI ( patch 3/6 and 4/6).
These patches need possibility to attach private data to control.
If patches 3/6 and 4/6 are rejected, for sure i will rework it to use
existing API.
Today It is more on compromise than an optimized solution...
But, creating a generic iec control also implies a compatibility with
ASoC and none ASoC drivers...
>
> Lastly, this doesn't help our usecase of DPCM where the HDMI codec is
> connected to a BE, so that rtd cannot be used and we need to link to FE, so
> not sure how we can solve that...
DPCM seems another story... I'm not fully up to date on DPCM concept,
but as i can remember no link between FE and BE except DAPM routing.
Perhaps, for DPCM, a solution should be to use index field for control,
instead of trying to dynamically link the codec control to PCM device?
Thanks
Arnaud
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-10 9:08 ` Arnaud Pouliquen
@ 2016-03-10 12:58 ` Subhransu S. Prusty
2016-03-10 14:03 ` Arnaud Pouliquen
2016-03-12 6:11 ` Mark Brown
0 siblings, 2 replies; 12+ messages in thread
From: Subhransu S. Prusty @ 2016-03-10 12:58 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Jean-Francois Moine, alsa-devel@alsa-project.org,
Lars-Peter Clausen, Russell King - ARM Linux, Philipp Zabel,
Koul, Vinod, Liam Girdwood, Jyri Sarha, David Airlie, Mark Brown,
Takashi Iwai, Moise GERGAUD
On Thu, Mar 10, 2016 at 02:38:01PM +0530, Arnaud Pouliquen wrote:
>
>
> On 03/10/2016 06:06 AM, Vinod Koul wrote:
> > On Tue, Mar 08, 2016 at 01:53:56PM +0100, Arnaud Pouliquen wrote:
> >> Add helper function to register DAI controls that need to be
> >> linked to pcm device.
> >> A list is handled in case controls are created before dai_link probe
> >
> > Overall this patch looks good to us. But on first read it is not very clear
> > how PCM and DAIs are inter related and why you need to do this. Since we are
> > having similar issues we were able to quickly understand this, the
> > suggestion would be to elborate a bit more in changelog.
> Right, i will provide more details in commit message.
> >
> > Second, why do we need a new API for this. Why not use existing asoc
> > concepts and add one more field in dai_driver for dai_controls.
> > Core can automagically create those controls and link to PCM.
> Yes this was my first approach. Finally, i created a separate API, to be
> able to support iec generic control in DAI ( patch 3/6 and 4/6).
> These patches need possibility to attach private data to control.
> If patches 3/6 and 4/6 are rejected, for sure i will rework it to use
> existing API.
> Today It is more on compromise than an optimized solution...
> But, creating a generic iec control also implies a compatibility with
> ASoC and none ASoC drivers...
In our view, iec controls should be made generic alsa controls and asoc
should have wrapper over these and create these controls on card enumeration
and use these wrappers.
>
> >
> > Lastly, this doesn't help our usecase of DPCM where the HDMI codec is
> > connected to a BE, so that rtd cannot be used and we need to link to FE, so
> > not sure how we can solve that...
> DPCM seems another story... I'm not fully up to date on DPCM concept,
> but as i can remember no link between FE and BE except DAPM routing.
> Perhaps, for DPCM, a solution should be to use index field for control,
> instead of trying to dynamically link the codec control to PCM device?
That calls for a discussion on how the controls should be represented for
DPCM. Takashi/Mark/Liam we need your opinion on how the PCM related controls
should be represented in DPCM concepts.
>
> Thanks
> Arnaud
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-10 12:58 ` Subhransu S. Prusty
@ 2016-03-10 14:03 ` Arnaud Pouliquen
2016-03-12 6:11 ` Mark Brown
1 sibling, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-10 14:03 UTC (permalink / raw)
To: Subhransu S. Prusty
Cc: Jean-Francois Moine, alsa-devel@alsa-project.org,
Lars-Peter Clausen, Russell King - ARM Linux, Philipp Zabel,
Koul, Vinod, Liam Girdwood, Jyri Sarha, David Airlie, Mark Brown,
Takashi Iwai, Moise GERGAUD
On 03/10/2016 01:58 PM, Subhransu S. Prusty wrote:
> On Thu, Mar 10, 2016 at 02:38:01PM +0530, Arnaud Pouliquen wrote:
>>
>>
>> On 03/10/2016 06:06 AM, Vinod Koul wrote:
>>> On Tue, Mar 08, 2016 at 01:53:56PM +0100, Arnaud Pouliquen wrote:
>>>> Add helper function to register DAI controls that need to be
>>>> linked to pcm device.
>>>> A list is handled in case controls are created before dai_link probe
>>>
>>> Overall this patch looks good to us. But on first read it is not very clear
>>> how PCM and DAIs are inter related and why you need to do this. Since we are
>>> having similar issues we were able to quickly understand this, the
>>> suggestion would be to elborate a bit more in changelog.
>> Right, i will provide more details in commit message.
>>>
>>> Second, why do we need a new API for this. Why not use existing asoc
>>> concepts and add one more field in dai_driver for dai_controls.
>>> Core can automagically create those controls and link to PCM.
>> Yes this was my first approach. Finally, i created a separate API, to be
>> able to support iec generic control in DAI ( patch 3/6 and 4/6).
>> These patches need possibility to attach private data to control.
>> If patches 3/6 and 4/6 are rejected, for sure i will rework it to use
>> existing API.
>> Today It is more on compromise than an optimized solution...
>> But, creating a generic iec control also implies a compatibility with
>> ASoC and none ASoC drivers...
>
> In our view, iec controls should be made generic alsa controls and asoc
> should have wrapper over these and create these controls on card enumeration
> and use these wrappers.
Please, could you details what you have in mind please, because I
thought it was exactly what I proposed my patch-set...
My approach is that i would like to take into account the creation of
generic controls in the way of handling pcm control in ASoC.
I tried to treat iec generic control as another PCM control in soc core,
not as an exception. Because this could also be reused in future for
some other generic pcm controls.
>
>>
>>>
>>> Lastly, this doesn't help our usecase of DPCM where the HDMI codec is
>>> connected to a BE, so that rtd cannot be used and we need to link to FE, so
>>> not sure how we can solve that...
>> DPCM seems another story... I'm not fully up to date on DPCM concept,
>> but as i can remember no link between FE and BE except DAPM routing.
>> Perhaps, for DPCM, a solution should be to use index field for control,
>> instead of trying to dynamically link the codec control to PCM device?
>
> That calls for a discussion on how the controls should be represented for
> DPCM. Takashi/Mark/Liam we need your opinion on how the PCM related controls
> should be represented in DPCM concepts.
>
>>
>> Thanks
>> Arnaud
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper
2016-03-10 12:58 ` Subhransu S. Prusty
2016-03-10 14:03 ` Arnaud Pouliquen
@ 2016-03-12 6:11 ` Mark Brown
1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-03-12 6:11 UTC (permalink / raw)
To: Subhransu S. Prusty
Cc: Jean-Francois Moine, alsa-devel@alsa-project.org,
Lars-Peter Clausen, Russell King - ARM Linux, Philipp Zabel,
Koul, Vinod, Arnaud Pouliquen, Liam Girdwood, Jyri Sarha,
David Airlie, Takashi Iwai, Moise GERGAUD
[-- Attachment #1.1: Type: text/plain, Size: 375 bytes --]
On Thu, Mar 10, 2016 at 06:28:18PM +0530, Subhransu S. Prusty wrote:
> That calls for a discussion on how the controls should be represented for
> DPCM. Takashi/Mark/Liam we need your opinion on how the PCM related controls
> should be represented in DPCM concepts.
I think we should be moving away from DPCM towards something that scales
to cover off-SoC devices as well.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/6] ASoC: sti: use snd_soc_add_dai_pcm_controls helper
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
2016-03-08 12:53 ` [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper Arnaud Pouliquen
@ 2016-03-08 12:53 ` Arnaud Pouliquen
2016-03-08 12:53 ` [PATCH v4 3/6] ALSA: pcm: add IEC958 channel status control helper Arnaud Pouliquen
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:53 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
sound/soc/sti/sti_uniperif.c | 37 +++++++------------------------------
1 file changed, 7 insertions(+), 30 deletions(-)
diff --git a/sound/soc/sti/sti_uniperif.c b/sound/soc/sti/sti_uniperif.c
index 39bcefe..0c87380 100644
--- a/sound/soc/sti/sti_uniperif.c
+++ b/sound/soc/sti/sti_uniperif.c
@@ -11,34 +11,6 @@
#include "uniperif.h"
/*
- * 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->info->id;
- ctrl->device = uni->info->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,
@@ -112,6 +84,7 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai)
{
struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
struct sti_uniperiph_dai *dai_data = &priv->dai_data;
+ struct uniperif *uni = priv->dai_data.uni;
/* DMA settings*/
if (of_device_is_compatible(dai->dev->of_node, "st,sti-uni-player"))
@@ -119,10 +92,14 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai)
else
snd_soc_dai_init_dma_data(dai, NULL, &dai_data->dma_data);
- dai_data->dma_data.addr = dai_data->uni->fifo_phys_address;
+ dai_data->dma_data.addr = uni->fifo_phys_address;
dai_data->dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- return sti_uniperiph_dai_create_ctrl(dai);
+ if (uni->num_ctrls)
+ return snd_soc_add_dai_pcm_controls(dai, uni->snd_ctrls,
+ uni->num_ctrls);
+
+ return 0;
}
static const struct snd_soc_dai_driver sti_uniperiph_dai_template = {
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 3/6] ALSA: pcm: add IEC958 channel status control helper
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
2016-03-08 12:53 ` [PATCH v4 1/6] ASoC: core: add snd_soc_add_dai_pcm_controls helper Arnaud Pouliquen
2016-03-08 12:53 ` [PATCH v4 2/6] ASoC: sti: use " Arnaud Pouliquen
@ 2016-03-08 12:53 ` Arnaud Pouliquen
2016-03-08 12:53 ` [PATCH v4 4/6] ASoC: core: allow private data for snd_soc_add_dai_pcm_controls Arnaud Pouliquen
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:53 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
Add IEC958 channel status helper that creates control to handle the
IEC60958 status bits.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
include/sound/pcm_iec958.h | 21 ++++++++
sound/core/pcm_iec958.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 146 insertions(+)
diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
index 0eed397..6d5e3d1 100644
--- a/include/sound/pcm_iec958.h
+++ b/include/sound/pcm_iec958.h
@@ -3,7 +3,28 @@
#include <linux/types.h>
+/**
+ * struct snd_pcm_iec958_params: IEC 60958 controls parameters
+ * @ctrl_set: control set callback
+ * This callback is optional and shall be used to set associated driver
+ * configuration. Calls under mutex protection.
+ * @iec: Mandatory pointer to iec958 structure.
+ * @pdata: Optional pointer to driver context.
+ * @mutex: Mandatory pointer to mutex
+ * Provided by driver to handle race conditions.
+ */
+struct snd_pcm_iec958_params {
+ int (*ctrl_set)(void *pdata, u8 *status);
+ struct snd_aes_iec958 *iec;
+ void *pdata;
+ struct mutex *mutex;
+};
+
int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
size_t len);
+int snd_pcm_create_iec958_ctl(struct snd_pcm *pcm, int subdevice,
+ struct snd_pcm_iec958_params *params,
+ int stream);
+const struct snd_kcontrol_new *snd_pcm_iec958_ctl_new(int stream);
#endif
diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
index 36b2d7a..24d6a8e 100644
--- a/sound/core/pcm_iec958.c
+++ b/sound/core/pcm_iec958.c
@@ -7,10 +7,97 @@
*/
#include <linux/export.h>
#include <linux/types.h>
+#include <linux/wait.h>
#include <sound/asoundef.h>
+#include <sound/control.h>
#include <sound/pcm.h>
#include <sound/pcm_iec958.h>
+static int snd_pcm_iec958_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
+ uinfo->count = 1;
+ return 0;
+}
+
+/*
+ * IEC958 channel status default controls callbacks
+ *
+ * Callbacks are protected by a mutex provided by user.
+ */
+static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *uctl)
+{
+ struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
+ int i;
+
+ if (!params->mutex)
+ return -EINVAL;
+
+ mutex_lock(params->mutex);
+ for (i = 0; i < 5; i++)
+ uctl->value.iec958.status[i] = params->iec->status[i];
+ mutex_unlock(params->mutex);
+
+ return 0;
+}
+
+static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *uctl)
+{
+ struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
+ int err = 0;
+ int i, updated = 0;
+
+ if (!params->mutex)
+ return -EINVAL;
+
+ mutex_lock(params->mutex);
+ for (i = 0; i < 5; i++) {
+ if (params->iec->status[i] != uctl->value.iec958.status[i])
+ updated = 1;
+ }
+
+ if (!updated)
+ goto unlock;
+
+ if (params->ctrl_set)
+ err = params->ctrl_set(params->pdata,
+ uctl->value.iec958.status);
+ if (err < 0) {
+ mutex_unlock(params->mutex);
+ return err;
+ }
+
+ for (i = 0; i < 5; i++)
+ params->iec->status[i] = uctl->value.iec958.status[i];
+
+ unlock:
+ mutex_unlock(params->mutex);
+ return updated;
+}
+
+static const struct snd_kcontrol_new iec958_ctls[] = {
+ {
+ .access = (SNDRV_CTL_ELEM_ACCESS_READWRITE |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE),
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
+ .info = snd_pcm_iec958_info,
+ .get = snd_pcm_iec958_get,
+ .put = snd_pcm_iec958_put,
+ },
+ {
+ .access = (SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE),
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = SNDRV_CTL_NAME_IEC958("", CAPTURE, DEFAULT),
+ .info = snd_pcm_iec958_info,
+ .get = snd_pcm_iec958_get,
+ },
+};
+
/**
* snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status
* @runtime: pcm runtime structure with ->rate filled in
@@ -93,3 +180,41 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
return len;
}
EXPORT_SYMBOL(snd_pcm_create_iec958_consumer);
+
+/**
+ * snd_pcm_create_iec958_ctl - Add a IEC958 control associated to the pcm device
+ * @pcm: pcm device to associate to the control.
+ * @subdevice: subdevice index.Must be set to 0 if unused
+ * @iec958: snd_pcm_iec958_params structure that contains callbacks
+ * and channel status buffer.
+ * @stream: stream type SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CATURE.
+ * Returns: negative error code if something failed.
+ */
+int snd_pcm_create_iec958_ctl(struct snd_pcm *pcm, int subdevice,
+ struct snd_pcm_iec958_params *params, int stream)
+{
+ struct snd_kcontrol_new knew;
+
+ if (stream > SNDRV_PCM_STREAM_LAST)
+ return -EINVAL;
+
+ knew = iec958_ctls[stream];
+ knew.device = pcm->device;
+ knew.subdevice = subdevice;
+ return snd_ctl_add(pcm->card, snd_ctl_new1(&knew, params));
+}
+EXPORT_SYMBOL(snd_pcm_create_iec958_ctl);
+
+/**
+ * snd_pcm_iec958_ctl_new - create a IEC958 control instance from the template
+ * @stream: stream type SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CATURE.
+ * Returns: pointer to snd_kcontrol_new structure, NULL if something failed.
+ */
+const struct snd_kcontrol_new *snd_pcm_iec958_ctl_new(int stream)
+{
+ if (stream > SNDRV_PCM_STREAM_LAST)
+ return NULL;
+
+ return &iec958_ctls[stream];
+}
+EXPORT_SYMBOL(snd_pcm_iec958_ctl_new);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 4/6] ASoC: core: allow private data for snd_soc_add_dai_pcm_controls
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
` (2 preceding siblings ...)
2016-03-08 12:53 ` [PATCH v4 3/6] ALSA: pcm: add IEC958 channel status control helper Arnaud Pouliquen
@ 2016-03-08 12:53 ` Arnaud Pouliquen
2016-03-08 12:54 ` [PATCH v4 5/6] ASoC: sti: use iec channel status control helper Arnaud Pouliquen
2016-03-08 12:54 ` [PATCH v4 6/6] ASoC: hdmi-codec: add IEC control Arnaud Pouliquen
5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:53 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
To support generic iec958 control implementation, dai needs
to provide control private data to allow to retrieve context.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
include/sound/soc.h | 3 ++-
sound/soc/soc-core.c | 10 +++++++---
sound/soc/sti/sti_uniperif.c | 2 +-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 044adcf..e277e726 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -599,7 +599,8 @@ int snd_soc_add_card_controls(struct snd_soc_card *soc_card,
int snd_soc_add_dai_controls(struct snd_soc_dai *dai,
const struct snd_kcontrol_new *controls, int num_controls);
int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
- const struct snd_kcontrol_new *controls, int num_controls);
+ const struct snd_kcontrol_new *controls,
+ int num_controls, void *data);
int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo);
int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 08a1b9f..780192b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1606,6 +1606,7 @@ static int snd_soc_add_controls(struct snd_card *card, struct device *dev,
struct snd_soc_dai_pcm_ctls {
struct snd_kcontrol_new *controls;
int num_controls;
+ void *controls_data;
struct list_head list;
};
@@ -1623,7 +1624,8 @@ static int soc_link_dai_pcm_controls(struct snd_soc_card *card,
kctl[i].device = rtd->pcm->device;
ret = snd_soc_add_controls(card->snd_card, dai->dev, kctl,
- ctls->num_controls, NULL, dai);
+ ctls->num_controls, NULL,
+ ctls->controls_data);
if (ret < 0)
return ret;
kfree(kctl);
@@ -2387,12 +2389,13 @@ EXPORT_SYMBOL_GPL(snd_soc_add_dai_controls);
* @dai: DAI to add controls to
* @controls: array of controls to add
* @num_controls: number of elements in the array
+ * @data: control private data
*
* Return 0 for success, else error.
*/
int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
const struct snd_kcontrol_new *controls,
- int num_controls)
+ int num_controls, void *data)
{
struct snd_soc_card *card = dai->component->card;
struct snd_soc_pcm_runtime *rtd;
@@ -2425,13 +2428,14 @@ int snd_soc_add_dai_pcm_controls(struct snd_soc_dai *dai,
for (i = 0; i < num_controls; i++)
kctl[i].device = rtd->pcm->device;
snd_soc_add_controls(card->snd_card, dai->dev, kctl,
- num_controls, NULL, dai);
+ num_controls, NULL, data);
kfree(kctl);
} else {
/* pcm device does not exists. Postpone to dai link creation */
ctls_list = kzalloc(sizeof(*ctls_list), GFP_KERNEL);
ctls_list->controls = kctl;
ctls_list->num_controls = num_controls;
+ ctls_list->controls_data = data;
list_add(&ctls_list->list, &dai->list_pcm_ctl);
}
diff --git a/sound/soc/sti/sti_uniperif.c b/sound/soc/sti/sti_uniperif.c
index 0c87380..267c44f 100644
--- a/sound/soc/sti/sti_uniperif.c
+++ b/sound/soc/sti/sti_uniperif.c
@@ -97,7 +97,7 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai)
if (uni->num_ctrls)
return snd_soc_add_dai_pcm_controls(dai, uni->snd_ctrls,
- uni->num_ctrls);
+ uni->num_ctrls, dai);
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/6] ASoC: sti: use iec channel status control helper
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
` (3 preceding siblings ...)
2016-03-08 12:53 ` [PATCH v4 4/6] ASoC: core: allow private data for snd_soc_add_dai_pcm_controls Arnaud Pouliquen
@ 2016-03-08 12:54 ` Arnaud Pouliquen
2016-03-08 12:54 ` [PATCH v4 6/6] ASoC: hdmi-codec: add IEC control Arnaud Pouliquen
5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:54 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
Use helper function instead of internal function for iec control
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
sound/soc/sti/Kconfig | 1 +
sound/soc/sti/sti_uniperif.c | 22 ++++++++++--
sound/soc/sti/uniperif.h | 1 +
sound/soc/sti/uniperif_player.c | 78 ++++++++++-------------------------------
4 files changed, 41 insertions(+), 61 deletions(-)
diff --git a/sound/soc/sti/Kconfig b/sound/soc/sti/Kconfig
index 64a6900..8e616a4 100644
--- a/sound/soc/sti/Kconfig
+++ b/sound/soc/sti/Kconfig
@@ -6,6 +6,7 @@ menuconfig SND_SOC_STI
depends on SND_SOC
depends on ARCH_STI || COMPILE_TEST
select SND_SOC_GENERIC_DMAENGINE_PCM
+ select SND_PCM_IEC958
help
Say Y if you want to enable ASoC-support for
any of the STI platforms (e.g. STIH416).
diff --git a/sound/soc/sti/sti_uniperif.c b/sound/soc/sti/sti_uniperif.c
index 267c44f..97e777c 100644
--- a/sound/soc/sti/sti_uniperif.c
+++ b/sound/soc/sti/sti_uniperif.c
@@ -9,6 +9,7 @@
#include <linux/pinctrl/consumer.h>
#include "uniperif.h"
+#include <sound/pcm_iec958.h>
/*
* DAI
@@ -84,10 +85,14 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai)
{
struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
struct sti_uniperiph_dai *dai_data = &priv->dai_data;
- struct uniperif *uni = priv->dai_data.uni;
+ struct uniperif *uni = dai_data->uni;
+ int ret, stream;
/* DMA settings*/
- if (of_device_is_compatible(dai->dev->of_node, "st,sti-uni-player"))
+ stream = of_device_is_compatible(dai->dev->of_node,
+ "st,sti-uni-player") ?
+ SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE;
+ if (stream == SNDRV_PCM_STREAM_PLAYBACK)
snd_soc_dai_init_dma_data(dai, &dai_data->dma_data, NULL);
else
snd_soc_dai_init_dma_data(dai, NULL, &dai_data->dma_data);
@@ -95,6 +100,19 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai)
dai_data->dma_data.addr = uni->fifo_phys_address;
dai_data->dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ if (uni->iec_param) {
+ const struct snd_kcontrol_new *ctrl;
+ /* create generic iec control*/
+ ctrl = snd_pcm_iec958_ctl_new(stream);
+ ret = snd_soc_add_dai_pcm_controls(dai, ctrl, 1,
+ uni->iec_param);
+ if (ret < 0) {
+ dev_err(dai->dev, "%s: Failed to create iec control",
+ __func__);
+ return ret;
+ }
+ }
+
if (uni->num_ctrls)
return snd_soc_add_dai_pcm_controls(dai, uni->snd_ctrls,
uni->num_ctrls, dai);
diff --git a/sound/soc/sti/uniperif.h b/sound/soc/sti/uniperif.h
index f0fd5a9..ab06b4c 100644
--- a/sound/soc/sti/uniperif.h
+++ b/sound/soc/sti/uniperif.h
@@ -1189,6 +1189,7 @@ struct uniperif {
/*alsa ctrl*/
struct snd_kcontrol_new *snd_ctrls;
int num_ctrls;
+ struct snd_pcm_iec958_params *iec_param;
/* dai properties */
unsigned int daifmt;
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c
index 7aca6b9..ee69443 100644
--- a/sound/soc/sti/uniperif_player.c
+++ b/sound/soc/sti/uniperif_player.c
@@ -12,6 +12,7 @@
#include <sound/asoundef.h>
#include <sound/soc.h>
+#include <sound/pcm_iec958.h>
#include "uniperif.h"
@@ -250,7 +251,6 @@ static void uni_player_set_channel_status(struct uniperif *player,
* sampling frequency. If no sample rate is already specified, then
* set one.
*/
- mutex_lock(&player->ctrl_lock);
if (runtime) {
switch (runtime->rate) {
case 22050:
@@ -327,7 +327,6 @@ static void uni_player_set_channel_status(struct uniperif *player,
player->stream_settings.iec958.status[3 + (n * 4)] << 24;
SET_UNIPERIF_CHANNEL_STA_REGN(player, n, status);
}
- mutex_unlock(&player->ctrl_lock);
/* Update the channel status */
if (player->ver < SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0)
@@ -390,7 +389,9 @@ static int uni_player_prepare_iec958(struct uniperif *player,
SET_UNIPERIF_CTRL_ZERO_STUFF_HW(player);
/* Update the channel status */
+ mutex_lock(&player->ctrl_lock);
uni_player_set_channel_status(player, runtime);
+ mutex_unlock(&player->ctrl_lock);
/* Clear the user validity user bits */
SET_UNIPERIF_USER_VALIDITY_VALIDITY_LR(player, 0);
@@ -541,60 +542,16 @@ static int uni_player_prepare_pcm(struct uniperif *player,
/*
* ALSA uniperipheral iec958 controls
*/
-static int uni_player_ctl_iec958_info(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_info *uinfo)
-{
- uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
- uinfo->count = 1;
-
- return 0;
-}
-static int uni_player_ctl_iec958_get(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
+static int uni_player_ctl_iec958_set(void *pdata, u8 *status)
{
- struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
- struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
- struct uniperif *player = priv->dai_data.uni;
- struct snd_aes_iec958 *iec958 = &player->stream_settings.iec958;
-
- mutex_lock(&player->ctrl_lock);
- ucontrol->value.iec958.status[0] = iec958->status[0];
- ucontrol->value.iec958.status[1] = iec958->status[1];
- ucontrol->value.iec958.status[2] = iec958->status[2];
- ucontrol->value.iec958.status[3] = iec958->status[3];
- mutex_unlock(&player->ctrl_lock);
- return 0;
-}
-
-static int uni_player_ctl_iec958_put(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
- struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
- struct uniperif *player = priv->dai_data.uni;
- struct snd_aes_iec958 *iec958 = &player->stream_settings.iec958;
-
- mutex_lock(&player->ctrl_lock);
- iec958->status[0] = ucontrol->value.iec958.status[0];
- iec958->status[1] = ucontrol->value.iec958.status[1];
- iec958->status[2] = ucontrol->value.iec958.status[2];
- iec958->status[3] = ucontrol->value.iec958.status[3];
- mutex_unlock(&player->ctrl_lock);
+ struct uniperif *player = pdata;
uni_player_set_channel_status(player, NULL);
return 0;
}
-static struct snd_kcontrol_new uni_player_iec958_ctl = {
- .iface = SNDRV_CTL_ELEM_IFACE_PCM,
- .name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
- .info = uni_player_ctl_iec958_info,
- .get = uni_player_ctl_iec958_get,
- .put = uni_player_ctl_iec958_put,
-};
-
/*
* uniperif rate adjustement control
*/
@@ -654,12 +611,7 @@ static struct snd_kcontrol_new uni_player_clk_adj_ctl = {
.put = snd_sti_clk_adjustment_put,
};
-static struct snd_kcontrol_new *snd_sti_pcm_ctl[] = {
- &uni_player_clk_adj_ctl,
-};
-
-static struct snd_kcontrol_new *snd_sti_iec_ctl[] = {
- &uni_player_iec958_ctl,
+static struct snd_kcontrol_new *snd_sti_ctl[] = {
&uni_player_clk_adj_ctl,
};
@@ -1106,13 +1058,21 @@ int uni_player_init(struct platform_device *pdev,
IEC958_AES4_CON_MAX_WORDLEN_24 |
IEC958_AES4_CON_WORDLEN_24_20;
- player->num_ctrls = ARRAY_SIZE(snd_sti_iec_ctl);
- player->snd_ctrls = snd_sti_iec_ctl[0];
- } else {
- player->num_ctrls = ARRAY_SIZE(snd_sti_pcm_ctl);
- player->snd_ctrls = snd_sti_pcm_ctl[0];
+ player->iec_param = devm_kzalloc(player->dev,
+ sizeof(*player->iec_param),
+ GFP_KERNEL);
+ if (!player->iec_param)
+ return -ENOMEM;
+
+ player->iec_param->iec = &player->stream_settings.iec958;
+ player->iec_param->mutex = &player->ctrl_lock;
+ player->iec_param->pdata = player;
+ player->iec_param->ctrl_set = uni_player_ctl_iec958_set;
}
+ player->num_ctrls = ARRAY_SIZE(snd_sti_ctl);
+ player->snd_ctrls = snd_sti_ctl[0];
+
return 0;
}
EXPORT_SYMBOL_GPL(uni_player_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 6/6] ASoC: hdmi-codec: add IEC control.
2016-03-08 12:53 [PATCH v4 0/6] add IEC958 channel status control helpers Arnaud Pouliquen
` (4 preceding siblings ...)
2016-03-08 12:54 ` [PATCH v4 5/6] ASoC: sti: use iec channel status control helper Arnaud Pouliquen
@ 2016-03-08 12:54 ` Arnaud Pouliquen
5 siblings, 0 replies; 12+ messages in thread
From: Arnaud Pouliquen @ 2016-03-08 12:54 UTC (permalink / raw)
To: alsa-devel
Cc: Jean-Francois Moine, Lars-Peter Clausen, Russell King - ARM Linux,
David Airlie, arnaud.pouliquen, Liam Girdwood, Jyri Sarha,
Takashi Iwai, Mark Brown, Philipp Zabel, Moise Gergaud
Create 'IEC958 Playback Default' controls to support IEC61937 formats.
the use of the alsa control is optional, using 'iec_ctl' flag.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
include/sound/hdmi-codec.h | 1 +
sound/soc/codecs/hdmi-codec.c | 54 ++++++++++++++++++++++++++++++++-----------
2 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index fc3a481..4366d9f 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -92,6 +92,7 @@ struct hdmi_codec_pdata {
const struct hdmi_codec_ops *ops;
uint i2s:1;
uint spdif:1;
+ uint iec_ctl:1;
int max_i2s_channels;
};
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index bc47b9a..6d156ea 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -32,6 +32,7 @@ struct hdmi_codec_priv {
struct snd_pcm_substream *current_stream;
struct snd_pcm_hw_constraint_list ratec;
uint8_t eld[MAX_ELD_BYTES];
+ struct snd_aes_iec958 iec;
};
static const struct snd_soc_dapm_widget hdmi_widgets[] = {
@@ -122,26 +123,29 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
- struct hdmi_codec_params hp = {
- .iec = {
- .status = { 0 },
- .subcode = { 0 },
- .pad = 0,
- .dig_subframe = { 0 },
- }
- };
+ struct hdmi_codec_params hp;
int ret;
dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
params_width(params), params_rate(params),
params_channels(params));
- ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status,
- sizeof(hp.iec.status));
- if (ret < 0) {
- dev_err(dai->dev, "Creating IEC958 channel status failed %d\n",
- ret);
- return ret;
+ mutex_lock(&hcp->current_stream_lock);
+ hp.iec = hcp->iec;
+ mutex_unlock(&hcp->current_stream_lock);
+
+ if (!hcp->hcd.iec_ctl) {
+ /*
+ * only PCM format supported
+ *channel status set according to runtime parameters
+ */
+ ret = snd_pcm_create_iec958_consumer_hw_params(params,
+ hp.iec.status, sizeof(hp.iec.status));
+ if (ret < 0) {
+ dev_err(dai->dev, "Creating IEC958 status failed %d\n",
+ ret);
+ return ret;
+ }
}
ret = hdmi_codec_new_stream(substream, dai);
@@ -248,6 +252,27 @@ static int hdmi_codec_digital_mute(struct snd_soc_dai *dai, int mute)
return 0;
}
+static int hdmi_codec_dai_probe(struct snd_soc_dai *dai)
+{
+ struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+ struct snd_pcm_iec958_params *iec958_params;
+ const struct snd_kcontrol_new *ctrl;
+
+ if (!hcp->hcd.iec_ctl)
+ return 0;
+
+ iec958_params = devm_kzalloc(dai->dev, sizeof(*iec958_params),
+ GFP_KERNEL);
+ if (!iec958_params)
+ return -ENOMEM;
+
+ iec958_params->iec = &hcp->iec;
+ iec958_params->mutex = &hcp->current_stream_lock;
+
+ ctrl = snd_pcm_iec958_ctl_new(SNDRV_PCM_STREAM_PLAYBACK);
+ return snd_soc_add_dai_pcm_controls(dai, ctrl, 1, iec958_params);
+}
+
static const struct snd_soc_dai_ops hdmi_dai_ops = {
.startup = hdmi_codec_startup,
.shutdown = hdmi_codec_shutdown,
@@ -293,6 +318,7 @@ static struct snd_soc_dai_driver hdmi_i2s_dai = {
.sig_bits = 24,
},
.ops = &hdmi_dai_ops,
+ .probe = hdmi_codec_dai_probe,
};
static const struct snd_soc_dai_driver hdmi_spdif_dai = {
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread