Re: [alsa-devel] [PATCH v2 116/146] ASoC: sof: use modern dai_link style

Hi Morimoto-san,

One question inline:

On Thu, Jun 6, 2019 at 8:26 AM Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

sound/soc/sof/nocodec.c | 21 +++++++++++++++++---- sound/soc/sof/topology.c | 20 +++++++++----------- 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sof/nocodec.c b/sound/soc/sof/nocodec.c index f84b434..3d128e5 100644 --- a/sound/soc/sof/nocodec.c +++ b/sound/soc/sof/nocodec.c @@ -21,6 +21,7 @@ static int sof_nocodec_bes_setup(struct device *dev, struct snd_soc_dai_link *links, int link_num, struct snd_soc_card *card) {

•   struct snd_soc_dai_link_component *dlc;
    int i;
  
    if (!ops || !links || !card)
  

@@ -28,17 +29,29 @@ static int sof_nocodec_bes_setup(struct device *dev,

    /* set up BE dai_links */
    for (i = 0; i < link_num; i++) {

•           dlc = devm_kzalloc(dev, 3 * sizeof(*dlc), GFP_KERNEL);
  

•           if (!dlc)
  

•                   return -ENOMEM;
  

•           links[i].name = devm_kasprintf(dev, GFP_KERNEL,
                                           "NoCodec-%d", i);
            if (!links[i].name)
                    return -ENOMEM;
  
  

•           links[i].cpus = &dlc[0];
  

•           links[i].codecs = &dlc[1];
  

•           links[i].platforms = &dlc[2];
  

•           links[i].num_cpus = 1;
  

•           links[i].num_codecs = 1;
  

•           links[i].num_platforms = 1;
  

•           links[i].id = i;
            links[i].no_pcm = 1;
  

•           links[i].cpu_dai_name = ops->drv[i].name;
  

•           links[i].platform_name = dev_name(dev);
  

•           links[i].codec_dai_name = "snd-soc-dummy-dai";
  

•           links[i].codec_name = "snd-soc-dummy";
  

•           links[i].cpus->dai_name = ops->drv[i].name;
  

•           links[i].platforms->name = dev_name(dev);
  

•           links[i].codecs->dai_name = "snd-soc-dummy-dai";
  

•           links[i].codecs->name = "snd-soc-dummy";
            links[i].dpcm_playback = 1;
            links[i].dpcm_capture = 1;
    }
  

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index b969686f..a13233a 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2639,7 +2639,6 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index, struct sof_ipc_dai_config *config) { struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);

•   struct snd_soc_dai_link_component dai_component;
    struct snd_soc_tplg_private *private = &cfg->priv;
    struct snd_soc_dai *dai;
    u32 size = sizeof(*config);
  

@@ -2650,7 +2649,6 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index, int ret;

    /* init IPC */

•   memset(&dai_component, 0, sizeof(dai_component));
    memset(&config->hda, 0, sizeof(struct sof_ipc_dai_hda_params));
    config->hdr.size = size;
  
  

@@ -2664,11 +2662,10 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index, return ret; }

•   dai_component.dai_name = link->cpu_dai_name;
  

•   dai = snd_soc_find_dai(&dai_component);
  

•   dai = snd_soc_find_dai(link->cpus);
    if (!dai) {
            dev_err(sdev->dev, "error: failed to find dai %s in %s",
  

•                   dai_component.dai_name, __func__);
  

•                   link->cpus->dai_name, __func__);
            return -EINVAL;
    }
  
  

@@ -2708,7 +2705,11 @@ static int sof_link_load(struct snd_soc_component *scomp, int index, int ret; int i = 0;

•   link->platform_name = dev_name(sdev->dev);
  

•   if (!link->platforms) {
  

•           dev_err(sdev->dev, "error: no platforms\n");
  

•           return -EINVAL;
  

Why do we need this check? With linux-next this check fails for me.

I do also have some local changes so take this question with a grain of salt.

This check fails for FE links as they are created as follows:

/* create the FE DAI link */ static int soc_tplg_fe_link_create

=> link->cpus» = &dlc[0]; => link->codecs» = &dlc[1]; => link->num_cpus» = 1; => link->num_codecs = 1;

So, there is no platforms set up for FE links.

I could get rid of the failure with the following patch:

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index f485f7f751a1..ee73318135fc 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1883,7 +1883,7 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, int ret;

/* link + cpu + codec */ - link = kzalloc(sizeof(*link) + (2 * sizeof(*dlc)), GFP_KERNEL); + link = kzalloc(sizeof(*link) + (3 * sizeof(*dlc)), GFP_KERNEL); if (link == NULL) return -ENOMEM;

@@ -1891,9 +1891,11 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,

link->cpus = &dlc[0]; link->codecs = &dlc[1]; + link->platforms = &dlc[2];

link->num_cpus = 1; link->num_codecs = 1; + link->num_platforms = 1;

Can you please help me figure this out?

thanks, Daniel.