[alsa-devel] [PATCH v2 00/10] ASoC: support adding PCM dynamically from topology
From: Mengdong Lin mengdong.lin@intel.com
This series allows the topology core to create PCM devices dynamically. The user can define different DAIs in the topology files for different versions of firmware, but share a generic platform and machine driver.
A dummy DAI and DAI link can be used to register the soc card and specify the platform with topology. Then real DAIs are created in platform probing phase by the topology core, and the machine driver will be notified to create relavant DAI links.
We tested using topology to create FE DAI/DAI links on Broadwell.
Mengdong Lin (10): ASoC: Change the PCM runtime array to a list ASoC: Define soc_init_dai_link() to wrap link intialization. ASoC: Change 2nd argument of soc_bind_dai_link() to DAI link pointer ASoC: Implement DAI links in a list ASoC: Add support for dummy DAI links and PCM runtimes ASoC: Bind new DAI links after probing components ASoC: Support adding a DAI dynamically ASoC: topology: Change pass number of DAI smaller than graph ASoC: topology: Change stream formats to bitwise flag ASOC: topology: Add PCM DAIs dynamically when loading them
include/sound/soc-dai.h | 1 + include/sound/soc-topology.h | 1 - include/sound/soc.h | 28 +- include/uapi/sound/asoc.h | 2 +- sound/soc/fsl/fsl-asoc-card.c | 10 +- sound/soc/fsl/imx-wm8962.c | 10 +- sound/soc/generic/simple-card.c | 12 +- sound/soc/intel/atom/sst-mfld-platform-pcm.c | 12 +- sound/soc/intel/boards/cht_bsw_rt5645.c | 7 +- sound/soc/intel/boards/cht_bsw_rt5672.c | 7 +- sound/soc/pxa/mioa701_wm9713.c | 6 +- sound/soc/samsung/bells.c | 40 +- sound/soc/samsung/littlemill.c | 32 +- sound/soc/samsung/odroidx2_max98090.c | 9 +- sound/soc/samsung/snow.c | 9 +- sound/soc/samsung/speyside.c | 12 +- sound/soc/samsung/tobermory.c | 21 +- sound/soc/soc-core.c | 642 +++++++++++++++++++-------- sound/soc/soc-dapm.c | 7 +- sound/soc/soc-pcm.c | 22 +- sound/soc/soc-topology.c | 99 ++++- sound/soc/tegra/tegra_wm8903.c | 3 +- 22 files changed, 701 insertions(+), 291 deletions(-)
From: Mengdong Lin mengdong.lin@intel.com
Currently the number of DAI links is statically defined by the machine driver at build time using an array. This makes it difficult to shrink/ grow the number of DAI links at runtime in order to reflect any changes in topology.
We can change the DAI link array in the core to a list so that PCMs and FE DAI links can be added and deleted at runtime to reflect changes in use case and DSP topology. The machine driver can still register DAI links as an array.
As the 1st step, this patch change the PCM runtime array to a list. A new PCM runtime is added to the list when a DAI link is bound successfully.
Later patches will further implement the DAI link list.
More: - define snd_soc_new/free_pcm_runtime() to create/free a runtime. - define soc_add_pcm_runtime() to add a runtime to the rtd list. - define soc_remove_pcm_runtimes() to clean up the runtime list.
- traverse the rtd list to probe the link components and dais.
- Add a field "num" to PCM runtime struct, used to specify the device number when creating the pcm device, and for a simple card to access its dai_props array.
- The following 3rd party machine/platform drivers either iterate the rtd list or use snd_soc_get_pcm_runtime() to access a rtd:
intel/atom/sst-mfld-platform-pcm.c intel/boards/cht_bsw_rt5672 intel/boards/cht_bsw_rt5645 fsl/imx-wm8962.c fsl/fsl-asoc-card.c samsung/bells.c samsung/littlemill.c samsung/odroidx2_max98090.c samsung/snow.c samsung/speyside.c samsung/tobermory.c pxa/mioa701_wm9713.c tegra/tegra_wm8903.c
More 3rd party drivers will be modified later, if the basic idea is acceptable.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 4cef20e..e2b7b0f 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1080,7 +1080,7 @@ struct snd_soc_card { /* CPU <--> Codec DAI links */ struct snd_soc_dai_link *dai_link; int num_links; - struct snd_soc_pcm_runtime *rtd; + struct list_head rtd_list; int num_rtd;
/* optional codec specific configuration */ @@ -1175,6 +1175,9 @@ struct snd_soc_pcm_runtime { struct dentry *debugfs_dpcm_root; struct dentry *debugfs_dpcm_state; #endif + + unsigned int num; /* 0-based and monotonic increasing */ + struct list_head list; /* rtd list of the soc card */ };
/* mixer control */ diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c index de43887..b80cf09 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -211,12 +211,15 @@ static int fsl_asoc_card_set_bias_level(struct snd_soc_card *card, enum snd_soc_bias_level level) { struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card); - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; struct codec_priv *codec_priv = &priv->codec_priv; struct device *dev = card->dev; unsigned int pll_out; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; if (dapm->dev != codec_dai->dev) return 0;
@@ -383,11 +386,14 @@ static int fsl_asoc_card_audmux_init(struct device_node *np, static int fsl_asoc_card_late_probe(struct snd_soc_card *card) { struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card); - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; struct codec_priv *codec_priv = &priv->codec_priv; struct device *dev = card->dev; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id, codec_priv->mclk_freq, SND_SOC_CLOCK_IN); if (ret) { diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c index b38b98c..201a70d 100644 --- a/sound/soc/fsl/imx-wm8962.c +++ b/sound/soc/fsl/imx-wm8962.c @@ -69,13 +69,16 @@ static int imx_wm8962_set_bias_level(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; struct imx_priv *priv = &card_priv; struct imx_wm8962_data *data = snd_soc_card_get_drvdata(card); struct device *dev = &priv->pdev->dev; unsigned int pll_out; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; if (dapm->dev != codec_dai->dev) return 0;
@@ -135,12 +138,15 @@ static int imx_wm8962_set_bias_level(struct snd_soc_card *card,
static int imx_wm8962_late_probe(struct snd_soc_card *card) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; struct imx_priv *priv = &card_priv; struct imx_wm8962_data *data = snd_soc_card_get_drvdata(card); struct device *dev = &priv->pdev->dev; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK, data->clk_frequency, SND_SOC_CLOCK_IN); if (ret < 0) diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 3ff76d4..95a2f91 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -45,7 +45,7 @@ static int asoc_simple_card_startup(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card); struct simple_dai_props *dai_props = - &priv->dai_props[rtd - rtd->card->rtd]; + &priv->dai_props[rtd->num]; int ret;
ret = clk_prepare_enable(dai_props->cpu_dai.clk); @@ -64,7 +64,7 @@ static void asoc_simple_card_shutdown(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card); struct simple_dai_props *dai_props = - &priv->dai_props[rtd - rtd->card->rtd]; + &priv->dai_props[rtd->num];
clk_disable_unprepare(dai_props->cpu_dai.clk);
@@ -78,8 +78,7 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card); - struct simple_dai_props *dai_props = - &priv->dai_props[rtd - rtd->card->rtd]; + struct simple_dai_props *dai_props = &priv->dai_props[rtd->num]; unsigned int mclk, mclk_fs = 0; int ret = 0;
@@ -172,10 +171,9 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) struct snd_soc_dai *codec = rtd->codec_dai; struct snd_soc_dai *cpu = rtd->cpu_dai; struct simple_dai_props *dai_props; - int num, ret; + int ret;
- num = rtd - rtd->card->rtd; - dai_props = &priv->dai_props[num]; + dai_props = &priv->dai_props[rtd->num]; ret = __asoc_simple_card_dai_init(codec, &dai_props->codec_dai); if (ret < 0) return ret; diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c index 683e501..5ae8ae2 100644 --- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c +++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c @@ -777,15 +777,15 @@ static int sst_platform_remove(struct platform_device *pdev) static int sst_soc_prepare(struct device *dev) { struct sst_data *drv = dev_get_drvdata(dev); - int i; + struct snd_soc_pcm_runtime *rtd;
/* suspend all pcms first */ snd_soc_suspend(drv->soc_card->dev); snd_soc_poweroff(drv->soc_card->dev);
/* set the SSPs to idle */ - for (i = 0; i < drv->soc_card->num_rtd; i++) { - struct snd_soc_dai *dai = drv->soc_card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &drv->soc_card->rtd_list, list) { + struct snd_soc_dai *dai = rtd->cpu_dai;
if (dai->active) { send_ssp_cmd(dai, dai->name, 0); @@ -799,11 +799,11 @@ static int sst_soc_prepare(struct device *dev) static void sst_soc_complete(struct device *dev) { struct sst_data *drv = dev_get_drvdata(dev); - int i; + struct snd_soc_pcm_runtime *rtd;
/* restart SSPs */ - for (i = 0; i < drv->soc_card->num_rtd; i++) { - struct snd_soc_dai *dai = drv->soc_card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &drv->soc_card->rtd_list, list) { + struct snd_soc_dai *dai = rtd->cpu_dai;
if (dai->active) { sst_handle_vb_timer(dai, true); diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index bdcaf46..fade1a2 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -47,12 +47,9 @@ struct cht_mc_private {
static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card) { - int i; - - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd; + struct snd_soc_pcm_runtime *rtd;
- rtd = card->rtd + i; + list_for_each_entry(rtd, &card->rtd_list, list) { if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI, strlen(CHT_CODEC_DAI))) return rtd->codec_dai; diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index 2c9cc5b..fee43bd 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -46,12 +46,9 @@ static struct snd_soc_jack_pin cht_bsw_headset_pins[] = {
static inline struct snd_soc_dai *cht_get_codec_dai(struct snd_soc_card *card) { - int i; + struct snd_soc_pcm_runtime *rtd;
- for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd; - - rtd = card->rtd + i; + list_for_each_entry(rtd, &card->rtd_list, list) { if (!strncmp(rtd->codec_dai->name, CHT_CODEC_DAI, strlen(CHT_CODEC_DAI))) return rtd->codec_dai; diff --git a/sound/soc/pxa/mioa701_wm9713.c b/sound/soc/pxa/mioa701_wm9713.c index a9615a57..4f6cafe 100644 --- a/sound/soc/pxa/mioa701_wm9713.c +++ b/sound/soc/pxa/mioa701_wm9713.c @@ -81,8 +81,12 @@ static int rear_amp_power(struct snd_soc_codec *codec, int power) static int rear_amp_event(struct snd_soc_dapm_widget *widget, struct snd_kcontrol *kctl, int event) { - struct snd_soc_codec *codec = widget->dapm->card->rtd[0].codec; + struct snd_soc_card *card = widget->dapm->card; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_codec *codec;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec = rtd->codec; return rear_amp_power(codec, SND_SOC_DAPM_EVENT_ON(event)); }
diff --git a/sound/soc/samsung/bells.c b/sound/soc/samsung/bells.c index e5f05e6..3dd246f 100644 --- a/sound/soc/samsung/bells.c +++ b/sound/soc/samsung/bells.c @@ -58,11 +58,16 @@ static int bells_set_bias_level(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[DAI_DSP_CODEC].codec_dai; - struct snd_soc_codec *codec = codec_dai->codec; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; + struct snd_soc_codec *codec; struct bells_drvdata *bells = card->drvdata; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_DSP_CODEC].name); + codec_dai = rtd->codec_dai; + codec = codec_dai->codec; + if (dapm->dev != codec_dai->dev) return 0;
@@ -99,11 +104,16 @@ static int bells_set_bias_level_post(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[DAI_DSP_CODEC].codec_dai; - struct snd_soc_codec *codec = codec_dai->codec; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; + struct snd_soc_codec *codec; struct bells_drvdata *bells = card->drvdata; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_DSP_CODEC].name); + codec_dai = rtd->codec_dai; + codec = codec_dai->codec; + if (dapm->dev != codec_dai->dev) return 0;
@@ -137,14 +147,22 @@ static int bells_set_bias_level_post(struct snd_soc_card *card, static int bells_late_probe(struct snd_soc_card *card) { struct bells_drvdata *bells = card->drvdata; - struct snd_soc_codec *wm0010 = card->rtd[DAI_AP_DSP].codec; - struct snd_soc_codec *codec = card->rtd[DAI_DSP_CODEC].codec; - struct snd_soc_dai *aif1_dai = card->rtd[DAI_DSP_CODEC].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_codec *wm0010; + struct snd_soc_codec *codec; + struct snd_soc_dai *aif1_dai; struct snd_soc_dai *aif2_dai; struct snd_soc_dai *aif3_dai; struct snd_soc_dai *wm9081_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_AP_DSP].name); + wm0010 = rtd->codec; + + rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_DSP_CODEC].name); + codec = rtd->codec; + aif1_dai = rtd->codec_dai; + ret = snd_soc_codec_set_sysclk(codec, ARIZONA_CLK_SYSCLK, ARIZONA_CLK_SRC_FLL1, bells->sysclk_rate, @@ -181,7 +199,8 @@ static int bells_late_probe(struct snd_soc_card *card) return ret; }
- aif2_dai = card->rtd[DAI_CODEC_CP].cpu_dai; + rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_CODEC_CP].name); + aif2_dai = rtd->cpu_dai;
ret = snd_soc_dai_set_sysclk(aif2_dai, ARIZONA_CLK_ASYNCCLK, 0, 0); if (ret != 0) { @@ -192,8 +211,9 @@ static int bells_late_probe(struct snd_soc_card *card) if (card->num_rtd == DAI_CODEC_SUB) return 0;
- aif3_dai = card->rtd[DAI_CODEC_SUB].cpu_dai; - wm9081_dai = card->rtd[DAI_CODEC_SUB].codec_dai; + rtd = snd_soc_get_pcm_runtime(card, card->dai_link[DAI_CODEC_SUB].name); + aif3_dai = rtd->cpu_dai; + wm9081_dai = rtd->codec_dai;
ret = snd_soc_dai_set_sysclk(aif3_dai, ARIZONA_CLK_SYSCLK, 0, 0); if (ret != 0) { diff --git a/sound/soc/samsung/littlemill.c b/sound/soc/samsung/littlemill.c index 31a820e..7cb204e 100644 --- a/sound/soc/samsung/littlemill.c +++ b/sound/soc/samsung/littlemill.c @@ -23,9 +23,13 @@ static int littlemill_set_bias_level(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *aif1_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *aif1_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + aif1_dai = rtd->codec_dai; + if (dapm->dev != aif1_dai->dev) return 0;
@@ -66,9 +70,13 @@ static int littlemill_set_bias_level_post(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *aif1_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *aif1_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + aif1_dai = rtd->codec_dai; + if (dapm->dev != aif1_dai->dev) return 0;
@@ -168,9 +176,13 @@ static int bbclk_ev(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { struct snd_soc_card *card = w->dapm->card; - struct snd_soc_dai *aif2_dai = card->rtd[1].cpu_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *aif2_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name); + aif2_dai = rtd->cpu_dai; + switch (event) { case SND_SOC_DAPM_PRE_PMU: ret = snd_soc_dai_set_pll(aif2_dai, WM8994_FLL2, @@ -245,11 +257,19 @@ static struct snd_soc_jack littlemill_headset;
static int littlemill_late_probe(struct snd_soc_card *card) { - struct snd_soc_codec *codec = card->rtd[0].codec; - struct snd_soc_dai *aif1_dai = card->rtd[0].codec_dai; - struct snd_soc_dai *aif2_dai = card->rtd[1].cpu_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_codec *codec; + struct snd_soc_dai *aif1_dai; + struct snd_soc_dai *aif2_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec = rtd->codec; + aif1_dai = rtd->codec_dai; + + rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name); + aif2_dai = rtd->cpu_dai; + ret = snd_soc_dai_set_sysclk(aif1_dai, WM8994_SYSCLK_MCLK2, 32768, SND_SOC_CLOCK_IN); if (ret < 0) diff --git a/sound/soc/samsung/odroidx2_max98090.c b/sound/soc/samsung/odroidx2_max98090.c index 596f118..0421727 100644 --- a/sound/soc/samsung/odroidx2_max98090.c +++ b/sound/soc/samsung/odroidx2_max98090.c @@ -25,10 +25,15 @@ static struct snd_soc_dai_link odroidx2_dai[];
static int odroidx2_late_probe(struct snd_soc_card *card) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; - struct snd_soc_dai *cpu_dai = card->rtd[0].cpu_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; + struct snd_soc_dai *cpu_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; + cpu_dai = rtd->cpu_dai; + ret = snd_soc_dai_set_sysclk(codec_dai, 0, MAX98090_MCLK, SND_SOC_CLOCK_IN);
diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c index 7651dc9..0969577 100644 --- a/sound/soc/samsung/snow.c +++ b/sound/soc/samsung/snow.c @@ -35,10 +35,15 @@ static struct snd_soc_dai_link snow_dai[] = {
static int snow_late_probe(struct snd_soc_card *card) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; - struct snd_soc_dai *cpu_dai = card->rtd[0].cpu_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; + struct snd_soc_dai *cpu_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; + cpu_dai = rtd->cpu_dai; + /* Set the MCLK rate for the codec */ ret = snd_soc_dai_set_sysclk(codec_dai, 0, FIN_PLL_RATE, SND_SOC_CLOCK_IN); diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c index d1ae21c..083ef5e 100644 --- a/sound/soc/samsung/speyside.c +++ b/sound/soc/samsung/speyside.c @@ -25,9 +25,13 @@ static int speyside_set_bias_level(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[1].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name); + codec_dai = rtd->codec_dai; + if (dapm->dev != codec_dai->dev) return 0;
@@ -57,9 +61,13 @@ static int speyside_set_bias_level_post(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[1].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name); + codec_dai = rtd->codec_dai; + if (dapm->dev != codec_dai->dev) return 0;
diff --git a/sound/soc/samsung/tobermory.c b/sound/soc/samsung/tobermory.c index 85ccfb7..3310eda 100644 --- a/sound/soc/samsung/tobermory.c +++ b/sound/soc/samsung/tobermory.c @@ -23,9 +23,13 @@ static int tobermory_set_bias_level(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; + if (dapm->dev != codec_dai->dev) return 0;
@@ -62,9 +66,13 @@ static int tobermory_set_bias_level_post(struct snd_soc_card *card, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level) { - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai *codec_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec_dai = rtd->codec_dai; + if (dapm->dev != codec_dai->dev) return 0;
@@ -170,10 +178,15 @@ static struct snd_soc_jack_pin tobermory_headset_pins[] = {
static int tobermory_late_probe(struct snd_soc_card *card) { - struct snd_soc_codec *codec = card->rtd[0].codec; - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; + struct snd_soc_pcm_runtime *rtd; + struct snd_soc_codec *codec; + struct snd_soc_dai *codec_dai; int ret;
+ rtd = snd_soc_get_pcm_runtime(card, card->dai_link[0].name); + codec = rtd->codec; + codec_dai = rtd->codec_dai; + ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK, 32768, SND_SOC_CLOCK_IN); if (ret < 0) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 1b63a03..fb78674 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -537,26 +537,76 @@ static inline void snd_soc_debugfs_exit(void) struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card, const char *dai_link, int stream) { - int i; + struct snd_soc_pcm_runtime *rtd;
- for (i = 0; i < card->num_links; i++) { - if (card->rtd[i].dai_link->no_pcm && - !strcmp(card->rtd[i].dai_link->name, dai_link)) - return card->rtd[i].pcm->streams[stream].substream; + list_for_each_entry(rtd, &card->rtd_list, list) { + if (rtd->dai_link->no_pcm && + !strcmp(rtd->dai_link->name, dai_link)) + return rtd->pcm->streams[stream].substream; } dev_dbg(card->dev, "ASoC: failed to find dai link %s\n", dai_link); return NULL; } EXPORT_SYMBOL_GPL(snd_soc_get_dai_substream);
+static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( + struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) +{ + struct snd_soc_pcm_runtime *rtd; + + rtd = devm_kzalloc(card->dev, + sizeof(struct snd_soc_pcm_runtime), GFP_KERNEL); + if (!rtd) + return NULL; + + rtd->card = card; + rtd->dai_link = dai_link; + rtd->codec_dais = devm_kzalloc(card->dev, + sizeof(struct snd_soc_dai *) * + dai_link->num_codecs, + GFP_KERNEL); + if (!rtd->codec_dais) + return NULL; + + return rtd; +} + +static void soc_free_pcm_runtime(struct snd_soc_card *card, + struct snd_soc_pcm_runtime *rtd) +{ + if (rtd->codec_dai) + devm_kfree(card->dev, rtd->codec_dais); + devm_kfree(card->dev, rtd); +} + +static void soc_add_pcm_runtime(struct snd_soc_card *card, + struct snd_soc_pcm_runtime *rtd) +{ + list_add_tail(&rtd->list, &card->rtd_list); + rtd->num = card->num_rtd; + card->num_rtd++; +} + +static void soc_remove_pcm_runtimes(struct snd_soc_card *card) +{ + struct snd_soc_pcm_runtime *rtd, *_rtd; + + list_for_each_entry_safe(rtd, _rtd, &card->rtd_list, list) { + list_del(&rtd->list); + soc_free_pcm_runtime(card, rtd); + } + + card->num_rtd = 0; +} + struct snd_soc_pcm_runtime *snd_soc_get_pcm_runtime(struct snd_soc_card *card, const char *dai_link) { - int i; + struct snd_soc_pcm_runtime *rtd;
- for (i = 0; i < card->num_links; i++) { - if (!strcmp(card->rtd[i].dai_link->name, dai_link)) - return &card->rtd[i]; + list_for_each_entry(rtd, &card->rtd_list, list) { + if (!strcmp(rtd->dai_link->name, dai_link)) + return rtd; } dev_dbg(card->dev, "ASoC: failed to find rtd %s\n", dai_link); return NULL; @@ -578,7 +628,8 @@ int snd_soc_suspend(struct device *dev) { struct snd_soc_card *card = dev_get_drvdata(dev); struct snd_soc_codec *codec; - int i, j; + struct snd_soc_pcm_runtime *rtd; + int i;
/* If the card is not initialized yet there is nothing to do */ if (!card->instantiated) @@ -595,13 +646,13 @@ int snd_soc_suspend(struct device *dev) snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D3hot);
/* mute any active DACs */ - for (i = 0; i < card->num_rtd; i++) { + list_for_each_entry(rtd, &card->rtd_list, list) {
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
- for (j = 0; j < card->rtd[i].num_codecs; j++) { - struct snd_soc_dai *dai = card->rtd[i].codec_dais[j]; + for (i = 0; i < rtd->num_codecs; i++) { + struct snd_soc_dai *dai = rtd->codec_dais[i]; struct snd_soc_dai_driver *drv = dai->driver;
if (drv->ops->digital_mute && dai->playback_active) @@ -610,20 +661,20 @@ int snd_soc_suspend(struct device *dev) }
/* suspend all pcms */ - for (i = 0; i < card->num_rtd; i++) { - if (card->rtd[i].dai_link->ignore_suspend) + list_for_each_entry(rtd, &card->rtd_list, list) { + if (rtd->dai_link->ignore_suspend) continue;
- snd_pcm_suspend_all(card->rtd[i].pcm); + snd_pcm_suspend_all(rtd->pcm); }
if (card->suspend_pre) card->suspend_pre(card);
- for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai *cpu_dai = card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &card->rtd_list, list) { + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
if (cpu_dai->driver->suspend && !cpu_dai->driver->bus_control) @@ -631,19 +682,19 @@ int snd_soc_suspend(struct device *dev) }
/* close any waiting streams */ - for (i = 0; i < card->num_rtd; i++) - flush_delayed_work(&card->rtd[i].delayed_work); + list_for_each_entry(rtd, &card->rtd_list, list) + flush_delayed_work(&rtd->delayed_work);
- for (i = 0; i < card->num_rtd; i++) { + list_for_each_entry(rtd, &card->rtd_list, list) {
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
- snd_soc_dapm_stream_event(&card->rtd[i], + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_SUSPEND);
- snd_soc_dapm_stream_event(&card->rtd[i], + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE, SND_SOC_DAPM_STREAM_SUSPEND); } @@ -690,10 +741,10 @@ int snd_soc_suspend(struct device *dev) } }
- for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai *cpu_dai = card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &card->rtd_list, list) { + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
if (cpu_dai->driver->suspend && cpu_dai->driver->bus_control) @@ -717,8 +768,9 @@ static void soc_resume_deferred(struct work_struct *work) { struct snd_soc_card *card = container_of(work, struct snd_soc_card, deferred_resume_work); + struct snd_soc_pcm_runtime *rtd; struct snd_soc_codec *codec; - int i, j; + int i;
/* our power state is still SNDRV_CTL_POWER_D3hot from suspend time, * so userspace apps are blocked from touching us @@ -733,10 +785,10 @@ static void soc_resume_deferred(struct work_struct *work) card->resume_pre(card);
/* resume control bus DAIs */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai *cpu_dai = card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &card->rtd_list, list) { + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
if (cpu_dai->driver->resume && cpu_dai->driver->bus_control) @@ -751,28 +803,28 @@ static void soc_resume_deferred(struct work_struct *work) } }
- for (i = 0; i < card->num_rtd; i++) { + list_for_each_entry(rtd, &card->rtd_list, list) {
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
- snd_soc_dapm_stream_event(&card->rtd[i], + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_RESUME);
- snd_soc_dapm_stream_event(&card->rtd[i], + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE, SND_SOC_DAPM_STREAM_RESUME); }
/* unmute any active DACs */ - for (i = 0; i < card->num_rtd; i++) { + list_for_each_entry(rtd, &card->rtd_list, list) {
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
- for (j = 0; j < card->rtd[i].num_codecs; j++) { - struct snd_soc_dai *dai = card->rtd[i].codec_dais[j]; + for (i = 0; i < rtd->num_codecs; i++) { + struct snd_soc_dai *dai = rtd->codec_dais[i]; struct snd_soc_dai_driver *drv = dai->driver;
if (drv->ops->digital_mute && dai->playback_active) @@ -780,10 +832,10 @@ static void soc_resume_deferred(struct work_struct *work) } }
- for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai *cpu_dai = card->rtd[i].cpu_dai; + list_for_each_entry(rtd, &card->rtd_list, list) { + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- if (card->rtd[i].dai_link->ignore_suspend) + if (rtd->dai_link->ignore_suspend) continue;
if (cpu_dai->driver->resume && !cpu_dai->driver->bus_control) @@ -808,15 +860,14 @@ int snd_soc_resume(struct device *dev) { struct snd_soc_card *card = dev_get_drvdata(dev); bool bus_control = false; - int i; + struct snd_soc_pcm_runtime *rtd;
/* If the card is not initialized yet there is nothing to do */ if (!card->instantiated) return 0;
/* activate pins from sleep state */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + 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; int j; @@ -837,8 +888,8 @@ int snd_soc_resume(struct device *dev) * have that problem and may take a substantial amount of time to resume * due to I/O costs and anti-pop so handle them out of line. */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai *cpu_dai = card->rtd[i].cpu_dai; + 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; } if (bus_control) { @@ -913,16 +964,26 @@ static struct snd_soc_dai *snd_soc_find_dai( static int soc_bind_dai_link(struct snd_soc_card *card, int num) { struct snd_soc_dai_link *dai_link = &card->dai_link[num]; - struct snd_soc_pcm_runtime *rtd = &card->rtd[num]; + 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 **codec_dais = rtd->codec_dais; + struct snd_soc_dai **codec_dais; struct snd_soc_platform *platform; const char *platform_name; int i;
dev_dbg(card->dev, "ASoC: binding %s at idx %d\n", dai_link->name, num);
+ if (snd_soc_get_pcm_runtime(card, dai_link->name)) { + dev_dbg(card->dev, "ASoC: dai link %s already bound\n", + dai_link->name); + return 0; + } + + 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; @@ -930,18 +991,19 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num) if (!rtd->cpu_dai) { dev_err(card->dev, "ASoC: CPU DAI %s not registered\n", dai_link->cpu_dai_name); - return -EPROBE_DEFER; + goto _err_defer; }
rtd->num_codecs = dai_link->num_codecs;
/* Find CODEC from registered CODECs */ + codec_dais = rtd->codec_dais; for (i = 0; i < rtd->num_codecs; i++) { codec_dais[i] = snd_soc_find_dai(&codecs[i]); if (!codec_dais[i]) { dev_err(card->dev, "ASoC: CODEC DAI %s not registered\n", codecs[i].dai_name); - return -EPROBE_DEFER; + goto _err_defer; } }
@@ -973,9 +1035,12 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num) return -EPROBE_DEFER; }
- card->num_rtd++; - + soc_add_pcm_runtime(card, rtd); return 0; + +_err_defer: + soc_free_pcm_runtime(card, rtd); + return -EPROBE_DEFER; }
static void soc_remove_component(struct snd_soc_component *component) @@ -1014,9 +1079,9 @@ static void soc_remove_dai(struct snd_soc_dai *dai, int order) } }
-static void soc_remove_link_dais(struct snd_soc_card *card, int num, int order) +static void soc_remove_link_dais(struct snd_soc_card *card, + struct snd_soc_pcm_runtime *rtd, int order) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[num]; int i;
/* unregister the rtd device */ @@ -1032,10 +1097,9 @@ static void soc_remove_link_dais(struct snd_soc_card *card, int num, int order) soc_remove_dai(rtd->cpu_dai, order); }
-static void soc_remove_link_components(struct snd_soc_card *card, int num, - int order) +static void soc_remove_link_components(struct snd_soc_card *card, + struct snd_soc_pcm_runtime *rtd, int order) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[num]; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_platform *platform = rtd->platform; struct snd_soc_component *component; @@ -1061,21 +1125,20 @@ static void soc_remove_link_components(struct snd_soc_card *card, int num,
static void soc_remove_dai_links(struct snd_soc_card *card) { - int dai, order; + int order; + struct snd_soc_pcm_runtime *rtd;
for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - for (dai = 0; dai < card->num_rtd; dai++) - soc_remove_link_dais(card, dai, order); + list_for_each_entry(rtd, &card->rtd_list, list) + soc_remove_link_dais(card, rtd, order); }
for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - for (dai = 0; dai < card->num_rtd; dai++) - soc_remove_link_components(card, dai, order); + list_for_each_entry(rtd, &card->rtd_list, list) + soc_remove_link_components(card, rtd, order); } - - card->num_rtd = 0; }
static void soc_set_name_prefix(struct snd_soc_card *card, @@ -1220,10 +1283,10 @@ static int soc_post_component_init(struct snd_soc_pcm_runtime *rtd, return 0; }
-static int soc_probe_link_components(struct snd_soc_card *card, int num, +static int soc_probe_link_components(struct snd_soc_card *card, + struct snd_soc_pcm_runtime *rtd, int order) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[num]; struct snd_soc_platform *platform = rtd->platform; struct snd_soc_component *component; int i, ret; @@ -1319,15 +1382,15 @@ static int soc_link_dai_widgets(struct snd_soc_card *card, return 0; }
-static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order) +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 = &card->dai_link[num]; - struct snd_soc_pcm_runtime *rtd = &card->rtd[num]; + 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", - card->name, num, order); + card->name, rtd->num, order);
/* set default power off timeout */ rtd->pmdown_time = pmdown_time; @@ -1372,7 +1435,7 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order)
if (cpu_dai->driver->compress_dai) { /*create compress_device"*/ - ret = soc_new_compress(rtd, num); + ret = soc_new_compress(rtd, rtd->num); if (ret < 0) { dev_err(card->dev, "ASoC: can't create compress %s\n", dai_link->stream_name); @@ -1382,7 +1445,7 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order)
if (!dai_link->params) { /* create the pcm */ - ret = soc_new_pcm(rtd, num); + ret = soc_new_pcm(rtd, rtd->num); if (ret < 0) { dev_err(card->dev, "ASoC: can't create pcm %s :%d\n", dai_link->stream_name, ret); @@ -1552,6 +1615,7 @@ EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt); static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec; + struct snd_soc_pcm_runtime *rtd; int ret, i, order;
mutex_lock(&client_mutex); @@ -1624,8 +1688,8 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) /* probe all components used by DAI links on this card */ for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - for (i = 0; i < card->num_links; i++) { - ret = soc_probe_link_components(card, i, order); + list_for_each_entry(rtd, &card->rtd_list, list) { + ret = soc_probe_link_components(card, rtd, order); if (ret < 0) { dev_err(card->dev, "ASoC: failed to instantiate card %d\n", @@ -1638,8 +1702,8 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) /* probe all DAI links on this card */ for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - for (i = 0; i < card->num_links; i++) { - ret = soc_probe_link_dais(card, i, order); + list_for_each_entry(rtd, &card->rtd_list, list) { + ret = soc_probe_link_dais(card, rtd, order); if (ret < 0) { dev_err(card->dev, "ASoC: failed to instantiate card %d\n", @@ -1733,6 +1797,7 @@ card_probe_error: snd_card_free(card->snd_card);
base_error: + soc_remove_pcm_runtimes(card); mutex_unlock(&card->mutex); mutex_unlock(&client_mutex);
@@ -1763,13 +1828,12 @@ static int soc_probe(struct platform_device *pdev)
static int soc_cleanup_card_resources(struct snd_soc_card *card) { + struct snd_soc_pcm_runtime *rtd; int i;
/* make sure any delayed work runs */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + list_for_each_entry(rtd, &card->rtd_list, list) flush_delayed_work(&rtd->delayed_work); - }
/* remove auxiliary devices */ for (i = 0; i < card->num_aux_devs; i++) @@ -1777,6 +1841,7 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
/* remove and free each DAI */ soc_remove_dai_links(card); + soc_remove_pcm_runtimes(card);
soc_cleanup_card_debugfs(card);
@@ -1803,29 +1868,26 @@ static int soc_remove(struct platform_device *pdev) int snd_soc_poweroff(struct device *dev) { struct snd_soc_card *card = dev_get_drvdata(dev); - int i; + struct snd_soc_pcm_runtime *rtd;
if (!card->instantiated) return 0;
/* Flush out pmdown_time work - we actually do want to run it * now, we're shutting down so no imminent restart. */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + list_for_each_entry(rtd, &card->rtd_list, list) flush_delayed_work(&rtd->delayed_work); - }
snd_soc_dapm_shutdown(card);
/* deactivate pins to sleep state */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + list_for_each_entry(rtd, &card->rtd_list, list) { struct snd_soc_dai *cpu_dai = rtd->cpu_dai; - int j; + int i;
pinctrl_pm_select_sleep_state(cpu_dai->dev); - for (j = 0; j < rtd->num_codecs; j++) { - struct snd_soc_dai *codec_dai = rtd->codec_dais[j]; + 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); } } @@ -2337,6 +2399,7 @@ static int snd_soc_init_multicodec(struct snd_soc_card *card, int snd_soc_register_card(struct snd_soc_card *card) { int i, j, ret; + struct snd_soc_pcm_runtime *rtd;
if (!card->name || !card->dev) return -EINVAL; @@ -2408,25 +2471,15 @@ int snd_soc_register_card(struct snd_soc_card *card)
snd_soc_initialize_card_lists(card);
- card->rtd = devm_kzalloc(card->dev, + INIT_LIST_HEAD(&card->rtd_list); + card->num_rtd = 0; + + card->rtd_aux = devm_kzalloc(card->dev, sizeof(struct snd_soc_pcm_runtime) * - (card->num_links + card->num_aux_devs), + card->num_aux_devs, GFP_KERNEL); - if (card->rtd == NULL) + if (card->rtd_aux == NULL) return -ENOMEM; - card->num_rtd = 0; - card->rtd_aux = &card->rtd[card->num_links]; - - for (i = 0; i < card->num_links; i++) { - card->rtd[i].card = card; - card->rtd[i].dai_link = &card->dai_link[i]; - card->rtd[i].codec_dais = devm_kzalloc(card->dev, - sizeof(struct snd_soc_dai *) * - (card->rtd[i].dai_link->num_codecs), - GFP_KERNEL); - if (card->rtd[i].codec_dais == NULL) - return -ENOMEM; - }
for (i = 0; i < card->num_aux_devs; i++) card->rtd_aux[i].card = card; @@ -2442,8 +2495,7 @@ int snd_soc_register_card(struct snd_soc_card *card) return ret;
/* deactivate pins to sleep state */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + list_for_each_entry(rtd, &card->rtd_list, list) { struct snd_soc_dai *cpu_dai = rtd->cpu_dai; int j;
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 7482c5d..6d1f066 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3861,13 +3861,10 @@ static void soc_dapm_dai_stream_event(struct snd_soc_dai *dai, int stream,
void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) { - struct snd_soc_pcm_runtime *rtd = card->rtd; - int i; + struct snd_soc_pcm_runtime *rtd;
/* for each BE DAI link... */ - for (i = 0; i < card->num_rtd; i++) { - rtd = &card->rtd[i]; - + list_for_each_entry(rtd, &card->rtd_list, list) { /* * dynamic FE links have no fixed DAI mapping. * CODEC<->CODEC links have no direct connection. diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 70e4b9d..0246019 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1166,11 +1166,10 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, struct snd_soc_dapm_widget *widget, int stream) { struct snd_soc_pcm_runtime *be; - int i, j; + int i;
if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - for (i = 0; i < card->num_links; i++) { - be = &card->rtd[i]; + list_for_each_entry(be, &card->rtd_list, list) {
if (!be->dai_link->no_pcm) continue; @@ -1178,16 +1177,15 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, if (be->cpu_dai->playback_widget == widget) return be;
- for (j = 0; j < be->num_codecs; j++) { - struct snd_soc_dai *dai = be->codec_dais[j]; + for (i = 0; i < be->num_codecs; i++) { + struct snd_soc_dai *dai = be->codec_dais[i]; if (dai->playback_widget == widget) return be; } } } else {
- for (i = 0; i < card->num_links; i++) { - be = &card->rtd[i]; + list_for_each_entry(be, &card->rtd_list, list) {
if (!be->dai_link->no_pcm) continue; @@ -1195,8 +1193,8 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, if (be->cpu_dai->capture_widget == widget) return be;
- for (j = 0; j < be->num_codecs; j++) { - struct snd_soc_dai *dai = be->codec_dais[j]; + for (i = 0; i < be->num_codecs; i++) { + struct snd_soc_dai *dai = be->codec_dais[i]; if (dai->capture_widget == widget) return be; } @@ -2296,12 +2294,12 @@ static int dpcm_run_old_update(struct snd_soc_pcm_runtime *fe, int stream) */ int soc_dpcm_runtime_update(struct snd_soc_card *card) { - int i, old, new, paths; + struct snd_soc_pcm_runtime *fe; + int old, new, paths;
mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_RUNTIME); - for (i = 0; i < card->num_rtd; i++) { + list_for_each_entry(fe, &card->rtd_list, list) { struct snd_soc_dapm_widget_list *list; - struct snd_soc_pcm_runtime *fe = &card->rtd[i];
/* make sure link is FE */ if (!fe->dai_link->dynamic) diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 2160400..e485278 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -199,7 +199,8 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
static int tegra_wm8903_remove(struct snd_soc_card *card) { - struct snd_soc_pcm_runtime *rtd = &(card->rtd[0]); + struct snd_soc_pcm_runtime *rtd = + snd_soc_get_pcm_runtime(card, card->dai_link[0].name); struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_codec *codec = codec_dai->codec; struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card);
On Mon, Aug 10, 2015 at 10:45:28PM +0800, mengdong.lin@intel.com wrote:
- rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name);
- codec_dai = rtd->codec_dai;
For ease of review please split the addition of this new interface for getting the runtime from the change to the implementation - the same function can be written in terms of a list. As ever please make one change per commit :(
+static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
- struct snd_soc_card *card, struct snd_soc_dai_link *dai_link)
+{
- struct snd_soc_pcm_runtime *rtd;
- rtd = devm_kzalloc(card->dev,
sizeof(struct snd_soc_pcm_runtime), GFP_KERNEL);- if (!rtd)
return NULL;
Why are we allocating something with devm_kzalloc()...
+static void soc_free_pcm_runtime(struct snd_soc_card *card,
- struct snd_soc_pcm_runtime *rtd)
+{
- if (rtd->codec_dai)
devm_kfree(card->dev, rtd->codec_dais);- devm_kfree(card->dev, rtd);
+}
...things that we later explicitly free in (hopefully?) all cases.
+static void soc_remove_pcm_runtimes(struct snd_soc_card *card) +{
- struct snd_soc_pcm_runtime *rtd, *_rtd;
- list_for_each_entry_safe(rtd, _rtd, &card->rtd_list, list) {
list_del(&rtd->list);soc_free_pcm_runtime(card, rtd);- }
- card->num_rtd = 0;
+}
Why doesn't the free function do the list removal?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Saturday, August 15, 2015 4:03 AM
On Mon, Aug 10, 2015 at 10:45:28PM +0800, mengdong.lin@intel.com wrote:
- rtd = snd_soc_get_pcm_runtime(card, card->dai_link[1].name);
- codec_dai = rtd->codec_dai;
For ease of review please split the addition of this new interface for getting the runtime from the change to the implementation - the same function can be written in terms of a list. As ever please make one change per commit :(
Okay. I'll split adding snd_soc_get_pcm_runtime() to a new commit.
+static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
- struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) {
- struct snd_soc_pcm_runtime *rtd;
- rtd = devm_kzalloc(card->dev,
sizeof(struct snd_soc_pcm_runtime), GFP_KERNEL);- if (!rtd)
return NULL;Why are we allocating something with devm_kzalloc()...
I'll change to use kzalloc() and free the memory for the error path.
+static void soc_free_pcm_runtime(struct snd_soc_card *card,
- struct snd_soc_pcm_runtime *rtd)
+{
- if (rtd->codec_dai)
devm_kfree(card->dev, rtd->codec_dais);- devm_kfree(card->dev, rtd);
+}
...things that we later explicitly free in (hopefully?) all cases.
Okay.
+static void soc_remove_pcm_runtimes(struct snd_soc_card *card) {
- struct snd_soc_pcm_runtime *rtd, *_rtd;
- list_for_each_entry_safe(rtd, _rtd, &card->rtd_list, list) {
list_del(&rtd->list);soc_free_pcm_runtime(card, rtd);- }
- card->num_rtd = 0;
+}
Why doesn't the free function do the list removal?
Sorry, I'll add the list removal.
Thanks Mengdong
From: Mengdong Lin mengdong.lin@intel.com
Define soc_init_dai_link() to wrap link initialization, to reuse it later by snd_soc_instantiate_card() when adding new DAI links from topology in component probing phase.
Move static func snd_soc_init_multicodec(), so that it can be reused by soc_init_dai_link(). This saves adding a function declaration for snd_soc_init_multicodec().
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index fb78674..e9e8956 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1141,6 +1141,100 @@ static void soc_remove_dai_links(struct snd_soc_card *card) } }
+static int snd_soc_init_multicodec(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link) +{ + /* Legacy codec/codec_dai link is a single entry in multicodec */ + if (dai_link->codec_name || dai_link->codec_of_node || + dai_link->codec_dai_name) { + dai_link->num_codecs = 1; + + dai_link->codecs = devm_kzalloc(card->dev, + sizeof(struct snd_soc_dai_link_component), + GFP_KERNEL); + if (!dai_link->codecs) + return -ENOMEM; + + dai_link->codecs[0].name = dai_link->codec_name; + dai_link->codecs[0].of_node = dai_link->codec_of_node; + dai_link->codecs[0].dai_name = dai_link->codec_dai_name; + } + + if (!dai_link->codecs) { + dev_err(card->dev, "ASoC: DAI link has no CODECs\n"); + return -EINVAL; + } + + return 0; +} + +static int soc_init_dai_link(struct snd_soc_card *card, + struct snd_soc_dai_link *link) +{ + int i, ret; + + ret = snd_soc_init_multicodec(card, link); + if (ret) { + dev_err(card->dev, "ASoC: failed to init multicodec\n"); + return ret; + } + + for (i = 0; i < link->num_codecs; i++) { + /* + * Codec must be specified by 1 of name or OF node, + * not both or neither. + */ + if (!!link->codecs[i].name == + !!link->codecs[i].of_node) { + dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n", + link->name); + return -EINVAL; + } + /* Codec DAI name must be specified */ + if (!link->codecs[i].dai_name) { + dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n", + link->name); + return -EINVAL; + } + } + + /* + * Platform may be specified by either name or OF node, but + * can be left unspecified, and a dummy platform will be used. + */ + if (link->platform_name && link->platform_of_node) { + dev_err(card->dev, + "ASoC: Both platform name/of_node are set for %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->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; +} + static void soc_set_name_prefix(struct snd_soc_card *card, struct snd_soc_component *component) { @@ -2363,33 +2457,6 @@ int snd_soc_dai_digital_mute(struct snd_soc_dai *dai, int mute, } EXPORT_SYMBOL_GPL(snd_soc_dai_digital_mute);
-static int snd_soc_init_multicodec(struct snd_soc_card *card, - struct snd_soc_dai_link *dai_link) -{ - /* Legacy codec/codec_dai link is a single entry in multicodec */ - if (dai_link->codec_name || dai_link->codec_of_node || - dai_link->codec_dai_name) { - dai_link->num_codecs = 1; - - dai_link->codecs = devm_kzalloc(card->dev, - sizeof(struct snd_soc_dai_link_component), - GFP_KERNEL); - if (!dai_link->codecs) - return -ENOMEM; - - dai_link->codecs[0].name = dai_link->codec_name; - dai_link->codecs[0].of_node = dai_link->codec_of_node; - dai_link->codecs[0].dai_name = dai_link->codec_dai_name; - } - - if (!dai_link->codecs) { - dev_err(card->dev, "ASoC: DAI link has no CODECs\n"); - return -EINVAL; - } - - return 0; -} - /** * snd_soc_register_card - Register a card with the ASoC core * @@ -2398,7 +2465,7 @@ static int snd_soc_init_multicodec(struct snd_soc_card *card, */ int snd_soc_register_card(struct snd_soc_card *card) { - int i, j, ret; + int i, ret; struct snd_soc_pcm_runtime *rtd;
if (!card->name || !card->dev) @@ -2407,63 +2474,11 @@ int snd_soc_register_card(struct snd_soc_card *card) for (i = 0; i < card->num_links; i++) { struct snd_soc_dai_link *link = &card->dai_link[i];
- ret = snd_soc_init_multicodec(card, link); + ret = soc_init_dai_link(card, link); if (ret) { - dev_err(card->dev, "ASoC: failed to init multicodec\n"); - return ret; - } - - for (j = 0; j < link->num_codecs; j++) { - /* - * Codec must be specified by 1 of name or OF node, - * not both or neither. - */ - if (!!link->codecs[j].name == - !!link->codecs[j].of_node) { - dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n", - link->name); - return -EINVAL; - } - /* Codec DAI name must be specified */ - if (!link->codecs[j].dai_name) { - dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n", - link->name); - return -EINVAL; - } - } - - /* - * Platform may be specified by either name or OF node, but - * can be left unspecified, and a dummy platform will be used. - */ - if (link->platform_name && link->platform_of_node) { - dev_err(card->dev, - "ASoC: Both platform name/of_node are set for %s\n", + dev_err(card->dev, "ASoC: failed to init link %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->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 ret; } }
From: Mengdong Lin mengdong.lin@intel.com
Just code refactoring, to reuse it if new DAI Links are added later based on topology in component probing phase.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e9e8956..ddfdd1d 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -961,9 +961,9 @@ static struct snd_soc_dai *snd_soc_find_dai( return NULL; }
-static int soc_bind_dai_link(struct snd_soc_card *card, int num) +static int soc_bind_dai_link(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link) { - struct snd_soc_dai_link *dai_link = &card->dai_link[num]; 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; @@ -972,7 +972,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num) const char *platform_name; int i;
- dev_dbg(card->dev, "ASoC: binding %s at idx %d\n", dai_link->name, num); + dev_dbg(card->dev, "ASoC: binding %s\n", dai_link->name);
if (snd_soc_get_pcm_runtime(card, dai_link->name)) { dev_dbg(card->dev, "ASoC: dai link %s already bound\n", @@ -1717,7 +1717,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
/* bind DAIs */ for (i = 0; i < card->num_links; i++) { - ret = soc_bind_dai_link(card, i); + ret = soc_bind_dai_link(card, &card->dai_link[i]); if (ret != 0) goto base_error; }
From: Mengdong Lin mengdong.lin@intel.com
Implement a dai link list for the soc card. And API snd_soc_add_dai_link() is defined for machine drivers to add DAI links dynamically based on topology.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index e2b7b0f..79891e2 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1011,6 +1011,8 @@ struct snd_soc_dai_link {
/* pmdown_time is ignored at stop */ unsigned int ignore_pmdown_time:1; + + struct list_head list; /* dai link list of the soc card */ };
struct snd_soc_codec_conf { @@ -1078,8 +1080,11 @@ struct snd_soc_card { long pmdown_time;
/* CPU <--> Codec DAI links */ - struct snd_soc_dai_link *dai_link; - int num_links; + struct snd_soc_dai_link *dai_link; /* predefined links only */ + int num_links; /* predefined links only */ + struct list_head dai_link_list; /* all links except the dummy ones */ + int num_dai_links; + struct list_head rtd_list; int num_rtd;
@@ -1619,6 +1624,9 @@ int snd_soc_of_get_dai_link_codecs(struct device *dev, struct device_node *of_node, struct snd_soc_dai_link *dai_link);
+void snd_soc_add_dai_link(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link); + #include <sound/soc-dai.h>
#ifdef CONFIG_DEBUG_FS diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index ddfdd1d..58eac57 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1235,6 +1235,27 @@ static int soc_init_dai_link(struct snd_soc_card *card, return 0; }
+/** + * snd_soc_add_dai_link - Add a DAI link dynamically + * @card: The ASoC card to which the DAI link is added + * @dai_link: The new DAI link to add + * + * This function adds a DAI link to the ASoC card, and creates a ASoC + * runtime for the link. + * + * Note: For adding DAI links dynamically by machine drivers based on + * the topology info when probing the platform component. And machine + * drivers can still define static DAI links in dai_link array. + */ +void snd_soc_add_dai_link(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link) +{ + lockdep_assert_held(&client_mutex); + list_add_tail(&dai_link->list, &card->dai_link_list); + card->num_dai_links++; +} +EXPORT_SYMBOL_GPL(snd_soc_add_dai_link); + static void soc_set_name_prefix(struct snd_soc_card *card, struct snd_soc_component *component) { @@ -1729,6 +1750,10 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) goto base_error; }
+ /* all predefined resources are ready, store non-dummy links */ + for (i = 0; i < card->num_links; i++) + snd_soc_add_dai_link(card, card->dai_link+i); + /* initialize the register cache for each available codec */ list_for_each_entry(codec, &codec_list, list) { if (codec->cache_init) @@ -2486,6 +2511,9 @@ int snd_soc_register_card(struct snd_soc_card *card)
snd_soc_initialize_card_lists(card);
+ INIT_LIST_HEAD(&card->dai_link_list); + card->num_dai_links = 0; + INIT_LIST_HEAD(&card->rtd_list); card->num_rtd = 0;
From: Mengdong Lin mengdong.lin@intel.com
Add dummy flag to a DAI link and PCM runtime. A dummy link will create a dummy PCM runtime.
A machine driver can define dummy dai links to specify the expected platform and codec. And its dummy runtime is used to probe the platform and codec components, which can bring real DAI/DAI links based on topology. After components probing, all dummy PCM runtimes will be removed.
Only non-dummy PCM runtimes will be assigned a valid number and create PCM streams.
The ASoC core will find the platform for a dummy DAI link, assuming the cpu DAI can be dummy but the platform can bring the topology info.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 79891e2..7e04ec0 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -970,6 +970,11 @@ struct snd_soc_dai_link {
enum snd_soc_dpcm_trigger trigger[2]; /* trigger type for DPCM */
+ /* This DAI link is dummy. Machine drivers can define static dummy + * links to specify platform/codec with topology. + */ + bool dummy; + /* codec/machine specific init - e.g. add machine controls */ int (*init)(struct snd_soc_pcm_runtime *rtd);
@@ -1087,6 +1092,7 @@ struct snd_soc_card {
struct list_head rtd_list; int num_rtd; + int num_rtd_dev; /* monotonic increase and exclude dummy ones */
/* optional codec specific configuration */ struct snd_soc_codec_conf *codec_conf; @@ -1181,6 +1187,7 @@ struct snd_soc_pcm_runtime { struct dentry *debugfs_dpcm_state; #endif
+ bool dummy; unsigned int num; /* 0-based and monotonic increasing */ struct list_head list; /* rtd list of the soc card */ }; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 58eac57..f552fa0 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -561,6 +561,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
rtd->card = card; rtd->dai_link = dai_link; + rtd->dummy = dai_link->dummy; rtd->codec_dais = devm_kzalloc(card->dev, sizeof(struct snd_soc_dai *) * dai_link->num_codecs, @@ -583,10 +584,28 @@ static void soc_add_pcm_runtime(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd) { list_add_tail(&rtd->list, &card->rtd_list); - rtd->num = card->num_rtd; + + if (!rtd->dummy) { + rtd->num = card->num_rtd_dev; + card->num_rtd_dev++; + } + card->num_rtd++; }
+static void soc_remove_dummy_pcm_runtimes(struct snd_soc_card *card) +{ + struct snd_soc_pcm_runtime *rtd, *_rtd; + + list_for_each_entry_safe(rtd, _rtd, &card->rtd_list, list) { + if (rtd->dummy) { + list_del(&rtd->list); + soc_free_pcm_runtime(card, rtd); + card->num_rtd--; + } + } +} + static void soc_remove_pcm_runtimes(struct snd_soc_card *card) { struct snd_soc_pcm_runtime *rtd, *_rtd; @@ -1007,6 +1026,15 @@ static int soc_bind_dai_link(struct snd_soc_card *card, } }
+ /* Dummy link can use dummy cpu DAI but set platform to get topology */ + if (dai_link->dummy && dai_link->platform_name) { + if (!soc_find_component(NULL, dai_link->platform_name)) { + dev_err(card->dev, "ASoC: Platform %s not registered\n", + dai_link->platform_name); + goto _err_defer; + } + } + /* Single codec links expect codec and codec_dai in runtime data */ rtd->codec_dai = codec_dais[0]; rtd->codec = rtd->codec_dai->codec; @@ -1250,6 +1278,10 @@ static int soc_init_dai_link(struct snd_soc_card *card, void snd_soc_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) { + /* Dummy links should be static, so overlook them */ + if (dai_link->dummy) + return; + lockdep_assert_held(&client_mutex); list_add_tail(&dai_link->list, &card->dai_link_list); card->num_dai_links++; @@ -1818,6 +1850,9 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) } }
+ /* no longer need dummy runtimes */ + soc_remove_dummy_pcm_runtimes(card); + /* probe all DAI links on this card */ for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { @@ -2516,6 +2551,7 @@ int snd_soc_register_card(struct snd_soc_card *card)
INIT_LIST_HEAD(&card->rtd_list); card->num_rtd = 0; + card->num_rtd_dev = 0;
card->rtd_aux = devm_kzalloc(card->dev, sizeof(struct snd_soc_pcm_runtime) *
On Mon, Aug 10, 2015 at 10:47:47PM +0800, mengdong.lin@intel.com wrote:
Add dummy flag to a DAI link and PCM runtime. A dummy link will create a dummy PCM runtime.
A machine driver can define dummy dai links to specify the expected platform and codec. And its dummy runtime is used to probe the platform and codec components, which can bring real DAI/DAI links based on topology. After components probing, all dummy PCM runtimes will be removed.
This isn't very clear to me either in terms of the description or the interface. As far as I can tell these links that are being created here are placeholders representing links that will actually be instantiated later but if that's the case they're not really dummies as I'd understand them, they are real links which physically exist and will be created later by topology data. If we are going to do things this way we need a better way of talking about these not yet instantiated links.
I'm really confused as to why physical links that exist in the board are coming in via topology information loaded from userspace - it's not like the DSP firmware is going to be able to physically connect the DSP to things where no links existed before. I'd expect that these links are connections between physical devices which just exist in the system and that while the topology information might leave some of them dangling they wouldn't be created or removed.
Presumably we're also going to need to handle the case where the firmware is unloaded and we destroy the links if we do this...
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Saturday, August 15, 2015 4:22 AM
On Mon, Aug 10, 2015 at 10:47:47PM +0800, mengdong.lin@intel.com wrote:
Add dummy flag to a DAI link and PCM runtime. A dummy link will create a dummy PCM runtime.
A machine driver can define dummy dai links to specify the expected platform and codec. And its dummy runtime is used to probe the platform and codec components, which can bring real DAI/DAI links based
on topology.
After components probing, all dummy PCM runtimes will be removed.
This isn't very clear to me either in terms of the description or the interface. As far as I can tell these links that are being created here are placeholders representing links that will actually be instantiated later but if that's the case they're not really dummies as I'd understand them, they are real links which physically exist and will be created later by topology data. If we are going to do things this way we need a better way of talking about these not yet instantiated links.
(Sorry. I'm still trying to enable my 2nd mail account & client. So this reply can still have format problems.)
These *dummy* links are not place holders of real physical links. So they cannot come from ACPI but from topology.
Here is an example used in my test on Broadwell:
/* broadwell digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link broadwell_rt286_dais[] = { /* Front End dummy DAI links */ { .name = "Dummy System PCM", .stream_name = "Dummy System Playback/Capture", .cpu_dai_name = "snd-soc-dummy-dai", .platform_name = "haswell-pcm-audio", .dynamic = 1, .codec_name = "snd-soc-dummy", .codec_dai_name = "snd-soc-dummy-dai", } ... All real FE DAI links will no longer be in this link array.
We use this dummy DAI link to avoid snd_soc_instantiate_card() fail on binding DAI links.
And since this dummy link specify the platform "haswell-pcm-audio", soc_bind_dai_link() will promise the platform component is there. And then platform probing will load topology and the topology core will create the real FE DAIs.
And the machine driver will get notified when a new FE DAI is added to the ASoC core. It can create FE links in the callback context according to the new FE DAI info.
This seems to give us small code change.
Thanks Mengdong
I'm really confused as to why physical links that exist in the board are coming in via topology information loaded from userspace - it's not like the DSP firmware is going to be able to physically connect the DSP to things where no links existed before. I'd expect that these links are connections between physical devices which just exist in the system and that while the topology information might leave some of them dangling they wouldn't be created or removed.
Presumably we're also going to need to handle the case where the firmware is unloaded and we destroy the links if we do this...
Yes. The DAI/DAI links created by topology should be destroyed when unloading the topology component.
Now there is no product request to unload/change the firmware as well as its topology in the middle. We may consider this later.
Thanks Mengdong
On Mon, Aug 17, 2015 at 10:01:16AM +0000, Lin, Mengdong wrote:
These *dummy* links are not place holders of real physical links. So they cannot come from ACPI but from topology.
OK, so this is something that should have been apparent from the changelog and if that's what's supposed to happen that shouldn be what the code does - it needs to be limited to these software only links.
But honestly this just doesn't seem like the ideal design - what this means is that we're going to need to have a dummy link in place for any link that is defined by the topology which means that we've got a hard coded limit in the drivers for this dynamically defined thing from the firmware. It'd be much nicer if we just created the links from scratch when we detect them in firmware rather than requiring these stubs.
From: Mengdong Lin mengdong.lin@intel.com
Probing components can bring new DAI or DAI links based on the topology info. This patch finds the unbound DAI links and bind them.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index f552fa0..2818709 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1169,6 +1169,19 @@ static void soc_remove_dai_links(struct snd_soc_card *card) } }
+static bool soc_is_dai_link_bound(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link) +{ + struct snd_soc_pcm_runtime *rtd; + + list_for_each_entry(rtd, &card->rtd_list, list) { + if (rtd->dai_link == dai_link) + return true; + } + + return false; +} + static int snd_soc_init_multicodec(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) { @@ -1763,6 +1776,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec; struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dai_link *dai_link; int ret, i, order;
mutex_lock(&client_mutex); @@ -1853,6 +1867,21 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) /* no longer need dummy runtimes */ soc_remove_dummy_pcm_runtimes(card);
+ /* Find new DAI links added during probing components and bind them. + * Components with topology may bring new DAIs and DAI links. + */ + list_for_each_entry(dai_link, &card->dai_link_list, list) { + if (dai_link->dummy || soc_is_dai_link_bound(card, dai_link)) + continue; + + ret = soc_init_dai_link(card, dai_link); + if (ret) + goto probe_dai_err; + ret = soc_bind_dai_link(card, dai_link); + if (ret) + goto probe_dai_err; + } + /* probe all DAI links on this card */ for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) {
From: Mengdong Lin mengdong.lin@intel.com
API snd_soc_add_dai() is defined to register a DAI dynamically, create its DAI widget, and notify the machine driver. This API can be used by the topology core to add DAIs dynamically.
And a callback add_dai() is defined for the soc card. The machine driver can implement this callback. Adding a new DAI dynmaically will trigger this callback, and the machine driver can create relavant DAI link in the callback context.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 7e04ec0..2a474c8 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1081,6 +1081,7 @@ struct snd_soc_card { int (*set_bias_level_post)(struct snd_soc_card *, struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level); + int (*add_dai)(struct snd_soc_card *, struct snd_soc_dai_driver*);
long pmdown_time;
@@ -1634,6 +1635,9 @@ int snd_soc_of_get_dai_link_codecs(struct device *dev, void snd_soc_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link);
+int snd_soc_add_dai(struct snd_soc_component *component, + struct snd_soc_dai_driver *dai_drv); + #include <sound/soc-dai.h>
#ifdef CONFIG_DEBUG_FS diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2818709..b330676 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2792,6 +2792,98 @@ err: return ret; }
+static struct snd_soc_dai *soc_register_dai(struct snd_soc_component *component, + struct snd_soc_dai_driver *dai_drv, + bool legacy_dai_naming) +{ + struct device *dev = component->dev; + struct snd_soc_dai *dai; + int ret; + + dev_dbg(dev, "ASoC: dynamically register DAI %s\n", dev_name(dev)); + + dai = kzalloc(sizeof(struct snd_soc_dai), GFP_KERNEL); + if (dai == NULL) { + ret = -ENOMEM; + goto err; + } + + /* + * Back in the old days when we still had component-less DAIs, + * instead of having a static name, component-less DAIs would + * inherit the name of the parent device so it is possible to + * register multiple instances of the DAI. We still need to keep + * the same naming style even though those DAIs are not + * component-less anymore. + */ + if (legacy_dai_naming) { + dai->name = fmt_single_name(dev, &dai->id); + } else { + dai->name = fmt_multiple_name(dev, dai_drv); + if (dai_drv->id) + dai->id = dai_drv->id; + else + dai->id = component->num_dai; + } + if (dai->name == NULL) { + kfree(dai); + ret = -ENOMEM; + goto err; + } + + dai->component = component; + dai->dev = dev; + dai->driver = dai_drv; + if (!dai->driver->ops) + dai->driver->ops = &null_dai_ops; + + list_add(&dai->list, &component->dai_list); + component->num_dai++; + + dev_dbg(dev, "ASoC: Registered DAI '%s'\n", dai->name); + return dai; + +err: + kfree(dai); + return NULL; +} + +/** + * snd_soc_add_dai - Add a DAI dynamically with the ASoC core + * + * @component: The component the DAIs are registered for + * @dai_drv: DAI driver to use for the DAI + */ +int snd_soc_add_dai(struct snd_soc_component *component, + struct snd_soc_dai_driver *dai_drv) +{ + struct snd_soc_dapm_context *dapm = + snd_soc_component_get_dapm(component); + struct snd_soc_card *card = component->card; + struct snd_soc_dai *dai; + int ret; + + lockdep_assert_held(&client_mutex); + dai = soc_register_dai(component, dai_drv, false); + if (!dai) + return -ENOMEM; + + ret = snd_soc_dapm_new_dai_widgets(dapm, dai); + if (ret != 0) { + dev_err(component->dev, + "Failed to create DAI widgets %d\n", ret); + } + + /* Notify the machine driver a new DAI is added, so the machine driver + * can add new DAI links in the callback context. + */ + if (card && card->add_dai) + card->add_dai(card, dai_drv); + + return ret; +} +EXPORT_SYMBOL_GPL(snd_soc_add_dai); + static void snd_soc_component_seq_notifier(struct snd_soc_dapm_context *dapm, enum snd_soc_dapm_type type, int subseq) {
From: Mengdong Lin mengdong.lin@intel.com
The PCM DAIs need to be loaded and added to ASoC core ealier than the graph (route). Otherwise, adding routes will fail for missing DAIs.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 1b6728b..0f8fa0d 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -45,12 +45,12 @@ #define SOC_TPLG_PASS_VENDOR 1 #define SOC_TPLG_PASS_MIXER 2 #define SOC_TPLG_PASS_WIDGET 3 -#define SOC_TPLG_PASS_GRAPH 4 -#define SOC_TPLG_PASS_PINS 5 -#define SOC_TPLG_PASS_PCM_DAI 6 +#define SOC_TPLG_PASS_PCM_DAI 4 +#define SOC_TPLG_PASS_GRAPH 5 +#define SOC_TPLG_PASS_PINS 6
#define SOC_TPLG_PASS_START SOC_TPLG_PASS_MANIFEST -#define SOC_TPLG_PASS_END SOC_TPLG_PASS_PCM_DAI +#define SOC_TPLG_PASS_END SOC_TPLG_PASS_PINS
struct soc_tplg { const struct firmware *fw;
From: Mengdong Lin mengdong.lin@intel.com
The toplogy user space tool will generate this bitwise flag by using SNDRV_PCM_FORMAT_* exposed by asound.h, and the topology core will copy this flag when generating DAI streams.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h index 69383b0..eb2ad59 100644 --- a/include/uapi/sound/asoc.h +++ b/include/uapi/sound/asoc.h @@ -191,7 +191,7 @@ struct snd_soc_tplg_ctl_hdr { struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; - __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported formats SNDRV_PCM_FMTBIT_* */ + __le64 formats; /* supported formats SNDRV_PCM_FORMAT_* */ __le32 rates; /* supported rates SNDRV_PCM_RATE_* */ __le32 rate_min; /* min rate */ __le32 rate_max; /* max rate */
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_* */
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be limited.
This should have at the very least been sent at the start of the series, and really should have been sent separately. It is also worrying that there is nothing in the subject or changelog talking about the fact that this is an ABI change or explaining why it is required. The changelog was simply a statement of the content of the diff, not a rationale:
| The toplogy user space tool will generate this bitwise flag by using | SNDRV_PCM_FORMAT_* exposed by asound.h, and the topology core will copy | this flag when generating DAI streams.
Once the release goes out this sort of incompatible change will be unacceptable. Given the number of changes that are going into the ABI (we also had some others went in last week) I'm now giving some serious thought to the idea that we should ask Linus to revert the topology code for v4.2 and waiting for v4.3 so we've got more chance to make sure the ABI is one we actually want to stick with. Liam, Takashi I don't know what you think?
On Fri, 14 Aug 2015 22:34:28 +0200, Mark Brown wrote:
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_* */
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be limited.
Thanks for checking this. Mengdong, if there is a change in uapi/*, this means touching ABI. So, this change isn't acceptable once when the kernel is released. At least, the backward compatibility *must* be kept no matter which version is.
Also, the patch (and even the current code) looks wrong. The current formats[] would receive SNDRV_PCM_FORMAT_* while the comment shows SNDRV_PCM_FMT_BIT_*. And your patch changes it again wrongly in the comment. Quite confusing.
This should have at the very least been sent at the start of the series, and really should have been sent separately. It is also worrying that there is nothing in the subject or changelog talking about the fact that this is an ABI change or explaining why it is required. The changelog was simply a statement of the content of the diff, not a rationale:
| The toplogy user space tool will generate this bitwise flag by using | SNDRV_PCM_FORMAT_* exposed by asound.h, and the topology core will copy | this flag when generating DAI streams.
Once the release goes out this sort of incompatible change will be unacceptable. Given the number of changes that are going into the ABI (we also had some others went in last week) I'm now giving some serious thought to the idea that we should ask Linus to revert the topology code for v4.2 and waiting for v4.3 so we've got more chance to make sure the ABI is one we actually want to stick with. Liam, Takashi I don't know what you think?
We can make the topology stuff disabled in Makefile (and the only call in soc-core.c) instead of the whole revert. This will be the minimal impact and safe enough.
BTW, I wonder why there is no Kconfig for topology stuff. If we had it, it would have been easy to disable.
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 3:38 PM
On Fri, 14 Aug 2015 22:34:28 +0200, Mark Brown wrote:
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com
wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported
formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_* */
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be
limited.
Thanks for checking this. Mengdong, if there is a change in uapi/*, this means touching ABI. So, this change isn't acceptable once when the kernel is released. At least, the backward compatibility *must* be kept no matter which version is.
Thanks for your review, Mark and Takashi. This series including the ABI change, is not for v4.2, but is an RFC to adding DAI/DAI links support in topology. I'm sorry that I should have explicitly said this in my comments.
Is there some topic branch which can accept topology patches for next kernel release? Can we change topology ABI in v4.3?
Now only Intel SKL driver is using topology code, so the affect should be limited. And I feel using bitwise flag is simpler in both user space and in kernel coding. We can increase the topology ABI number when there is ABI change, and the topology driver always checks this version.
Also, the patch (and even the current code) looks wrong. The current formats[] would receive SNDRV_PCM_FORMAT_* while the comment shows SNDRV_PCM_FMT_BIT_*. And your patch changes it again wrongly in the comment. Quite confusing.
Sorry. I will change the comments. I hope to change "formats" to a bitwise flag of _SNDRV_PCM_FMTBIT_*. Current upstreamed topology driver does not really handle DAIs (streams caps) so the error is not discovered.
This should have at the very least been sent at the start of the series, and really should have been sent separately. It is also worrying that there is nothing in the subject or changelog talking about the fact that this is an ABI change or explaining why it is required. The changelog was simply a statement of the content of the diff,
not a rationale:
| The toplogy user space tool will generate this bitwise flag by using | SNDRV_PCM_FORMAT_* exposed by asound.h, and the topology core will | copy this flag when generating DAI streams.
Once the release goes out this sort of incompatible change will be unacceptable. Given the number of changes that are going into the ABI (we also had some others went in last week) I'm now giving some serious thought to the idea that we should ask Linus to revert the topology code for v4.2 and waiting for v4.3 so we've got more chance to make sure the ABI is one we actually want to stick with. Liam, Takashi I don't know what you think?
We can make the topology stuff disabled in Makefile (and the only call in soc-core.c) instead of the whole revert. This will be the minimal impact and safe enough.
BTW, I wonder why there is no Kconfig for topology stuff. If we had it, it would have been easy to disable.
We'll add Kconfig for topology.
Thanks Mengdong
On Sat, 15 Aug 2015 15:49:42 +0200, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 3:38 PM
On Fri, 14 Aug 2015 22:34:28 +0200, Mark Brown wrote:
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com
wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported
formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_* */
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be
limited.
Thanks for checking this. Mengdong, if there is a change in uapi/*, this means touching ABI. So, this change isn't acceptable once when the kernel is released. At least, the backward compatibility *must* be kept no matter which version is.
Thanks for your review, Mark and Takashi. This series including the ABI change, is not for v4.2, but is an RFC to adding DAI/DAI links support in topology. I'm sorry that I should have explicitly said this in my comments.
Is there some topic branch which can accept topology patches for next kernel release? Can we change topology ABI in v4.3?
In general "yes, but only if it's backward-compatible".
Now only Intel SKL driver is using topology code, so the affect should be limited. And I feel using bitwise flag is simpler in both user space and in kernel coding.
Yes, using FMTBIT_* is better, but this must not justify to break ABI. Either we disable this API/ABI for 4.2, or make the change backward compatible.
We can increase the topology ABI number when there is ABI change, and the topology driver always checks this version.
It's a big NO, sorry, this isn't acceptable. Again, the kernel must give the backward compatibility. That is, the kernel must accept the old dialect the user space talks if user space wants. You cannot force user space to communicate in a new ABI out of sudden.
So, yes, ABI version bump is needed if you change ABI. But it means that the kernel accepts all ABIs up to this version, not meaning *only* this ABI version.
Hopefully this clarifies the situation.
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 10:46 PM
On Sat, 15 Aug 2015 15:49:42 +0200, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 3:38 PM
On Fri, 14 Aug 2015 22:34:28 +0200, Mark Brown wrote:
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com
wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported
formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_*
*/
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be
limited.
Thanks for checking this. Mengdong, if there is a change in uapi/*, this means touching ABI. So, this change isn't acceptable once when the kernel is released. At least, the backward compatibility *must* be kept no matter which version is.
Thanks for your review, Mark and Takashi. This series including the ABI change, is not for v4.2, but is an RFC to adding
DAI/DAI links support in topology.
I'm sorry that I should have explicitly said this in my comments.
Is there some topic branch which can accept topology patches for next
kernel release?
Can we change topology ABI in v4.3?
In general "yes, but only if it's backward-compatible".
Now only Intel SKL driver is using topology code, so the affect should be
limited.
And I feel using bitwise flag is simpler in both user space and in kernel
coding.
Yes, using FMTBIT_* is better, but this must not justify to break ABI. Either we disable this API/ABI for 4.2, or make the change backward compatible.
Many thanks for your clarification! We'd better keep current ABI unchanged to keep backward compatibility.
In addition, does "disable this API/ABI for 4.2" mean disabling all topology features, or only some structures for v4.2? But alsa-lib already integrated this ABI version.
I'm worried that If we have to add new fields to ABI structures, how to keep the backward compatibility.
Thanks Mengdong
We can increase the topology ABI number when there is ABI change, and
the topology driver always checks this version.
It's a big NO, sorry, this isn't acceptable. Again, the kernel must give the backward compatibility. That is, the kernel must accept the old dialect the user space talks if user space wants. You cannot force user space to communicate in a new ABI out of sudden.
So, yes, ABI version bump is needed if you change ABI. But it means that the kernel accepts all ABIs up to this version, not meaning *only* this ABI version.
Hopefully this clarifies the situation.
thanks,
Takashi
On Sat, 15 Aug 2015 17:25:18 +0200, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 10:46 PM
On Sat, 15 Aug 2015 15:49:42 +0200, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, August 15, 2015 3:38 PM
On Fri, 14 Aug 2015 22:34:28 +0200, Mark Brown wrote:
On Mon, Aug 10, 2015 at 10:48:37PM +0800, mengdong.lin@intel.com
wrote:
struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- __le64 formats[SND_SOC_TPLG_MAX_FORMATS]; /* supported
formats SNDRV_PCM_FMTBIT_* */
- __le64 formats; /* supported formats SNDRV_PCM_FORMAT_*
*/
Argh, this is *another* ABI change which you've buried in the middle of a series you're sending right at the end of the release cycle (this was sent after -rc6, we may not even get a -rc7...). Given how close we are to the release and the invasiveness of the changes in this series it's very lucky I even saw it before release, I'm reading this on my flight to Plumbers which means my bandwidth next week will be
limited.
Thanks for checking this. Mengdong, if there is a change in uapi/*, this means touching ABI. So, this change isn't acceptable once when the kernel is released. At least, the backward compatibility *must* be kept no matter which version is.
Thanks for your review, Mark and Takashi. This series including the ABI change, is not for v4.2, but is an RFC to adding
DAI/DAI links support in topology.
I'm sorry that I should have explicitly said this in my comments.
Is there some topic branch which can accept topology patches for next
kernel release?
Can we change topology ABI in v4.3?
In general "yes, but only if it's backward-compatible".
Now only Intel SKL driver is using topology code, so the affect should be
limited.
And I feel using bitwise flag is simpler in both user space and in kernel
coding.
Yes, using FMTBIT_* is better, but this must not justify to break ABI. Either we disable this API/ABI for 4.2, or make the change backward compatible.
Many thanks for your clarification! We'd better keep current ABI unchanged to keep backward compatibility.
In addition, does "disable this API/ABI for 4.2" mean disabling all topology features, or only some structures for v4.2?
I suppose the whole topology feature.
But alsa-lib already integrated this ABI version.
Yes, but it's no big problem, as we haven't released alsa-lib changes yet.
I'm worried that If we have to add new fields to ABI structures, how to keep the backward compatibility.
There are reserved areas for the future extension, and usually take a part of them for the additional fields. Even if doing that, it's still very sensitive to change the structure in general, though.
Takashi
On Sat, Aug 15, 2015 at 03:25:18PM +0000, Lin, Mengdong wrote:
In addition, does "disable this API/ABI for 4.2" mean disabling all topology features, or only some structures for v4.2? But alsa-lib already integrated this ABI version.
I'm suggesting not including this at all in the v4.2 release so we can continue to make changes, or perhaps just leaving the code there but making sure it can't be used as Takashi suggested. alsa-lib hasn't released yet as far as I'm aware so we can continue to make changes there.
I'm worried that If we have to add new fields to ABI structures, how to keep the backward compatibility.
Right, it's tricky - there's a bunch of techniques like the versioning that's in the current structures but they all take work and effort.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Sunday, August 16, 2015 12:52 AM
On Sat, Aug 15, 2015 at 03:25:18PM +0000, Lin, Mengdong wrote:
In addition, does "disable this API/ABI for 4.2" mean disabling all topology features, or only some structures for v4.2? But alsa-lib already integrated this ABI version.
I'm suggesting not including this at all in the v4.2 release so we can continue to make changes, or perhaps just leaving the code there but making sure it can't be used as Takashi suggested. alsa-lib hasn't released yet as far as I'm aware so we can continue to make changes there.
Hi Takashi/Mark,
After syncing with Liam, we hope to disable topology in the v4.2 release atm but leave the code there.
There may be topology ABI changes later and keeping backward compatibility would be difficult.
Sorry for the inconvenience.
Thanks Mengdong
I'm worried that If we have to add new fields to ABI structures, how to keep the backward compatibility.
Right, it's tricky - there's a bunch of techniques like the versioning that's in the current structures but they all take work and effort.
On Sat, Aug 15, 2015 at 01:49:42PM +0000, Lin, Mengdong wrote:
Is there some topic branch which can accept topology patches for next kernel release? Can we change topology ABI in v4.3?
You're not quite getting the issue here - the problem is that once we have released the ABI we need to keep supporting applications using it so we can't just change the ABI, we also need to keep compatibility code for the original ABI. This makes it a lot more effort to change things.
Now only Intel SKL driver is using topology code, so the affect should be limited. And I feel using bitwise flag is simpler in both user space and in kernel coding. We can increase the topology ABI number when there is ABI change, and the topology driver always checks this version.
Someone else may take this kernel and use it in their product, or some distro may decide to ship v4.2 as their kernel. Once it's in a release we have to assume someone is going to start relying on the ABI.
On Sat, Aug 15, 2015 at 09:37:34AM +0200, Takashi Iwai wrote:
We can make the topology stuff disabled in Makefile (and the only call in soc-core.c) instead of the whole revert. This will be the minimal impact and safe enough.
Yes, though it does leave the UAPI files there for people to look at which is a big part of the risk since they end up getting shipped separately to the kernel binary. Might be sufficient, though - we could just put some #errors in the headers to catch potential users. Or perhaps we're OK with the ABI as it stands.
BTW, I wonder why there is no Kconfig for topology stuff. If we had it, it would have been easy to disable.
It's probably worth adding that anyway - I know Intel at least are keen on having the capability to minimise memory usage!
On Sat, 15 Aug 2015 16:59:04 +0200, Mark Brown wrote:
On Sat, Aug 15, 2015 at 09:37:34AM +0200, Takashi Iwai wrote:
We can make the topology stuff disabled in Makefile (and the only call in soc-core.c) instead of the whole revert. This will be the minimal impact and safe enough.
Yes, though it does leave the UAPI files there for people to look at which is a big part of the risk since they end up getting shipped separately to the kernel binary.
True, if we want really 120% safety...
Might be sufficient, though - we could just put some #errors in the headers to catch potential users. Or perhaps we're OK with the ABI as it stands.
Yeah, #error or a big fat comment should suffice, I suppose.
BTW, I wonder why there is no Kconfig for topology stuff. If we had it, it would have been easy to disable.
It's probably worth adding that anyway - I know Intel at least are keen on having the capability to minimise memory usage!
Right, I had it in my mind, too.
Takashi
From: Mengdong Lin mengdong.lin@intel.com
Add PCM DAIs to ASoC core when loading them from topology. Embed dobj in the DAI to free it when unloading.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 2df96b1..e8f0404 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -207,6 +207,7 @@ struct snd_soc_dai_driver { const char *name; unsigned int id; unsigned int base; + struct snd_soc_dobj dobj;
/* DAI driver callbacks */ int (*probe)(struct snd_soc_dai *dai); diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h index 5405f64..a49c134 100644 --- a/include/sound/soc-topology.h +++ b/include/sound/soc-topology.h @@ -71,7 +71,6 @@ struct snd_soc_dobj { union { struct snd_soc_dobj_control control; struct snd_soc_dobj_widget widget; - struct snd_soc_dobj_pcm_dai pcm_dai; }; void *private; /* core does not touch this */ }; diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 0f8fa0d..6c0dd3f 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -499,6 +499,9 @@ static void remove_widget(struct snd_soc_component *comp, static void remove_pcm_dai(struct snd_soc_component *comp, struct snd_soc_dobj *dobj, int pass) { + struct snd_soc_dai_driver *dai = + container_of(dobj, struct snd_soc_dai_driver, dobj); + if (pass != SOC_TPLG_PASS_PCM_DAI) return;
@@ -506,7 +509,7 @@ static void remove_pcm_dai(struct snd_soc_component *comp, dobj->ops->pcm_dai_unload(comp, dobj);
list_del(&dobj->list); - kfree(dobj); + kfree(dai); }
/* bind a kcontrol to it's IO handlers */ @@ -1558,12 +1561,75 @@ static int soc_tplg_dapm_complete(struct soc_tplg *tplg) return 0; }
+static unsigned int get_rates(unsigned int rate_min, unsigned int rate_max) +{ + static int pcm_rates[] = { + 5512, 8000, 11025, 16000, 22050, 32000, 44100, 48000, + 64000, 88200, 96000, 176400, 192000, + }; + unsigned int rates = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(pcm_rates); i++) { + if (pcm_rates[i] >= rate_min && pcm_rates[i] <= rate_max) + rates |= 1 << i; + } + + return rates; +} + +static int soc_tplg_pcm_dai_create(struct soc_tplg *tplg, + struct snd_soc_tplg_pcm_dai *dai) +{ + struct snd_soc_dai_driver *dai_drv; + struct snd_soc_pcm_stream *stream; + struct snd_soc_tplg_stream_caps *caps; + + dai_drv = kzalloc(sizeof(struct snd_soc_dai_driver), GFP_KERNEL); + if (dai_drv == NULL) + return -ENOMEM; + + dai_drv->name = dai->name; + dai_drv->id = dai->id; + dai_drv->compress_dai = dai->compress; + + if (dai->playback) { + stream = &dai_drv->playback; + caps = &dai->capconf[SND_SOC_TPLG_STREAM_PLAYBACK].caps; + + stream->stream_name = kstrdup(caps->name, GFP_KERNEL); + stream->channels_min = caps->channels_min; + stream->channels_max = caps->channels_max; + stream->rates = get_rates(caps->rate_min, caps->rate_max); + stream->formats = caps->formats; + } + + if (dai->capture) { + stream = &dai_drv->capture; + caps = &dai->capconf[SND_SOC_TPLG_STREAM_CAPTURE].caps; + + stream->stream_name = kstrdup(caps->name, GFP_KERNEL); + stream->channels_min = caps->channels_min; + stream->channels_max = caps->channels_max; + stream->rates = get_rates(caps->rate_min, caps->rate_max); + stream->formats = caps->formats; + } + + dai_drv->dobj.index = tplg->index; + dai_drv->dobj.ops = tplg->ops; + dai_drv->dobj.type = SND_SOC_DOBJ_PCM; + list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list); + + return snd_soc_add_dai(tplg->comp, dai_drv); +} + static int soc_tplg_pcm_dai_elems_load(struct soc_tplg *tplg, struct snd_soc_tplg_hdr *hdr) { struct snd_soc_tplg_pcm_dai *pcm_dai; struct snd_soc_dobj *dobj; int count = hdr->count; + int i; int ret;
if (tplg->pass != SOC_TPLG_PASS_PCM_DAI) @@ -1579,26 +1645,21 @@ static int soc_tplg_pcm_dai_elems_load(struct soc_tplg *tplg, return -EINVAL; }
- dev_dbg(tplg->dev, "ASoC: adding %d PCM DAIs\n", count); - tplg->pos += sizeof(struct snd_soc_tplg_pcm_dai) * count; - - dobj = kzalloc(sizeof(struct snd_soc_dobj), GFP_KERNEL); - if (dobj == NULL) - return -ENOMEM; - - /* Call the platform driver call back to register the dais */ + /* pass control to driver for optional further init */ ret = soc_tplg_pcm_dai_load(tplg, pcm_dai, count); if (ret < 0) { dev_err(tplg->comp->dev, "ASoC: PCM DAI loading failed\n"); goto err; }
- dobj->type = get_dobj_type(hdr, NULL); - dobj->pcm_dai.count = count; - dobj->pcm_dai.pd = pcm_dai; - dobj->ops = tplg->ops; - dobj->index = tplg->index; - list_add(&dobj->list, &tplg->comp->dobj_list); + /* register the dais */ + for (i = 0; i < count; i++) { + soc_tplg_pcm_dai_create(tplg, pcm_dai); + pcm_dai++; + } + + dev_dbg(tplg->dev, "ASoC: adding %d PCM DAIs\n", count); + tplg->pos += sizeof(struct snd_soc_tplg_pcm_dai) * count; return 0;
err:
On 08/10/2015 04:45 PM, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
This series allows the topology core to create PCM devices dynamically. The user can define different DAIs in the topology files for different versions of firmware, but share a generic platform and machine driver.
A dummy DAI and DAI link can be used to register the soc card and specify the platform with topology. Then real DAIs are created in platform probing phase by the topology core, and the machine driver will be notified to create relavant DAI links.
This all sounds as if the dynamically loaded topology information does not describe dynamic topology, e.g. like DSP firmware, but rather the static topology that would usually come from things like devicetree or ACPI. Maybe you need to rethink when the topology information is loaded and make sure that the parts that describe the static topology are available before the card is instantiated.
- Lars
On Sat, Aug 15, 2015 at 09:56:24AM +0200, Lars-Peter Clausen wrote:
This all sounds as if the dynamically loaded topology information does not describe dynamic topology, e.g. like DSP firmware, but rather the static topology that would usually come from things like devicetree or ACPI. Maybe you need to rethink when the topology information is loaded and make sure that the parts that describe the static topology are available before the card is instantiated.
I do think that's some of it (as I said in my replies in the series), but I got the impression from some other discussion that some of this was about host<->DSP channels that exist purely in software which does seem like something that could reasonably be in the topology, but this does seem more like the fixed links stuff.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Saturday, August 15, 2015 11:51 PM To: Lars-Peter Clausen Cc: Lin, Mengdong; alsa-devel@alsa-project.org; tiwai@suse.de; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH v2 00/10] ASoC: support adding PCM dynamically from topology
On Sat, Aug 15, 2015 at 09:56:24AM +0200, Lars-Peter Clausen wrote:
This all sounds as if the dynamically loaded topology information does not describe dynamic topology, e.g. like DSP firmware, but rather the static topology that would usually come from things like devicetree or ACPI. Maybe you need to rethink when the topology information is loaded and make sure that the parts that describe the static topology are available before the card is instantiated.
I do think that's some of it (as I said in my replies in the series), but I got the impression from some other discussion that some of this was about host<->DSP channels that exist purely in software which does seem like something that could reasonably be in the topology, but this does seem more like the fixed links stuff.
We hope to release different firmware for different OEM products. Due to license restrictions, these firmware may have different features and may support different FE DAIs.
ACPI only describes physical links and does not handle FE DAIs. This is why we use topology here.
Thanks Mengdong
On Mon, 2015-08-17 at 06:13 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Saturday, August 15, 2015 11:51 PM To: Lars-Peter Clausen Cc: Lin, Mengdong; alsa-devel@alsa-project.org; tiwai@suse.de; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH v2 00/10] ASoC: support adding PCM dynamically from topology
On Sat, Aug 15, 2015 at 09:56:24AM +0200, Lars-Peter Clausen wrote:
This all sounds as if the dynamically loaded topology information does not describe dynamic topology, e.g. like DSP firmware, but rather the static topology that would usually come from things like devicetree or ACPI. Maybe you need to rethink when the topology information is loaded and make sure that the parts that describe the static topology are available before the card is instantiated.
I do think that's some of it (as I said in my replies in the series), but I got the impression from some other discussion that some of this was about host<->DSP channels that exist purely in software which does seem like something that could reasonably be in the topology, but this does seem more like the fixed links stuff.
We hope to release different firmware for different OEM products. Due to license restrictions, these firmware may have different features and may support different FE DAIs.
ACPI only describes physical links and does not handle FE DAIs. This is why we use topology here.
To further clarify.
The intention is that the topology data for BE and codec style links is not to define the link (as this should come from DT or ACPI) but to define the links config/capabilities for a given FW. e.g. we have two different FWs that run BE0 (SSP0) with different number of channels (one is stereo, the other is 4 channel TDM). Currently the link definitions will be hard coded until the ACPI data is ready.
The FE uses the above too for config/capabilities but it can also be defined and created by the topology data (since it's a FW entity).
Liam
On Mon, Aug 17, 2015 at 11:39:59AM +0100, Liam Girdwood wrote:
The intention is that the topology data for BE and codec style links is not to define the link (as this should come from DT or ACPI) but to define the links config/capabilities for a given FW. e.g. we have two different FWs that run BE0 (SSP0) with different number of channels (one is stereo, the other is 4 channel TDM). Currently the link definitions will be hard coded until the ACPI data is ready.
The FE uses the above too for config/capabilities but it can also be defined and created by the topology data (since it's a FW entity).
So this is actually for adding further constraints to the existing physical links, not just for defining new virtual links. That is definitely a reasonable application but it seems like what we should be doing here is adding constraints to a fully defined link rather than just having a dummy link. The link is real and there, it's just that we want to further restrict how it can be used. Something that says "this is a dummy link" definitely feels wrong.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Tuesday, August 18, 2015 4:05 AM To: Liam Girdwood Cc: Lars-Peter Clausen; tiwai@suse.de; alsa-devel@alsa-project.org; Lin, Mengdong Subject: Re: [alsa-devel] [PATCH v2 00/10] ASoC: support adding PCM dynamically from topology
On Mon, Aug 17, 2015 at 11:39:59AM +0100, Liam Girdwood wrote:
The intention is that the topology data for BE and codec style links is not to define the link (as this should come from DT or ACPI) but to define the links config/capabilities for a given FW. e.g. we have two different FWs that run BE0 (SSP0) with different number of channels (one is stereo, the other is 4 channel TDM). Currently the link definitions will be hard coded until the ACPI data is ready.
The FE uses the above too for config/capabilities but it can also be defined and created by the topology data (since it's a FW entity).
So this is actually for adding further constraints to the existing physical links, not just for defining new virtual links. That is definitely a reasonable application but it seems like what we should be doing here is adding constraints to a fully defined link rather than just having a dummy link. The link is real and there, it's just that we want to further restrict how it can be used. Something that says "this is a dummy link" definitely feels wrong.
I feel it's not enough to just "adding constraints to a fully defined link" for FE DAIs. We need to determine the number of FE DAIs (thus FE DAI links as well) for a specific version of FW based on its topology info.
We use a "dummy FE DAI link" in the Broadwell machine driver just to - pass the soc_bind_dai_link() on registering the soc card and use the deferred probe mechanism; - pull in the platform component which carries topology the info
If you prefer not using any dummy DAI link but creating the links from scratch when we detect them in firmware, can we add new field into "struct snd_soc_card" to specify the components for topology? After all, topology may not only come from the platform component.
Thanks Mengdong
On Tue, Aug 18, 2015 at 05:17:36AM +0000, Lin, Mengdong wrote:
I feel it's not enough to just "adding constraints to a fully defined link" for FE DAIs. We need to determine the number of FE DAIs (thus FE DAI links as well) for a specific version of FW based on its topology info.
There's two things going on here. One is configuring the limits on the DAIs that represent physical connections in the system, the other is creating new fully software DAIs for DSP<->AP links. The two cases are separate.
If you prefer not using any dummy DAI link but creating the links from scratch when we detect them in firmware, can we add new field into "struct snd_soc_card" to specify the components for topology? After all, topology may not only come from the platform component.
We need to come up with some mechanism for it. I'm not clear what a new field in the card would be, I'd more expect some runtime calls from the topology code but perhaps there's some way of doing this in a data driven way that I've just not thought of.
participants (6)
-
Lars-Peter Clausen -
Liam Girdwood -
Lin, Mengdong -
Mark Brown -
mengdong.lin＠intel.com
-
Takashi Iwai