Re: [RFC 07/13] ASoC: Intel: avs: Add topology loading operations

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 2/7/22 07:25, Cezary Rojewski wrote:
> AVS topology is split into two major parts: dictionaries - found within
> ASoC topology manifest - and path templates - found within DAPM widget
> private data. 

Well, one would hope that the path templates are initially represented
in the topology and set when parsing the topology.

If not, who defines that those 'path templates' are?

It's also unclear which 'DAPM widget' you are referring to?


> +static int avs_route_load(struct snd_soc_component *comp, int index,
> +			  struct snd_soc_dapm_route *route)

I have to ask: what is the difference between stream, path, path
template, route?

> +{
> +	struct snd_soc_acpi_mach *mach = dev_get_platdata(comp->card->dev);
> +	size_t len = SNDRV_CTL_ELEM_ID_NAME_MAXLEN;
> +	char buf[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	u32 port;
> +
> +	/* See parse_link_formatted_string() for dynamic naming when(s). */
> +	if (hweight_long(mach->link_mask) == 1) {
> +		port = __ffs(mach->link_mask);
> +
> +		snprintf(buf, len, route->source, port);
> +		strncpy((char *)route->source, buf, len);
> +		snprintf(buf, len, route->sink, port);
> +		strncpy((char *)route->sink, buf, len);
> +		if (route->control) {
> +			snprintf(buf, len, route->control, port);
> +			strncpy((char *)route->control, buf, len);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int avs_widget_load(struct snd_soc_component *comp, int index,
> +			   struct snd_soc_dapm_widget *w,
> +			   struct snd_soc_tplg_dapm_widget *dw)
> +{
> +	struct snd_soc_acpi_mach *mach;
> +	struct avs_tplg_path_template *template;
> +	struct avs_soc_component *acomp = to_avs_soc_component(comp);
> +	struct avs_tplg *tplg;
> +
> +	if (!le32_to_cpu(dw->priv.size))
> +		return 0;
> +
> +	tplg = acomp->tplg;
> +	mach = dev_get_platdata(comp->card->dev);
> +
> +	/* See parse_link_formatted_string() for dynamic naming when(s). */
> +	if (hweight_long(mach->link_mask) == 1) {
> +		kfree(w->name);
> +		/* ->name is freed later by soc_tplg_dapm_widget_create() */

->name? missing something here
w->name?

> +		w->name = kasprintf(GFP_KERNEL, dw->name, __ffs(mach->link_mask));
> +		if (!w->name)
> +			return -ENOMEM;
> +	}
> +
> +	template = avs_tplg_path_template_create(comp, tplg, dw->priv.array,
> +						 le32_to_cpu(dw->priv.size));
> +	if (IS_ERR(template)) {
> +		dev_err(comp->dev, "widget %s load failed: %ld\n", dw->name,
> +			PTR_ERR(template));
> +		return PTR_ERR(template);
> +	}
> +
> +	w->priv = template; /* link path information to widget */
> +	list_add_tail(&template->node, &tplg->path_tmpl_list);
> +	return 0;
> +}
> +
> +static int avs_dai_load(struct snd_soc_component *comp, int index,
> +			struct snd_soc_dai_driver *dai_drv, struct snd_soc_tplg_pcm *pcm,
> +			struct snd_soc_dai *dai)
> +{
> +	if (pcm)
> +		dai_drv->ops = &avs_dai_fe_ops;
> +	return 0;
> +}
> +
> +static int avs_link_load(struct snd_soc_component *comp, int index, struct snd_soc_dai_link *link,
> +			 struct snd_soc_tplg_link_config *cfg)
> +{
> +	/* Stream control handled by IPCs. */
> +	link->nonatomic = true;

if this routine also takes care of BE dailinks, then it's not quite
correct to set nonatomic here.

Should this be set only within the test below?

> +
> +	if (!link->no_pcm) {
> +		/* Open LINK (BE) pipes last and close them first to prevent xruns. */
> +		link->trigger[0] = SND_SOC_DPCM_TRIGGER_PRE;
> +		link->trigger[1] = SND_SOC_DPCM_TRIGGER_PRE;
> +	}
> +
> +	return 0;
> +}