Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] ASoC: Add Multi CPU DAI support #1644

Closed
wants to merge 6 commits into from
Closed

[RFC] ASoC: Add Multi CPU DAI support #1644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
14c9c9e
ASoC: Add initial support for multiple CPU DAIs
shreyasnc1 Dec 17, 2019
81fa54a
ASoC: Add multiple CPU DAI support for PCM ops
shreyasnc1 Jan 20, 2020
19b41c7
ASoC: Add dapm_add_valid_dai_widget helper
shreyasnc1 Jan 20, 2020
213efa3
ASoC: Add multiple CPU DAI support in DAPM
shreyasnc1 Jan 20, 2020
c58d23e
ASoC: return error if the function is not support multi cpu yet.
bardliao Jan 1, 2020
918814f
ASoC: pcm: check if cpu DAI support the given stream
bardliao Jan 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions include/sound/soc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,11 @@ struct snd_soc_dai_link {
((platform) = &link->platforms[i]); \
(i)++)

#define for_each_link_cpus(link, i, cpu) \
for ((i) = 0; \
((i) < link->num_cpus) && ((cpu) = &link->cpus[i]); \
(i)++)

/*
* Sample 1 : Single CPU/Codec/Platform
*
Expand Down Expand Up @@ -1132,6 +1137,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_cpus;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we need to keep 'codec_dai' now that we have 'codec_dais" and 'num_codecs'?

Also why do we need 'cpu_dai' as well as 'cpus_dais' and 'num_cpu_dai'

Looks like room for additional cleanups

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we need to keep 'codec_dai' now that we have 'codec_dais" and 'num_codecs'?

Also why do we need 'cpu_dai' as well as 'cpus_dais' and 'num_cpu_dai'

Looks like room for additional cleanups

Indeed, we can send another patch to remove cpu_dai and codec_dai

struct delayed_work delayed_work;
void (*close_delayed_work_func)(struct snd_soc_pcm_runtime *rtd);
#ifdef CONFIG_DEBUG_FS
Expand Down Expand Up @@ -1159,6 +1167,13 @@ struct snd_soc_pcm_runtime {
#define for_each_rtd_codec_dai_rollback(rtd, i, dai) \
for (; ((--i) >= 0) && ((dai) = rtd->codec_dais[i]);)

#define for_each_rtd_cpu_dai(rtd, i, dai)\
for ((i) = 0; \
((i) < rtd->num_cpus) && ((dai) = rtd->cpu_dais[i]); \
(i)++)
#define for_each_rtd_cpu_dai_rollback(rtd, i, dai) \
for (; ((--i) >= 0) && ((dai) = rtd->cpu_dais[i]);)

void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);

/* mixer control */
Expand Down
5 changes: 3 additions & 2 deletions sound/soc/soc-compress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,10 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
int playback = 0, capture = 0;
int i;

if (rtd->num_codecs > 1) {
if (rtd->num_cpus > 1 ||
rtd->num_codecs > 1) {
dev_err(rtd->card->dev,
"Compress ASoC: Multicodec not supported\n");
"Compress ASoC: Multi CPU/Codec not supported\n");
return -EINVAL;
}

Expand Down
205 changes: 114 additions & 91 deletions sound/soc/soc-core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,14 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
if (!rtd->codec_dais)
goto free_rtd;

/*
* for rtd->cpu_dais
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not very helpful at all

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to be consistent with the existing code. ie

	/*
	 * for rtd->codec_dais
	 */

above.

rtd->cpu_dais = devm_kcalloc(dev, dai_link->num_cpus,
sizeof(struct snd_soc_dai *),
GFP_KERNEL);
if (!rtd->cpu_dais)
goto free_rtd;
/*
* rtd remaining settings
*/
Expand Down Expand Up @@ -559,13 +567,15 @@ int snd_soc_suspend(struct device *dev)
card->suspend_pre(card);

for_each_card_rtds(card, rtd) {
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->bus_control)
snd_soc_dai_suspend(cpu_dai);
for_each_rtd_cpu_dai(rtd, i, cpu_dai) {
if (!cpu_dai->driver->bus_control)
snd_soc_dai_suspend(cpu_dai);
}
}

/* close any waiting streams */
Expand Down Expand Up @@ -640,13 +650,14 @@ int snd_soc_suspend(struct device *dev)
}

for_each_card_rtds(card, rtd) {
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->bus_control)
snd_soc_dai_suspend(cpu_dai);
for_each_rtd_cpu_dai(rtd, i, cpu_dai)
if (cpu_dai->driver->bus_control)
snd_soc_dai_suspend(cpu_dai);
}

if (card->suspend_post)
Expand Down Expand Up @@ -684,13 +695,15 @@ static void soc_resume_deferred(struct work_struct *work)

/* resume control bus DAIs */
for_each_card_rtds(card, rtd) {
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->bus_control)
snd_soc_dai_resume(cpu_dai);
for_each_rtd_cpu_dai(rtd, i, cpu_dai) {
if (cpu_dai->driver->bus_control)
snd_soc_dai_resume(cpu_dai);
}
}

for_each_card_components(card, component) {
Expand Down Expand Up @@ -727,13 +740,15 @@ static void soc_resume_deferred(struct work_struct *work)
}

for_each_card_rtds(card, rtd) {
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->bus_control)
snd_soc_dai_resume(cpu_dai);
for_each_rtd_cpu_dai(rtd, i, cpu_dai) {
if (!cpu_dai->driver->bus_control)
snd_soc_dai_resume(cpu_dai);
}
}

if (card->resume_post)
Expand Down Expand Up @@ -773,9 +788,11 @@ int snd_soc_resume(struct device *dev)
* due to I/O costs and anti-pop so handle them out of line.
*/
for_each_card_rtds(card, rtd) {
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
struct snd_soc_dai *cpu_dai;
int i;

bus_control |= cpu_dai->driver->bus_control;
for_each_rtd_cpu_dai(rtd, i, cpu_dai)
bus_control |= cpu_dai->driver->bus_control;
}
if (bus_control) {
dev_dbg(dev, "ASoC: Resuming control bus master immediately\n");
Expand Down Expand Up @@ -900,7 +917,7 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
struct snd_soc_dai_link *link)
{
int i;
struct snd_soc_dai_link_component *codec, *platform;
struct snd_soc_dai_link_component *cpu, *codec, *platform;

for_each_link_codecs(link, i, codec) {
/*
Expand Down Expand Up @@ -949,44 +966,38 @@ static int soc_dai_link_sanity_check(struct snd_soc_card *card,
return -EPROBE_DEFER;
}

/* FIXME */
if (link->num_cpus > 1) {
dev_err(card->dev,
"ASoC: multi cpu is not yet supported %s\n",
link->name);
return -EINVAL;
}

/*
* 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->cpus->name && link->cpus->of_node) {
dev_err(card->dev,
"ASoC: Neither/both cpu name/of_node are set for %s\n",
link->name);
return -EINVAL;
}
for_each_link_cpus(link, i, cpu) {
/*
* 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 (cpu->name && cpu->of_node) {
dev_err(card->dev,
"ASoC: Neither/both cpu name/of_node are set for %s\n",
link->name);
return -EINVAL;
}

Copy link
Collaborator

@dbaluta dbaluta Jan 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is off here. Either the error message is wrong or the if condition is wrong. If none of cpu->name / cpu->of_node are set we won't hit the error message.

This is not a blocker for this PR as the issue was pre-existent. Will send an email to alsa-devel.

/*
* Defer card registration if cpu dai component is not added to
* component list.
*/
if ((link->cpus->of_node || link->cpus->name) &&
!soc_find_component(link->cpus))
return -EPROBE_DEFER;
/*
* Defer card registration if cpu dai component is not added to
* component list.
*/
if ((cpu->of_node || cpu->name) &&
!soc_find_component(cpu))
return -EPROBE_DEFER;

/*
* At least one of CPU DAI name or CPU device name/node must be
* specified
*/
if (!link->cpus->dai_name &&
!(link->cpus->name || link->cpus->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;
/*
* At least one of CPU DAI name or CPU device name/node must be
* specified
*/
if (!cpu->dai_name &&
!(cpu->name || 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;
Expand Down Expand Up @@ -1029,7 +1040,7 @@ int snd_soc_add_pcm_runtime(struct snd_soc_card *card,
struct snd_soc_dai_link *dai_link)
{
struct snd_soc_pcm_runtime *rtd;
struct snd_soc_dai_link_component *codec, *platform;
struct snd_soc_dai_link_component *codec, *platform, *cpu;
struct snd_soc_component *component;
int i, ret;

Expand All @@ -1054,14 +1065,19 @@ int snd_soc_add_pcm_runtime(struct snd_soc_card *card,
if (!rtd)
return -ENOMEM;

/* FIXME: we need multi CPU support in the future */
rtd->cpu_dai = snd_soc_find_dai(dai_link->cpus);
if (!rtd->cpu_dai) {
dev_info(card->dev, "ASoC: CPU DAI %s not registered\n",
dai_link->cpus->dai_name);
goto _err_defer;
rtd->num_cpus = dai_link->num_cpus;
for_each_link_cpus(dai_link, i, cpu) {
rtd->cpu_dais[i] = snd_soc_find_dai(cpu);
if (!rtd->cpu_dais[i]) {
dev_info(card->dev, "ASoC: CPU DAI %s not registered\n",
cpu->dai_name);
goto _err_defer;
}
snd_soc_rtd_add_component(rtd, rtd->cpu_dais[i]->component);
}
snd_soc_rtd_add_component(rtd, rtd->cpu_dai->component);

/* Single cpu links expect cpu and cpu_dai in runtime data */
rtd->cpu_dai = rtd->cpu_dais[0];

/* Find CODEC from registered CODECs */
rtd->num_codecs = dai_link->num_codecs;
Expand Down Expand Up @@ -1181,7 +1197,8 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card,
dai_link->stream_name, ret);
return ret;
}
ret = soc_dai_pcm_new(&cpu_dai, 1, rtd);
ret = soc_dai_pcm_new(rtd->cpu_dais,
rtd->num_cpus, rtd);
if (ret < 0)
return ret;
ret = soc_dai_pcm_new(rtd->codec_dais,
Expand Down Expand Up @@ -1373,6 +1390,7 @@ static void soc_remove_link_dais(struct snd_soc_card *card)
{
int i;
struct snd_soc_dai *codec_dai;
struct snd_soc_dai *cpu_dai;
struct snd_soc_pcm_runtime *rtd;
int order;

Expand All @@ -1382,14 +1400,15 @@ static void soc_remove_link_dais(struct snd_soc_card *card)
for_each_rtd_codec_dai(rtd, i, codec_dai)
soc_remove_dai(codec_dai, order);

soc_remove_dai(rtd->cpu_dai, order);
for_each_rtd_cpu_dai(rtd, i, cpu_dai)
soc_remove_dai(cpu_dai, order);
}
}
}

static int soc_probe_link_dais(struct snd_soc_card *card)
{
struct snd_soc_dai *codec_dai;
struct snd_soc_dai *codec_dai, *cpu_dai;
struct snd_soc_pcm_runtime *rtd;
int i, order, ret;

Expand All @@ -1400,9 +1419,12 @@ static int soc_probe_link_dais(struct snd_soc_card *card)
"ASoC: probe %s dai link %d late %d\n",
card->name, rtd->num, order);

ret = soc_probe_dai(rtd->cpu_dai, order);
if (ret)
return ret;
/* probe the CPU DAI */
for_each_rtd_cpu_dai(rtd, i, cpu_dai) {
ret = soc_probe_dai(cpu_dai, order);
if (ret)
return ret;
}

/* probe the CODEC DAI */
for_each_rtd_codec_dai(rtd, i, codec_dai) {
Expand Down Expand Up @@ -1534,8 +1556,9 @@ static void soc_remove_aux_devices(struct snd_soc_card *card)
int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
unsigned int dai_fmt)
{
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
struct snd_soc_dai *cpu_dai;
struct snd_soc_dai *codec_dai;
unsigned int inv_dai_fmt;
unsigned int i;
int ret;

Expand All @@ -1552,33 +1575,33 @@ 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;

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;
}

dai_fmt = inv_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;
}
for_each_rtd_cpu_dai(rtd, i, cpu_dai) {
unsigned int fmt = dai_fmt;

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;
if (cpu_dai->component->driver->non_legacy_dai_naming)
fmt = inv_dai_fmt;

ret = snd_soc_dai_set_fmt(cpu_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;
Expand Down
Loading