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@xxxxxxxxxxxxxxx>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> +// Amadeusz Slawinski <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> +//
> +
> +#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;
> +};
