| Message ID | 1526985623-2606-2-git-send-email-shreyas.nc@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@opensource.cirrus.com> > Signed-off-by: Vinod Koul <vkoul@kernel.org> > Signed-off-by: Shreyas NC <shreyas.nc@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;
> >@@ -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? rtd->cpu_dai is used in many drivers to get cpu_dai from soc_pcm_runtime. Similarly cpu_dais is addded to get multiple cpu_dais from runtime. TBH, I have shadowed the codec_dais implementation for sake of convenience and being conformal :) > 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? > As mentioned in [1] (in cover letter), there are discussions to unify cpu and codec dais and this is the first step towards that. So, yes, eventually we should port the code as you have suggested :) > > /* 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. Yes, will fix this. Thanks :) > >+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? > pasting the code added from the function above to show the mapping b/w name, of_node and dai_name: 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; and the original condition was: if (!link->cpu_dai_name && !(link->cpu_name || link->cpu_of_node)) { So, it does look correct to me, unless I am missing something obvious :( > >+ 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]? I am not able to comprehend that we can have multi CPU DAIS in a DAI Link with one of the DAI being compress and the other being a PCM DAI. Any such systems that you can think of ? Thanks for the review! --Shreyas
On 05/24/2018 12:34 AM, Shreyas NC wrote: >>> @@ -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? > rtd->cpu_dai is used in many drivers to get cpu_dai from soc_pcm_runtime. > Similarly cpu_dais is addded to get multiple cpu_dais from runtime. > > TBH, I have shadowed the codec_dais implementation for sake of > convenience and being conformal :) > >> 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? >> > As mentioned in [1] (in cover letter), there are discussions to unify cpu and > codec dais and this is the first step towards that. So, yes, eventually we > should port the code as you have suggested :) ok, maybe add a comment in the commit message then? > >>> /* 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. > Yes, will fix this. Thanks :) ok > > - /* > - * 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? >> > pasting the code added from the function above to show the mapping b/w name, > of_node and dai_name: > > 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; > > and the original condition was: > if (!link->cpu_dai_name && > !(link->cpu_name || link->cpu_of_node)) { > > So, it does look correct to me, unless I am missing something obvious :( The original code I was referring to is this: /* * CPU device may be specified by either name or OF node, but * 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; } return 0; The new code is this: 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)) { dev_err(card->dev, "ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n", link->name); return -EINVAL; } } Yes, it's equivalent for i==0, but it's not clear to me for non-zero indices. I don't get how/where link->cpu_dai[i].name and link->cpu_dai[i].of_node are initialized for i>0. >>> - 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]? > I am not able to comprehend that we can have multi CPU DAIS in a DAI Link > with one of the DAI being compress and the other being a PCM DAI. > > Any such systems that you can think of ? Yes I agree, my point was that you test only for the first cpu_dai (index 0), but if rtd->cpu_dais[1]->drivers->compress_new is non NULL then it's also wrong. You should test for all cpu_dais to make sure they are all PCM.
On Thu, May 24, 2018 at 03:45:40PM -0500, Pierre-Louis Bossart wrote: > > > On 05/24/2018 12:34 AM, Shreyas NC wrote: > >>>@@ -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? > >rtd->cpu_dai is used in many drivers to get cpu_dai from soc_pcm_runtime. > >Similarly cpu_dais is addded to get multiple cpu_dais from runtime. > > > >TBH, I have shadowed the codec_dais implementation for sake of > >convenience and being conformal :) > > > >>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? > >> > >As mentioned in [1] (in cover letter), there are discussions to unify cpu and > >codec dais and this is the first step towards that. So, yes, eventually we > >should port the code as you have suggested :) > ok, maybe add a comment in the commit message then? Ok > > > >- /* > >- * 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? > >> > >pasting the code added from the function above to show the mapping b/w name, > >of_node and dai_name: > > > > 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; > > > >and the original condition was: > > if (!link->cpu_dai_name && > > !(link->cpu_name || link->cpu_of_node)) { > > > >So, it does look correct to me, unless I am missing something obvious :( > The original code I was referring to is this: > > /* > * CPU device may be specified by either name or OF node, but > * 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; > } > > return 0; > > The new code is this: > > 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)) { > dev_err(card->dev, > "ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for > %s\n", > link->name); > return -EINVAL; > } > } > > Yes, it's equivalent for i==0, but it's not clear to me for non-zero > indices. I don't get how/where > link->cpu_dai[i].name and link->cpu_dai[i].of_node are initialized for i>0. > Aaah ok, I understand your question now :) That is added in the dai_link description in the machine driver. Like this: static struct snd_soc_dai_link_component test_multi_cpu_component[] = { { /* Left */ .name = "int-sdw.1", .dai_name = "SDW1 Pin0", }, { /* Right */ .name = "int-sdw.2", .dai_name = "SDW2 Pin0", }, }; struct snd_soc_dai_link test_dailink[] = { { .name = "SDW0-Codec", .cpu_dai = test_multi_cpu_component, .num_cpu_dai = ARRAY_SIZE(sdw_multi_cpu_comp), ... } } > >>>- 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]? > >I am not able to comprehend that we can have multi CPU DAIS in a DAI Link > >with one of the DAI being compress and the other being a PCM DAI. > > > >Any such systems that you can think of ? > Yes I agree, my point was that you test only for the first cpu_dai (index > 0), but if rtd->cpu_dais[1]->drivers->compress_new is non NULL then it's > also wrong. You should test for all cpu_dais to make sure they are all PCM. > Ok --Shreyas
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; + 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; - 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; - 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 (dai_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)) { + 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"); + /*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;