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

[Cezary Rojewski]

On 2022-02-25 8:17 PM, Pierre-Louis Bossart wrote:
> 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?

Just like skylake-driver, avs-driver has several dictionaries which 
provide a list of primitive elements each (e.g. module configs) so the 
more 'advanced' structures such as struct avs_tplg_path_template can 
refer to them later.

Yes, path templates are represented in the topology file and are set to 
instance of struct avs_tplg_path_template each when the file is being 
parsed.

DAPM widget - widget which represents either FE or BE path.

>> +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?

That's a ->route_load() topology operation. So, the route in known 
upfront from ASoC topology documentation.

All others were already explained earlier in the series as it's not the 
first time question is being asked. Trying to keep the number of 
'threads' in check so it's easier to follow.

>> +{
>> +	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?

Ack, thanks!

>> +		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?

Hmm.. You're right, there are just FE links being loaded here. Guess 
this one should be moved into the if-statement 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;
>> +}
>