* [PATCH v5 0/3] ASoC: Add Multi CPU DAI support
@ 2018-05-22 10:40 Shreyas NC
2018-05-22 10:40 ` [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs Shreyas NC
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Shreyas NC @ 2018-05-22 10:40 UTC (permalink / raw)
To: alsa-devel; +Cc: liam.r.girdwood, patches.audio, broonie, ckeepax, Shreyas NC
As discussed in [1], ASoC core supports multi codec DAIs
on a DAI link. However it does not do so for CPU DAIs.
So, add support for multi CPU DAIs on a DAI Link by adding
multi CPU DAI in Card instantiation, suspend and resume
functions, PCM ops, stream handling functions and DAPM.
[1]: https://www.spinics.net/lists/alsa-devel/msg71369.html
changes in v5:
- Addressed review comments from Charles in calculating delays,
soc_pcm_init_runtime_hw() and other nits
- rebased on latest for-next branch
Shreyas NC (3):
ASoC: Add initial support for multiple CPU DAIs
ASoC: Add multiple CPU DAI support for PCM ops
ASoC: Add multiple CPU DAI support in DAPM
include/sound/soc.h | 6 +
sound/soc/soc-core.c | 280 ++++++++++++++++++++---------
sound/soc/soc-dapm.c | 71 +++++---
sound/soc/soc-pcm.c | 489 ++++++++++++++++++++++++++++++++++-----------------
4 files changed, 574 insertions(+), 272 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs
2018-05-22 10:40 [PATCH v5 0/3] ASoC: Add Multi CPU DAI support Shreyas NC
@ 2018-05-22 10:40 ` Shreyas NC
2018-05-23 21:38 ` Pierre-Louis Bossart
2018-05-22 10:40 ` [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops Shreyas NC
2018-05-22 10:40 ` [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM Shreyas NC
2 siblings, 1 reply; 12+ messages in thread
From: Shreyas NC @ 2018-05-22 10:40 UTC (permalink / raw)
To: alsa-devel
Cc: ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie, Shreyas NC
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;
+
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;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops
2018-05-22 10:40 [PATCH v5 0/3] ASoC: Add Multi CPU DAI support Shreyas NC
2018-05-22 10:40 ` [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs Shreyas NC
@ 2018-05-22 10:40 ` Shreyas NC
2018-05-23 9:22 ` Charles Keepax
2018-05-24 21:29 ` Pierre-Louis Bossart
2018-05-22 10:40 ` [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM Shreyas NC
2 siblings, 2 replies; 12+ messages in thread
From: Shreyas NC @ 2018-05-22 10:40 UTC (permalink / raw)
To: alsa-devel
Cc: ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie, Shreyas NC
Add support in PCM operations to invoke multiple cpu dais as we do
for multiple codec dais. Also the symmetry calculations are updated to
reflect multiple cpu dais.
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Shreyas NC <shreyas.nc@intel.com>
---
sound/soc/soc-pcm.c | 489 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 329 insertions(+), 160 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 2df4719..f9fcda5 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -64,23 +64,27 @@ static bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream)
*/
void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream)
{
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
int i;
lockdep_assert_held(&rtd->pcm_mutex);
if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
- cpu_dai->playback_active++;
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ rtd->cpu_dais[i]->playback_active++;
for (i = 0; i < rtd->num_codecs; i++)
rtd->codec_dais[i]->playback_active++;
} else {
- cpu_dai->capture_active++;
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ rtd->cpu_dais[i]->capture_active++;
for (i = 0; i < rtd->num_codecs; i++)
rtd->codec_dais[i]->capture_active++;
}
- cpu_dai->active++;
- cpu_dai->component->active++;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ rtd->cpu_dais[i]->component->active++;
+ rtd->cpu_dais[i]->active++;
+ }
+
for (i = 0; i < rtd->num_codecs; i++) {
rtd->codec_dais[i]->active++;
rtd->codec_dais[i]->component->active++;
@@ -99,23 +103,27 @@ void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream)
*/
void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream)
{
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
int i;
lockdep_assert_held(&rtd->pcm_mutex);
if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
- cpu_dai->playback_active--;
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ rtd->cpu_dais[i]->playback_active--;
for (i = 0; i < rtd->num_codecs; i++)
rtd->codec_dais[i]->playback_active--;
} else {
- cpu_dai->capture_active--;
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ rtd->cpu_dais[i]->capture_active--;
for (i = 0; i < rtd->num_codecs; i++)
rtd->codec_dais[i]->capture_active--;
}
- cpu_dai->active--;
- cpu_dai->component->active--;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ rtd->cpu_dais[i]->component->active--;
+ rtd->cpu_dais[i]->active--;
+ }
+
for (i = 0; i < rtd->num_codecs; i++) {
rtd->codec_dais[i]->component->active--;
rtd->codec_dais[i]->active--;
@@ -258,7 +266,6 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
unsigned int rate, channels, sample_bits, symmetry, i;
rate = params_rate(params);
@@ -266,41 +273,54 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
sample_bits = snd_pcm_format_physical_width(params_format(params));
/* reject unmatched parameters when applying symmetry */
- symmetry = cpu_dai->driver->symmetric_rates ||
- rtd->dai_link->symmetric_rates;
+ symmetry = rtd->dai_link->symmetric_rates;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ symmetry |= rtd->cpu_dais[i]->driver->symmetric_rates;
for (i = 0; i < rtd->num_codecs; i++)
symmetry |= rtd->codec_dais[i]->driver->symmetric_rates;
- if (symmetry && cpu_dai->rate && cpu_dai->rate != rate) {
- dev_err(rtd->dev, "ASoC: unmatched rate symmetry: %d - %d\n",
- cpu_dai->rate, rate);
- return -EINVAL;
- }
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ if (symmetry && rtd->cpu_dais[i]->rate &&
+ rtd->cpu_dais[i]->rate != rate) {
+ dev_err(rtd->dev, "ASoC: unmatched rate symmetry: %d - %d\n",
+ rtd->cpu_dais[i]->rate, rate);
+ return -EINVAL;
+ }
- symmetry = cpu_dai->driver->symmetric_channels ||
- rtd->dai_link->symmetric_channels;
+ symmetry = rtd->dai_link->symmetric_channels;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ symmetry |= rtd->cpu_dais[i]->driver->symmetric_channels;
for (i = 0; i < rtd->num_codecs; i++)
symmetry |= rtd->codec_dais[i]->driver->symmetric_channels;
- if (symmetry && cpu_dai->channels && cpu_dai->channels != channels) {
- dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
- cpu_dai->channels, channels);
- return -EINVAL;
- }
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ if (symmetry && rtd->cpu_dais[i]->channels &&
+ rtd->cpu_dais[i]->channels != channels) {
+ dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
+ rtd->cpu_dais[i]->channels, channels);
+ return -EINVAL;
+ }
- symmetry = cpu_dai->driver->symmetric_samplebits ||
- rtd->dai_link->symmetric_samplebits;
+ symmetry = rtd->dai_link->symmetric_samplebits;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ symmetry |= rtd->cpu_dais[i]->driver->symmetric_samplebits;
for (i = 0; i < rtd->num_codecs; i++)
symmetry |= rtd->codec_dais[i]->driver->symmetric_samplebits;
- if (symmetry && cpu_dai->sample_bits && cpu_dai->sample_bits != sample_bits) {
- dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
- cpu_dai->sample_bits, sample_bits);
- return -EINVAL;
- }
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ if (symmetry && rtd->cpu_dais[i]->sample_bits &&
+ rtd->cpu_dais[i]->sample_bits != sample_bits) {
+ dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
+ rtd->cpu_dais[i]->sample_bits,
+ sample_bits);
+ return -EINVAL;
+ }
return 0;
}
@@ -308,13 +328,18 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai_driver *cpu_driver = rtd->cpu_dai->driver;
struct snd_soc_dai_link *link = rtd->dai_link;
unsigned int symmetry, i;
- symmetry = cpu_driver->symmetric_rates || link->symmetric_rates ||
- cpu_driver->symmetric_channels || link->symmetric_channels ||
- cpu_driver->symmetric_samplebits || link->symmetric_samplebits;
+ symmetry = link->symmetric_rates || link->symmetric_channels ||
+ link->symmetric_samplebits;
+
+ /* Apply symmetery for multiple cpu dais */
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ symmetry = symmetry ||
+ rtd->cpu_dais[i]->driver->symmetric_rates ||
+ rtd->cpu_dais[i]->driver->symmetric_channels ||
+ rtd->cpu_dais[i]->driver->symmetric_samplebits;
for (i = 0; i < rtd->num_codecs; i++)
symmetry = symmetry ||
@@ -342,10 +367,10 @@ static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits)
static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
int i;
- unsigned int bits = 0, cpu_bits;
+ unsigned int bits = 0, cpu_bits = 0;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
for (i = 0; i < rtd->num_codecs; i++) {
@@ -356,7 +381,16 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
}
bits = max(codec_dai->driver->playback.sig_bits, bits);
}
- cpu_bits = cpu_dai->driver->playback.sig_bits;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->playback.sig_bits == 0) {
+ cpu_bits = 0;
+ break;
+ }
+
+ cpu_bits = max(cpu_dai->driver->playback.sig_bits, cpu_bits);
+ }
} else {
for (i = 0; i < rtd->num_codecs; i++) {
codec_dai = rtd->codec_dais[i];
@@ -366,7 +400,15 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
}
bits = max(codec_dai->driver->capture.sig_bits, bits);
}
- cpu_bits = cpu_dai->driver->capture.sig_bits;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->capture.sig_bits == 0) {
+ cpu_bits = 0;
+ break;
+ }
+ cpu_bits = max(cpu_dai->driver->capture.sig_bits, bits);
+ }
}
soc_pcm_set_msb(substream, bits);
@@ -378,21 +420,18 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_pcm_hardware *hw = &runtime->hw;
struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai_driver *cpu_dai_drv = rtd->cpu_dai->driver;
+ struct snd_soc_dai_driver *cpu_dai_drv;
struct snd_soc_dai_driver *codec_dai_drv;
struct snd_soc_pcm_stream *codec_stream;
struct snd_soc_pcm_stream *cpu_stream;
unsigned int chan_min = 0, chan_max = UINT_MAX;
+ unsigned int cpu_chan_min = 0, cpu_chan_max = UINT_MAX;
unsigned int rate_min = 0, rate_max = UINT_MAX;
- unsigned int rates = UINT_MAX;
+ unsigned int cpu_rate_min = 0, cpu_rate_max = UINT_MAX;
+ unsigned int rates = UINT_MAX, cpu_rates = UINT_MAX;
u64 formats = ULLONG_MAX;
int i;
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- cpu_stream = &cpu_dai_drv->playback;
- else
- cpu_stream = &cpu_dai_drv->capture;
-
/* first calculate min/max only for CODECs in the DAI link */
for (i = 0; i < rtd->num_codecs; i++) {
@@ -422,30 +461,55 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
rates = snd_pcm_rate_mask_intersect(codec_stream->rates, rates);
}
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai_drv = rtd->cpu_dais[i]->driver;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ cpu_stream = &cpu_dai_drv->playback;
+ else
+ cpu_stream = &cpu_dai_drv->capture;
+
+ cpu_chan_min = max(cpu_chan_min,
+ cpu_stream->channels_min);
+ cpu_chan_max = min(cpu_chan_max,
+ cpu_stream->channels_max);
+
+ if (hw->formats)
+ hw->formats &= cpu_stream->formats;
+ else
+ hw->formats = cpu_stream->formats;
+
+ cpu_rates = snd_pcm_rate_mask_intersect(cpu_rates,
+ cpu_stream->rates);
+
+ cpu_rate_min = max(cpu_rate_min, cpu_stream->rate_min);
+ cpu_rate_max = min_not_zero(cpu_rate_max, cpu_stream->rate_max);
+ }
+
/*
- * chan min/max cannot be enforced if there are multiple CODEC DAIs
- * connected to a single CPU DAI, use CPU DAI's directly and let
- * channel allocation be fixed up later
+ * chan min/max cannot be enforced if there are multiple
+ * CODEC DAIs connected to CPU DAI(s), use CPU DAI's
+ * directly and let channel allocation be fixed up later
*/
if (rtd->num_codecs > 1) {
- chan_min = cpu_stream->channels_min;
- chan_max = cpu_stream->channels_max;
+ chan_min = cpu_chan_min;
+ chan_max = cpu_chan_max;
}
- hw->channels_min = max(chan_min, cpu_stream->channels_min);
- hw->channels_max = min(chan_max, cpu_stream->channels_max);
- if (hw->formats)
- hw->formats &= formats & cpu_stream->formats;
- else
- hw->formats = formats & cpu_stream->formats;
- hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
+ hw->channels_min = max(cpu_chan_min, chan_min);
+
+ hw->channels_max = min(cpu_chan_max, chan_max);
+
+ hw->rate_min = max(cpu_rate_min, rate_min);
+ hw->rate_max = min_not_zero(cpu_rate_max, rate_max);
+ hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_rates);
snd_pcm_limit_hw_rates(runtime);
- hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
- hw->rate_min = max(hw->rate_min, rate_min);
- hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max);
- hw->rate_max = min_not_zero(hw->rate_max, rate_max);
+ if (hw->formats)
+ hw->formats &= formats;
+ else
+ hw->formats = formats;
}
/*
@@ -459,12 +523,15 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
const char *codec_dai_name = "multicodec";
- int i, ret = 0, __ret;
+ const char *cpu_dai_name = "multicpu";
+ int i, ret = 0, __ret, j;
+
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ pinctrl_pm_select_default_state(rtd->cpu_dais[i]->dev);
- pinctrl_pm_select_default_state(cpu_dai->dev);
for (i = 0; i < rtd->num_codecs; i++)
pinctrl_pm_select_default_state(rtd->codec_dais[i]->dev);
@@ -477,12 +544,15 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
/* startup the audio subsystem */
- if (cpu_dai->driver->ops->startup) {
- ret = cpu_dai->driver->ops->startup(substream, cpu_dai);
- if (ret < 0) {
- dev_err(cpu_dai->dev, "ASoC: can't open interface"
- " %s: %d\n", cpu_dai->name, ret);
- goto out;
+ for (j = 0; j < rtd->num_cpu_dai; j++) {
+ cpu_dai = rtd->cpu_dais[j];
+ if (cpu_dai->driver->ops->startup) {
+ ret = cpu_dai->driver->ops->startup(substream, cpu_dai);
+ if (ret < 0) {
+ dev_err(cpu_dai->dev, "ASoC: can't open interface %s: %d\n",
+ cpu_dai->name, ret);
+ goto interface_err;
+ }
}
}
@@ -543,34 +613,40 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
if (rtd->num_codecs == 1)
codec_dai_name = rtd->codec_dai->name;
+ if (rtd->num_cpu_dai == 1)
+ cpu_dai_name = rtd->cpu_dai->name;
+
if (soc_pcm_has_symmetry(substream))
runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
ret = -EINVAL;
if (!runtime->hw.rates) {
printk(KERN_ERR "ASoC: %s <-> %s No matching rates\n",
- codec_dai_name, cpu_dai->name);
+ codec_dai_name, cpu_dai_name);
goto config_err;
}
if (!runtime->hw.formats) {
printk(KERN_ERR "ASoC: %s <-> %s No matching formats\n",
- codec_dai_name, cpu_dai->name);
+ codec_dai_name, cpu_dai_name);
goto config_err;
}
if (!runtime->hw.channels_min || !runtime->hw.channels_max ||
runtime->hw.channels_min > runtime->hw.channels_max) {
printk(KERN_ERR "ASoC: %s <-> %s No matching channels\n",
- codec_dai_name, cpu_dai->name);
+ codec_dai_name, cpu_dai_name);
goto config_err;
}
soc_pcm_apply_msb(substream);
/* Symmetry only applies if we've already got an active stream. */
- if (cpu_dai->active) {
- ret = soc_pcm_apply_symmetry(substream, cpu_dai);
- if (ret != 0)
- goto config_err;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ if (rtd->cpu_dais[i]->active) {
+ ret = soc_pcm_apply_symmetry(substream,
+ rtd->cpu_dais[i]);
+ if (ret != 0)
+ goto config_err;
+ }
}
for (i = 0; i < rtd->num_codecs; i++) {
@@ -583,7 +659,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
}
pr_debug("ASoC: %s <-> %s info:\n",
- codec_dai_name, cpu_dai->name);
+ codec_dai_name, cpu_dai_name);
pr_debug("ASoC: rate mask 0x%x\n", runtime->hw.rates);
pr_debug("ASoC: min ch %d max ch %d\n", runtime->hw.channels_min,
runtime->hw.channels_max);
@@ -622,9 +698,15 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
component->driver->ops->close(substream);
}
- if (cpu_dai->driver->ops->shutdown)
- cpu_dai->driver->ops->shutdown(substream, cpu_dai);
-out:
+ j = rtd->num_cpu_dai;
+
+interface_err:
+ while (--j >= 0) {
+ cpu_dai = rtd->cpu_dais[j];
+ if (cpu_dai->driver->ops->shutdown)
+ cpu_dai->driver->ops->shutdown(substream, cpu_dai);
+ }
+
mutex_unlock(&rtd->pcm_mutex);
for_each_rtdcom(rtd, rtdcom) {
@@ -638,8 +720,11 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
if (!rtd->codec_dais[i]->active)
pinctrl_pm_select_sleep_state(rtd->codec_dais[i]->dev);
}
- if (!cpu_dai->active)
- pinctrl_pm_select_sleep_state(cpu_dai->dev);
+
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ if (!rtd->cpu_dais[i]->active)
+ pinctrl_pm_select_sleep_state(rtd->cpu_dais[i]->dev);
+ }
return ret;
}
@@ -682,7 +767,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
int i;
@@ -691,8 +776,11 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
snd_soc_runtime_deactivate(rtd, substream->stream);
/* clear the corresponding DAIs rate when inactive */
- if (!cpu_dai->active)
- cpu_dai->rate = 0;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (!cpu_dai->active)
+ cpu_dai->rate = 0;
+ }
for (i = 0; i < rtd->num_codecs; i++) {
codec_dai = rtd->codec_dais[i];
@@ -700,10 +788,16 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
codec_dai->rate = 0;
}
- snd_soc_dai_digital_mute(cpu_dai, 1, substream->stream);
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ snd_soc_dai_digital_mute(cpu_dai, 1, substream->stream);
+ }
- if (cpu_dai->driver->ops->shutdown)
- cpu_dai->driver->ops->shutdown(substream, cpu_dai);
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops->shutdown)
+ cpu_dai->driver->ops->shutdown(substream, cpu_dai);
+ }
for (i = 0; i < rtd->num_codecs; i++) {
codec_dai = rtd->codec_dais[i];
@@ -756,8 +850,11 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
if (!rtd->codec_dais[i]->active)
pinctrl_pm_select_sleep_state(rtd->codec_dais[i]->dev);
}
- if (!cpu_dai->active)
- pinctrl_pm_select_sleep_state(cpu_dai->dev);
+
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ if (!rtd->cpu_dais[i]->active)
+ pinctrl_pm_select_sleep_state(rtd->cpu_dais[i]->dev);
+ }
return 0;
}
@@ -772,7 +869,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
int i, ret = 0;
@@ -816,12 +913,16 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
}
}
- if (cpu_dai->driver->ops->prepare) {
- ret = cpu_dai->driver->ops->prepare(substream, cpu_dai);
- if (ret < 0) {
- dev_err(cpu_dai->dev,
- "ASoC: cpu DAI prepare error: %d\n", ret);
- goto out;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops->prepare) {
+ ret = cpu_dai->driver->ops->prepare(substream, cpu_dai);
+ if (ret < 0) {
+ dev_err(cpu_dai->dev,
+ "ASoC: cpu DAI prepare error: %d\n",
+ ret);
+ goto out;
+ }
}
}
@@ -838,7 +939,10 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
for (i = 0; i < rtd->num_codecs; i++)
snd_soc_dai_digital_mute(rtd->codec_dais[i], 0,
substream->stream);
- snd_soc_dai_digital_mute(cpu_dai, 0, substream->stream);
+
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ snd_soc_dai_digital_mute(rtd->cpu_dais[i], 0,
+ substream->stream);
out:
mutex_unlock(&rtd->pcm_mutex);
@@ -885,8 +989,8 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- int i, ret = 0, __ret;
+ struct snd_soc_dai *cpu_dai;
+ int i, ret = 0, __ret, j;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
if (rtd->dai_link->ops->hw_params) {
@@ -940,9 +1044,12 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
params_format(&codec_params));
}
- ret = soc_dai_hw_params(substream, params, cpu_dai);
- if (ret < 0)
- goto interface_err;
+ for (j = 0; j < rtd->num_cpu_dai; j++) {
+ cpu_dai = rtd->cpu_dais[j];
+ ret = soc_dai_hw_params(substream, params, cpu_dai);
+ if (ret < 0)
+ goto interface_err;
+ }
ret = 0;
for_each_rtdcom(rtd, rtdcom) {
@@ -963,11 +1070,14 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
if (ret < 0)
goto component_err;
- /* store the parameters for each DAIs */
- cpu_dai->rate = params_rate(params);
- cpu_dai->channels = params_channels(params);
- cpu_dai->sample_bits =
- snd_pcm_format_physical_width(params_format(params));
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ /* store the parameters for each DAIs */
+ cpu_dai = rtd->cpu_dais[i];
+ cpu_dai->rate = params_rate(params);
+ cpu_dai->channels = params_channels(params);
+ cpu_dai->sample_bits =
+ snd_pcm_format_physical_width(params_format(params));
+ }
ret = soc_pcm_params_symmetry(substream, params);
if (ret)
@@ -987,10 +1097,16 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
component->driver->ops->hw_free(substream);
}
- if (cpu_dai->driver->ops->hw_free)
- cpu_dai->driver->ops->hw_free(substream, cpu_dai);
+ j = rtd->num_cpu_dai;
interface_err:
+ while (--j >= 0) {
+ cpu_dai = rtd->cpu_dais[j];
+
+ if (cpu_dai->driver->ops && cpu_dai->driver->ops->hw_free)
+ cpu_dai->driver->ops->hw_free(substream, cpu_dai);
+ }
+
i = rtd->num_codecs;
codec_err:
@@ -1016,7 +1132,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
int i;
@@ -1024,10 +1140,13 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
/* clear the corresponding DAIs parameters when going to be inactive */
- if (cpu_dai->active == 1) {
- cpu_dai->rate = 0;
- cpu_dai->channels = 0;
- cpu_dai->sample_bits = 0;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->active == 1) {
+ cpu_dai->rate = 0;
+ cpu_dai->channels = 0;
+ cpu_dai->sample_bits = 0;
+ }
}
for (i = 0; i < rtd->num_codecs; i++) {
@@ -1069,8 +1188,11 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
codec_dai->driver->ops->hw_free(substream, codec_dai);
}
- if (cpu_dai->driver->ops->hw_free)
- cpu_dai->driver->ops->hw_free(substream, cpu_dai);
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops && cpu_dai->driver->ops->hw_free)
+ cpu_dai->driver->ops->hw_free(substream, cpu_dai);
+ }
mutex_unlock(&rtd->pcm_mutex);
return 0;
@@ -1081,7 +1203,7 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
int i, ret;
@@ -1107,10 +1229,14 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
return ret;
}
- if (cpu_dai->driver->ops->trigger) {
- ret = cpu_dai->driver->ops->trigger(substream, cmd, cpu_dai);
- if (ret < 0)
- return ret;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops->trigger) {
+ ret = cpu_dai->driver->ops->trigger(substream,
+ cmd, cpu_dai);
+ if (ret < 0)
+ return ret;
+ }
}
if (rtd->dai_link->ops->trigger) {
@@ -1126,7 +1252,7 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
int cmd)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
int i, ret;
@@ -1140,10 +1266,14 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
}
}
- if (cpu_dai->driver->ops->bespoke_trigger) {
- ret = cpu_dai->driver->ops->bespoke_trigger(substream, cmd, cpu_dai);
- if (ret < 0)
- return ret;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops->bespoke_trigger) {
+ ret = cpu_dai->driver->ops->bespoke_trigger(substream,
+ cmd, cpu_dai);
+ if (ret < 0)
+ return ret;
+ }
}
return 0;
}
@@ -1157,12 +1287,13 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_uframes_t offset = 0;
snd_pcm_sframes_t delay = 0;
snd_pcm_sframes_t codec_delay = 0;
+ snd_pcm_sframes_t cpu_delay = 0;
int i;
for_each_rtdcom(rtd, rtdcom) {
@@ -1177,8 +1308,15 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
break;
}
- if (cpu_dai->driver->ops->delay)
- delay += cpu_dai->driver->ops->delay(substream, cpu_dai);
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->ops->delay)
+ cpu_delay = max(cpu_delay,
+ cpu_dai->driver->ops->delay(substream,
+ cpu_dai));
+ }
+
+ delay += cpu_delay;
for (i = 0; i < rtd->num_codecs; i++) {
codec_dai = rtd->codec_dais[i];
@@ -1301,12 +1439,16 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card,
if (!be->dai_link->no_pcm)
continue;
- dev_dbg(card->dev, "ASoC: try BE : %s\n",
- be->cpu_dai->playback_widget ?
- be->cpu_dai->playback_widget->name : "(not set)");
+ for (i = 0; i < be->num_cpu_dai; i++) {
+ struct snd_soc_dai *cpu_dai = be->cpu_dais[i];
+
+ dev_dbg(card->dev, "ASoC: try BE : %s\n",
+ cpu_dai->playback_widget ?
+ cpu_dai->playback_widget->name : "(not set)");
- if (be->cpu_dai->playback_widget == widget)
- return be;
+ if (cpu_dai->playback_widget == widget)
+ return be;
+ }
for (i = 0; i < be->num_codecs; i++) {
struct snd_soc_dai *dai = be->codec_dais[i];
@@ -1321,12 +1463,16 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card,
if (!be->dai_link->no_pcm)
continue;
- dev_dbg(card->dev, "ASoC: try BE %s\n",
- be->cpu_dai->capture_widget ?
- be->cpu_dai->capture_widget->name : "(not set)");
+ for (i = 0; i < be->num_cpu_dai; i++) {
+ struct snd_soc_dai *cpu_dai = be->cpu_dais[i];
- if (be->cpu_dai->capture_widget == widget)
- return be;
+ dev_dbg(card->dev, "ASoC: try BE %s\n",
+ cpu_dai->capture_widget ?
+ cpu_dai->capture_widget->name : "(not set)");
+
+ if (cpu_dai->capture_widget == widget)
+ return be;
+ }
for (i = 0; i < be->num_codecs; i++) {
struct snd_soc_dai *dai = be->codec_dais[i];
@@ -1376,8 +1522,12 @@ static bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget,
if (!rtd->dai_link->no_pcm)
continue;
- if (rtd->cpu_dai->playback_widget == widget)
- return true;
+ for (i = 0; i < rtd->num_cpu_dai; ++i) {
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dais[i];
+
+ if (cpu_dai->playback_widget == widget)
+ return true;
+ }
for (i = 0; i < rtd->num_codecs; ++i) {
struct snd_soc_dai *dai = rtd->codec_dais[i];
@@ -1390,8 +1540,12 @@ static bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget,
if (!rtd->dai_link->no_pcm)
continue;
- if (rtd->cpu_dai->capture_widget == widget)
- return true;
+ for (i = 0; i < rtd->num_cpu_dai; ++i) {
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dais[i];
+
+ if (cpu_dai->capture_widget == widget)
+ return true;
+ }
for (i = 0; i < rtd->num_codecs; ++i) {
struct snd_soc_dai *dai = rtd->codec_dais[i];
@@ -1433,11 +1587,15 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream,
unsigned int i;
/* is there a valid CPU DAI widget for this BE */
- widget = dai_get_widget(dpcm->be->cpu_dai, stream);
+ for (i = 0; i < dpcm->be->num_cpu_dai; i++) {
+ struct snd_soc_dai *dai = dpcm->be->cpu_dais[i];
- /* prune the BE if it's no longer in our active list */
- if (widget && widget_in_list(list, widget))
- continue;
+ widget = dai_get_widget(dai, stream);
+
+ /* prune the BE if it's no longer in our active list */
+ if (widget && widget_in_list(list, widget))
+ continue;
+ }
/* is there a valid CODEC DAI widget for this BE */
for (i = 0; i < dpcm->be->num_codecs; i++) {
@@ -1778,10 +1936,13 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
be_substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
/* Symmetry only applies if we've got an active stream. */
- if (rtd->cpu_dai->active) {
- err = soc_pcm_apply_symmetry(be_substream, rtd->cpu_dai);
- if (err < 0)
- return err;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ if (rtd->cpu_dais[i]->active) {
+ err = soc_pcm_apply_symmetry(be_substream,
+ rtd->cpu_dais[i]);
+ if (err < 0)
+ return err;
+ }
}
for (i = 0; i < rtd->num_codecs; i++) {
@@ -2881,13 +3042,13 @@ static int soc_rtdcom_mmap(struct snd_pcm_substream *substream,
int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
{
struct snd_soc_dai *codec_dai;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *cpu_dai;
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
struct snd_pcm *pcm;
char new_name[64];
int ret = 0, playback = 0, capture = 0;
- int i;
+ int i, cpu_capture = 0, cpu_playback = 0;
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
playback = rtd->dai_link->dpcm_playback;
@@ -2901,8 +3062,16 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
capture = 1;
}
- capture = capture && cpu_dai->driver->capture.channels_min;
- playback = playback && cpu_dai->driver->playback.channels_min;
+ for (i = 0; i < rtd->num_cpu_dai; i++) {
+ cpu_dai = rtd->cpu_dais[i];
+ if (cpu_dai->driver->playback.channels_min)
+ cpu_playback = 1;
+ if (cpu_dai->driver->capture.channels_min)
+ cpu_capture = 1;
+ }
+
+ playback = playback && cpu_playback;
+ capture = capture && cpu_capture;
}
if (rtd->dai_link->playback_only) {
@@ -3023,7 +3192,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
out:
dev_info(rtd->card->dev, "%s <-> %s mapping ok\n",
(rtd->num_codecs > 1) ? "multicodec" : rtd->codec_dai->name,
- cpu_dai->name);
+ (rtd->num_cpu_dai > 1) ? "multicpu" : rtd->cpu_dai->name);
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM
2018-05-22 10:40 [PATCH v5 0/3] ASoC: Add Multi CPU DAI support Shreyas NC
2018-05-22 10:40 ` [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs Shreyas NC
2018-05-22 10:40 ` [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops Shreyas NC
@ 2018-05-22 10:40 ` Shreyas NC
2018-05-24 21:37 ` Pierre-Louis Bossart
2 siblings, 1 reply; 12+ messages in thread
From: Shreyas NC @ 2018-05-22 10:40 UTC (permalink / raw)
To: alsa-devel
Cc: ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie, Shreyas NC
DAPM handles DAIs during soc_dapm_stream_event() and during addition
and creation of DAI widgets i.e., dapm_add_valid_dai_widget() and
dapm_connect_dai_link_widgets().
Extend these functions to handle multiple cpu dai.
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>
---
sound/soc/soc-dapm.c | 71 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 46 insertions(+), 25 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 2d97091..79f5f61 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4108,38 +4108,57 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card)
return 0;
}
-static void dapm_connect_dai_link_widgets(struct snd_soc_card *card,
- struct snd_soc_pcm_runtime *rtd)
+static void dapm_add_valid_dai_widget(struct snd_soc_card *card,
+ struct snd_soc_pcm_runtime *rtd,
+ struct snd_soc_dai *codec_dai,
+ struct snd_soc_dai *cpu_dai)
{
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
struct snd_soc_dapm_widget *sink, *source;
- int i;
- for (i = 0; i < rtd->num_codecs; i++) {
- struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
+ /* connect BE DAI playback if widgets are valid */
+ if (codec_dai->playback_widget && cpu_dai->playback_widget) {
+ source = cpu_dai->playback_widget;
+ sink = codec_dai->playback_widget;
+ dev_err(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
+ cpu_dai->component->name,
+ source->name,
+ codec_dai->component->name,
+ sink->name);
+
+ snd_soc_dapm_add_path(&card->dapm, source, sink,
+ NULL, NULL);
+ }
- /* connect BE DAI playback if widgets are valid */
- if (codec_dai->playback_widget && cpu_dai->playback_widget) {
- source = cpu_dai->playback_widget;
- sink = codec_dai->playback_widget;
- dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
- cpu_dai->component->name, source->name,
- codec_dai->component->name, sink->name);
+ /* connect BE DAI capture if widgets are valid */
+ if (codec_dai->capture_widget && cpu_dai->capture_widget) {
+ source = codec_dai->capture_widget;
+ sink = cpu_dai->capture_widget;
+ dev_err(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
+ codec_dai->component->name,
+ source->name,
+ cpu_dai->component->name,
+ sink->name);
- snd_soc_dapm_add_path(&card->dapm, source, sink,
+ snd_soc_dapm_add_path(&card->dapm, source, sink,
NULL, NULL);
- }
+ }
+
+}
- /* connect BE DAI capture if widgets are valid */
- if (codec_dai->capture_widget && cpu_dai->capture_widget) {
- source = codec_dai->capture_widget;
- sink = cpu_dai->capture_widget;
- dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
- codec_dai->component->name, source->name,
- cpu_dai->component->name, sink->name);
+static void dapm_connect_dai_link_widgets(struct snd_soc_card *card,
+ struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_dai *cpu_dai;
+ int i, j;
- snd_soc_dapm_add_path(&card->dapm, source, sink,
- NULL, NULL);
+ for (i = 0; i < rtd->num_codecs; i++) {
+ struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
+
+ for (j = 0; j < rtd->num_cpu_dai; j++) {
+ cpu_dai = rtd->cpu_dais[j];
+
+ dapm_add_valid_dai_widget(card, rtd,
+ codec_dai, cpu_dai);
}
}
}
@@ -4206,7 +4225,9 @@ static void soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream,
{
int i;
- soc_dapm_dai_stream_event(rtd->cpu_dai, stream, event);
+ for (i = 0; i < rtd->num_cpu_dai; i++)
+ soc_dapm_dai_stream_event(rtd->cpu_dais[i], stream, event);
+
for (i = 0; i < rtd->num_codecs; i++)
soc_dapm_dai_stream_event(rtd->codec_dais[i], stream, event);
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops
2018-05-22 10:40 ` [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops Shreyas NC
@ 2018-05-23 9:22 ` Charles Keepax
2018-05-24 21:29 ` Pierre-Louis Bossart
1 sibling, 0 replies; 12+ messages in thread
From: Charles Keepax @ 2018-05-23 9:22 UTC (permalink / raw)
To: Shreyas NC
Cc: liam.r.girdwood, patches.audio, alsa-devel, broonie, Vinod Koul
On Tue, May 22, 2018 at 04:10:22PM +0530, Shreyas NC wrote:
> Add support in PCM operations to invoke multiple cpu dais as we do
> for multiple codec dais. Also the symmetry calculations are updated to
> reflect multiple cpu dais.
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Shreyas NC <shreyas.nc@intel.com>
> ---
For the non-DPCM parts:
Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Thanks,
Charles
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs
2018-05-22 10:40 ` [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs Shreyas NC
@ 2018-05-23 21:38 ` Pierre-Louis Bossart
2018-05-24 5:34 ` Shreyas NC
0 siblings, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-23 21:38 UTC (permalink / raw)
To: Shreyas NC, alsa-devel
Cc: patches.audio, liam.r.girdwood, Vinod Koul, broonie, ckeepax
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;
_______________________________________________
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 v5 1/3] ASoC: Add initial support for multiple CPU DAIs
2018-05-23 21:38 ` Pierre-Louis Bossart
@ 2018-05-24 5:34 ` Shreyas NC
2018-05-24 20:45 ` Pierre-Louis Bossart
0 siblings, 1 reply; 12+ messages in thread
From: Shreyas NC @ 2018-05-24 5:34 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie
> >@@ -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
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs
2018-05-24 5:34 ` Shreyas NC
@ 2018-05-24 20:45 ` Pierre-Louis Bossart
2018-05-25 2:16 ` Shreyas NC
0 siblings, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-24 20:45 UTC (permalink / raw)
To: Shreyas NC
Cc: alsa-devel, ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie
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.
_______________________________________________
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 v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops
2018-05-22 10:40 ` [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops Shreyas NC
2018-05-23 9:22 ` Charles Keepax
@ 2018-05-24 21:29 ` Pierre-Louis Bossart
2018-05-25 4:46 ` Shreyas NC
1 sibling, 1 reply; 12+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-24 21:29 UTC (permalink / raw)
To: Shreyas NC, alsa-devel
Cc: patches.audio, liam.r.girdwood, Vinod Koul, broonie, ckeepax
This one is also quite dense, I could use clarifications on how channels
will be handled in a multi-cpu context. I believe for the multi-codec
case there was an assumption of symmetry, not sure this works or is
required in a multi-cpu case, see below.
>
> - symmetry = cpu_dai->driver->symmetric_channels ||
> - rtd->dai_link->symmetric_channels;
> + symmetry = rtd->dai_link->symmetric_channels;
> +
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + symmetry |= rtd->cpu_dais[i]->driver->symmetric_channels;
>
> for (i = 0; i < rtd->num_codecs; i++)
> symmetry |= rtd->codec_dais[i]->driver->symmetric_channels;
>
> - if (symmetry && cpu_dai->channels && cpu_dai->channels != channels) {
> - dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
> - cpu_dai->channels, channels);
> - return -EINVAL;
> - }
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + if (symmetry && rtd->cpu_dais[i]->channels &&
> + rtd->cpu_dais[i]->channels != channels) {
> + dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
> + rtd->cpu_dais[i]->channels, channels);
> + return -EINVAL;
> + }
I am not sure I get this part - but maybe I am connecting too many dots
with the SoundWire 'stream' patches.
This code is assuming all cpu_dais have the same number of channels,
defined by the hw_params.
Is this right? In the SoundWire case, we can have one port with 2
channels and another with 4, for a total of 6 channels for the stream.
Am I missing something or how should I reconcile the concepts?
making the assumption that the rates and sample_bits are identical is ok.
>
> - symmetry = cpu_dai->driver->symmetric_samplebits ||
> - rtd->dai_link->symmetric_samplebits;
> + symmetry = rtd->dai_link->symmetric_samplebits;
> +
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + symmetry |= rtd->cpu_dais[i]->driver->symmetric_samplebits;
>
> for (i = 0; i < rtd->num_codecs; i++)
> symmetry |= rtd->codec_dais[i]->driver->symmetric_samplebits;
>
> - if (symmetry && cpu_dai->sample_bits && cpu_dai->sample_bits != sample_bits) {
> - dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
> - cpu_dai->sample_bits, sample_bits);
> - return -EINVAL;
> - }
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + if (symmetry && rtd->cpu_dais[i]->sample_bits &&
> + rtd->cpu_dais[i]->sample_bits != sample_bits) {
> + dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
> + rtd->cpu_dais[i]->sample_bits,
> + sample_bits);
> + return -EINVAL;
> + }
>
> return 0;
> }
> @@ -308,13 +328,18 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
> static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream)
> {
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> - struct snd_soc_dai_driver *cpu_driver = rtd->cpu_dai->driver;
> struct snd_soc_dai_link *link = rtd->dai_link;
> unsigned int symmetry, i;
>
> - symmetry = cpu_driver->symmetric_rates || link->symmetric_rates ||
> - cpu_driver->symmetric_channels || link->symmetric_channels ||
> - cpu_driver->symmetric_samplebits || link->symmetric_samplebits;
> + symmetry = link->symmetric_rates || link->symmetric_channels ||
> + link->symmetric_samplebits;
> +
> + /* Apply symmetery for multiple cpu dais */
I've never seen this spelling for cemetery :-)
[...]
>
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai_drv = rtd->cpu_dais[i]->driver;
> +
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + cpu_stream = &cpu_dai_drv->playback;
> + else
> + cpu_stream = &cpu_dai_drv->capture;
> +
> + cpu_chan_min = max(cpu_chan_min,
> + cpu_stream->channels_min);
> + cpu_chan_max = min(cpu_chan_max,
> + cpu_stream->channels_max);
> +
> + if (hw->formats)
> + hw->formats &= cpu_stream->formats;
> + else
> + hw->formats = cpu_stream->formats;
> +
> + cpu_rates = snd_pcm_rate_mask_intersect(cpu_rates,
> + cpu_stream->rates);
> +
> + cpu_rate_min = max(cpu_rate_min, cpu_stream->rate_min);
> + cpu_rate_max = min_not_zero(cpu_rate_max, cpu_stream->rate_max);
> + }
> +
> /*
> - * chan min/max cannot be enforced if there are multiple CODEC DAIs
> - * connected to a single CPU DAI, use CPU DAI's directly and let
> - * channel allocation be fixed up later
> + * chan min/max cannot be enforced if there are multiple
> + * CODEC DAIs connected to CPU DAI(s), use CPU DAI's
> + * directly and let channel allocation be fixed up later
What does 'later' mean?
I guess I don't quite get the channel management, same issue as my
feedback above.
[...]
>
> @@ -963,11 +1070,14 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
> if (ret < 0)
> goto component_err;
>
> - /* store the parameters for each DAIs */
> - cpu_dai->rate = params_rate(params);
> - cpu_dai->channels = params_channels(params);
> - cpu_dai->sample_bits =
> - snd_pcm_format_physical_width(params_format(params));
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + /* store the parameters for each DAIs */
> + cpu_dai = rtd->cpu_dais[i];
> + cpu_dai->rate = params_rate(params);
> + cpu_dai->channels = params_channels(params);
same here, are we again making the assumption that all cpu_dais can
transmit the same number of channels?
[...]
>
> @@ -1107,10 +1229,14 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> return ret;
> }
>
> - if (cpu_dai->driver->ops->trigger) {
> - ret = cpu_dai->driver->ops->trigger(substream, cmd, cpu_dai);
> - if (ret < 0)
> - return ret;
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> + if (cpu_dai->driver->ops->trigger) {
> + ret = cpu_dai->driver->ops->trigger(substream,
> + cmd, cpu_dai);
How do I reconcile this sequential trigger with the notion of
bank-switch in SoundWire? It seems we are missing a global trigger for
all cpu_dais who are part of the same dailink? Or am I in the weeds again?
[...]
> @@ -1157,12 +1287,13 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> struct snd_soc_component *component;
> struct snd_soc_rtdcom_list *rtdcom;
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_dai *cpu_dai;
> struct snd_soc_dai *codec_dai;
> struct snd_pcm_runtime *runtime = substream->runtime;
> snd_pcm_uframes_t offset = 0;
> snd_pcm_sframes_t delay = 0;
> snd_pcm_sframes_t codec_delay = 0;
> + snd_pcm_sframes_t cpu_delay = 0;
> int i;
>
> for_each_rtdcom(rtd, rtdcom) {
> @@ -1177,8 +1308,15 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> break;
> }
>
> - if (cpu_dai->driver->ops->delay)
> - delay += cpu_dai->driver->ops->delay(substream, cpu_dai);
> + for (i = 0; i < rtd->num_cpu_dai; i++) {
> + cpu_dai = rtd->cpu_dais[i];
> + if (cpu_dai->driver->ops->delay)
> + cpu_delay = max(cpu_delay,
> + cpu_dai->driver->ops->delay(substream,
> + cpu_dai));
> + }
> +
> + delay += cpu_delay;
Oh, this is weird. If you are checking the delay sequentially for each
cpu_dai, what are the odds that you get a consistent reply? I think it's
fundamentally different from the codec side since you will in theory be
able to check delays on each cpu_dai fairly quickly over IPC, whereas
for codecs the delay is likely to be a long-term estimate, not an
immediate value. In addition you would probably expect that all cpu_dais
are triggered at the same time and hence have the same delay, so you
could use the cpu_dais[0] instead of querying the values multiple times.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM
2018-05-22 10:40 ` [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM Shreyas NC
@ 2018-05-24 21:37 ` Pierre-Louis Bossart
0 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-24 21:37 UTC (permalink / raw)
To: Shreyas NC, alsa-devel
Cc: patches.audio, liam.r.girdwood, Vinod Koul, broonie, ckeepax
On 05/22/2018 05:40 AM, Shreyas NC wrote:
> DAPM handles DAIs during soc_dapm_stream_event() and during addition
> and creation of DAI widgets i.e., dapm_add_valid_dai_widget() and
> dapm_connect_dai_link_widgets().
>
> Extend these functions to handle multiple cpu dai.
nit-pick: the diffs are difficult to look at, it might be easier on the
reader if you first added a new helper function, then added multi
cpu_dais in a second patch.
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
> 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>
> ---
> sound/soc/soc-dapm.c | 71 ++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 2d97091..79f5f61 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -4108,38 +4108,57 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card)
> return 0;
> }
>
> -static void dapm_connect_dai_link_widgets(struct snd_soc_card *card,
> - struct snd_soc_pcm_runtime *rtd)
> +static void dapm_add_valid_dai_widget(struct snd_soc_card *card,
> + struct snd_soc_pcm_runtime *rtd,
> + struct snd_soc_dai *codec_dai,
> + struct snd_soc_dai *cpu_dai)
> {
> - struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> struct snd_soc_dapm_widget *sink, *source;
> - int i;
>
> - for (i = 0; i < rtd->num_codecs; i++) {
> - struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
> + /* connect BE DAI playback if widgets are valid */
> + if (codec_dai->playback_widget && cpu_dai->playback_widget) {
> + source = cpu_dai->playback_widget;
> + sink = codec_dai->playback_widget;
> + dev_err(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
> + cpu_dai->component->name,
> + source->name,
> + codec_dai->component->name,
> + sink->name);
> +
> + snd_soc_dapm_add_path(&card->dapm, source, sink,
> + NULL, NULL);
> + }
>
> - /* connect BE DAI playback if widgets are valid */
> - if (codec_dai->playback_widget && cpu_dai->playback_widget) {
> - source = cpu_dai->playback_widget;
> - sink = codec_dai->playback_widget;
> - dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
> - cpu_dai->component->name, source->name,
> - codec_dai->component->name, sink->name);
> + /* connect BE DAI capture if widgets are valid */
> + if (codec_dai->capture_widget && cpu_dai->capture_widget) {
> + source = codec_dai->capture_widget;
> + sink = cpu_dai->capture_widget;
> + dev_err(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
> + codec_dai->component->name,
> + source->name,
> + cpu_dai->component->name,
> + sink->name);
>
> - snd_soc_dapm_add_path(&card->dapm, source, sink,
> + snd_soc_dapm_add_path(&card->dapm, source, sink,
> NULL, NULL);
> - }
> + }
> +
> +}
>
> - /* connect BE DAI capture if widgets are valid */
> - if (codec_dai->capture_widget && cpu_dai->capture_widget) {
> - source = codec_dai->capture_widget;
> - sink = cpu_dai->capture_widget;
> - dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n",
> - codec_dai->component->name, source->name,
> - cpu_dai->component->name, sink->name);
> +static void dapm_connect_dai_link_widgets(struct snd_soc_card *card,
> + struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_dai *cpu_dai;
> + int i, j;
>
> - snd_soc_dapm_add_path(&card->dapm, source, sink,
> - NULL, NULL);
> + for (i = 0; i < rtd->num_codecs; i++) {
> + struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
> +
> + for (j = 0; j < rtd->num_cpu_dai; j++) {
> + cpu_dai = rtd->cpu_dais[j];
> +
> + dapm_add_valid_dai_widget(card, rtd,
> + codec_dai, cpu_dai);
> }
> }
> }
> @@ -4206,7 +4225,9 @@ static void soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream,
> {
> int i;
>
> - soc_dapm_dai_stream_event(rtd->cpu_dai, stream, event);
> + for (i = 0; i < rtd->num_cpu_dai; i++)
> + soc_dapm_dai_stream_event(rtd->cpu_dais[i], stream, event);
> +
> for (i = 0; i < rtd->num_codecs; i++)
> soc_dapm_dai_stream_event(rtd->codec_dais[i], stream, event);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs
2018-05-24 20:45 ` Pierre-Louis Bossart
@ 2018-05-25 2:16 ` Shreyas NC
0 siblings, 0 replies; 12+ messages in thread
From: Shreyas NC @ 2018-05-25 2:16 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie
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
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops
2018-05-24 21:29 ` Pierre-Louis Bossart
@ 2018-05-25 4:46 ` Shreyas NC
0 siblings, 0 replies; 12+ messages in thread
From: Shreyas NC @ 2018-05-25 4:46 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, ckeepax, patches.audio, liam.r.girdwood, Vinod Koul, broonie
On Thu, May 24, 2018 at 04:29:45PM -0500, Pierre-Louis Bossart wrote:
> This one is also quite dense, I could use clarifications on how channels
> will be handled in a multi-cpu context. I believe for the multi-codec case
> there was an assumption of symmetry, not sure this works or is required in a
> multi-cpu case, see below.
I am not sure if I understand symmetry correctly. Mark, can you please help us
here ?
> >- symmetry = cpu_dai->driver->symmetric_channels ||
> >- rtd->dai_link->symmetric_channels;
> >+ symmetry = rtd->dai_link->symmetric_channels;
> >+
> >+ for (i = 0; i < rtd->num_cpu_dai; i++)
> >+ symmetry |= rtd->cpu_dais[i]->driver->symmetric_channels;
> > for (i = 0; i < rtd->num_codecs; i++)
> > symmetry |= rtd->codec_dais[i]->driver->symmetric_channels;
> >- if (symmetry && cpu_dai->channels && cpu_dai->channels != channels) {
> >- dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
> >- cpu_dai->channels, channels);
> >- return -EINVAL;
> >- }
> >+ for (i = 0; i < rtd->num_cpu_dai; i++)
> >+ if (symmetry && rtd->cpu_dais[i]->channels &&
> >+ rtd->cpu_dais[i]->channels != channels) {
> >+ dev_err(rtd->dev, "ASoC: unmatched channel symmetry: %d - %d\n",
> >+ rtd->cpu_dais[i]->channels, channels);
> >+ return -EINVAL;
> >+ }
> I am not sure I get this part - but maybe I am connecting too many dots with
> the SoundWire 'stream' patches.
>
> This code is assuming all cpu_dais have the same number of channels, defined
> by the hw_params.
Yes
> Is this right? In the SoundWire case, we can have one port with 2 channels
> and another with 4, for a total of 6 channels for the stream. Am I missing
> something or how should I reconcile the concepts?
>
In the case you have explained, the stream has 6 channels. But, from the
machine driver we can have set channel masks on each of these DAIs
accordingly.
> making the assumption that the rates and sample_bits are identical is ok.
>
> >- symmetry = cpu_dai->driver->symmetric_samplebits ||
> >- rtd->dai_link->symmetric_samplebits;
> >+ symmetry = rtd->dai_link->symmetric_samplebits;
> >+
> >+ for (i = 0; i < rtd->num_cpu_dai; i++)
> >+ symmetry |= rtd->cpu_dais[i]->driver->symmetric_samplebits;
> > for (i = 0; i < rtd->num_codecs; i++)
> > symmetry |= rtd->codec_dais[i]->driver->symmetric_samplebits;
> >- if (symmetry && cpu_dai->sample_bits && cpu_dai->sample_bits != sample_bits) {
> >- dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
> >- cpu_dai->sample_bits, sample_bits);
> >- return -EINVAL;
> >- }
> >+ for (i = 0; i < rtd->num_cpu_dai; i++)
> >+ if (symmetry && rtd->cpu_dais[i]->sample_bits &&
> >+ rtd->cpu_dais[i]->sample_bits != sample_bits) {
> >+ dev_err(rtd->dev, "ASoC: unmatched sample bits symmetry: %d - %d\n",
> >+ rtd->cpu_dais[i]->sample_bits,
> >+ sample_bits);
> >+ return -EINVAL;
> >+ }
> > return 0;
> > }
> >@@ -308,13 +328,18 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
> > static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream)
> > {
> > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> >- struct snd_soc_dai_driver *cpu_driver = rtd->cpu_dai->driver;
> > struct snd_soc_dai_link *link = rtd->dai_link;
> > unsigned int symmetry, i;
> >- symmetry = cpu_driver->symmetric_rates || link->symmetric_rates ||
> >- cpu_driver->symmetric_channels || link->symmetric_channels ||
> >- cpu_driver->symmetric_samplebits || link->symmetric_samplebits;
> >+ symmetry = link->symmetric_rates || link->symmetric_channels ||
> >+ link->symmetric_samplebits;
> >+
> >+ /* Apply symmetery for multiple cpu dais */
> I've never seen this spelling for cemetery :-)
>
Never meant it to be cemetery for sure :D
Will correct it, Thanks!
> [...]
> >+ for (i = 0; i < rtd->num_cpu_dai; i++) {
> >+ cpu_dai_drv = rtd->cpu_dais[i]->driver;
> >+
> >+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> >+ cpu_stream = &cpu_dai_drv->playback;
> >+ else
> >+ cpu_stream = &cpu_dai_drv->capture;
> >+
> >+ cpu_chan_min = max(cpu_chan_min,
> >+ cpu_stream->channels_min);
> >+ cpu_chan_max = min(cpu_chan_max,
> >+ cpu_stream->channels_max);
> >+
> >+ if (hw->formats)
> >+ hw->formats &= cpu_stream->formats;
> >+ else
> >+ hw->formats = cpu_stream->formats;
> >+
> >+ cpu_rates = snd_pcm_rate_mask_intersect(cpu_rates,
> >+ cpu_stream->rates);
> >+
> >+ cpu_rate_min = max(cpu_rate_min, cpu_stream->rate_min);
> >+ cpu_rate_max = min_not_zero(cpu_rate_max, cpu_stream->rate_max);
> >+ }
> >+
> > /*
> >- * chan min/max cannot be enforced if there are multiple CODEC DAIs
> >- * connected to a single CPU DAI, use CPU DAI's directly and let
> >- * channel allocation be fixed up later
> >+ * chan min/max cannot be enforced if there are multiple
> >+ * CODEC DAIs connected to CPU DAI(s), use CPU DAI's
> >+ * directly and let channel allocation be fixed up later
> What does 'later' mean?
> I guess I don't quite get the channel management, same issue as my feedback
> above.
>
I think 'later' here means channel allocation can then be fixed by using
_set_channel_map(), be_hw_params_fixup()
> [...]
> >
> >@@ -963,11 +1070,14 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
> > if (ret < 0)
> > goto component_err;
> >- /* store the parameters for each DAIs */
> >- cpu_dai->rate = params_rate(params);
> >- cpu_dai->channels = params_channels(params);
> >- cpu_dai->sample_bits =
> >- snd_pcm_format_physical_width(params_format(params));
> >+ for (i = 0; i < rtd->num_cpu_dai; i++) {
> >+ /* store the parameters for each DAIs */
> >+ cpu_dai = rtd->cpu_dais[i];
> >+ cpu_dai->rate = params_rate(params);
> >+ cpu_dai->channels = params_channels(params);
> same here, are we again making the assumption that all cpu_dais can transmit
> the same number of channels?
>
Yes. But, as explained earlier machine can then set the channels masks on
these CPU DAIs
> [...]
> >@@ -1107,10 +1229,14 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> > return ret;
> > }
> >- if (cpu_dai->driver->ops->trigger) {
> >- ret = cpu_dai->driver->ops->trigger(substream, cmd, cpu_dai);
> >- if (ret < 0)
> >- return ret;
> >+ for (i = 0; i < rtd->num_cpu_dai; i++) {
> >+ cpu_dai = rtd->cpu_dais[i];
> >+ if (cpu_dai->driver->ops->trigger) {
> >+ ret = cpu_dai->driver->ops->trigger(substream,
> >+ cmd, cpu_dai);
> How do I reconcile this sequential trigger with the notion of bank-switch in
> SoundWire? It seems we are missing a global trigger for all cpu_dais who are
> part of the same dailink? Or am I in the weeds again?
>
Yes, there is no global trigger for all cpu_dais specifically.
And for Soundwire, this is the reason we chose to call from machine/platform
> [...]
> >@@ -1157,12 +1287,13 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > struct snd_soc_component *component;
> > struct snd_soc_rtdcom_list *rtdcom;
> >- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> >+ struct snd_soc_dai *cpu_dai;
> > struct snd_soc_dai *codec_dai;
> > struct snd_pcm_runtime *runtime = substream->runtime;
> > snd_pcm_uframes_t offset = 0;
> > snd_pcm_sframes_t delay = 0;
> > snd_pcm_sframes_t codec_delay = 0;
> >+ snd_pcm_sframes_t cpu_delay = 0;
> > int i;
> > for_each_rtdcom(rtd, rtdcom) {
> >@@ -1177,8 +1308,15 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> > break;
> > }
> >- if (cpu_dai->driver->ops->delay)
> >- delay += cpu_dai->driver->ops->delay(substream, cpu_dai);
> >+ for (i = 0; i < rtd->num_cpu_dai; i++) {
> >+ cpu_dai = rtd->cpu_dais[i];
> >+ if (cpu_dai->driver->ops->delay)
> >+ cpu_delay = max(cpu_delay,
> >+ cpu_dai->driver->ops->delay(substream,
> >+ cpu_dai));
> >+ }
> >+
> >+ delay += cpu_delay;
> Oh, this is weird. If you are checking the delay sequentially for each
> cpu_dai, what are the odds that you get a consistent reply? I think it's
> fundamentally different from the codec side since you will in theory be able
> to check delays on each cpu_dai fairly quickly over IPC, whereas for codecs
> the delay is likely to be a long-term estimate, not an immediate value. In
> addition you would probably expect that all cpu_dais are triggered at the
> same time and hence have the same delay, so you could use the cpu_dais[0]
> instead of querying the values multiple times.
>
That sounds like a fair arguement to me. Just wondering if there can be a
case of multiple CPU DAIs but you would not want them to be triggered at the
same time.
Thanks for the review!
--Shreyas
--
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-05-25 4:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22 10:40 [PATCH v5 0/3] ASoC: Add Multi CPU DAI support Shreyas NC
2018-05-22 10:40 ` [PATCH v5 1/3] ASoC: Add initial support for multiple CPU DAIs Shreyas NC
2018-05-23 21:38 ` Pierre-Louis Bossart
2018-05-24 5:34 ` Shreyas NC
2018-05-24 20:45 ` Pierre-Louis Bossart
2018-05-25 2:16 ` Shreyas NC
2018-05-22 10:40 ` [PATCH v5 2/3] ASoC: Add multiple CPU DAI support for PCM ops Shreyas NC
2018-05-23 9:22 ` Charles Keepax
2018-05-24 21:29 ` Pierre-Louis Bossart
2018-05-25 4:46 ` Shreyas NC
2018-05-22 10:40 ` [PATCH v5 3/3] ASoC: Add multiple CPU DAI support in DAPM Shreyas NC
2018-05-24 21:37 ` Pierre-Louis Bossart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.