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

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@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@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@intel.com +// Amadeusz Slawinski amadeuszx.slawinski@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;

+};