Re: [RFC 18/37] ASoC: Intel: avs: Topology parsing

From: Mark Brown <broonie@kernel.org>
To: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: alsa-devel@alsa-project.org, upstream@semihalf.com,
	harshapriya.n@intel.com, yung-chuan.liao@linux.intel.com,
	rad@semihalf.com, pierre-louis.bossart@linux.intel.com,
	tiwai@suse.com, hdegoede@redhat.com,
	ranjani.sridharan@linux.intel.com,
	amadeuszx.slawinski@linux.intel.com, cujomalainey@chromium.org,
	peter.ujfalusi@linux.intel.com, lma@semihalf.com
Subject: Re: [RFC 18/37] ASoC: Intel: avs: Topology parsing
Date: Tue, 21 Dec 2021 17:39:17 +0000	[thread overview]
Message-ID: <YcIRRevkBpM/jIKV@sirena.org.uk> (raw)
In-Reply-To: <20211208111301.1817725-19-cezary.rojewski@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

On Wed, Dec 08, 2021 at 12:12:42PM +0100, Cezary Rojewski wrote:

> Implementation of ASoC topology feature for AVS driver. AudioDSP
> firmware supports a wide range of audio formats, module configurations
> and multi-pipeline streams. To represent all of this in form of static
> ALSA topology file, which resides usually in /lib/firmware/, while
> simultaneously not hindering user from any of the possibilities, 'path
> template' and its 'path variants' concept is introduced. These are later
> converted into actual runtime path. This part is explained in follow-up
> change.

This sounds like it should be extending the topology code (it's talking
about "ALSA topologies") but it seems to be outside of that.

> Path template is just a pattern like its name suggests. It is tied to
> DAPM widget which represents a FE or a BE and is used during path
> instantiation when substream is opened for streaming. It carries a range
> of available variants and only these represent actual implementation of
> a runtime path in AudioDSP. Only one variant of given path template can
> be instantiated at a time and selection is based off of audio format
> provided from userspace and currently selected one on the codec.

So this sounds like it's baking a table of use cases into the firmware
rather than a separate UCM type configuration file?

> AVS topology is split into two major parts: dictionaries - found within
> ASoC topology manifest - and path templates - found within DAPM widget
> private data. Dictionaries job is to reduce the total amount of memory

Or are the use cases baked into the driver code if they're in the DAPM
widget private data?

> +struct avs_tplg_token_parser {
> +	enum avs_tplg_token token;
> +	u32 type;
> +	u32 offset;
> +	int (*parse)(struct snd_soc_component *comp, void *elem, void *object, u32 offset);
> +};
> +
> +static int
> +avs_parse_uuid_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple = elem;
> +	guid_t *val = (guid_t *)((u8 *)object + offset);
> +
> +	guid_copy((guid_t *)val, (const guid_t *)&tuple->value);
> +
> +	return 0;
> +}

I have to say I'm having a hard time telling if these parsers are doing
the right thing - the interface is a bit obscure and all the void *s
make it hard to follow, and of course the format is undocumented.  I
looked through a lot of it but I've definitely not gone through this
code properly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]