[RFC 02/13] ASoC: Intel: avs: Add topology parsing infrastructure

[Pierre-Louis Bossart]

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

what is a "path template"? this is the third time I review your patches
and I have yet to find a description of all this.

If you introduce a new concept you really need to explain what problem
you are trying to solve, why it's important and what other alternatives
could be considered. Consider adding a Documentation file to explain
what you are trying to accomplish.

Jumping to optimizations of memory footprint through dictionaries is too
early.

> private data. Dictionaries job is to reduce the total amount of memory
> occupied by topology elements. Rather than having every pipeline and
> module carry its own information, each refers to specific entry in
> specific dictionary by provided (from topology file) indexes. In
> consequence, most struct avs_tplg_xxx are made out of pointers.
> To support the above, range of parsing helpers for all value-types known
> to ALSA: uuid, bool, byte, short, word and string are added. Additional
> handlers help translate pointer-types and more complex objects such as
> audio formats and module base configs.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski at linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski at intel.com>
> ---
>  sound/soc/intel/avs/avs.h      |  14 +
>  sound/soc/intel/avs/topology.c | 595 +++++++++++++++++++++++++++++++++
>  sound/soc/intel/avs/topology.h |  44 +++
>  3 files changed, 653 insertions(+)
>  create mode 100644 sound/soc/intel/avs/topology.c
>  create mode 100644 sound/soc/intel/avs/topology.h
> 
> diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
> index 20987c7744a3..61842720c894 100644
> --- a/sound/soc/intel/avs/avs.h
> +++ b/sound/soc/intel/avs/avs.h
> @@ -13,10 +13,12 @@
>  #include <linux/firmware.h>
>  #include <sound/hda_codec.h>
>  #include <sound/hda_register.h>
> +#include <sound/soc-component.h>
>  #include "messages.h"
>  #include "registers.h"
>  
>  struct avs_dev;
> +struct avs_tplg;
>  
>  struct avs_dsp_ops {
>  	int (* const power)(struct avs_dev *, u32, bool);
> @@ -223,4 +225,16 @@ int avs_hda_load_library(struct avs_dev *adev, struct firmware *lib, u32 id);
>  int avs_hda_transfer_modules(struct avs_dev *adev, bool load,
>  			     struct avs_module_entry *mods, u32 num_mods);
>  
> +/* Soc component members */
> +
> +struct avs_soc_component {
> +	struct snd_soc_component base;
> +	struct avs_tplg *tplg;
> +
> +	struct list_head node;
> +};
> +
> +#define to_avs_soc_component(comp) \
> +	container_of(comp, struct avs_soc_component, base)
> +
>  #endif /* __SOUND_SOC_INTEL_AVS_H */
> diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
> new file mode 100644
> index 000000000000..4b8b415ca006
> --- /dev/null
> +++ b/sound/soc/intel/avs/topology.c
> @@ -0,0 +1,595 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright(c) 2021 Intel Corporation. All rights reserved.
> +//
> +// Authors: Cezary Rojewski <cezary.rojewski at intel.com>
> +//          Amadeusz Slawinski <amadeuszx.slawinski at linux.intel.com>
> +//
> +
> +#include <linux/uuid.h>
> +#include <sound/soc.h>
> +#include <sound/soc-acpi.h>
> +#include <sound/soc-topology.h>
> +#include <uapi/sound/intel/avs/tokens.h>
> +#include "avs.h"
> +#include "topology.h"
> +
> +/* Get pointer to vendor array at the specified offset. */
> +#define avs_tplg_vendor_array_at(array, offset) \
> +	((struct snd_soc_tplg_vendor_array *)((u8 *)array + offset))
> +
> +/* Get pointer to vendor array that is next in line. */
> +#define avs_tplg_vendor_array_next(array) \
> +	(avs_tplg_vendor_array_at(array, le32_to_cpu((array)->size)))
> +
> +/*
> + * Scan provided block of tuples for the specified token. If found,
> + * @offset is updated with position at which first matching token is
> + * located.
> + *
> + * Returns 0 on success, -ENOENT if not found and error code otherwise.
> + */
> +static int
> +avs_tplg_vendor_array_lookup(struct snd_soc_tplg_vendor_array *tuples,
> +			     u32 block_size, u32 token, u32 *offset)
> +{
> +	u32 pos = 0;
> +
> +	while (block_size > 0) {
> +		struct snd_soc_tplg_vendor_value_elem *tuple;
> +		u32 tuples_size = le32_to_cpu(tuples->size);
> +
> +		if (tuples_size > block_size)
> +			return -EINVAL;
> +
> +		tuple = tuples->value;
> +		if (le32_to_cpu(tuple->token) == token) {
> +			*offset = pos;
> +			return 0;
> +		}
> +
> +		block_size -= tuples_size;
> +		pos += tuples_size;
> +		tuples = avs_tplg_vendor_array_next(tuples);
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +/*
> + * See avs_tplg_vendor_array_lookup() for description.
> + *
> + * Behaves exactly like its precursor but starts from the next vendor
> + * array in line. Useful when searching for the finish line of an
> + * arbitrary entry in a list of entries where each is composed of
> + * several vendor tuples and a specific token marks the beginning of
> + * a new entry block.

please try and reword such comments for people who didn't take part in
the development.

I really have no idea what this is about.

> + */
> +static int
> +avs_tplg_vendor_array_lookup_next(struct snd_soc_tplg_vendor_array *tuples,
> +				  u32 block_size, u32 token, u32 *offset)
> +{
> +	u32 tuples_size = le32_to_cpu(tuples->size);
> +	int ret;
> +
> +	if (tuples_size > block_size)
> +		return -EINVAL;
> +
> +	tuples = avs_tplg_vendor_array_next(tuples);
> +	block_size -= tuples_size;
> +
> +	ret = avs_tplg_vendor_array_lookup(tuples, block_size, token, offset);
> +	if (!ret)
> +		*offset += tuples_size;
> +	return ret;
> +}
> +
> +/*
> + * Scan provided block of tuples for the specified token which marks
> + * the boarder of an entry block. Behavior is similar to

boarder looks like a typo. Did you mean border? boundary? position?
location?

> + * avs_tplg_vendor_array_lookup() except 0 is also returned if no
> + * matching token has been found. In such case, returned @size is
> + * assigned to @block_size as the entire block belongs to the current
> + * entry.
> + *
> + * Returns 0 on success, error code otherwise.
> + */
> +static int
> +avs_tplg_vendor_entry_size(struct snd_soc_tplg_vendor_array *tuples,
> +			   u32 block_size, u32 entry_id_token, u32 *size)
> +{
> +	int ret;
> +
> +	ret = avs_tplg_vendor_array_lookup_next(tuples, block_size, entry_id_token, size);
> +	if (ret == -ENOENT) {
> +		*size = block_size;
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Vendor tuple parsing descriptor.
> + *
> + * @token: vendor specific token that identifies tuple
> + * @type: tuple type, one of SND_SOC_TPLG_TUPLE_TYPE_XXX
> + * @offset: offset of a struct's field to initialize
> + * @parse: parsing function, extracts and assigns value to object's field
> + */
> +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;
> +}
> +
> +static int
> +avs_parse_bool_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple = elem;
> +	bool *val = (bool *)((u8 *)object + offset);
> +
> +	*val = le32_to_cpu(tuple->value);
> +
> +	return 0;
> +}
> +
> +static int
> +avs_parse_byte_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple = elem;
> +	u8 *val = ((u8 *)object + offset);
> +
> +	*val = le32_to_cpu(tuple->value);
> +
> +	return 0;
> +}
> +
> +static int
> +avs_parse_short_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple = elem;
> +	u16 *val = (u16 *)((u8 *)object + offset);
> +
> +	*val = le32_to_cpu(tuple->value);
> +
> +	return 0;
> +}
> +
> +static int
> +avs_parse_word_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple = elem;
> +	u32 *val = (u32 *)((u8 *)object + offset);
> +
> +	*val = le32_to_cpu(tuple->value);
> +
> +	return 0;
> +}
> +
> +static int
> +avs_parse_string_token(struct snd_soc_component *comp, void *elem, void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_string_elem *tuple = elem;
> +	char *val = (char *)((u8 *)object + offset);
> +
> +	snprintf(val, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "%s", tuple->string);
> +
> +	return 0;
> +}
> +
> +static int avs_parse_uuid_tokens(struct snd_soc_component *comp, void *object,
> +				 const struct avs_tplg_token_parser *parsers, int count,
> +				 struct snd_soc_tplg_vendor_array *tuples)
> +{
> +	struct snd_soc_tplg_vendor_uuid_elem *tuple;
> +	int ret, i, j;
> +
> +	/* Parse element by element. */
> +	for (i = 0; i < le32_to_cpu(tuples->num_elems); i++) {
> +		tuple = &tuples->uuid[i];
> +
> +		for (j = 0; j < count; j++) {
> +			/* Ignore non-UUID tokens. */
> +			if (parsers[j].type != SND_SOC_TPLG_TUPLE_TYPE_UUID ||
> +			    parsers[j].token != le32_to_cpu(tuple->token))
> +				continue;
> +
> +			ret = parsers[j].parse(comp, tuple, object, parsers[j].offset);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int avs_parse_string_tokens(struct snd_soc_component *comp, void *object,
> +				   const struct avs_tplg_token_parser *parsers, int count,
> +				   struct snd_soc_tplg_vendor_array *tuples)
> +{
> +	struct snd_soc_tplg_vendor_string_elem *tuple;
> +	int ret, i, j;
> +
> +	/* Parse element by element. */
> +	for (i = 0; i < le32_to_cpu(tuples->num_elems); i++) {
> +		tuple = &tuples->string[i];
> +
> +		for (j = 0; j < count; j++) {
> +			/* Ignore non-string tokens. */
> +			if (parsers[j].type != SND_SOC_TPLG_TUPLE_TYPE_STRING ||
> +			    parsers[j].token != le32_to_cpu(tuple->token))
> +				continue;
> +
> +			ret = parsers[j].parse(comp, tuple, object, parsers[j].offset);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int avs_parse_word_tokens(struct snd_soc_component *comp, void *object,
> +				 const struct avs_tplg_token_parser *parsers, int count,
> +				 struct snd_soc_tplg_vendor_array *tuples)
> +{
> +	struct snd_soc_tplg_vendor_value_elem *tuple;
> +	int ret, i, j;
> +
> +	/* Parse element by element. */
> +	for (i = 0; i < le32_to_cpu(tuples->num_elems); i++) {
> +		tuple = &tuples->value[i];
> +
> +		for (j = 0; j < count; j++) {
> +			/* Ignore non-integer tokens. */
> +			if (!(parsers[j].type == SND_SOC_TPLG_TUPLE_TYPE_WORD ||
> +			      parsers[j].type == SND_SOC_TPLG_TUPLE_TYPE_SHORT ||
> +			      parsers[j].type == SND_SOC_TPLG_TUPLE_TYPE_BYTE ||
> +			      parsers[j].type == SND_SOC_TPLG_TUPLE_TYPE_BOOL))
> +				continue;
> +
> +			if (parsers[j].token != le32_to_cpu(tuple->token))
> +				continue;
> +
> +			ret = parsers[j].parse(comp, tuple, object, parsers[j].offset);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int avs_parse_tokens(struct snd_soc_component *comp, void *object,
> +			    const struct avs_tplg_token_parser *parsers, size_t count,
> +			    struct snd_soc_tplg_vendor_array *tuples, int priv_size)
> +{
> +	int array_size, ret;
> +
> +	while (priv_size > 0) {
> +		array_size = le32_to_cpu(tuples->size);
> +
> +		if (array_size <= 0) {
> +			dev_err(comp->dev, "invalid array size 0x%x\n", array_size);
> +			return -EINVAL;
> +		}
> +
> +		/* Make sure there is enough data before parsing. */
> +		priv_size -= array_size;
> +		if (priv_size < 0) {
> +			dev_err(comp->dev, "invalid array size 0x%x\n", array_size);
> +			return -EINVAL;
> +		}
> +
> +		switch (le32_to_cpu(tuples->type)) {
> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> +			ret = avs_parse_uuid_tokens(comp, object, parsers, count, tuples);
> +			break;
> +		case SND_SOC_TPLG_TUPLE_TYPE_STRING:
> +			ret = avs_parse_string_tokens(comp, object, parsers, count, tuples);
> +			break;
> +		case SND_SOC_TPLG_TUPLE_TYPE_BOOL:
> +		case SND_SOC_TPLG_TUPLE_TYPE_BYTE:
> +		case SND_SOC_TPLG_TUPLE_TYPE_SHORT:
> +		case SND_SOC_TPLG_TUPLE_TYPE_WORD:
> +			ret = avs_parse_word_tokens(comp, object, parsers, count, tuples);

avs_parse_bool_token(struct snd_soc_component *comp, void *elem, void
*object, u32 offset)
avs_parse_byte_token(struct snd_soc_component *comp, void *elem, void
*object, u32 offset)
avs_parse_short_token(struct snd_soc_component *comp, void *elem, void
*object, u32 offset)

why did you introduce such helpers, if you only use word_tokens?

> +			break;
> +		default:
> +			dev_err(comp->dev, "unknown token type %d\n", tuples->type);
> +			ret = -EINVAL;
> +		}
> +
> +		if (ret) {
> +			dev_err(comp->dev, "parsing %ld tokens of %d type failed: %d\n",
> +				count, tuples->type, ret);
> +			return ret;
> +		}
> +
> +		tuples = avs_tplg_vendor_array_next(tuples);
> +	}
> +
> +	return 0;
> +}

> +static int parse_link_formatted_string(struct snd_soc_component *comp, void *elem,
> +				       void *object, u32 offset)
> +{
> +	struct snd_soc_tplg_vendor_string_elem *tuple = elem;
> +	struct snd_soc_acpi_mach *mach = dev_get_platdata(comp->card->dev);
> +	char *val = (char *)((u8 *)object + offset);
> +
> +	/*
> +	 * Dynamic naming - string formats, e.g.: ssp%d - supported only for
> +	 * topologies describing single device e.g.: an I2S codec on SSP0.
> +	 */

what are you trying to optimize here? the topology will contain the name
in all cases?

> +	if (hweight_long(mach->link_mask) != 1)
> +		return avs_parse_string_token(comp, elem, object, offset);
> +
> +	snprintf(val, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, tuple->string,
> +		 __ffs(mach->link_mask));
> +
> +	return 0;
> +}

> +struct avs_tplg {
> +	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	u32 version;

version of what and where does it come from (manifest)?

does this contain an ABI information? if yes, how is it defined?

> +	struct snd_soc_component *comp;
> +
> +	struct avs_tplg_library *libs;
> +	u32 num_libs;
> +	struct avs_audio_format *fmts;
> +	u32 num_fmts;
> +	struct avs_tplg_modcfg_base *modcfgs_base;
> +	u32 num_modcfgs_base;
> +};