[alsa-devel] [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Wed May 23 23:38:46 CEST 2018
This series is pretty dense, I could only review the first in the series
today...
On 05/22/2018 05:40 AM, Shreyas NC wrote:
> ASoC core supports multiple codec DAIs but supports only a CPU DAI.
> To support multiple cpu DAIs, add cpu_dai and num_cpu_dai in
> snd_soc_dai_link and snd_soc_pcm_runtime structures similar to
> support for codec_dai.
>
> Inline with multiple codec DAI approach, add support to allocate,
> init, bind and probe multiple cpu_dai on init if driver specifies
> that. Also add support to loop over multiple cpu_dai during
> suspend and resume
>
> Reviewed-by: Charles Keepax <ckeepax at opensource.cirrus.com>
> Signed-off-by: Vinod Koul <vkoul at kernel.org>
> Signed-off-by: Shreyas NC <shreyas.nc at intel.com>
> ---
> include/sound/soc.h | 6 ++
> sound/soc/soc-core.c | 280 +++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 199 insertions(+), 87 deletions(-)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 600a7eb..09bddc4 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -898,6 +898,9 @@ struct snd_soc_dai_link {
> struct snd_soc_dai_link_component *codecs;
> unsigned int num_codecs;
>
> + struct snd_soc_dai_link_component *cpu_dai;
> + unsigned int num_cpu_dai;
> +
> /*
> * You MAY specify the link's platform/PCM/DMA driver, either by
> * device name, or by DT/OF node, but not both. Some forms of link
> @@ -1120,6 +1123,9 @@ struct snd_soc_pcm_runtime {
> struct snd_soc_dai **codec_dais;
> unsigned int num_codecs;
>
> + struct snd_soc_dai **cpu_dais;
> + unsigned int num_cpu_dai;
> +
This structure is becoming difficult to interpret:
struct snd_soc_dai *codec_dai;
struct snd_soc_dai *cpu_dai;
struct snd_soc_dai **codec_dais;
unsigned int num_codecs;
struct snd_soc_dai **cpu_dais;
unsigned int num_cpu_dai;
What is the intended usage of codec_dai vs. codec_dais and cpu_dai vs.
cpu_dais?
If this is only to handle the single codec_dai/single cpu_dai as an
exception, should we port the code to support multiple cpu_dais/multiple
codec_dais?
> struct delayed_work delayed_work;
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs_dpcm_root;
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 3d56f1f..9908c29 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -381,12 +381,21 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
> return NULL;
> }
>
> + rtd->cpu_dais = kzalloc(sizeof(struct snd_soc_dai *) *
> + dai_link->num_cpu_dai, GFP_KERNEL);
> + if (!rtd->cpu_dais) {
> + kfree(rtd->codec_dais);
> + kfree(rtd);
> + return NULL;
> + }
> +
> return rtd;
> }
>
> static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd)
> {
> kfree(rtd->codec_dais);
> + kfree(rtd->cpu_dais);
> snd_soc_rtdcom_del_all(rtd);
> kfree(rtd);
> }
> @@ -482,13 +491,17 @@ int snd_soc_suspend(struct device *dev)
> card->suspend_pre(card);
>
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *cpu_dai;
>
> if (rtd->dai_link->ignore_suspend)
> continue;
>
> - if (cpu_dai->driver->suspend && !cpu_dai->driver->bus_control)
> - cpu_dai->driver->suspend(cpu_dai);
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> + if (cpu_dai->driver->suspend &&
> + !cpu_dai->driver->bus_control)
> + cpu_dai->driver->suspend(cpu_dai);
> + }
> }
>
> /* close any waiting streams */
> @@ -552,16 +565,21 @@ int snd_soc_suspend(struct device *dev)
> }
>
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *cpu_dai;
>
> - if (rtd->dai_link->ignore_suspend)
> - continue;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> +
> + if (rtd->dai_link->ignore_suspend)
> + continue;
this test needs to be outside of the for loop? the 'continue' should
result in moving to the next element of the list, not to the next cpu_dai.
see below, this statement is indeed outside for the resume part.
>
> - if (cpu_dai->driver->suspend && cpu_dai->driver->bus_control)
> - cpu_dai->driver->suspend(cpu_dai);
> + if (cpu_dai->driver->suspend &&
> + cpu_dai->driver->bus_control)
> + cpu_dai->driver->suspend(cpu_dai);
>
> - /* deactivate pins to sleep state */
> - pinctrl_pm_select_sleep_state(cpu_dai->dev);
> + /* deactivate pins to sleep state */
> + pinctrl_pm_select_sleep_state(cpu_dai->dev);
> + }
> }
>
> if (card->suspend_post)
> @@ -596,13 +614,18 @@ static void soc_resume_deferred(struct work_struct *work)
>
> /* resume control bus DAIs */
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *cpu_dai;
>
> if (rtd->dai_link->ignore_suspend)
> continue;
here the test is outside of the forall(cpu_dais)
>
> - if (cpu_dai->driver->resume && cpu_dai->driver->bus_control)
> - cpu_dai->driver->resume(cpu_dai);
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> +
> + if (cpu_dai->driver->resume &&
> + cpu_dai->driver->bus_control)
> + cpu_dai->driver->resume(cpu_dai);
> + }
> }
>
> list_for_each_entry(component, &card->component_dev_list, card_list) {
> @@ -648,8 +671,13 @@ static void soc_resume_deferred(struct work_struct *work)
> if (rtd->dai_link->ignore_suspend)
> continue;
>
> - if (cpu_dai->driver->resume && !cpu_dai->driver->bus_control)
> - cpu_dai->driver->resume(cpu_dai);
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> +
> + if (cpu_dai->driver->resume &&
> + !cpu_dai->driver->bus_control)
> + cpu_dai->driver->resume(cpu_dai);
> + }
> }
>
> if (card->resume_post)
> @@ -671,6 +699,7 @@ int snd_soc_resume(struct device *dev)
> struct snd_soc_card *card = dev_get_drvdata(dev);
> bool bus_control = false;
> struct snd_soc_pcm_runtime *rtd;
> + int i;
>
> /* If the card is not initialized yet there is nothing to do */
> if (!card->instantiated)
> @@ -679,11 +708,16 @@ int snd_soc_resume(struct device *dev)
> /* activate pins from sleep state */
> list_for_each_entry(rtd, &card->rtd_list, list) {
> struct snd_soc_dai **codec_dais = rtd->codec_dais;
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai **cpu_dais = rtd->cpu_dais;
> + struct snd_soc_dai *cpu_dai;
> int j;
>
> - if (cpu_dai->active)
> - pinctrl_pm_select_default_state(cpu_dai->dev);
> + for (j = 0; j < rtd->num_cpu_dai; j++) {
> + cpu_dai = cpu_dais[j];
> +
> + if (cpu_dai->active)
> + pinctrl_pm_select_default_state(cpu_dai->dev);
> + }
>
> for (j = 0; j < rtd->num_codecs; j++) {
> struct snd_soc_dai *codec_dai = codec_dais[j];
> @@ -699,8 +733,11 @@ int snd_soc_resume(struct device *dev)
> * due to I/O costs and anti-pop so handle them out of line.
> */
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> - bus_control |= cpu_dai->driver->bus_control;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + struct snd_soc_dai *cpu_dai = rtd->cpu_dais[i];
> +
> + bus_control |= cpu_dai->driver->bus_control;
> + }
> }
> if (bus_control) {
> dev_dbg(dev, "ASoC: Resuming control bus master immediately\n");
> @@ -845,35 +882,47 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
> {
> struct snd_soc_pcm_runtime *rtd;
> struct snd_soc_dai_link_component *codecs = dai_link->codecs;
> - struct snd_soc_dai_link_component cpu_dai_component;
> + struct snd_soc_dai_link_component *cpu_dai_component;
> struct snd_soc_component *component;
> - struct snd_soc_dai **codec_dais;
> + struct snd_soc_dai **codec_dais, **cpu_dais;
> struct device_node *platform_of_node;
> const char *platform_name;
> int i;
>
> dev_dbg(card->dev, "ASoC: binding %s\n", dai_link->name);
>
> + cpu_dai_component = dai_link->cpu_dai;
> +
> if (soc_is_dai_link_bound(card, dai_link)) {
> dev_dbg(card->dev, "ASoC: dai link %s already bound\n",
> dai_link->name);
> return 0;
> }
>
> + if (daiif (rtd->cpu_dais[0]->driver->compress_new) {
> _link->dynamic && dai_link->num_cpu_dai > 1) {
> + dev_err(card->dev, "ASoC: Multi CPU DAI not supported for FE");
> + return -EINVAL;
> + }
> +
> rtd = soc_new_pcm_runtime(card, dai_link);
> if (!rtd)
> return -ENOMEM;
>
> - cpu_dai_component.name = dai_link->cpu_name;
> - cpu_dai_component.of_node = dai_link->cpu_of_node;
> - cpu_dai_component.dai_name = dai_link->cpu_dai_name;
> - rtd->cpu_dai = snd_soc_find_dai(&cpu_dai_component);
> - if (!rtd->cpu_dai) {
> - dev_info(card->dev, "ASoC: CPU DAI %s not registered\n",
> - dai_link->cpu_dai_name);
> - goto _err_defer;
> + rtd->num_cpu_dai = dai_link->num_cpu_dai;
> +
> + cpu_dais = rtd->cpu_dais;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dais[i] = snd_soc_find_dai(&cpu_dai_component[i]);
> + if (!cpu_dais[i]) {
> + dev_err(card->dev, "ASoC: CPU DAI %s not registered\n",
> + cpu_dai_component[i].dai_name);
> + goto _err_defer;
> + }
> + snd_soc_rtdcom_add(rtd, cpu_dais[i]->component);
> }
> - snd_soc_rtdcom_add(rtd, rtd->cpu_dai->component);
> +
> + /* Fill cpu_dai in the runtime data */
> + rtd->cpu_dai = cpu_dais[0];
>
> rtd->num_codecs = dai_link->num_codecs;
>
> @@ -971,7 +1020,8 @@ static void soc_remove_link_dais(struct snd_soc_card *card,
> for (i = 0; i < rtd->num_codecs; i++)
> soc_remove_dai(rtd->codec_dais[i], order);
>
> - soc_remove_dai(rtd->cpu_dai, order);
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + soc_remove_dai(rtd->cpu_dais[i], order);
> }
>
> static void soc_remove_link_components(struct snd_soc_card *card,
> @@ -1043,6 +1093,30 @@ static int snd_soc_init_multicodec(struct snd_soc_card *card,
> return 0;
> }
>
> +static int snd_soc_init_single_cpu_dai(struct snd_soc_card *card,
> + struct snd_soc_dai_link *dai_link)
> +{
> + if (dai_link->cpu_name || dai_link->cpu_of_node ||
> + dai_link->cpu_dai_name) {
> + dai_link->num_cpu_dai = 1;
> + dai_link->cpu_dai = devm_kzalloc(card->dev,
> + sizeof(struct snd_soc_dai_link_component),
> + GFP_KERNEL);
> +
> + if (!dai_link->cpu_dai)
> + return -ENOMEM;
> +
> + dai_link->cpu_dai[0].name = dai_link->cpu_name;
> + dai_link->cpu_dai[0].of_node = dai_link->cpu_of_node;
> + dai_link->cpu_dai[0].dai_name = dai_link->cpu_dai_name;
> + } else if (!dai_link->cpu_dai) {
> + dev_err(card->dev, "ASoC: DAI link has no DAIs\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int soc_init_dai_link(struct snd_soc_card *card,
> struct snd_soc_dai_link *link)
> {
> @@ -1054,6 +1128,12 @@ static int soc_init_dai_link(struct snd_soc_card *card,
> return ret;
> }
>
> + ret = snd_soc_init_single_cpu_dai(card, link);
> + if (ret) {
> + dev_err(card->dev, "ASoC: failed to init cpu\n");
> + return ret;
> + }
> +
> for (i = 0; i < link->num_codecs; i++) {
> /*
> * Codec must be specified by 1 of name or OF node,
> @@ -1089,24 +1169,28 @@ static int soc_init_dai_link(struct snd_soc_card *card,
> * can be left unspecified, and will be matched based on DAI
> * name alone..
> */
> - if (link->cpu_name && link->cpu_of_node) {
> - dev_err(card->dev,
> - "ASoC: Neither/both cpu name/of_node are set for %s\n",
> - link->name);
> - return -EINVAL;
> - }
> - /*
> - * At least one of CPU DAI name or CPU device name/node must be
> - * specified
> - */
> - if (!link->cpu_dai_name &&
> - !(link->cpu_name || link->cpu_of_node)) {
> - dev_err(card->dev,
> - "ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
> - link->name);
> - return -EINVAL;
> - }
>
> + for (i = 0; i < link->num_cpu_dai; i++) {
> + if (link->cpu_dai[i].name &&
> + link->cpu_dai[i].of_node) {
> + dev_err(card->dev,
> + "ASoC: Neither/both cpu name/of_node are set for %s\n",
> + link->cpu_dai[i].name);
> + return -EINVAL;
> + }
> +
> + /*
> + * At least one of CPU DAI name or CPU device name/node must be
> + * specified
> + */
> + if (!link->cpu_dai[i].dai_name &&
> + !(link->cpu_dai[i].name || link->cpu_dai[i].of_node)) {
This doesn't seem to be the logical translation of the initial condition
based on link->cpu_name and link->cpu_of_node?
> + dev_err(card->dev,
> + "ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n",
> + link->name);
> + return -EINVAL;
> + }
> + }
> return 0;
> }
>
> @@ -1426,6 +1510,9 @@ static int soc_link_dai_widgets(struct snd_soc_card *card,
> if (rtd->num_codecs > 1)
> dev_warn(card->dev, "ASoC: Multiple codecs not supported yet\n");
>
> + if (rtd->num_cpu_dai > 1)
> + dev_warn(card->dev, "ASoC: Multiple CPU DAIs not supported yet\n");
> +
> /* link the DAI widgets */
> sink = codec_dai->playback_widget;
> source = cpu_dai->capture_widget;
> @@ -1460,7 +1547,6 @@ 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;
> int i, ret;
>
> dev_dbg(card->dev, "ASoC: probe %s dai link %d late %d\n",
> @@ -1469,9 +1555,11 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
> /* set default power off timeout */
> rtd->pmdown_time = pmdown_time;
>
> - ret = soc_probe_dai(cpu_dai, order);
> - if (ret)
> - return ret;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + ret = soc_probe_dai(rtd->cpu_dais[i], order);
> + if (ret)
> + return ret;
> + }
>
> /* probe the CODEC DAI */
> for (i = 0; i < rtd->num_codecs; i++) {
> @@ -1507,9 +1595,13 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
> soc_dpcm_debugfs_add(rtd);
> #endif
>
> - if (cpu_dai->driver->compress_new) {
> + if (rtd->cpu_dais[0]->driver->compress_new) {
> + if (rtd->num_cpu_dai > 1)
> + dev_warn(card->dev,
> + "ASoC: multi-cpu compress dais not supported");
Not sure this is right, you could have a case where the compress dai is
not on the cpu_dai[0]?
> +
> /*create compress_device"*/
> - ret = cpu_dai->driver->compress_new(rtd, rtd->num);
> + ret = rtd->cpu_dais[0]->driver->compress_new(rtd, rtd->num);
> if (ret < 0) {
> dev_err(card->dev, "ASoC: can't create compress %s\n",
> dai_link->stream_name);
> @@ -1525,7 +1617,8 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
> dai_link->stream_name, ret);
> return ret;
> }
> - ret = soc_link_dai_pcm_new(&cpu_dai, 1, rtd);
> + ret = soc_link_dai_pcm_new(rtd->cpu_dais,
> + rtd->num_cpu_dai, rtd);
> if (ret < 0)
> return ret;
> ret = soc_link_dai_pcm_new(rtd->codec_dais,
> @@ -1644,7 +1737,7 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
> unsigned int dai_fmt)
> {
> struct snd_soc_dai **codec_dais = rtd->codec_dais;
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai **cpu_dais = rtd->cpu_dais;
> unsigned int i;
> int ret;
>
> @@ -1659,35 +1752,44 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
> }
> }
>
> - /* Flip the polarity for the "CPU" end of a CODEC<->CODEC link */
> /* the component which has non_legacy_dai_naming is Codec */
> - if (cpu_dai->component->driver->non_legacy_dai_naming) {
> - unsigned int inv_dai_fmt;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + struct snd_soc_dai *cpu_dai = cpu_dais[i];
> + unsigned int inv_dai_fmt, temp_dai_fmt;
>
> - inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_MASTER_MASK;
> - switch (dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> - case SND_SOC_DAIFMT_CBM_CFM:
> - inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
> - break;
> - case SND_SOC_DAIFMT_CBM_CFS:
> - inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
> - break;
> - case SND_SOC_DAIFMT_CBS_CFM:
> - inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFS;
> - break;
> - case SND_SOC_DAIFMT_CBS_CFS:
> - inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
> - break;
> - }
> + temp_dai_fmt = dai_fmt;
> + if (cpu_dai->component->driver->non_legacy_dai_naming) {
>
> - dai_fmt = inv_dai_fmt;
> - }
> + inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_MASTER_MASK;
>
> - ret = snd_soc_dai_set_fmt(cpu_dai, dai_fmt);
> - if (ret != 0 && ret != -ENOTSUPP) {
> - dev_warn(cpu_dai->dev,
> - "ASoC: Failed to set DAI format: %d\n", ret);
> - return ret;
> + switch (dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBM_CFM:
> + inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
> + break;
> +
> + case SND_SOC_DAIFMT_CBM_CFS:
> + inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
> + break;
> +
> + case SND_SOC_DAIFMT_CBS_CFM:
> + inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFS;
> + break;
> +
> + case SND_SOC_DAIFMT_CBS_CFS:
> + inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
> + break;
> +
> + }
> +
> + temp_dai_fmt = inv_dai_fmt;
> + }
> +
> + ret = snd_soc_dai_set_fmt(cpu_dai, temp_dai_fmt);
> + if (ret != 0 && ret != -ENOTSUPP) {
> + dev_warn(cpu_dai->dev,
> + "ASoC: Failed to set DAI format: %d\n", ret);
> + return ret;
> + }
> }
>
> return 0;
> @@ -2121,10 +2223,11 @@ int snd_soc_poweroff(struct device *dev)
>
> /* deactivate pins to sleep state */
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> int i;
>
> - pinctrl_pm_select_sleep_state(cpu_dai->dev);
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + pinctrl_pm_select_sleep_state(rtd->cpu_dais[i]->dev);
> +
> for (i = 0; i < rtd->num_codecs; i++) {
> struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
> pinctrl_pm_select_sleep_state(codec_dai->dev);
> @@ -2609,7 +2712,7 @@ int snd_soc_register_card(struct snd_soc_card *card)
>
> /* deactivate pins to sleep state */
> list_for_each_entry(rtd, &card->rtd_list, list) {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *cpu_dai;
> int j;
>
> for (j = 0; j < rtd->num_codecs; j++) {
> @@ -2618,8 +2721,11 @@ int snd_soc_register_card(struct snd_soc_card *card)
> pinctrl_pm_select_sleep_state(codec_dai->dev);
> }
>
> - if (!cpu_dai->active)
> - pinctrl_pm_select_sleep_state(cpu_dai->dev);
> + for (j = 0; j < rtd->num_cpu_dai; j++) {
> + cpu_dai = rtd->cpu_dais[j];
> + if (!cpu_dai->active)
> + pinctrl_pm_select_sleep_state(cpu_dai->dev);
> + }
> }
>
> return ret;
More information about the Alsa-devel
mailing list