[RFC 00/13] ASoC: Intel: avs: Topology and path management
A continuation of avs-driver initial series [1]. This chapter covers path management and topology parsing part which was ealier path of the main series. The two patches that represented these two subjects in the initial series, have been split into many to allow for easier review and discussion.
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 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.
A 'path' represents a DSP side of audio stream in runtime - is a logical container for pipelines which are themselves composed of modules - processing units. Due to high range of possible audio format combinations, there can be more variants of given path (and thus, its pipelines and modules) than total number of pipelines and module instances which firmware supports concurrently, all the instance IDs are allocated dynamically with help of IDA interface. 'Path templates' come from topology file and describe a pattern which is later used to actually create runtime 'path'.
[1]: https://lore.kernel.org/alsa-devel/20220207122108.3780926-1-cezary.rojewski@...
Cezary Rojewski (13): ASoC: Intel: avs: Declare vendor tokens ASoC: Intel: avs: Add topology parsing infrastructure ASoC: Intel: avs: Parse module-extension tuples ASoC: Intel: avs: Parse pplcfg and binding tuples ASoC: Intel: avs: Parse pipeline and module tuples ASoC: Intel: avs: Parse path and path templates tuples ASoC: Intel: avs: Add topology loading operations ASoC: Intel: avs: Declare path and its components ASoC: Intel: avs: Path creation and freeing ASoC: Intel: avs: Path state management ASoC: Intel: avs: Arm paths after creating them ASoC: Intel: avs: Prepare modules before bindings them ASoC: Intel: avs: Configure modules according to their type
include/uapi/sound/intel/avs/tokens.h | 126 ++ sound/soc/intel/Kconfig | 2 + sound/soc/intel/avs/Makefile | 3 +- sound/soc/intel/avs/avs.h | 23 + sound/soc/intel/avs/path.c | 1008 ++++++++++++++++ sound/soc/intel/avs/path.h | 72 ++ sound/soc/intel/avs/topology.c | 1600 +++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 195 +++ 8 files changed, 3028 insertions(+), 1 deletion(-) create mode 100644 include/uapi/sound/intel/avs/tokens.h create mode 100644 sound/soc/intel/avs/path.c create mode 100644 sound/soc/intel/avs/path.h create mode 100644 sound/soc/intel/avs/topology.c create mode 100644 sound/soc/intel/avs/topology.h
Expose all vendor tokens that help shape AVS topology. Parsing helpers introduced in follow up patches make use of these to know which block they are currently dealing with and to verify their correctness.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- include/uapi/sound/intel/avs/tokens.h | 126 ++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 include/uapi/sound/intel/avs/tokens.h
diff --git a/include/uapi/sound/intel/avs/tokens.h b/include/uapi/sound/intel/avs/tokens.h new file mode 100644 index 000000000000..754f02b2f444 --- /dev/null +++ b/include/uapi/sound/intel/avs/tokens.h @@ -0,0 +1,126 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Copyright(c) 2021 Intel Corporation. All rights reserved. + * + * Authors: Cezary Rojewski cezary.rojewski@intel.com + * Amadeusz Slawinski amadeuszx.slawinski@linux.intel.com + */ + +#ifndef __UAPI_SOUND_INTEL_AVS_TOKENS_H +#define __UAPI_SOUND_INTEL_AVS_TOKENS_H + +enum avs_tplg_token { + /* struct avs_tplg */ + AVS_TKN_MANIFEST_NAME_STRING = 1, + AVS_TKN_MANIFEST_VERSION_U32 = 2, + AVS_TKN_MANIFEST_NUM_LIBRARIES_U32 = 3, + AVS_TKN_MANIFEST_NUM_AFMTS_U32 = 4, + AVS_TKN_MANIFEST_NUM_MODCFGS_BASE_U32 = 5, + AVS_TKN_MANIFEST_NUM_MODCFGS_EXT_U32 = 6, + AVS_TKN_MANIFEST_NUM_PPLCFGS_U32 = 7, + AVS_TKN_MANIFEST_NUM_BINDINGS_U32 = 8, + + /* struct avs_tplg_library */ + AVS_TKN_LIBRARY_ID_U32 = 101, + AVS_TKN_LIBRARY_NAME_STRING = 102, + + /* struct avs_audio_format */ + AVS_TKN_AFMT_ID_U32 = 201, + AVS_TKN_AFMT_SAMPLE_RATE_U32 = 202, + AVS_TKN_AFMT_BIT_DEPTH_U32 = 203, + AVS_TKN_AFMT_CHANNEL_MAP_U32 = 204, + AVS_TKN_AFMT_CHANNEL_CFG_U32 = 205, + AVS_TKN_AFMT_INTERLEAVING_U32 = 206, + AVS_TKN_AFMT_NUM_CHANNELS_U32 = 207, + AVS_TKN_AFMT_VALID_BIT_DEPTH_U32 = 208, + AVS_TKN_AFMT_SAMPLE_TYPE_U32 = 209, + + /* struct avs_tplg_modcfg_base */ + AVS_TKN_MODCFG_BASE_ID_U32 = 301, + AVS_TKN_MODCFG_BASE_CPC_U32 = 302, + AVS_TKN_MODCFG_BASE_IBS_U32 = 303, + AVS_TKN_MODCFG_BASE_OBS_U32 = 304, + AVS_TKN_MODCFG_BASE_PAGES_U32 = 305, + + /* struct avs_tplg_modcfg_ext */ + AVS_TKN_MODCFG_EXT_ID_U32 = 401, + AVS_TKN_MODCFG_EXT_TYPE_UUID = 402, + AVS_TKN_MODCFG_CPR_OUT_AFMT_ID_U32 = 403, + AVS_TKN_MODCFG_CPR_FEATURE_MASK_U32 = 404, + AVS_TKN_MODCFG_CPR_DMA_TYPE_U32 = 405, + AVS_TKN_MODCFG_CPR_DMABUFF_SIZE_U32 = 406, + AVS_TKN_MODCFG_CPR_VINDEX_U8 = 407, + AVS_TKN_MODCFG_CPR_BLOB_FMT_ID_U32 = 408, + AVS_TKN_MODCFG_MICSEL_OUT_AFMT_ID_U32 = 409, + AVS_TKN_MODCFG_INTELWOV_CPC_LP_MODE_U32 = 410, + AVS_TKN_MODCFG_SRC_OUT_FREQ_U32 = 411, + AVS_TKN_MODCFG_MUX_REF_AFMT_ID_U32 = 412, + AVS_TKN_MODCFG_MUX_OUT_AFMT_ID_U32 = 413, + AVS_TKN_MODCFG_AEC_REF_AFMT_ID_U32 = 414, + AVS_TKN_MODCFG_AEC_OUT_AFMT_ID_U32 = 415, + AVS_TKN_MODCFG_AEC_CPC_LP_MODE_U32 = 416, + AVS_TKN_MODCFG_ASRC_OUT_FREQ_U32 = 417, + AVS_TKN_MODCFG_ASRC_MODE_U8 = 418, + AVS_TKN_MODCFG_ASRC_DISABLE_JITTER_U8 = 419, + AVS_TKN_MODCFG_UPDOWN_MIX_OUT_CHAN_CFG_U32 = 420, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_SELECT_U32 = 421, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_0_S32 = 422, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_1_S32 = 423, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_2_S32 = 424, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_3_S32 = 425, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_4_S32 = 426, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_5_S32 = 427, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_6_S32 = 428, + AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_7_S32 = 429, + AVS_TKN_MODCFG_UPDOWN_MIX_CHAN_MAP_U32 = 430, + AVS_TKN_MODCFG_EXT_NUM_INPUT_PINS_U16 = 431, + AVS_TKN_MODCFG_EXT_NUM_OUTPUT_PINS_U16 = 432, + + /* struct avs_tplg_pplcfg */ + AVS_TKN_PPLCFG_ID_U32 = 1401, + AVS_TKN_PPLCFG_REQ_SIZE_U16 = 1402, + AVS_TKN_PPLCFG_PRIORITY_U8 = 1403, + AVS_TKN_PPLCFG_LOW_POWER_BOOL = 1404, + AVS_TKN_PPLCFG_ATTRIBUTES_U16 = 1405, + AVS_TKN_PPLCFG_TRIGGER_U32 = 1406, + + /* struct avs_tplg_binding */ + AVS_TKN_BINDING_ID_U32 = 1501, + AVS_TKN_BINDING_TARGET_TPLG_NAME_STRING = 1502, + AVS_TKN_BINDING_TARGET_PATH_TMPL_ID_U32 = 1503, + AVS_TKN_BINDING_TARGET_PPL_ID_U32 = 1504, + AVS_TKN_BINDING_TARGET_MOD_ID_U32 = 1505, + AVS_TKN_BINDING_TARGET_MOD_PIN_U8 = 1506, + AVS_TKN_BINDING_MOD_ID_U32 = 1507, + AVS_TKN_BINDING_MOD_PIN_U8 = 1508, + AVS_TKN_BINDING_IS_SINK_U8 = 1509, + + /* struct avs_tplg_pipeline */ + AVS_TKN_PPL_ID_U32 = 1601, + AVS_TKN_PPL_PPLCFG_ID_U32 = 1602, + AVS_TKN_PPL_NUM_BINDING_IDS_U32 = 1603, + AVS_TKN_PPL_BINDING_ID_U32 = 1604, + + /* struct avs_tplg_module */ + AVS_TKN_MOD_ID_U32 = 1701, + AVS_TKN_MOD_MODCFG_BASE_ID_U32 = 1702, + AVS_TKN_MOD_IN_AFMT_ID_U32 = 1703, + AVS_TKN_MOD_CORE_ID_U8 = 1704, + AVS_TKN_MOD_PROC_DOMAIN_U8 = 1705, + AVS_TKN_MOD_MODCFG_EXT_ID_U32 = 1706, + + /* struct avs_tplg_path_template */ + AVS_TKN_PATH_TMPL_ID_U32 = 1801, + + /* struct avs_tplg_path */ + AVS_TKN_PATH_ID_U32 = 1901, + AVS_TKN_PATH_FE_FMT_ID_U32 = 1902, + AVS_TKN_PATH_BE_FMT_ID_U32 = 1903, + + /* struct avs_tplg_pin_format */ + AVS_TKN_PIN_FMT_INDEX_U32 = 2201, + AVS_TKN_PIN_FMT_IOBS_U32 = 2202, + AVS_TKN_PIN_FMT_AFMT_ID_U32 = 2203, +}; + +#endif
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 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. + */ +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 + * 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); + 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; +} + +#define AVS_DEFINE_PTR_PARSER(name, type, member) \ +static int \ +avs_parse_##name##_ptr(struct snd_soc_component *comp, void *elem, void *object, u32 offset) \ +{ \ + struct snd_soc_tplg_vendor_value_elem *tuple = elem; \ + struct avs_soc_component *acomp = to_avs_soc_component(comp); \ + type **val = (type **)(object + offset); \ + u32 idx; \ + \ + idx = le32_to_cpu(tuple->value); \ + if (idx >= acomp->tplg->num_##member) \ + return -EINVAL; \ + \ + *val = &acomp->tplg->member[idx]; \ + \ + return 0; \ +} + +AVS_DEFINE_PTR_PARSER(audio_format, struct avs_audio_format, fmts); +AVS_DEFINE_PTR_PARSER(modcfg_base, struct avs_tplg_modcfg_base, modcfgs_base); + +static int +parse_audio_format_bitfield(struct snd_soc_component *comp, void *elem, void *object, u32 offset) +{ + struct snd_soc_tplg_vendor_value_elem *velem = elem; + struct avs_audio_format *audio_format = object; + + switch (offset) { + case AVS_TKN_AFMT_NUM_CHANNELS_U32: + audio_format->num_channels = le32_to_cpu(velem->value); + break; + case AVS_TKN_AFMT_VALID_BIT_DEPTH_U32: + audio_format->valid_bit_depth = le32_to_cpu(velem->value); + break; + case AVS_TKN_AFMT_SAMPLE_TYPE_U32: + audio_format->sample_type = le32_to_cpu(velem->value); + break; + } + + 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. + */ + 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; +} + +static int +parse_dictionary_header(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + void **dict, u32 *num_entries, size_t entry_size, + u32 num_entries_token) +{ + struct snd_soc_tplg_vendor_value_elem *tuple; + + /* Dictionary header consists of single tuple - entry count. */ + tuple = tuples->value; + if (le32_to_cpu(tuple->token) != num_entries_token) { + dev_err(comp->dev, "invalid dictionary header, expected: %d\n", + num_entries_token); + return -EINVAL; + } + + *num_entries = le32_to_cpu(tuple->value); + *dict = devm_kcalloc(comp->card->dev, *num_entries, entry_size, GFP_KERNEL); + if (!*dict) + return -ENOMEM; + + return 0; +} + +static int +parse_dictionary_entries(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size, + void *dict, u32 num_entries, size_t entry_size, + u32 entry_id_token, + const struct avs_tplg_token_parser *parsers, size_t num_parsers) +{ + void *pos = dict; + int i; + + for (i = 0; i < num_entries; i++) { + u32 esize; + int ret; + + ret = avs_tplg_vendor_entry_size(tuples, block_size, + entry_id_token, &esize); + if (ret) + return ret; + + ret = avs_parse_tokens(comp, pos, parsers, num_parsers, tuples, esize); + if (ret < 0) { + dev_err(comp->dev, "parse entry: %d of type: %d failed: %d\n", + i, entry_id_token, ret); + return ret; + } + + pos += entry_size; + block_size -= esize; + tuples = avs_tplg_vendor_array_at(tuples, esize); + } + + return 0; +} + +static int parse_dictionary(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size, + void **dict, u32 *num_entries, size_t entry_size, + u32 num_entries_token, u32 entry_id_token, + const struct avs_tplg_token_parser *parsers, size_t num_parsers) +{ + int ret; + + ret = parse_dictionary_header(comp, tuples, dict, num_entries, + entry_size, num_entries_token); + if (ret) + return ret; + + block_size -= le32_to_cpu(tuples->size); + /* With header parsed, move on to parsing entries. */ + tuples = avs_tplg_vendor_array_next(tuples); + + return parse_dictionary_entries(comp, tuples, block_size, *dict, + *num_entries, entry_size, + entry_id_token, parsers, num_parsers); +} + +static const struct avs_tplg_token_parser library_parsers[] = { + { + .token = AVS_TKN_LIBRARY_NAME_STRING, + .type = SND_SOC_TPLG_TUPLE_TYPE_STRING, + .offset = offsetof(struct avs_tplg_library, name), + .parse = avs_parse_string_token, + }, +}; + +static int avs_tplg_parse_libraries(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + + return parse_dictionary(comp, tuples, block_size, (void **)&tplg->libs, + &tplg->num_libs, sizeof(*tplg->libs), + AVS_TKN_MANIFEST_NUM_LIBRARIES_U32, + AVS_TKN_LIBRARY_ID_U32, + library_parsers, ARRAY_SIZE(library_parsers)); +} + +static const struct avs_tplg_token_parser audio_format_parsers[] = { + { + .token = AVS_TKN_AFMT_SAMPLE_RATE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_audio_format, sampling_freq), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_AFMT_BIT_DEPTH_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_audio_format, bit_depth), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_AFMT_CHANNEL_MAP_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_audio_format, channel_map), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_AFMT_CHANNEL_CFG_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_audio_format, channel_config), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_AFMT_INTERLEAVING_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_audio_format, interleaving), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_AFMT_NUM_CHANNELS_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = AVS_TKN_AFMT_NUM_CHANNELS_U32, + .parse = parse_audio_format_bitfield, + }, + { + .token = AVS_TKN_AFMT_VALID_BIT_DEPTH_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = AVS_TKN_AFMT_VALID_BIT_DEPTH_U32, + .parse = parse_audio_format_bitfield, + }, + { + .token = AVS_TKN_AFMT_SAMPLE_TYPE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = AVS_TKN_AFMT_SAMPLE_TYPE_U32, + .parse = parse_audio_format_bitfield, + }, +}; + +static int avs_tplg_parse_audio_formats(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + + return parse_dictionary(comp, tuples, block_size, (void **)&tplg->fmts, + &tplg->num_fmts, sizeof(*tplg->fmts), + AVS_TKN_MANIFEST_NUM_AFMTS_U32, + AVS_TKN_AFMT_ID_U32, + audio_format_parsers, ARRAY_SIZE(audio_format_parsers)); +} + +static const struct avs_tplg_token_parser modcfg_base_parsers[] = { + { + .token = AVS_TKN_MODCFG_BASE_CPC_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_base, cpc), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_BASE_IBS_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_base, ibs), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_BASE_OBS_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_base, obs), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_BASE_PAGES_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_base, is_pages), + .parse = avs_parse_word_token, + }, +}; + +static int avs_tplg_parse_modcfgs_base(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + + return parse_dictionary(comp, tuples, block_size, (void **)&tplg->modcfgs_base, + &tplg->num_modcfgs_base, sizeof(*tplg->modcfgs_base), + AVS_TKN_MANIFEST_NUM_MODCFGS_BASE_U32, + AVS_TKN_MODCFG_BASE_ID_U32, + modcfg_base_parsers, ARRAY_SIZE(modcfg_base_parsers)); +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h new file mode 100644 index 000000000000..a3ab5d15c9ee --- /dev/null +++ b/sound/soc/intel/avs/topology.h @@ -0,0 +1,44 @@ +/* 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 + */ + +#ifndef __SOUND_SOC_INTEL_AVS_TPLG_H +#define __SOUND_SOC_INTEL_AVS_TPLG_H + +#include <linux/list.h> +#include "messages.h" + +#define INVALID_OBJECT_ID UINT_MAX + +struct snd_soc_component; + +struct avs_tplg { + char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; + u32 version; + 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; +}; + +struct avs_tplg_library { + char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; +}; + +/* Matches header of struct avs_mod_cfg_base. */ +struct avs_tplg_modcfg_base { + u32 cpc; + u32 ibs; + u32 obs; + u32 is_pages; +}; + +#endif
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;
+};
On 2022-02-25 6:20 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
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.
Hello,
I don't believe it's early for optimization and such. ASoC topology feature has not been invented yesterday and most of the topology files we see used are far from perfect.
I've been trying to explaining "path template" on several occasions, also during our meeting last year. Now, there's no separate Documentation for "path template" as is not a new concept really, it's a different name for already existing thing. Every driver which makes use of ASoC topology needs to have a "path template". skylake-driver has a "path template", sof-driver has one too. Topology information does not match 1:1 to runtime, it never did. You use topology to describe how the stream shall look like in runtime, kernel takes that information and instantiates the runtime.
If you do not believe, please see the skylake-driver topology which is made of: - ModuleType, ModuleResource, ModuleInterface (...) dictionaries - Path and PathDescription
There two blocks looks very, very similar to: - ModuleConfigBase, ModuleConfigExt (...) dictionaries - Path and PathTemplate
which are supposedly 'new' in avs-driver. Yes, we provided several optimizations, but the "path template"/"path pattern"/"path description" was already there.
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.
...
+/*
- 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.
Please provide suggestion - "don't understand" does not help me in rewording the comment.
ASoC topology is not the easiest to digest feature in general. Comments found here assume the layout and organization of sections, such as vendor tokens and vendor tuples with ASoC topology file are known to the reader.
- */
+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?
Indeed, it is supposed to be "border". Thanks!
- 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_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?
Huh? we do make use of all of these. Perhaps you missed these being used in the follow up patches (in this very series). This patch defines the parsing infrastructure so its declaration is separated from module and pipeline parsing details.
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?
I'll probably separate the name%d part so it's not clouding the core part of topology parsing.
These if-statements are here to allow %d to be filled automatically by kernel for SSP boards with ->link_mask found in ACPI board descriptor.
Example for avs_rt298 with snd_soc_acpi_mach::link_mask=BIT(0): 1) Topology file for avs_rt298 provides widget with name: ssp%d_be 2) Runtime topology parsing formats that name to: ssp0_be
- 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?
Yes, this one comes from topology manifest. Right now we decided to use single-digit versioning for simplicity, similarly to ASoC topology one.
- 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;
+};
Anything that goes beyond module base config is an extension config. It covers all fields for all specific module types available in ADSP firmware. Add parsing helpers to support loading such information from the topology file.
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/topology.c | 297 +++++++++++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 59 +++++++ 2 files changed, 356 insertions(+)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 4b8b415ca006..6c682fed9f5f 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -344,6 +344,7 @@ avs_parse_##name##_ptr(struct snd_soc_component *comp, void *elem, void *object,
AVS_DEFINE_PTR_PARSER(audio_format, struct avs_audio_format, fmts); AVS_DEFINE_PTR_PARSER(modcfg_base, struct avs_tplg_modcfg_base, modcfgs_base); +AVS_DEFINE_PTR_PARSER(modcfg_ext, struct avs_tplg_modcfg_ext, modcfgs_ext);
static int parse_audio_format_bitfield(struct snd_soc_component *comp, void *elem, void *object, u32 offset) @@ -593,3 +594,299 @@ static int avs_tplg_parse_modcfgs_base(struct snd_soc_component *comp, AVS_TKN_MODCFG_BASE_ID_U32, modcfg_base_parsers, ARRAY_SIZE(modcfg_base_parsers)); } + +static const struct avs_tplg_token_parser modcfg_ext_parsers[] = { + { + .token = AVS_TKN_MODCFG_EXT_TYPE_UUID, + .type = SND_SOC_TPLG_TUPLE_TYPE_UUID, + .offset = offsetof(struct avs_tplg_modcfg_ext, type), + .parse = avs_parse_uuid_token, + }, + { + .token = AVS_TKN_MODCFG_CPR_OUT_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.out_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_CPR_FEATURE_MASK_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.feature_mask), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_CPR_VINDEX_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.vindex), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_MODCFG_CPR_DMA_TYPE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.dma_type), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_CPR_DMABUFF_SIZE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.dma_buffer_size), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_CPR_BLOB_FMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, copier.blob_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_MICSEL_OUT_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, micsel.out_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_INTELWOV_CPC_LP_MODE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, wov.cpc_lp_mode), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_SRC_OUT_FREQ_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, src.out_freq), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_MUX_REF_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, mux.ref_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_MUX_OUT_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, mux.out_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_AEC_REF_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, aec.ref_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_AEC_OUT_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, aec.out_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MODCFG_AEC_CPC_LP_MODE_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, aec.cpc_lp_mode), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_ASRC_OUT_FREQ_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, asrc.out_freq), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_ASRC_MODE_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_modcfg_ext, asrc.mode), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_MODCFG_ASRC_DISABLE_JITTER_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_modcfg_ext, asrc.disable_jitter_buffer), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_OUT_CHAN_CFG_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.out_channel_config), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_SELECT_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients_select), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_0_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[0]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_1_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[1]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_2_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[2]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_3_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[3]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_4_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[4]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_5_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[5]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_6_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[6]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_COEFF_7_S32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.coefficients[7]), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_UPDOWN_MIX_CHAN_MAP_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_modcfg_ext, updown_mix.channel_map), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MODCFG_EXT_NUM_INPUT_PINS_U16, + .type = SND_SOC_TPLG_TUPLE_TYPE_SHORT, + .offset = offsetof(struct avs_tplg_modcfg_ext, generic.num_input_pins), + .parse = avs_parse_short_token, + }, + { + .token = AVS_TKN_MODCFG_EXT_NUM_OUTPUT_PINS_U16, + .type = SND_SOC_TPLG_TUPLE_TYPE_SHORT, + .offset = offsetof(struct avs_tplg_modcfg_ext, generic.num_output_pins), + .parse = avs_parse_short_token, + }, +}; + +static const struct avs_tplg_token_parser pin_format_parsers[] = { + { + .token = AVS_TKN_PIN_FMT_INDEX_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pin_format, pin_index), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_PIN_FMT_IOBS_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pin_format, iobs), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_PIN_FMT_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pin_format, fmt), + .parse = avs_parse_audio_format_ptr, + }, +}; + +static int avs_tplg_parse_modcfg_ext(struct snd_soc_component *comp, + struct avs_tplg_modcfg_ext *cfg, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + u32 esize; + int ret; + + /* See where pin block starts. */ + ret = avs_tplg_vendor_entry_size(tuples, block_size, + AVS_TKN_PIN_FMT_INDEX_U32, &esize); + if (ret) + return ret; + + ret = avs_parse_tokens(comp, cfg, modcfg_ext_parsers, + ARRAY_SIZE(modcfg_ext_parsers), tuples, esize); + if (ret) + return ret; + + block_size -= esize; + /* Parse trailing in/out pin formats if any. */ + if (block_size) { + struct avs_tplg_pin_format *pins; + u32 num_pins; + + num_pins = cfg->generic.num_input_pins + cfg->generic.num_output_pins; + if (!num_pins) + return -EINVAL; + + pins = devm_kcalloc(comp->card->dev, num_pins, sizeof(*pins), GFP_KERNEL); + if (!pins) + return -ENOMEM; + + tuples = avs_tplg_vendor_array_at(tuples, esize); + ret = parse_dictionary_entries(comp, tuples, block_size, + pins, num_pins, sizeof(*pins), + AVS_TKN_PIN_FMT_INDEX_U32, + pin_format_parsers, + ARRAY_SIZE(pin_format_parsers)); + if (ret) + return ret; + cfg->generic.pin_fmts = pins; + } + + return 0; +} + +static int avs_tplg_parse_modcfgs_ext(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + int ret, i; + + ret = parse_dictionary_header(comp, tuples, (void **)&tplg->modcfgs_ext, + &tplg->num_modcfgs_ext, + sizeof(*tplg->modcfgs_ext), + AVS_TKN_MANIFEST_NUM_MODCFGS_EXT_U32); + if (ret) + return ret; + + block_size -= le32_to_cpu(tuples->size); + /* With header parsed, move on to parsing entries. */ + tuples = avs_tplg_vendor_array_next(tuples); + + for (i = 0; i < tplg->num_modcfgs_ext; i++) { + struct avs_tplg_modcfg_ext *cfg = &tplg->modcfgs_ext[i]; + u32 esize; + + ret = avs_tplg_vendor_entry_size(tuples, block_size, + AVS_TKN_MODCFG_EXT_ID_U32, &esize); + if (ret) + return ret; + + ret = avs_tplg_parse_modcfg_ext(comp, cfg, tuples, esize); + if (ret) + return ret; + + block_size -= esize; + tuples = avs_tplg_vendor_array_at(tuples, esize); + } + + return 0; +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h index a3ab5d15c9ee..ce51fd7a99de 100644 --- a/sound/soc/intel/avs/topology.h +++ b/sound/soc/intel/avs/topology.h @@ -27,6 +27,8 @@ struct avs_tplg { u32 num_fmts; struct avs_tplg_modcfg_base *modcfgs_base; u32 num_modcfgs_base; + struct avs_tplg_modcfg_ext *modcfgs_ext; + u32 num_modcfgs_ext; };
struct avs_tplg_library { @@ -41,4 +43,61 @@ struct avs_tplg_modcfg_base { u32 is_pages; };
+struct avs_tplg_pin_format { + u32 pin_index; + u32 iobs; + struct avs_audio_format *fmt; +}; + +struct avs_tplg_modcfg_ext { + guid_t type; + + union { + struct { + u16 num_input_pins; + u16 num_output_pins; + struct avs_tplg_pin_format *pin_fmts; + } generic; + struct { + struct avs_audio_format *out_fmt; + struct avs_audio_format *blob_fmt; /* optional override */ + u32 feature_mask; + union avs_virtual_index vindex; + u32 dma_type; + u32 dma_buffer_size; + u32 config_length; + /* config_data part of priv data */ + } copier; + struct { + u32 out_channel_config; + u32 coefficients_select; + s32 coefficients[AVS_CHANNELS_MAX]; + u32 channel_map; + } updown_mix; + struct { + u32 out_freq; + } src; + struct { + u32 out_freq; + u8 mode; + u8 disable_jitter_buffer; + } asrc; + struct { + u32 cpc_lp_mode; + } wov; + struct { + struct avs_audio_format *ref_fmt; + struct avs_audio_format *out_fmt; + u32 cpc_lp_mode; + } aec; + struct { + struct avs_audio_format *ref_fmt; + struct avs_audio_format *out_fmt; + } mux; + struct { + struct avs_audio_format *out_fmt; + } micsel; + }; +}; + #endif
+struct avs_tplg_modcfg_ext {
- guid_t type;
- union {
struct {u16 num_input_pins;u16 num_output_pins;struct avs_tplg_pin_format *pin_fmts;} generic;struct {struct avs_audio_format *out_fmt;struct avs_audio_format *blob_fmt; /* optional override */u32 feature_mask;union avs_virtual_index vindex;u32 dma_type;u32 dma_buffer_size;u32 config_length;/* config_data part of priv data */} copier;struct {u32 out_channel_config;u32 coefficients_select;s32 coefficients[AVS_CHANNELS_MAX];u32 channel_map;} updown_mix;struct {u32 out_freq;} src;struct {u32 out_freq;u8 mode;u8 disable_jitter_buffer;} asrc;struct {u32 cpc_lp_mode;} wov;struct {struct avs_audio_format *ref_fmt;struct avs_audio_format *out_fmt;u32 cpc_lp_mode;} aec;struct {struct avs_audio_format *ref_fmt;struct avs_audio_format *out_fmt;} mux;struct {struct avs_audio_format *out_fmt;} micsel;- };
+};
I am struggling to reconcile the notion of 'extension' with a fixed structure that deals with Intel-specific modules.
How would a 3rd party add their own modules and expose parameters/tokens through the topology?
On 2022-02-25 6:24 PM, Pierre-Louis Bossart wrote:
+struct avs_tplg_modcfg_ext {
- guid_t type;
- union {
struct {u16 num_input_pins;u16 num_output_pins;struct avs_tplg_pin_format *pin_fmts;} generic;struct {struct avs_audio_format *out_fmt;struct avs_audio_format *blob_fmt; /* optional override */u32 feature_mask;union avs_virtual_index vindex;u32 dma_type;u32 dma_buffer_size;u32 config_length;/* config_data part of priv data */} copier;struct {u32 out_channel_config;u32 coefficients_select;s32 coefficients[AVS_CHANNELS_MAX];u32 channel_map;} updown_mix;struct {u32 out_freq;} src;struct {u32 out_freq;u8 mode;u8 disable_jitter_buffer;} asrc;struct {u32 cpc_lp_mode;} wov;struct {struct avs_audio_format *ref_fmt;struct avs_audio_format *out_fmt;u32 cpc_lp_mode;} aec;struct {struct avs_audio_format *ref_fmt;struct avs_audio_format *out_fmt;} mux;struct {struct avs_audio_format *out_fmt;} micsel;- };
+};
I am struggling to reconcile the notion of 'extension' with a fixed structure that deals with Intel-specific modules.
How would a 3rd party add their own modules and expose parameters/tokens through the topology?
All vendor modules go into 'generic' bucket. Vendor cannot define any specific fields i.e. extend the 'generic' header structure. Everything that is specific to them has to go into so called payload that is part of almost every INIT_INSTANCE.
Regards, Czarek
Stream on ADSP firmware is represented by one or more pipelines. Just like modules, these are described by a config structure. Add parsing helpers to support loading such information from the topology file.
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/topology.c | 114 +++++++++++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 29 +++++++++ 2 files changed, 143 insertions(+)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 6c682fed9f5f..7d454a0ea000 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -345,6 +345,8 @@ avs_parse_##name##_ptr(struct snd_soc_component *comp, void *elem, void *object, AVS_DEFINE_PTR_PARSER(audio_format, struct avs_audio_format, fmts); AVS_DEFINE_PTR_PARSER(modcfg_base, struct avs_tplg_modcfg_base, modcfgs_base); AVS_DEFINE_PTR_PARSER(modcfg_ext, struct avs_tplg_modcfg_ext, modcfgs_ext); +AVS_DEFINE_PTR_PARSER(pplcfg, struct avs_tplg_pplcfg, pplcfgs); +AVS_DEFINE_PTR_PARSER(binding, struct avs_tplg_binding, bindings);
static int parse_audio_format_bitfield(struct snd_soc_component *comp, void *elem, void *object, u32 offset) @@ -890,3 +892,115 @@ static int avs_tplg_parse_modcfgs_ext(struct snd_soc_component *comp,
return 0; } + +static const struct avs_tplg_token_parser pplcfg_parsers[] = { + { + .token = AVS_TKN_PPLCFG_REQ_SIZE_U16, + .type = SND_SOC_TPLG_TUPLE_TYPE_SHORT, + .offset = offsetof(struct avs_tplg_pplcfg, req_size), + .parse = avs_parse_short_token, + }, + { + .token = AVS_TKN_PPLCFG_PRIORITY_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_pplcfg, priority), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_PPLCFG_LOW_POWER_BOOL, + .type = SND_SOC_TPLG_TUPLE_TYPE_BOOL, + .offset = offsetof(struct avs_tplg_pplcfg, lp), + .parse = avs_parse_bool_token, + }, + { + .token = AVS_TKN_PPLCFG_ATTRIBUTES_U16, + .type = SND_SOC_TPLG_TUPLE_TYPE_SHORT, + .offset = offsetof(struct avs_tplg_pplcfg, attributes), + .parse = avs_parse_short_token, + }, + { + .token = AVS_TKN_PPLCFG_TRIGGER_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pplcfg, trigger), + .parse = avs_parse_word_token, + }, +}; + +static int avs_tplg_parse_pplcfgs(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + + return parse_dictionary(comp, tuples, block_size, (void **)&tplg->pplcfgs, + &tplg->num_pplcfgs, sizeof(*tplg->pplcfgs), + AVS_TKN_MANIFEST_NUM_PPLCFGS_U32, + AVS_TKN_PPLCFG_ID_U32, + pplcfg_parsers, ARRAY_SIZE(pplcfg_parsers)); +} + +static const struct avs_tplg_token_parser binding_parsers[] = { + { + .token = AVS_TKN_BINDING_TARGET_TPLG_NAME_STRING, + .type = SND_SOC_TPLG_TUPLE_TYPE_STRING, + .offset = offsetof(struct avs_tplg_binding, target_tplg_name), + .parse = parse_link_formatted_string, + }, + { + .token = AVS_TKN_BINDING_TARGET_PATH_TMPL_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_binding, target_path_tmpl_id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_BINDING_TARGET_PPL_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_binding, target_ppl_id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_BINDING_TARGET_MOD_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_binding, target_mod_id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_BINDING_TARGET_MOD_PIN_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_binding, target_mod_pin), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_BINDING_MOD_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_binding, mod_id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_BINDING_MOD_PIN_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_binding, mod_pin), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_BINDING_IS_SINK_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_binding, is_sink), + .parse = avs_parse_byte_token, + }, +}; + +static int avs_tplg_parse_bindings(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, + u32 block_size) +{ + struct avs_soc_component *acomp = to_avs_soc_component(comp); + struct avs_tplg *tplg = acomp->tplg; + + return parse_dictionary(comp, tuples, block_size, (void **)&tplg->bindings, + &tplg->num_bindings, sizeof(*tplg->bindings), + AVS_TKN_MANIFEST_NUM_BINDINGS_U32, + AVS_TKN_BINDING_ID_U32, + binding_parsers, ARRAY_SIZE(binding_parsers)); +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h index ce51fd7a99de..d23f4aba0bcc 100644 --- a/sound/soc/intel/avs/topology.h +++ b/sound/soc/intel/avs/topology.h @@ -29,6 +29,10 @@ struct avs_tplg { u32 num_modcfgs_base; struct avs_tplg_modcfg_ext *modcfgs_ext; u32 num_modcfgs_ext; + struct avs_tplg_pplcfg *pplcfgs; + u32 num_pplcfgs; + struct avs_tplg_binding *bindings; + u32 num_bindings; };
struct avs_tplg_library { @@ -100,4 +104,29 @@ struct avs_tplg_modcfg_ext { }; };
+/* Specifies path behaviour during PCM ->trigger(START) commnand. */ +enum avs_tplg_trigger { + AVS_TPLG_TRIGGER_AUTO = 0, + AVS_TPLG_TRIGGER_USERSPACE = 1, /* via sysfs */ +}; + +struct avs_tplg_pplcfg { + u16 req_size; + u8 priority; + bool lp; + u16 attributes; + enum avs_tplg_trigger trigger; +}; + +struct avs_tplg_binding { + char target_tplg_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; + u32 target_path_tmpl_id; + u32 target_ppl_id; + u32 target_mod_id; + u8 target_mod_pin; + u32 mod_id; + u8 mod_pin; + u8 is_sink; +}; + #endif
On 2/7/22 07:25, Cezary Rojewski wrote:
Stream on ADSP firmware is represented by one or more pipelines. Just
I have a lot of questions on this one line...
what is a 'stream'?
'stream' historically means 'direction' in ALSA.
Then we have sdw_stream, which describes how source and sink ports are connected on a platform.
We also have DPCM front-ends, visible mostly through the PCM device they expose to users.
In windows we have stream effects that are really meant to be on a single dedicated pipeline.
other questions: can a stream represent data moving in different directions, e.g. in full-duplex mode.
How would a loopback be described?
What happens when a data path is split (demux) or merged (mixer)?
How is a 'stream' different from a 'path template' that you referred to in Patch RFC 02/13
You would need at least 10 lines of plain English to make sure no one will misunderstand what 'stream' means.
like modules, these are described by a config structure. Add parsing helpers to support loading such information from the topology file.
+/* Specifies path behaviour during PCM ->trigger(START) commnand. */
typo: command.
+enum avs_tplg_trigger {
- AVS_TPLG_TRIGGER_AUTO = 0,
- AVS_TPLG_TRIGGER_USERSPACE = 1, /* via sysfs */
The feedback in previous versions was that we should not expose any sysfs interface that would interfere with decisions made by the driver.
This is very controversial and a major hurdle to upstream any of this.
Debugfs is fine, as suggested by Mark as well
" If it's mainly used for debugging then it could be exposed through debugfs with less worry. "
+};
+struct avs_tplg_pplcfg {
- u16 req_size;
what does this describe?
- u8 priority;
- bool lp;
- u16 attributes;
- enum avs_tplg_trigger trigger;
+};
+struct avs_tplg_binding {
- char target_tplg_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- u32 target_path_tmpl_id;
- u32 target_ppl_id;
- u32 target_mod_id;
- u8 target_mod_pin;
you really need to elaborate on what a template is, and how you use a template and a ppl id concurrently.
- u32 mod_id;
- u8 mod_pin;
- u8 is_sink;
+};
#endif
On 2022-02-25 6:40 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
Stream on ADSP firmware is represented by one or more pipelines. Just
I have a lot of questions on this one line...
what is a 'stream'?
'stream' historically means 'direction' in ALSA.
That ambiguity should be fixed long ago, it's probably the most frequently asked question/error done by ALSA newcomers.
Many drivers use 'stream' without implying direction e.g.: hda. It's also part of framework language anyway e.g.: substream (is that supposed to imply: subdirection?)
Then we have sdw_stream, which describes how source and sink ports are connected on a platform.
We also have DPCM front-ends, visible mostly through the PCM device they expose to users.
In windows we have stream effects that are really meant to be on a single dedicated pipeline.
other questions: can a stream represent data moving in different directions, e.g. in full-duplex mode.
How would a loopback be described?
What happens when a data path is split (demux) or merged (mixer)?
How is a 'stream' different from a 'path template' that you referred to in Patch RFC 02/13
You would need at least 10 lines of plain English to make sure no one will misunderstand what 'stream' means.
If that's the case, then we should re-think how and when 'stream' is used within the kernel.
Now, everyone from Windows team is perfectly fine with using 'stream' as is, it's common there. There are many types of effects, and the 'effect' subject has an ambiguity of its own but let's not have that subject here on the ALSA mailing list :)
I believe you would like a reword, as it seems my usage of 'stream' brought a ton of questions. 'Path' perhaps?
like modules, these are described by a config structure. Add parsing helpers to support loading such information from the topology file.
+/* Specifies path behaviour during PCM ->trigger(START) commnand. */
typo: command.
Ack, thanks!
+enum avs_tplg_trigger {
- AVS_TPLG_TRIGGER_AUTO = 0,
- AVS_TPLG_TRIGGER_USERSPACE = 1, /* via sysfs */
The feedback in previous versions was that we should not expose any sysfs interface that would interfere with decisions made by the driver.
This is very controversial and a major hurdle to upstream any of this.
Debugfs is fine, as suggested by Mark as well
" If it's mainly used for debugging then it could be exposed through debugfs with less worry. "
I'll remove 'USERSPACE' entry for now. I'll come back to that later on, in a different series.
+};
+struct avs_tplg_pplcfg {
- u16 req_size;
what does this describe?
Pipeline configuration i.e. all the information required when sending CREATE_PIPELINE IPC.
- u8 priority;
- bool lp;
- u16 attributes;
- enum avs_tplg_trigger trigger;
+};
+struct avs_tplg_binding {
- char target_tplg_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
- u32 target_path_tmpl_id;
- u32 target_ppl_id;
- u32 target_mod_id;
- u8 target_mod_pin;
you really need to elaborate on what a template is, and how you use a template and a ppl id concurrently.
As stated, such thing exists already, e.g.: in the skylake-driver. "Binding" here is called "Link" there. It's a different representation of the same thing.
Here we have all the information to without a question identify modules to bind during runtime.
- u32 mod_id;
- u8 mod_pin;
- u8 is_sink;
+};
- #endif
Shape of a stream on DSP side, that is, the number and the layout of its pipelines and modules is paramount for streaming to be efficient and low power-consuming. Add parsing helpers to support loading such information from the topology file.
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/topology.c | 177 +++++++++++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 31 ++++++ 2 files changed, 208 insertions(+)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 7d454a0ea000..8889ceae19b9 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -1004,3 +1004,180 @@ static int avs_tplg_parse_bindings(struct snd_soc_component *comp, AVS_TKN_BINDING_ID_U32, binding_parsers, ARRAY_SIZE(binding_parsers)); } + +static const struct avs_tplg_token_parser module_parsers[] = { + { + .token = AVS_TKN_MOD_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_module, id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_MOD_MODCFG_BASE_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_module, cfg_base), + .parse = avs_parse_modcfg_base_ptr, + }, + { + .token = AVS_TKN_MOD_IN_AFMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_module, in_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_MOD_CORE_ID_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_module, core_id), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_MOD_PROC_DOMAIN_U8, + .type = SND_SOC_TPLG_TUPLE_TYPE_BYTE, + .offset = offsetof(struct avs_tplg_module, domain), + .parse = avs_parse_byte_token, + }, + { + .token = AVS_TKN_MOD_MODCFG_EXT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_module, cfg_ext), + .parse = avs_parse_modcfg_ext_ptr, + }, +}; + +static struct avs_tplg_module * +avs_tplg_module_create(struct snd_soc_component *comp, struct avs_tplg_pipeline *owner, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size) +{ + struct avs_tplg_module *module; + int ret; + + module = devm_kzalloc(comp->card->dev, sizeof(*module), GFP_KERNEL); + if (!module) + return ERR_PTR(-ENOMEM); + + ret = avs_parse_tokens(comp, module, module_parsers, + ARRAY_SIZE(module_parsers), tuples, block_size); + if (ret < 0) + return ERR_PTR(ret); + + module->owner = owner; + INIT_LIST_HEAD(&module->node); + + return module; +} + +static const struct avs_tplg_token_parser pipeline_parsers[] = { + { + .token = AVS_TKN_PPL_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pipeline, id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_PPL_PPLCFG_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pipeline, cfg), + .parse = avs_parse_pplcfg_ptr, + }, + { + .token = AVS_TKN_PPL_NUM_BINDING_IDS_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_pipeline, num_bindings), + .parse = avs_parse_word_token, + }, +}; + +static const struct avs_tplg_token_parser bindings_parsers[] = { + { + .token = AVS_TKN_PPL_BINDING_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = 0, /* to treat pipeline->bindings as dictionary */ + .parse = avs_parse_binding_ptr, + }, +}; + +static struct avs_tplg_pipeline * +avs_tplg_pipeline_create(struct snd_soc_component *comp, struct avs_tplg_path *owner, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size) +{ + struct avs_tplg_pipeline *pipeline; + u32 modblk_size, offset; + int ret; + + pipeline = devm_kzalloc(comp->card->dev, sizeof(*pipeline), GFP_KERNEL); + if (!pipeline) + return ERR_PTR(-ENOMEM); + + pipeline->owner = owner; + INIT_LIST_HEAD(&pipeline->mod_list); + + /* Pipeline header MUST be followed by at least one module. */ + ret = avs_tplg_vendor_array_lookup(tuples, block_size, + AVS_TKN_MOD_ID_U32, &offset); + if (!ret && !offset) + ret = -EINVAL; + if (ret) + return ERR_PTR(ret); + + /* Process header which precedes module sections. */ + ret = avs_parse_tokens(comp, pipeline, pipeline_parsers, + ARRAY_SIZE(pipeline_parsers), tuples, offset); + if (ret < 0) + return ERR_PTR(ret); + + block_size -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + /* Optionally, binding sections follow module ones. */ + ret = avs_tplg_vendor_array_lookup_next(tuples, block_size, + AVS_TKN_PPL_BINDING_ID_U32, &offset); + if (ret) { + if (ret != -ENOENT) + return ERR_PTR(ret); + + /* Does header information match actual block layout? */ + if (pipeline->num_bindings) + return ERR_PTR(-EINVAL); + + modblk_size = block_size; + } else { + pipeline->bindings = devm_kcalloc(comp->card->dev, pipeline->num_bindings, + sizeof(*pipeline->bindings), GFP_KERNEL); + if (!pipeline->bindings) + return ERR_PTR(-ENOMEM); + + modblk_size = offset; + } + + block_size -= modblk_size; + do { + struct avs_tplg_module *module; + u32 esize; + + ret = avs_tplg_vendor_entry_size(tuples, modblk_size, + AVS_TKN_MOD_ID_U32, &esize); + if (ret) + return ERR_PTR(ret); + + module = avs_tplg_module_create(comp, pipeline, tuples, esize); + if (IS_ERR(module)) { + dev_err(comp->dev, "parse module failed: %ld\n", + PTR_ERR(module)); + return ERR_CAST(module); + } + + list_add_tail(&module->node, &pipeline->mod_list); + modblk_size -= esize; + tuples = avs_tplg_vendor_array_at(tuples, esize); + } while (modblk_size > 0); + + /* What's left is optional range of bindings. */ + ret = parse_dictionary_entries(comp, tuples, block_size, pipeline->bindings, + pipeline->num_bindings, sizeof(*pipeline->bindings), + AVS_TKN_PPL_BINDING_ID_U32, + bindings_parsers, ARRAY_SIZE(bindings_parsers)); + if (ret) + return ERR_PTR(ret); + + return pipeline; +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h index d23f4aba0bcc..b68c73afcde3 100644 --- a/sound/soc/intel/avs/topology.h +++ b/sound/soc/intel/avs/topology.h @@ -129,4 +129,35 @@ struct avs_tplg_binding { u8 is_sink; };
+struct avs_tplg_path { + u32 id; +}; + +struct avs_tplg_pipeline { + u32 id; + + struct avs_tplg_pplcfg *cfg; + struct avs_tplg_binding **bindings; + u32 num_bindings; + struct list_head mod_list; + + struct avs_tplg_path *owner; + /* Path pipelines management. */ + struct list_head node; +}; + +struct avs_tplg_module { + u32 id; + + struct avs_tplg_modcfg_base *cfg_base; + struct avs_audio_format *in_fmt; + u8 core_id; + u8 domain; + struct avs_tplg_modcfg_ext *cfg_ext; + + struct avs_tplg_pipeline *owner; + /* Pipeline modules management. */ + struct list_head node; +}; + #endif
On 2/7/22 07:25, Cezary Rojewski wrote:
Shape of a stream on DSP side, that is, the number and the layout of its pipelines and modules is paramount for streaming to be efficient and low power-consuming. Add parsing helpers to support loading such information from the topology file.
again what is a 'stream'?
+struct avs_tplg_path {
- u32 id;
+};
A concept that boils down to a single integer is really far from clear to me. What does this represent really?
+struct avs_tplg_pipeline {
- u32 id;
- struct avs_tplg_pplcfg *cfg;
- struct avs_tplg_binding **bindings;
- u32 num_bindings;
- struct list_head mod_list;
- struct avs_tplg_path *owner;
the cardinality between path and pipeline is far from clear. When you have topologies where the same data can be rendered on multiple outputs and demuxed into an echo reference, then what is the 'path'?
Worst case all connected pipelines could be a single path with this hierarchical definition of ownership, but is this desired?
What happens when the user uses switches to disconnects pipelines?
- /* Path pipelines management. */
what is a path pipeline?
- struct list_head node;
+};
+struct avs_tplg_module {
- u32 id;
what is the definition of id? is this local to the scope of a pipeline? global for the entire topology?
- struct avs_tplg_modcfg_base *cfg_base;
- struct avs_audio_format *in_fmt;
- u8 core_id;
- u8 domain;
- struct avs_tplg_modcfg_ext *cfg_ext;
- struct avs_tplg_pipeline *owner;
- /* Pipeline modules management. */
- struct list_head node;
+};
I would expect all modules to be seen as DAPM widgets, no?
#endif
On 2022-02-25 7:51 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
Shape of a stream on DSP side, that is, the number and the layout of its pipelines and modules is paramount for streaming to be efficient and low power-consuming. Add parsing helpers to support loading such information from the topology file.
again what is a 'stream'?
A collection of pipelines, usually connecting HOST (HDA DMA) gateway with LINK (GPDMA) gateway.
+struct avs_tplg_path {
- u32 id;
+};
A concept that boils down to a single integer is really far from clear to me. What does this represent really?
Nah, the structure is much larger. Here, to have pipeline parsing separated from the rest, some stub needed to be provided.
+struct avs_tplg_pipeline {
- u32 id;
- struct avs_tplg_pplcfg *cfg;
- struct avs_tplg_binding **bindings;
- u32 num_bindings;
- struct list_head mod_list;
- struct avs_tplg_path *owner;
the cardinality between path and pipeline is far from clear. When you have topologies where the same data can be rendered on multiple outputs and demuxed into an echo reference, then what is the 'path'?
Worst case all connected pipelines could be a single path with this hierarchical definition of ownership, but is this desired?
Just like in other DSP driver cases, here topology states all the possibilities. It's not random by any means.
If you want given data to be rendered on multiple outputs, then you make use of NxFEs -> 1xBE which ASoC supports through re-parenting. Multiple FEs in topology would be leading to the very same BE widget. On firmware side you have copier which supports several outputs. You just choose different output pin for each FE.
What happens when the user uses switches to disconnects pipelines?
"switches to disconnects pipelines"? How could user switch to a disconnected pipeline? Not following.
- /* Path pipelines management. */
what is a path pipeline?
Does this question mean you want "Path" part of the comment to be removed?
- struct list_head node;
+};
+struct avs_tplg_module {
- u32 id;
what is the definition of id? is this local to the scope of a pipeline? global for the entire topology?
It's module ID, so it's local. Basically every structure starts here with 'id'.
- struct avs_tplg_modcfg_base *cfg_base;
- struct avs_audio_format *in_fmt;
- u8 core_id;
- u8 domain;
- struct avs_tplg_modcfg_ext *cfg_ext;
- struct avs_tplg_pipeline *owner;
- /* Pipeline modules management. */
- struct list_head node;
+};
I would expect all modules to be seen as DAPM widgets, no?
Makes no sense since module alone have no real impact on whether audio path should be powered or not. Only the entire "path" has any saying in that.
- #endif
Path template is 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 - actual implementation of a runtime path on AudioDSP side. 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.
Add parsing helpers to support loading such information from the topology file.
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/topology.c | 158 +++++++++++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 27 ++++++ 2 files changed, 185 insertions(+)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 8889ceae19b9..92f7b9a361e1 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -1181,3 +1181,161 @@ avs_tplg_pipeline_create(struct snd_soc_component *comp, struct avs_tplg_path *o
return pipeline; } + +static const struct avs_tplg_token_parser path_parsers[] = { + { + .token = AVS_TKN_PATH_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_path, id), + .parse = avs_parse_word_token, + }, + { + .token = AVS_TKN_PATH_FE_FMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_path, fe_fmt), + .parse = avs_parse_audio_format_ptr, + }, + { + .token = AVS_TKN_PATH_BE_FMT_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_path, be_fmt), + .parse = avs_parse_audio_format_ptr, + }, +}; + +static struct avs_tplg_path * +avs_tplg_path_create(struct snd_soc_component *comp, struct avs_tplg_path_template *owner, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size, + const struct avs_tplg_token_parser *parsers, u32 num_parsers) +{ + struct avs_tplg_pipeline *pipeline; + struct avs_tplg_path *path; + u32 offset; + int ret; + + path = devm_kzalloc(comp->card->dev, sizeof(*path), GFP_KERNEL); + if (!path) + return ERR_PTR(-ENOMEM); + + path->owner = owner; + INIT_LIST_HEAD(&path->ppl_list); + INIT_LIST_HEAD(&path->node); + + /* Path header MAY be followed by one or more pipelines. */ + ret = avs_tplg_vendor_array_lookup(tuples, block_size, + AVS_TKN_PPL_ID_U32, &offset); + if (ret == -ENOENT) + offset = block_size; + else if (ret) + return ERR_PTR(ret); + else if (!offset) + return ERR_PTR(-EINVAL); + + /* Process header which precedes pipeline sections. */ + ret = avs_parse_tokens(comp, path, parsers, num_parsers, tuples, offset); + if (ret < 0) + return ERR_PTR(ret); + + block_size -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + while (block_size > 0) { + u32 esize; + + ret = avs_tplg_vendor_entry_size(tuples, block_size, + AVS_TKN_PPL_ID_U32, &esize); + if (ret) + return ERR_PTR(ret); + + pipeline = avs_tplg_pipeline_create(comp, path, tuples, esize); + if (IS_ERR(pipeline)) { + dev_err(comp->dev, "parse pipeline failed: %ld\n", + PTR_ERR(pipeline)); + return ERR_CAST(pipeline); + } + + list_add_tail(&pipeline->node, &path->ppl_list); + block_size -= esize; + tuples = avs_tplg_vendor_array_at(tuples, esize); + } + + return path; +} + +static const struct avs_tplg_token_parser path_tmpl_parsers[] = { + { + .token = AVS_TKN_PATH_TMPL_ID_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg_path_template, id), + .parse = avs_parse_word_token, + }, +}; + +static int parse_path_template(struct snd_soc_component *comp, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size, + struct avs_tplg_path_template *template, + const struct avs_tplg_token_parser *tmpl_tokens, u32 num_tmpl_tokens, + const struct avs_tplg_token_parser *path_tokens, u32 num_path_tokens) +{ + struct avs_tplg_path *path; + u32 offset; + int ret; + + /* Path template header MUST be followed by at least one path variant. */ + ret = avs_tplg_vendor_array_lookup(tuples, block_size, + AVS_TKN_PATH_ID_U32, &offset); + if (ret) + return ret; + + /* Process header which precedes path variants sections. */ + ret = avs_parse_tokens(comp, template, tmpl_tokens, num_tmpl_tokens, tuples, offset); + if (ret < 0) + return ret; + + block_size -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + do { + u32 esize; + + ret = avs_tplg_vendor_entry_size(tuples, block_size, + AVS_TKN_PATH_ID_U32, &esize); + if (ret) + return ret; + + path = avs_tplg_path_create(comp, template, tuples, esize, path_tokens, + num_path_tokens); + if (IS_ERR(path)) { + dev_err(comp->dev, "parse path failed: %ld\n", PTR_ERR(path)); + return PTR_ERR(path); + } + + list_add_tail(&path->node, &template->path_list); + block_size -= esize; + tuples = avs_tplg_vendor_array_at(tuples, esize); + } while (block_size > 0); + + return 0; +} + +static struct avs_tplg_path_template * +avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *owner, + struct snd_soc_tplg_vendor_array *tuples, u32 block_size) +{ + struct avs_tplg_path_template *template; + int ret; + + template = devm_kzalloc(comp->card->dev, sizeof(*template), GFP_KERNEL); + if (!template) + return ERR_PTR(-ENOMEM); + + template->owner = owner; /* Used when building sysfs hierarchy. */ + INIT_LIST_HEAD(&template->path_list); + INIT_LIST_HEAD(&template->node); + + ret = parse_path_template(comp, tuples, block_size, template, path_tmpl_parsers, + ARRAY_SIZE(path_tmpl_parsers), path_parsers, + ARRAY_SIZE(path_parsers)); + if (ret) + return ERR_PTR(ret); + + return template; +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h index b68c73afcde3..383931ab1d77 100644 --- a/sound/soc/intel/avs/topology.h +++ b/sound/soc/intel/avs/topology.h @@ -33,6 +33,8 @@ struct avs_tplg { u32 num_pplcfgs; struct avs_tplg_binding *bindings; u32 num_bindings; + + struct list_head path_tmpl_list; };
struct avs_tplg_library { @@ -129,8 +131,33 @@ struct avs_tplg_binding { u8 is_sink; };
+struct avs_tplg_path_template_id { + u32 id; + char tplg_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; +}; + +struct avs_tplg_path_template { + u32 id; + + struct list_head path_list; + + struct avs_tplg *owner; + /* Driver path templates management. */ + struct list_head node; +}; + struct avs_tplg_path { u32 id; + + /* Path format requirements. */ + struct avs_audio_format *fe_fmt; + struct avs_audio_format *be_fmt; + + struct list_head ppl_list; + + struct avs_tplg_path_template *owner; + /* Path template path-variants management. */ + struct list_head node; };
struct avs_tplg_pipeline {
On 2/7/22 07:25, Cezary Rojewski wrote:
Path template is a pattern like its name suggests. It is tied to DAPM
the name really doesn't suggest anything to me, and I have no idea what a 'pattern' represents for graph management.
You really ought to uplevel and expose the concepts earlier
widget which represents a FE or a BE and is used during path
'a widget which represents a FE or a BE'. I do not see a 1:1 relationship between widget and FE/BE. these are different concepts/levels.
instantiation when substream is opened for streaming. It carries a range of available variants - actual implementation of a runtime path on AudioDSP side. 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.
well, the last part is the fundamental problem that I am trying to explain.
The codec rate is not necessarily fixed as you seem to assume. There are many cases where the codec rate can be changed depending on the audio format changes happening in the DSP.
It is an invalid assumption to assume the codec rate is selected arbitrarily. It's often the case but that's more of a DPCM limitation than a design guiding principle we should build on.
+static struct avs_tplg_path_template * +avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *owner,
struct snd_soc_tplg_vendor_array *tuples, u32 block_size)+{
- struct avs_tplg_path_template *template;
- int ret;
- template = devm_kzalloc(comp->card->dev, sizeof(*template), GFP_KERNEL);
- if (!template)
return ERR_PTR(-ENOMEM);- template->owner = owner; /* Used when building sysfs hierarchy. */
sysfs is a showstopper if the intent is to let userspace modify the hierarchy.
Even for read-only information, it's not clear to me what application would ever read this information. debugfs seems more appropriate?
please clarify what the intended use might be.
- INIT_LIST_HEAD(&template->path_list);
- INIT_LIST_HEAD(&template->node);
- ret = parse_path_template(comp, tuples, block_size, template, path_tmpl_parsers,
ARRAY_SIZE(path_tmpl_parsers), path_parsers,ARRAY_SIZE(path_parsers));- if (ret)
return ERR_PTR(ret);- return template;
+}
struct avs_tplg_path { u32 id;
- /* Path format requirements. */
- struct avs_audio_format *fe_fmt;
- struct avs_audio_format *be_fmt;
this doesn't seem quite right or assumes a very narrow set of DPCM uses.
First I am not sure why there is only one format possible on both FE and BE sides. If you have multiples paths to represent all possible combinations of FE and BE formats, then it will become really confusing really quickly.
This representation would also not scale if there are multiple FEs are connected to the same BE, or conversely one FE is connected to multiple BEs. It's also quite possible that the different BE and FE have different formats, e.g. you could mix 24 and 32 bit data.
In addition, it is a really bad assumption to think that the BE is independent of the FE. In cases where its format can be adjusted, we would want constraints to be identified and select the 'best' possible BE format.
- struct list_head ppl_list;
- struct avs_tplg_path_template *owner;
- /* Path template path-variants management. */
- struct list_head node;
};
struct avs_tplg_pipeline {
On 2022-02-25 8:09 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
Path template is a pattern like its name suggests. It is tied to DAPM
the name really doesn't suggest anything to me, and I have no idea what a 'pattern' represents for graph management.
You really ought to uplevel and expose the concepts earlier
Again, such 'concept' already exists in kernel since skylake-driver. Topology never matched runtime 1:1, you are going to have some kind of template or pattern that you later convert into actual runtime stream.
Please state what would you like to see here as nether the ASoC topology nor the "template/pattern" is new here and I'm having trouble understanding what's need to be added.
widget which represents a FE or a BE and is used during path
'a widget which represents a FE or a BE'. I do not see a 1:1 relationship between widget and FE/BE. these are different concepts/levels.
Huh? For skylake-driver you have a widget per module which together make FE or BE. We simplified this here as duplicating widgets for no reason is not good.
instantiation when substream is opened for streaming. It carries a range of available variants - actual implementation of a runtime path on AudioDSP side. 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.
well, the last part is the fundamental problem that I am trying to explain.
The codec rate is not necessarily fixed as you seem to assume. There are many cases where the codec rate can be changed depending on the audio format changes happening in the DSP.
It is an invalid assumption to assume the codec rate is selected arbitrarily. It's often the case but that's more of a DPCM limitation than a design guiding principle we should build on.
That case is perfectly fine and is supported by the topology implemented here. I don't understand what's the issue here. Perhaps something is not worded correctly in the description.
+static struct avs_tplg_path_template * +avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *owner,
struct snd_soc_tplg_vendor_array *tuples, u32 block_size)+{
- struct avs_tplg_path_template *template;
- int ret;
- template = devm_kzalloc(comp->card->dev, sizeof(*template), GFP_KERNEL);
- if (!template)
return ERR_PTR(-ENOMEM);- template->owner = owner; /* Used when building sysfs hierarchy. */
sysfs is a showstopper if the intent is to let userspace modify the hierarchy.
Even for read-only information, it's not clear to me what application would ever read this information. debugfs seems more appropriate?
please clarify what the intended use might be.
I'll drop the 'owner' part and move it into a separate subject. The intent is not to modify the hierarchy, it's for having something to hook to to read info during runtime from DPS.
- INIT_LIST_HEAD(&template->path_list);
- INIT_LIST_HEAD(&template->node);
- ret = parse_path_template(comp, tuples, block_size, template, path_tmpl_parsers,
ARRAY_SIZE(path_tmpl_parsers), path_parsers,ARRAY_SIZE(path_parsers));- if (ret)
return ERR_PTR(ret);- return template;
+}
struct avs_tplg_path { u32 id;
- /* Path format requirements. */
- struct avs_audio_format *fe_fmt;
- struct avs_audio_format *be_fmt;
this doesn't seem quite right or assumes a very narrow set of DPCM uses.
First I am not sure why there is only one format possible on both FE and BE sides. If you have multiples paths to represent all possible combinations of FE and BE formats, then it will become really confusing really quickly.
This representation would also not scale if there are multiple FEs are connected to the same BE, or conversely one FE is connected to multiple BEs. It's also quite possible that the different BE and FE have different formats, e.g. you could mix 24 and 32 bit data.
In addition, it is a really bad assumption to think that the BE is independent of the FE. In cases where its format can be adjusted, we would want constraints to be identified and select the 'best' possible BE format.
struct avs_path_path_template can have a large list of struct avs_tplg_path defined, so it's not limited to one format. Remember that each format combination has its implication in form of need for different modules being engaged, changes in number of pipelines running and so on. And there's no running away from that. So, regardless of how you layout the struct which represents a "path" each combination will need a separate instance of it for its representation. Otherwise we enter the world where kernel driver has additional logic implemented so it modifies 'path' layouts on the fly. And that's a very dangerous path, especially when considering long term maintenance and backward compatibility subject.
Why would it not scale if multiple FEs are connected to the same BE? FE paths attached to single BE can have SRC/UpDownMixers modules within them to help with conversions. Remember that you have to take copier/mixin/mixout/fw modules limitations into account. You cannot just do random stuff and expect firmware to cope with that.
Sure, we want to select the best possible format. That's why you don't have to have different FE/BE format. It's a flexible approach.
struct list_head ppl_list;
struct avs_tplg_path_template *owner;
/* Path template path-variants management. */
struct list_head node; };
struct avs_tplg_pipeline {
AVS topology is split into two major parts: dictionaries - found within ASoC topology manifest - and path templates - found within DAPM widget private data. Add custom handlers for a range of operations available in struct snd_soc_tplg_ops to allow for actually loading the topology file.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/Kconfig | 1 + sound/soc/intel/avs/Makefile | 3 +- sound/soc/intel/avs/avs.h | 2 + sound/soc/intel/avs/topology.c | 259 +++++++++++++++++++++++++++++++++ sound/soc/intel/avs/topology.h | 5 + 5 files changed, 269 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 96aa702d831d..f0688c3d76a7 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -216,6 +216,7 @@ config SND_SOC_INTEL_AVS depends on SND_SOC_INTEL_SKYLAKE_FAMILY=n default n select SND_SOC_ACPI + select SND_SOC_TOPOLOGY select SND_HDA_EXT_CORE select SND_HDA_DSP_LOADER help diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile index f842bfc5e97e..5e12a3a89e64 100644 --- a/sound/soc/intel/avs/Makefile +++ b/sound/soc/intel/avs/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only
-snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o core.o loader.o +snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o core.o loader.o \ + topology.o snd-soc-avs-objs += cldma.o
obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h index 61842720c894..b0c17c45d7c6 100644 --- a/sound/soc/intel/avs/avs.h +++ b/sound/soc/intel/avs/avs.h @@ -237,4 +237,6 @@ struct avs_soc_component { #define to_avs_soc_component(comp) \ container_of(comp, struct avs_soc_component, base)
+extern const struct snd_soc_dai_ops avs_dai_fe_ops; + #endif /* __SOUND_SOC_INTEL_AVS_H */ diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c index 92f7b9a361e1..d7390359c39c 100644 --- a/sound/soc/intel/avs/topology.c +++ b/sound/soc/intel/avs/topology.c @@ -6,6 +6,7 @@ // Amadeusz Slawinski amadeuszx.slawinski@linux.intel.com //
+#include <linux/firmware.h> #include <linux/uuid.h> #include <sound/soc.h> #include <sound/soc-acpi.h> @@ -14,6 +15,8 @@ #include "avs.h" #include "topology.h"
+const struct snd_soc_dai_ops avs_dai_fe_ops; + /* 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)) @@ -1339,3 +1342,259 @@ avs_tplg_path_template_create(struct snd_soc_component *comp, struct avs_tplg *o
return template; } + +static int avs_route_load(struct snd_soc_component *comp, int index, + struct snd_soc_dapm_route *route) +{ + 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() */ + 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 (!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; +} + +static const struct avs_tplg_token_parser manifest_parsers[] = { + { + .token = AVS_TKN_MANIFEST_NAME_STRING, + .type = SND_SOC_TPLG_TUPLE_TYPE_STRING, + .offset = offsetof(struct avs_tplg, name), + .parse = parse_link_formatted_string, + }, + { + .token = AVS_TKN_MANIFEST_VERSION_U32, + .type = SND_SOC_TPLG_TUPLE_TYPE_WORD, + .offset = offsetof(struct avs_tplg, version), + .parse = avs_parse_word_token, + }, +}; + +static int avs_manifest(struct snd_soc_component *comp, int index, + struct snd_soc_tplg_manifest *manifest) +{ + struct snd_soc_tplg_vendor_array *tuples = manifest->priv.array; + struct avs_soc_component *acomp = to_avs_soc_component(comp); + size_t remaining = le32_to_cpu(manifest->priv.size); + u32 offset; + int ret; + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_LIBRARIES_U32, &offset); + /* Manifest MUST begin with a header. */ + if (!ret && !offset) + ret = -EINVAL; + if (ret) { + dev_err(comp->dev, "incorrect manifest format: %d\n", ret); + return ret; + } + + /* Process header which precedes any of the dictionaries. */ + ret = avs_parse_tokens(comp, acomp->tplg, manifest_parsers, + ARRAY_SIZE(manifest_parsers), tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_AFMTS_U32, &offset); + if (ret) { + dev_err(comp->dev, "audio formats lookup failed: %d\n", ret); + return ret; + } + + /* Libraries dictionary. */ + ret = avs_tplg_parse_libraries(comp, tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_MODCFGS_BASE_U32, &offset); + if (ret) { + dev_err(comp->dev, "modcfgs_base lookup failed: %d\n", ret); + return ret; + } + + /* Audio formats dictionary. */ + ret = avs_tplg_parse_audio_formats(comp, tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_MODCFGS_EXT_U32, &offset); + if (ret) { + dev_err(comp->dev, "modcfgs_ext lookup failed: %d\n", ret); + return ret; + } + + /* Module configs-base dictionary. */ + ret = avs_tplg_parse_modcfgs_base(comp, tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_PPLCFGS_U32, &offset); + if (ret) { + dev_err(comp->dev, "pplcfgs lookup failed: %d\n", ret); + return ret; + } + + /* Module configs-ext dictionary. */ + ret = avs_tplg_parse_modcfgs_ext(comp, tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + ret = avs_tplg_vendor_array_lookup(tuples, remaining, + AVS_TKN_MANIFEST_NUM_BINDINGS_U32, &offset); + if (ret) { + dev_err(comp->dev, "bindings lookup failed: %d\n", ret); + return ret; + } + + /* Pipeline configs dictionary. */ + ret = avs_tplg_parse_pplcfgs(comp, tuples, offset); + if (ret < 0) + return ret; + + remaining -= offset; + tuples = avs_tplg_vendor_array_at(tuples, offset); + + /* Bindings dictionary. */ + return avs_tplg_parse_bindings(comp, tuples, remaining); +} + +static struct snd_soc_tplg_ops avs_tplg_ops = { + .dapm_route_load = avs_route_load, + .widget_load = avs_widget_load, + .dai_load = avs_dai_load, + .link_load = avs_link_load, + .manifest = avs_manifest, +}; + +struct avs_tplg *avs_tplg_new(struct snd_soc_component *comp) +{ + struct avs_tplg *tplg; + + tplg = devm_kzalloc(comp->card->dev, sizeof(*tplg), GFP_KERNEL); + if (!tplg) + return NULL; + + tplg->comp = comp; + INIT_LIST_HEAD(&tplg->path_tmpl_list); + + return tplg; +} + +int avs_load_topology(struct snd_soc_component *comp, const char *filename) +{ + const struct firmware *fw; + int ret; + + ret = request_firmware(&fw, filename, comp->dev); + if (ret < 0) { + dev_err(comp->dev, "request topology "%s" failed: %d\n", filename, ret); + return ret; + } + + ret = snd_soc_tplg_component_load(comp, &avs_tplg_ops, fw); + if (ret < 0) + dev_err(comp->dev, "load topology "%s" failed: %d\n", filename, ret); + + release_firmware(fw); + return ret; +} + +int avs_remove_topology(struct snd_soc_component *comp) +{ + snd_soc_tplg_component_remove(comp); + + return 0; +} diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h index 383931ab1d77..6058d868f802 100644 --- a/sound/soc/intel/avs/topology.h +++ b/sound/soc/intel/avs/topology.h @@ -187,4 +187,9 @@ struct avs_tplg_module { struct list_head node; };
+struct avs_tplg *avs_tplg_new(struct snd_soc_component *comp); + +int avs_load_topology(struct snd_soc_component *comp, const char *filename); +int avs_remove_topology(struct snd_soc_component *comp); + #endif
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?
+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?
+{
- 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?
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?
- 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;
+}
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;
+}
Declare representatives for all crucial elements which stream on ADSP side is made of. That covers pipelines and modules subject which are presented by struct avs_path_pipeline and avs_path_module respetively. While struct avs_path_binding and struct avs_path do not represent any object on firmware side directly, they are needed to help track the interconnections and membership of every pipeline and module created.
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/path.h | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 sound/soc/intel/avs/path.h
diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h new file mode 100644 index 000000000000..ecfb687fdf36 --- /dev/null +++ b/sound/soc/intel/avs/path.h @@ -0,0 +1,60 @@ +/* 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 + */ + +#ifndef __SOUND_SOC_INTEL_AVS_PATH_H +#define __SOUND_SOC_INTEL_AVS_PATH_H + +#include <linux/list.h> +#include "avs.h" +#include "topology.h" + +struct avs_path { + u32 dma_id; + struct list_head ppl_list; + u32 state; + + struct avs_tplg_path *template; + struct avs_dev *owner; + /* device path management */ + struct list_head node; +}; + +struct avs_path_pipeline { + u8 instance_id; + struct list_head mod_list; + struct list_head binding_list; + + struct avs_tplg_pipeline *template; + struct avs_path *owner; + /* path pipelines management */ + struct list_head node; +}; + +struct avs_path_module { + u16 module_id; + u16 instance_id; + + struct avs_tplg_module *template; + struct avs_path_pipeline *owner; + /* pipeline modules management */ + struct list_head node; +}; + +struct avs_path_binding { + struct avs_path_module *source; + u8 source_pin; + struct avs_path_module *sink; + u8 sink_pin; + + struct avs_tplg_binding *template; + struct avs_path_pipeline *owner; + /* pipeline bindings management */ + struct list_head node; +}; + +#endif
On 2/7/22 07:25, Cezary Rojewski wrote:
Declare representatives for all crucial elements which stream on ADSP side is made of. That covers pipelines and modules subject which are presented by struct avs_path_pipeline and avs_path_module respetively.
respectively
While struct avs_path_binding and struct avs_path do not represent any object on firmware side directly, they are needed to help track the interconnections and membership of every pipeline and module created.
+struct avs_path {
- u32 dma_id;
that is very surprising...
This would seem to limit the concept of an avs path to a single host DMA channel, which somewhat contradicts that there can be multiple pipelines in the same path, or that a path can contain mixers.
And even if this is a single dma, what does this represent? the stream_tag? the BE DMA for SSP/DMIC?
Please clarify the concepts first, it's frustrating to discover this at patch 8.
- struct list_head ppl_list;
- u32 state;
- struct avs_tplg_path *template;
- struct avs_dev *owner;
- /* device path management */
- struct list_head node;
+};
+struct avs_path_binding {
- struct avs_path_module *source;
- u8 source_pin;
- struct avs_path_module *sink;
- u8 sink_pin;
- struct avs_tplg_binding *template;
I must admit not clearly seeing how the definitions of 'avs_path_binding' and 'avs_tplg_binding' are related.
More specifically, having a template for something that directly connects a source to a sink is far from intuitive.
- struct avs_path_pipeline *owner;
- /* pipeline bindings management */
- struct list_head node;
+};
+#endif
On 2022-02-25 8:25 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
Declare representatives for all crucial elements which stream on ADSP side is made of. That covers pipelines and modules subject which are presented by struct avs_path_pipeline and avs_path_module respetively.
respectively
Ack.
While struct avs_path_binding and struct avs_path do not represent any object on firmware side directly, they are needed to help track the interconnections and membership of every pipeline and module created.
+struct avs_path {
- u32 dma_id;
that is very surprising...
This would seem to limit the concept of an avs path to a single host DMA channel, which somewhat contradicts that there can be multiple pipelines in the same path, or that a path can contain mixers.
And even if this is a single dma, what does this represent? the stream_tag? the BE DMA for SSP/DMIC?
Please clarify the concepts first, it's frustrating to discover this at patch 8.
A single path is tied to either FE or BE. So at most to a single, user-visible endpoint if it's FE. If there are more FEs, then we move to NxFE <-> 1xBE scenario. You can have many pipelines forming the path - most of the pipelines do not contain module connected to any gateway (HDA/SSP/DMIC etc.) anyway.
Yes, dma_id represents any DMA i.e. HDA stream (DMA), SSP port, DMIC fifo etc. And it's not a concept, it's just a member of a structure. Similar field exists in skylake-driver topology too (it's called "port").
- struct list_head ppl_list;
- u32 state;
- struct avs_tplg_path *template;
- struct avs_dev *owner;
- /* device path management */
- struct list_head node;
+};
+struct avs_path_binding {
- struct avs_path_module *source;
- u8 source_pin;
- struct avs_path_module *sink;
- u8 sink_pin;
- struct avs_tplg_binding *template;
I must admit not clearly seeing how the definitions of 'avs_path_binding' and 'avs_tplg_binding' are related.
More specifically, having a template for something that directly connects a source to a sink is far from intuitive.
The IDs found within the topology file do not reflect the IDs that are going to be assigned dynamically during runtime streaming. We do not specify e.g.: pipeline id=8 is made of copier id=3 and copier id=7. Firmware has limited number of modules that can be instantiated per type so static assignment is a big no no.
- struct avs_path_pipeline *owner;
- /* pipeline bindings management */
- struct list_head node;
+};
+#endif
+struct avs_path { + u32 dma_id;
that is very surprising...
This would seem to limit the concept of an avs path to a single host DMA channel, which somewhat contradicts that there can be multiple pipelines in the same path, or that a path can contain mixers.
And even if this is a single dma, what does this represent? the stream_tag? the BE DMA for SSP/DMIC?
Please clarify the concepts first, it's frustrating to discover this at patch 8.
A single path is tied to either FE or BE. So at most to a single, user-visible endpoint if it's FE. If there are more FEs, then we move to NxFE <-> 1xBE scenario. You can have many pipelines forming the path - most of the pipelines do not contain module connected to any gateway (HDA/SSP/DMIC etc.) anyway.
This should have been explained in the cover letter.
Assuming that there's a single Back-End that can handle all possible routing cases is a very narrow interpretation of how DPCM is supposed to be used, and it adds quite a few opens on routing changes that can't be handled with regular triggers. What happens when not all interfaces are handled by the DSP 'gateway' is also interesting.
To implement ASoC PCM operations, DSP path handling is needed. With path template concept present, information carried by topology file can be converted into runtime path representation. Each may be composed of several pipelines and each pipeline can contain a number of processing modules inside. Number of templates and variants found within topology may vastly outnumber the total amount of pipelines and modules supported by AudioDSP firmware simultaneously (in runtime) so none of the IDs are specified in the topology. These are assigned dynamically when needed and account for limitations described by FIRMWARE_CONFIG and HARDWARE_CONFIG basefw parameters.
Paths are created on ->hw_params() and are freed on ->hw_free() ALSA PCM operations. This choice is based on firmware expectations - need for complete set of information when attempting to instantiate pipelines and modules on AudioDSP side. With DMA and audio format provided, search mechanism tests all path variants available in given path template until a matching variant is found. Once found, information already available is combined with all avs_tplg_* pieces pointed by matching path variant. This finally allows to begin a cascade of IPCs which goes is to reserve resources and prepare DSP for upcoming audio streaming.
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/Makefile | 2 +- sound/soc/intel/avs/avs.h | 6 + sound/soc/intel/avs/path.c | 287 +++++++++++++++++++++++++++++++++++ sound/soc/intel/avs/path.h | 6 + 4 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 sound/soc/intel/avs/path.c
diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile index 5e12a3a89e64..952f51977656 100644 --- a/sound/soc/intel/avs/Makefile +++ b/sound/soc/intel/avs/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only
snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o core.o loader.o \ - topology.o + topology.o path.o snd-soc-avs-objs += cldma.o
obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h index b0c17c45d7c6..313001b0455f 100644 --- a/sound/soc/intel/avs/avs.h +++ b/sound/soc/intel/avs/avs.h @@ -91,6 +91,12 @@ struct avs_dev { char **lib_names;
struct completion fw_ready; + + struct list_head comp_list; + struct mutex comp_list_mutex; + struct list_head path_list; + spinlock_t path_list_lock; + struct mutex path_mutex; };
/* from hda_bus to avs_dev */ diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c new file mode 100644 index 000000000000..272d868bedc9 --- /dev/null +++ b/sound/soc/intel/avs/path.c @@ -0,0 +1,287 @@ +// 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 <sound/intel-nhlt.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include "avs.h" +#include "path.h" +#include "topology.h" + +static bool avs_test_hw_params(struct snd_pcm_hw_params *params, + struct avs_audio_format *fmt) +{ + return (params_rate(params) == fmt->sampling_freq && + params_channels(params) == fmt->num_channels && + params_physical_width(params) == fmt->bit_depth && + params_width(params) == fmt->valid_bit_depth); +} + +static struct avs_tplg_path * +avs_path_find_variant(struct avs_dev *adev, + struct avs_tplg_path_template *template, + struct snd_pcm_hw_params *fe_params, + struct snd_pcm_hw_params *be_params) +{ + struct avs_tplg_path *variant; + + list_for_each_entry(variant, &template->path_list, node) { + dev_dbg(adev->dev, "check FE rate %d chn %d vbd %d bd %d\n", + variant->fe_fmt->sampling_freq, variant->fe_fmt->num_channels, + variant->fe_fmt->valid_bit_depth, variant->fe_fmt->bit_depth); + dev_dbg(adev->dev, "check BE rate %d chn %d vbd %d bd %d\n", + variant->be_fmt->sampling_freq, variant->be_fmt->num_channels, + variant->be_fmt->valid_bit_depth, variant->be_fmt->bit_depth); + + if (variant->fe_fmt && avs_test_hw_params(fe_params, variant->fe_fmt) && + variant->be_fmt && avs_test_hw_params(be_params, variant->be_fmt)) + return variant; + } + + return NULL; +} + +static void avs_path_module_free(struct avs_dev *adev, struct avs_path_module *mod) +{ + kfree(mod); +} + +static struct avs_path_module * +avs_path_module_create(struct avs_dev *adev, + struct avs_path_pipeline *owner, + struct avs_tplg_module *template) +{ + struct avs_path_module *mod; + int module_id; + + module_id = avs_get_module_id(adev, &template->cfg_ext->type); + if (module_id < 0) + return ERR_PTR(module_id); + + mod = kzalloc(sizeof(*mod), GFP_KERNEL); + if (!mod) + return ERR_PTR(-ENOMEM); + + mod->template = template; + mod->module_id = module_id; + mod->owner = owner; + INIT_LIST_HEAD(&mod->node); + + return mod; +} + +static void avs_path_binding_free(struct avs_dev *adev, struct avs_path_binding *binding) +{ + kfree(binding); +} + +static struct avs_path_binding *avs_path_binding_create(struct avs_dev *adev, + struct avs_path_pipeline *owner, + struct avs_tplg_binding *t) +{ + struct avs_path_binding *binding; + + binding = kzalloc(sizeof(*binding), GFP_KERNEL); + if (!binding) + return ERR_PTR(-ENOMEM); + + binding->template = t; + binding->owner = owner; + INIT_LIST_HEAD(&binding->node); + + return binding; +} + +static void avs_path_pipeline_free(struct avs_dev *adev, + struct avs_path_pipeline *ppl) +{ + struct avs_path_binding *binding, *bsave; + struct avs_path_module *mod, *save; + + list_for_each_entry_safe(binding, bsave, &ppl->binding_list, node) { + list_del(&binding->node); + avs_path_binding_free(adev, binding); + } + + avs_dsp_delete_pipeline(adev, ppl->instance_id); + + /* Unload resources occupied by owned modules */ + list_for_each_entry_safe(mod, save, &ppl->mod_list, node) { + avs_dsp_delete_module(adev, mod->module_id, mod->instance_id, + mod->owner->instance_id, + mod->template->core_id); + avs_path_module_free(adev, mod); + } + + list_del(&ppl->node); + kfree(ppl); +} + +static struct avs_path_pipeline * +avs_path_pipeline_create(struct avs_dev *adev, struct avs_path *owner, + struct avs_tplg_pipeline *template) +{ + struct avs_path_pipeline *ppl; + struct avs_tplg_pplcfg *cfg = template->cfg; + struct avs_tplg_module *tmod; + int ret, i; + + ppl = kzalloc(sizeof(*ppl), GFP_KERNEL); + if (!ppl) + return ERR_PTR(-ENOMEM); + + ppl->template = template; + ppl->owner = owner; + INIT_LIST_HEAD(&ppl->binding_list); + INIT_LIST_HEAD(&ppl->mod_list); + INIT_LIST_HEAD(&ppl->node); + + ret = avs_dsp_create_pipeline(adev, cfg->req_size, cfg->priority, + cfg->lp, cfg->attributes, + &ppl->instance_id); + if (ret) { + dev_err(adev->dev, "error creating pipeline %d\n", ret); + kfree(ppl); + return ERR_PTR(ret); + } + + list_for_each_entry(tmod, &template->mod_list, node) { + struct avs_path_module *mod; + + mod = avs_path_module_create(adev, ppl, tmod); + if (IS_ERR(mod)) { + ret = PTR_ERR(mod); + dev_err(adev->dev, "error creating module %d\n", ret); + goto init_err; + } + + list_add_tail(&mod->node, &ppl->mod_list); + } + + for (i = 0; i < template->num_bindings; i++) { + struct avs_path_binding *binding; + + binding = avs_path_binding_create(adev, ppl, template->bindings[i]); + if (IS_ERR(binding)) { + ret = PTR_ERR(binding); + dev_err(adev->dev, "error creating binding %d\n", ret); + goto init_err; + } + + list_add_tail(&binding->node, &ppl->binding_list); + } + + return ppl; + +init_err: + avs_path_pipeline_free(adev, ppl); + return ERR_PTR(ret); +} + +static int avs_path_init(struct avs_dev *adev, struct avs_path *path, + struct avs_tplg_path *template, u32 dma_id) +{ + struct avs_tplg_pipeline *tppl; + + path->owner = adev; + path->template = template; + path->dma_id = dma_id; + INIT_LIST_HEAD(&path->ppl_list); + INIT_LIST_HEAD(&path->node); + + /* create all the pipelines */ + list_for_each_entry(tppl, &template->ppl_list, node) { + struct avs_path_pipeline *ppl; + + ppl = avs_path_pipeline_create(adev, path, tppl); + if (IS_ERR(ppl)) + return PTR_ERR(ppl); + + list_add_tail(&ppl->node, &path->ppl_list); + } + + spin_lock(&adev->path_list_lock); + list_add_tail(&path->node, &adev->path_list); + spin_unlock(&adev->path_list_lock); + + return 0; +} + +static void avs_path_free_unlocked(struct avs_path *path) +{ + struct avs_path_pipeline *ppl, *save; + + spin_lock(&path->owner->path_list_lock); + list_del(&path->node); + spin_unlock(&path->owner->path_list_lock); + + list_for_each_entry_safe(ppl, save, &path->ppl_list, node) + avs_path_pipeline_free(path->owner, ppl); + + kfree(path); +} + +static struct avs_path *avs_path_create_unlocked(struct avs_dev *adev, u32 dma_id, + struct avs_tplg_path *template) +{ + struct avs_soc_component *acomp; + struct avs_path *path; + int ret; + + acomp = to_avs_soc_component(template->owner->owner->comp); + + path = kzalloc(sizeof(*path), GFP_KERNEL); + if (!path) + return ERR_PTR(-ENOMEM); + + ret = avs_path_init(adev, path, template, dma_id); + if (ret < 0) + goto err; + + path->state = AVS_PPL_STATE_INVALID; + return path; +err: + avs_path_free_unlocked(path); + return ERR_PTR(ret); +} + +void avs_path_free(struct avs_path *path) +{ + struct avs_dev *adev = path->owner; + + mutex_lock(&adev->path_mutex); + avs_path_free_unlocked(path); + mutex_unlock(&adev->path_mutex); +} + +struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, + struct avs_tplg_path_template *template, + struct snd_pcm_hw_params *fe_params, + struct snd_pcm_hw_params *be_params) +{ + struct avs_tplg_path *variant; + struct avs_path *path; + + variant = avs_path_find_variant(adev, template, fe_params, be_params); + if (!variant) { + dev_err(adev->dev, "no matching variant found\n"); + return ERR_PTR(-ENOENT); + } + + /* Serialize path and its components creation. */ + mutex_lock(&adev->path_mutex); + /* Satisfy needs of avs_path_find_tplg(). */ + mutex_lock(&adev->comp_list_mutex); + + path = avs_path_create_unlocked(adev, dma_id, variant); + + mutex_unlock(&adev->comp_list_mutex); + mutex_unlock(&adev->path_mutex); + + return path; +} diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index ecfb687fdf36..3843e5a062a1 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -57,4 +57,10 @@ struct avs_path_binding { struct list_head node; };
+void avs_path_free(struct avs_path *path); +struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, + struct avs_tplg_path_template *template, + struct snd_pcm_hw_params *fe_params, + struct snd_pcm_hw_params *be_params); + #endif
On 2/7/22 07:25, Cezary Rojewski wrote:
To implement ASoC PCM operations, DSP path handling is needed. With path template concept present, information carried by topology file can be converted into runtime path representation. Each may be composed of several pipelines and each pipeline can contain a number of processing
it's not quite clear how you can have different pipelines with a single dma_is per path?
modules inside. Number of templates and variants found within topology may vastly outnumber the total amount of pipelines and modules supported by AudioDSP firmware simultaneously (in runtime) so none of the IDs are specified in the topology. These are assigned dynamically when needed and account for limitations described by FIRMWARE_CONFIG and HARDWARE_CONFIG basefw parameters.
It's already quite hard to create a topology using static IDs that will work, this dynamic creation adds an element of risk and validation, doesn't it?
Can you clarify how you validated this dynamic capability?
Paths are created on ->hw_params() and are freed on ->hw_free() ALSA PCM operations. This choice is based on firmware expectations - need for
So a path seems to be completely tied to an FE then?
That would mean that a 'dai pipeline' with 'mixin-dai-copier' would not be managed by an avs-path, and would somehow need to be autonomously created?
You really need to clarify how basic topologies with mixers/demux/loopbacks are handled.
complete set of information when attempting to instantiate pipelines and modules on AudioDSP side. With DMA and audio format provided, search mechanism tests all path variants available in given path template until a matching variant is found. Once found, information already available is combined with all avs_tplg_* pieces pointed by matching path variant. This finally allows to begin a cascade of IPCs which goes is to reserve
this sentence makes no sense.
did you mean 'which goals'?
resources and prepare DSP for upcoming audio streaming.
+static struct avs_tplg_path * +avs_path_find_variant(struct avs_dev *adev,
struct avs_tplg_path_template *template,struct snd_pcm_hw_params *fe_params,struct snd_pcm_hw_params *be_params)+{
- struct avs_tplg_path *variant;
- list_for_each_entry(variant, &template->path_list, node) {
dev_dbg(adev->dev, "check FE rate %d chn %d vbd %d bd %d\n",variant->fe_fmt->sampling_freq, variant->fe_fmt->num_channels,variant->fe_fmt->valid_bit_depth, variant->fe_fmt->bit_depth);dev_dbg(adev->dev, "check BE rate %d chn %d vbd %d bd %d\n",variant->be_fmt->sampling_freq, variant->be_fmt->num_channels,variant->be_fmt->valid_bit_depth, variant->be_fmt->bit_depth);if (variant->fe_fmt && avs_test_hw_params(fe_params, variant->fe_fmt) &&variant->be_fmt && avs_test_hw_params(be_params, variant->be_fmt))return variant;- }
This matching between FE and BE formats is the key problem with this patchset IMHO.
We need the ability to reconfigure the BE based on constraint matching, not exact match with a fixed value!
- return NULL;
+}
+static void avs_path_module_free(struct avs_dev *adev, struct avs_path_module *mod) +{
- kfree(mod);
+}
+static struct avs_path_module * +avs_path_module_create(struct avs_dev *adev,
struct avs_path_pipeline *owner,struct avs_tplg_module *template)
so you have templates for paths AND modules?
Completely lost...
I'll skip the rest of this patch.
On 2022-02-25 8:36 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
To implement ASoC PCM operations, DSP path handling is needed. With path template concept present, information carried by topology file can be converted into runtime path representation. Each may be composed of several pipelines and each pipeline can contain a number of processing
it's not quite clear how you can have different pipelines with a single dma_is per path?
Pipelines do not need to have a module that is connected to a gateway.
Layout of a path is for architecture to decide i.e. one takes spreadsheet of all the scenarios to be supported and developers/archs discuss/decide what's the best and optimal way to shape the paths that implement all the possible scenarios.
modules inside. Number of templates and variants found within topology may vastly outnumber the total amount of pipelines and modules supported by AudioDSP firmware simultaneously (in runtime) so none of the IDs are specified in the topology. These are assigned dynamically when needed and account for limitations described by FIRMWARE_CONFIG and HARDWARE_CONFIG basefw parameters.
It's already quite hard to create a topology using static IDs that will work, this dynamic creation adds an element of risk and validation, doesn't it?
Can you clarify how you validated this dynamic capability?
Static ID assignment i.e. taking IDs found in the topology file directly when instantiating pipelines/modules in runtime is asking for trouble. Firmware has its limitations in terms of number of pipelines/modules supported simultaneously. Driver has to take these into account.
Here, we send an IPC to obtain all the limitations before any stream could be opened for streaming.
Paths are created on ->hw_params() and are freed on ->hw_free() ALSA PCM operations. This choice is based on firmware expectations - need for
So a path seems to be completely tied to an FE then?
That would mean that a 'dai pipeline' with 'mixin-dai-copier' would not be managed by an avs-path, and would somehow need to be autonomously created?
You really need to clarify how basic topologies with mixers/demux/loopbacks are handled.
Path is either tied to a single FE or a BE. Don't understand what's difficult with handling mixin/copier pipeliens or loopback scenario here. We have all the tools necessary to do the job, no?
complete set of information when attempting to instantiate pipelines and modules on AudioDSP side. With DMA and audio format provided, search mechanism tests all path variants available in given path template until a matching variant is found. Once found, information already available is combined with all avs_tplg_* pieces pointed by matching path variant. This finally allows to begin a cascade of IPCs which goes is to reserve
this sentence makes no sense.
did you mean 'which goals'?
Ack, thanks!
resources and prepare DSP for upcoming audio streaming.
+static struct avs_tplg_path * +avs_path_find_variant(struct avs_dev *adev,
struct avs_tplg_path_template *template,struct snd_pcm_hw_params *fe_params,struct snd_pcm_hw_params *be_params)+{
- struct avs_tplg_path *variant;
- list_for_each_entry(variant, &template->path_list, node) {
dev_dbg(adev->dev, "check FE rate %d chn %d vbd %d bd %d\n",variant->fe_fmt->sampling_freq, variant->fe_fmt->num_channels,variant->fe_fmt->valid_bit_depth, variant->fe_fmt->bit_depth);dev_dbg(adev->dev, "check BE rate %d chn %d vbd %d bd %d\n",variant->be_fmt->sampling_freq, variant->be_fmt->num_channels,variant->be_fmt->valid_bit_depth, variant->be_fmt->bit_depth);if (variant->fe_fmt && avs_test_hw_params(fe_params, variant->fe_fmt) &&variant->be_fmt && avs_test_hw_params(be_params, variant->be_fmt))return variant;- }
This matching between FE and BE formats is the key problem with this patchset IMHO.
We need the ability to reconfigure the BE based on constraint matching, not exact match with a fixed value!
We need to take into account what's set on the codec side too, can't just ignore it. Topology is written for concrete configuration, so we can crease a file that supports all possible configurations exposed by given codec.
- return NULL;
+}
+static void avs_path_module_free(struct avs_dev *adev, struct avs_path_module *mod) +{
- kfree(mod);
+}
+static struct avs_path_module * +avs_path_module_create(struct avs_dev *adev,
struct avs_path_pipeline *owner,struct avs_tplg_module *template)so you have templates for paths AND modules?
Completely lost...
I'll skip the rest of this patch.
All the objects here have topology and runtime representation both. Again, you cannot just take what's within a static topology file that knows nothing about firmware limitations and expect success.
Don't understand what's surprising here. skylake-driver also has a separate representation for module within topology and a separate one for runtime. Nothing new here.
Regards, Czarek
On 2022-03-21 6:19 PM, Cezary Rojewski wrote:
We need to take into account what's set on the codec side too, can't just ignore it. Topology is written for concrete configuration, so we can crease a file that supports all possible configurations exposed by given codec.
By "Topology is written for concrete configuration" I meant platform-level configuration e.g.: Skylake with single rt286 codec in I2S mode.
Regards, Czarek
Add functions to ease with state changing of all objects found in the path. Each represents either a BIND/UNBIND or SET_PIPEPILE_STATE IPC.
DSP pipelines follow simple state machine scheme: CREATE -> RESET -> PAUSE -> RUNNING -> PAUSE -> RESET -> DELETE
There is no STOP, PAUSE serves that purpose instead.
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/path.c | 130 +++++++++++++++++++++++++++++++++++++ sound/soc/intel/avs/path.h | 5 ++ 2 files changed, 135 insertions(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 272d868bedc9..c6db3f091e66 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -285,3 +285,133 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
return path; } + +int avs_path_bind(struct avs_path *path) +{ + struct avs_path_pipeline *ppl; + struct avs_dev *adev = path->owner; + int ret; + + list_for_each_entry(ppl, &path->ppl_list, node) { + struct avs_path_binding *binding; + + list_for_each_entry(binding, &ppl->binding_list, node) { + struct avs_path_module *source, *sink; + + source = binding->source; + sink = binding->sink; + + ret = avs_ipc_bind(adev, source->module_id, + source->instance_id, sink->module_id, + sink->instance_id, binding->sink_pin, + binding->source_pin); + if (ret) { + dev_err(adev->dev, "bind path failed: %d\n", ret); + return AVS_IPC_RET(ret); + } + } + } + + return 0; +} + +int avs_path_unbind(struct avs_path *path) +{ + struct avs_path_pipeline *ppl; + struct avs_dev *adev = path->owner; + int ret; + + list_for_each_entry(ppl, &path->ppl_list, node) { + struct avs_path_binding *binding; + + list_for_each_entry(binding, &ppl->binding_list, node) { + struct avs_path_module *source, *sink; + + source = binding->source; + sink = binding->sink; + + ret = avs_ipc_unbind(adev, source->module_id, + source->instance_id, sink->module_id, + sink->instance_id, binding->sink_pin, + binding->source_pin); + if (ret) { + dev_err(adev->dev, "unbind path failed: %d\n", ret); + return AVS_IPC_RET(ret); + } + } + } + + return 0; +} + +int avs_path_reset(struct avs_path *path) +{ + struct avs_path_pipeline *ppl; + struct avs_dev *adev = path->owner; + int ret; + + if (path->state == AVS_PPL_STATE_RESET) + return 0; + + list_for_each_entry(ppl, &path->ppl_list, node) { + ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id, + AVS_PPL_STATE_RESET); + if (ret) { + dev_err(adev->dev, "reset path failed: %d\n", ret); + path->state = AVS_PPL_STATE_INVALID; + return AVS_IPC_RET(ret); + } + } + + path->state = AVS_PPL_STATE_RESET; + return 0; +} + +int avs_path_pause(struct avs_path *path) +{ + struct avs_path_pipeline *ppl; + struct avs_dev *adev = path->owner; + int ret; + + if (path->state == AVS_PPL_STATE_PAUSED) + return 0; + + list_for_each_entry_reverse(ppl, &path->ppl_list, node) { + ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id, + AVS_PPL_STATE_PAUSED); + if (ret) { + dev_err(adev->dev, "pause path failed: %d\n", ret); + path->state = AVS_PPL_STATE_INVALID; + return AVS_IPC_RET(ret); + } + } + + path->state = AVS_PPL_STATE_PAUSED; + return 0; +} + +int avs_path_run(struct avs_path *path, int trigger) +{ + struct avs_path_pipeline *ppl; + struct avs_dev *adev = path->owner; + int ret; + + if (path->state == AVS_PPL_STATE_RUNNING && trigger == AVS_TPLG_TRIGGER_AUTO) + return 0; + + list_for_each_entry(ppl, &path->ppl_list, node) { + if (ppl->template->cfg->trigger != trigger) + continue; + + ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id, + AVS_PPL_STATE_RUNNING); + if (ret) { + dev_err(adev->dev, "run path failed: %d\n", ret); + path->state = AVS_PPL_STATE_INVALID; + return AVS_IPC_RET(ret); + } + } + + path->state = AVS_PPL_STATE_RUNNING; + return 0; +} diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index 3843e5a062a1..04a06473f04b 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -62,5 +62,10 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, struct avs_tplg_path_template *template, struct snd_pcm_hw_params *fe_params, struct snd_pcm_hw_params *be_params); +int avs_path_bind(struct avs_path *path); +int avs_path_unbind(struct avs_path *path); +int avs_path_reset(struct avs_path *path); +int avs_path_pause(struct avs_path *path); +int avs_path_run(struct avs_path *path, int trigger);
#endif
On 2/7/22 07:25, Cezary Rojewski wrote:
Add functions to ease with state changing of all objects found in the path. Each represents either a BIND/UNBIND or SET_PIPEPILE_STATE IPC.
SET_PIPELINE?
DSP pipelines follow simple state machine scheme: CREATE -> RESET -> PAUSE -> RUNNING -> PAUSE -> RESET -> DELETE> There is no STOP, PAUSE serves that purpose instead.
that's not fully correct, the STOP can be handled in two steps due to DMA programming flows.
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/path.c | 130 +++++++++++++++++++++++++++++++++++++ sound/soc/intel/avs/path.h | 5 ++ 2 files changed, 135 insertions(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 272d868bedc9..c6db3f091e66 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -285,3 +285,133 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
return path; }
+int avs_path_bind(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- list_for_each_entry(ppl, &path->ppl_list, node) {
struct avs_path_binding *binding;list_for_each_entry(binding, &ppl->binding_list, node) {struct avs_path_module *source, *sink;source = binding->source;sink = binding->sink;ret = avs_ipc_bind(adev, source->module_id,source->instance_id, sink->module_id,sink->instance_id, binding->sink_pin,binding->source_pin);if (ret) {dev_err(adev->dev, "bind path failed: %d\n", ret);return AVS_IPC_RET(ret);
so what happens for all the previously bound path?
Is there an error-unwinding loop somewhere?
}}- }
- return 0;
+}
+int avs_path_unbind(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- list_for_each_entry(ppl, &path->ppl_list, node) {
struct avs_path_binding *binding;list_for_each_entry(binding, &ppl->binding_list, node) {struct avs_path_module *source, *sink;source = binding->source;sink = binding->sink;ret = avs_ipc_unbind(adev, source->module_id,source->instance_id, sink->module_id,sink->instance_id, binding->sink_pin,binding->source_pin);if (ret) {dev_err(adev->dev, "unbind path failed: %d\n", ret);return AVS_IPC_RET(ret);
what happens then? reboot?
}}- }
- return 0;
+}
+int avs_path_reset(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_RESET)
return 0;- list_for_each_entry(ppl, &path->ppl_list, node) {
ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_RESET);if (ret) {dev_err(adev->dev, "reset path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_RESET;
- return 0;
+}
+int avs_path_pause(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_PAUSED)
return 0;- list_for_each_entry_reverse(ppl, &path->ppl_list, node) {
does the order actually matter?
ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_PAUSED);if (ret) {dev_err(adev->dev, "pause path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_PAUSED;
- return 0;
+}
it looks like you could use a helper since the flows are identical.
+int avs_path_run(struct avs_path *path, int trigger) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_RUNNING && trigger == AVS_TPLG_TRIGGER_AUTO)
return 0;- list_for_each_entry(ppl, &path->ppl_list, node) {
if (ppl->template->cfg->trigger != trigger)continue;ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_RUNNING);if (ret) {dev_err(adev->dev, "run path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_RUNNING;
- return 0;
+} diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index 3843e5a062a1..04a06473f04b 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -62,5 +62,10 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, struct avs_tplg_path_template *template, struct snd_pcm_hw_params *fe_params, struct snd_pcm_hw_params *be_params); +int avs_path_bind(struct avs_path *path); +int avs_path_unbind(struct avs_path *path); +int avs_path_reset(struct avs_path *path); +int avs_path_pause(struct avs_path *path); +int avs_path_run(struct avs_path *path, int trigger);
#endif
On 2022-02-25 8:42 PM, Pierre-Louis Bossart wrote:
On 2/7/22 07:25, Cezary Rojewski wrote:
Add functions to ease with state changing of all objects found in the path. Each represents either a BIND/UNBIND or SET_PIPEPILE_STATE IPC.
SET_PIPELINE?
A typo! Thanks for spotting this out!
DSP pipelines follow simple state machine scheme: CREATE -> RESET -> PAUSE -> RUNNING -> PAUSE -> RESET -> DELETE> There is no STOP, PAUSE serves that purpose instead.
that's not fully correct, the STOP can be handled in two steps due to DMA programming flows.
From DSP perspective, there's no stop. Literally one cannot send SET_PIPELINE_STATE with STOP as a requested state.
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/path.c | 130 +++++++++++++++++++++++++++++++++++++ sound/soc/intel/avs/path.h | 5 ++ 2 files changed, 135 insertions(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index 272d868bedc9..c6db3f091e66 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -285,3 +285,133 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
return path; }
+int avs_path_bind(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- list_for_each_entry(ppl, &path->ppl_list, node) {
struct avs_path_binding *binding;list_for_each_entry(binding, &ppl->binding_list, node) {struct avs_path_module *source, *sink;source = binding->source;sink = binding->sink;ret = avs_ipc_bind(adev, source->module_id,source->instance_id, sink->module_id,sink->instance_id, binding->sink_pin,binding->source_pin);if (ret) {dev_err(adev->dev, "bind path failed: %d\n", ret);return AVS_IPC_RET(ret);so what happens for all the previously bound path?
Is there an error-unwinding loop somewhere?
This is a good question. It's an unknown ground. Usually if anything wrong happens, all the pipelines that are part of the path would be forcefully deleted. What I can do, is add unwinding and check with validation using some unusual scenarios to see if no regression occurs.
Not promising anything though - see below.
}}- }
- return 0;
+}
+int avs_path_unbind(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- list_for_each_entry(ppl, &path->ppl_list, node) {
struct avs_path_binding *binding;list_for_each_entry(binding, &ppl->binding_list, node) {struct avs_path_module *source, *sink;source = binding->source;sink = binding->sink;ret = avs_ipc_unbind(adev, source->module_id,source->instance_id, sink->module_id,sink->instance_id, binding->sink_pin,binding->source_pin);if (ret) {dev_err(adev->dev, "unbind path failed: %d\n", ret);return AVS_IPC_RET(ret);what happens then? reboot?
Yeah, unfortunately when an IPC fails, it's usually for a very "bad" reason and only DSP recovery can help here.
}}- }
- return 0;
+}
+int avs_path_reset(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_RESET)
return 0;- list_for_each_entry(ppl, &path->ppl_list, node) {
ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_RESET);if (ret) {dev_err(adev->dev, "reset path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_RESET;
- return 0;
+}
+int avs_path_pause(struct avs_path *path) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_PAUSED)
return 0;- list_for_each_entry_reverse(ppl, &path->ppl_list, node) {
does the order actually matter?
Yes, it does. We do here what's recommended.
ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_PAUSED);if (ret) {dev_err(adev->dev, "pause path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_PAUSED;
- return 0;
+}
it looks like you could use a helper since the flows are identical.
I did try doing that in the past but the readability got hurt : ( avs_path_run() has an additional check when compared to the other two. avs_path_pause() and avs_path_reset() to the things in the opposite order. So, I still believe it's not good to provide a helper for these. If you are seeing something I'm not, please feel free to correct me.
+int avs_path_run(struct avs_path *path, int trigger) +{
- struct avs_path_pipeline *ppl;
- struct avs_dev *adev = path->owner;
- int ret;
- if (path->state == AVS_PPL_STATE_RUNNING && trigger == AVS_TPLG_TRIGGER_AUTO)
return 0;- list_for_each_entry(ppl, &path->ppl_list, node) {
if (ppl->template->cfg->trigger != trigger)continue;ret = avs_ipc_set_pipeline_state(adev, ppl->instance_id,AVS_PPL_STATE_RUNNING);if (ret) {dev_err(adev->dev, "run path failed: %d\n", ret);path->state = AVS_PPL_STATE_INVALID;return AVS_IPC_RET(ret);}- }
- path->state = AVS_PPL_STATE_RUNNING;
- return 0;
+} diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index 3843e5a062a1..04a06473f04b 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -62,5 +62,10 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, struct avs_tplg_path_template *template, struct snd_pcm_hw_params *fe_params, struct snd_pcm_hw_params *be_params); +int avs_path_bind(struct avs_path *path); +int avs_path_unbind(struct avs_path *path); +int avs_path_reset(struct avs_path *path); +int avs_path_pause(struct avs_path *path); +int avs_path_run(struct avs_path *path, int trigger);
#endif
Creating the pipelines and instantiating the modules alone is insufficient to have a fully operational stream. Before it can be run, stream components need to be bound. Add arming functions to ensure all necessary operations are completed before path is yielded back to the avs_path_create() caller.
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/path.c | 180 +++++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index c6db3f091e66..abeb6921fcce 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -13,6 +13,73 @@ #include "path.h" #include "topology.h"
+/* Must be called with adev->comp_list_mutex held. */ +static struct avs_tplg * +avs_path_find_tplg(struct avs_dev *adev, const char *name) +{ + struct avs_soc_component *acomp; + + list_for_each_entry(acomp, &adev->comp_list, node) + if (!strcmp(acomp->tplg->name, name)) + return acomp->tplg; + return NULL; +} + +static struct avs_path_module * +avs_path_find_module(struct avs_path_pipeline *ppl, u32 template_id) +{ + struct avs_path_module *mod; + + list_for_each_entry(mod, &ppl->mod_list, node) + if (mod->template->id == template_id) + return mod; + return NULL; +} + +static struct avs_path_pipeline * +avs_path_find_pipeline(struct avs_path *path, u32 template_id) +{ + struct avs_path_pipeline *ppl; + + list_for_each_entry(ppl, &path->ppl_list, node) + if (ppl->template->id == template_id) + return ppl; + return NULL; +} + +static struct avs_path * +avs_path_find_path(struct avs_dev *adev, const char *name, u32 template_id) +{ + struct avs_tplg_path_template *pos, *template = NULL; + struct avs_tplg *tplg; + struct avs_path *path; + + tplg = avs_path_find_tplg(adev, name); + if (!tplg) + return NULL; + + list_for_each_entry(pos, &tplg->path_tmpl_list, node) { + if (pos->id == template_id) { + template = pos; + break; + } + } + if (!template) + return NULL; + + spin_lock(&adev->path_list_lock); + /* Only one variant of given path template may be instantiated at a time. */ + list_for_each_entry(path, &adev->path_list, node) { + if (path->template->owner == template) { + spin_unlock(&adev->path_list_lock); + return path; + } + } + + spin_unlock(&adev->path_list_lock); + return NULL; +} + static bool avs_test_hw_params(struct snd_pcm_hw_params *params, struct avs_audio_format *fmt) { @@ -75,6 +142,58 @@ avs_path_module_create(struct avs_dev *adev, return mod; }
+static int avs_path_binding_arm(struct avs_dev *adev, struct avs_path_binding *binding) +{ + struct avs_path_module *this_mod, *target_mod; + struct avs_path_pipeline *target_ppl; + struct avs_path *target_path; + struct avs_tplg_binding *t; + + t = binding->template; + this_mod = avs_path_find_module(binding->owner, + t->mod_id); + if (!this_mod) { + dev_err(adev->dev, "path mod %d not found\n", t->mod_id); + return -EINVAL; + } + + /* update with target_tplg_name too */ + target_path = avs_path_find_path(adev, t->target_tplg_name, + t->target_path_tmpl_id); + if (!target_path) { + dev_err(adev->dev, "target path %s:%d not found\n", + t->target_tplg_name, t->target_path_tmpl_id); + return -EINVAL; + } + + target_ppl = avs_path_find_pipeline(target_path, + t->target_ppl_id); + if (!target_ppl) { + dev_err(adev->dev, "target ppl %d not found\n", t->target_ppl_id); + return -EINVAL; + } + + target_mod = avs_path_find_module(target_ppl, t->target_mod_id); + if (!target_mod) { + dev_err(adev->dev, "target mod %d not found\n", t->target_mod_id); + return -EINVAL; + } + + if (t->is_sink) { + binding->sink = this_mod; + binding->sink_pin = t->mod_pin; + binding->source = target_mod; + binding->source_pin = t->target_mod_pin; + } else { + binding->sink = target_mod; + binding->sink_pin = t->target_mod_pin; + binding->source = this_mod; + binding->source_pin = t->mod_pin; + } + + return 0; +} + static void avs_path_binding_free(struct avs_dev *adev, struct avs_path_binding *binding) { kfree(binding); @@ -97,6 +216,38 @@ static struct avs_path_binding *avs_path_binding_create(struct avs_dev *adev, return binding; }
+static int avs_path_pipeline_arm(struct avs_dev *adev, + struct avs_path_pipeline *ppl) +{ + struct avs_path_module *mod; + + list_for_each_entry(mod, &ppl->mod_list, node) { + struct avs_path_module *source, *sink; + int ret; + + /* + * Only one module (so it's implicitly last) or it is the last + * one, either way we don't have next module to bind it to. + */ + if (mod == list_last_entry(&ppl->mod_list, + struct avs_path_module, node)) + break; + + /* bind current module to next module on list */ + source = mod; + sink = list_next_entry(mod, node); + if (!source || !sink) + return -EINVAL; + + ret = avs_ipc_bind(adev, source->module_id, source->instance_id, + sink->module_id, sink->instance_id, 0, 0); + if (ret) + return AVS_IPC_RET(ret); + } + + return 0; +} + static void avs_path_pipeline_free(struct avs_dev *adev, struct avs_path_pipeline *ppl) { @@ -212,6 +363,31 @@ static int avs_path_init(struct avs_dev *adev, struct avs_path *path, return 0; }
+static int avs_path_arm(struct avs_dev *adev, struct avs_path *path) +{ + struct avs_path_pipeline *ppl; + struct avs_path_binding *binding; + int ret; + + list_for_each_entry(ppl, &path->ppl_list, node) { + /* + * Arm all ppl bindings before binding internal modules + * as it costs no IPCs which isn't true for the latter. + */ + list_for_each_entry(binding, &ppl->binding_list, node) { + ret = avs_path_binding_arm(adev, binding); + if (ret < 0) + return ret; + } + + ret = avs_path_pipeline_arm(adev, ppl); + if (ret < 0) + return ret; + } + + return 0; +} + static void avs_path_free_unlocked(struct avs_path *path) { struct avs_path_pipeline *ppl, *save; @@ -243,6 +419,10 @@ static struct avs_path *avs_path_create_unlocked(struct avs_dev *adev, u32 dma_i if (ret < 0) goto err;
+ ret = avs_path_arm(adev, path); + if (ret < 0) + goto err; + path->state = AVS_PPL_STATE_INVALID; return path; err:
When binding modules to pins other than pin0, sometimes additional preparations need to be made, depending on the module type. Add function that prepares modules when necessary before binding them.
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/path.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index abeb6921fcce..b5c0f89add4f 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -466,6 +466,37 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id, return path; }
+static int avs_path_bind_prepare(struct avs_dev *adev, + struct avs_path_binding *binding) +{ + const struct avs_audio_format *src_fmt, *sink_fmt; + struct avs_tplg_module *tsource = binding->source->template; + struct avs_path_module *source = binding->source; + int ret; + + /* + * only copier modules about to be bound + * to output pin other than 0 need preparation + */ + if (!binding->source_pin) + return 0; + if (!guid_equal(&tsource->cfg_ext->type, &AVS_COPIER_MOD_UUID)) + return 0; + + src_fmt = tsource->in_fmt; + sink_fmt = binding->sink->template->in_fmt; + + ret = avs_ipc_copier_set_sink_format(adev, source->module_id, + source->instance_id, binding->source_pin, + src_fmt, sink_fmt); + if (ret) { + dev_err(adev->dev, "config copier failed: %d\n", ret); + return AVS_IPC_RET(ret); + } + + return 0; +} + int avs_path_bind(struct avs_path *path) { struct avs_path_pipeline *ppl; @@ -481,6 +512,10 @@ int avs_path_bind(struct avs_path *path) source = binding->source; sink = binding->sink;
+ ret = avs_path_bind_prepare(adev, binding); + if (ret < 0) + return ret; + ret = avs_ipc_bind(adev, source->module_id, source->instance_id, sink->module_id, sink->instance_id, binding->sink_pin,
Each module on DSP side serves a processing purpose. Depending on its purpose, it needs different information during its initialization. Add functions responsible for creating instances of specific module types given the information coming from the topology file.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/Kconfig | 1 + sound/soc/intel/avs/avs.h | 1 + sound/soc/intel/avs/path.c | 378 ++++++++++++++++++++++++++++++++++++- sound/soc/intel/avs/path.h | 1 + 4 files changed, 380 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index f0688c3d76a7..b01c492d3514 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -219,6 +219,7 @@ config SND_SOC_INTEL_AVS select SND_SOC_TOPOLOGY select SND_HDA_EXT_CORE select SND_HDA_DSP_LOADER + select SND_INTEL_NHLT help Enable support for Intel(R) cAVS 1.5 platforms with DSP capabilities. This includes Skylake, Kabylake, Amberlake and diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h index 313001b0455f..4da8e16280fc 100644 --- a/sound/soc/intel/avs/avs.h +++ b/sound/soc/intel/avs/avs.h @@ -92,6 +92,7 @@ struct avs_dev {
struct completion fw_ready;
+ struct nhlt_acpi_table *nhlt; struct list_head comp_list; struct mutex comp_list_mutex; struct list_head path_list; diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c index b5c0f89add4f..8c5d2672a081 100644 --- a/sound/soc/intel/avs/path.c +++ b/sound/soc/intel/avs/path.c @@ -113,6 +113,375 @@ avs_path_find_variant(struct avs_dev *adev, return NULL; }
+__maybe_unused +static bool avs_dma_type_is_host(u32 dma_type) +{ + return dma_type == AVS_DMA_HDA_HOST_OUTPUT || + dma_type == AVS_DMA_HDA_HOST_INPUT; +} + +__maybe_unused +static bool avs_dma_type_is_link(u32 dma_type) +{ + return !avs_dma_type_is_host(dma_type); +} + +__maybe_unused +static bool avs_dma_type_is_output(u32 dma_type) +{ + return dma_type == AVS_DMA_HDA_HOST_OUTPUT || + dma_type == AVS_DMA_HDA_LINK_OUTPUT || + dma_type == AVS_DMA_I2S_LINK_OUTPUT; +} + +__maybe_unused +static bool avs_dma_type_is_input(u32 dma_type) +{ + return !avs_dma_type_is_output(dma_type); +} + +static int avs_copier_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct nhlt_acpi_table *nhlt = adev->nhlt; + struct avs_tplg_module *t = mod->template; + struct avs_copier_cfg *cfg; + struct nhlt_specific_cfg *ep_blob; + union avs_connector_node_id node_id = {0}; + size_t cfg_size, data_size = 0; + void *data = NULL; + u32 dma_type; + int ret; + + dma_type = t->cfg_ext->copier.dma_type; + node_id.dma_type = dma_type; + + switch (dma_type) { + struct avs_audio_format *fmt; + int direction; + + case AVS_DMA_I2S_LINK_OUTPUT: + case AVS_DMA_I2S_LINK_INPUT: + if (avs_dma_type_is_input(dma_type)) + direction = SNDRV_PCM_STREAM_CAPTURE; + else + direction = SNDRV_PCM_STREAM_PLAYBACK; + + if (t->cfg_ext->copier.blob_fmt) + fmt = t->cfg_ext->copier.blob_fmt; + else if (direction == SNDRV_PCM_STREAM_CAPTURE) + fmt = t->in_fmt; + else + fmt = t->cfg_ext->copier.out_fmt; + + ep_blob = intel_nhlt_get_endpoint_blob(adev->dev, + nhlt, t->cfg_ext->copier.vindex.i2s.instance, + NHLT_LINK_SSP, fmt->valid_bit_depth, fmt->bit_depth, + fmt->num_channels, fmt->sampling_freq, direction, + NHLT_DEVICE_I2S); + if (!ep_blob) { + dev_err(adev->dev, "no I2S ep_blob found\n"); + return -ENOENT; + } + + data = ep_blob->caps; + data_size = ep_blob->size; + /* I2S gateway's vindex is statically assigned in topology */ + node_id.vindex = t->cfg_ext->copier.vindex.val; + + break; + + case AVS_DMA_DMIC_LINK_INPUT: + direction = SNDRV_PCM_STREAM_CAPTURE; + + if (t->cfg_ext->copier.blob_fmt) + fmt = t->cfg_ext->copier.blob_fmt; + else + fmt = t->in_fmt; + + ep_blob = intel_nhlt_get_endpoint_blob(adev->dev, nhlt, 0, + NHLT_LINK_DMIC, fmt->valid_bit_depth, + fmt->bit_depth, fmt->num_channels, + fmt->sampling_freq, direction, NHLT_DEVICE_DMIC); + if (!ep_blob) { + dev_err(adev->dev, "no DMIC ep_blob found\n"); + return -ENOENT; + } + + data = ep_blob->caps; + data_size = ep_blob->size; + /* DMIC gateway's vindex is statically assigned in topology */ + node_id.vindex = t->cfg_ext->copier.vindex.val; + + break; + + case AVS_DMA_HDA_HOST_OUTPUT: + case AVS_DMA_HDA_HOST_INPUT: + /* HOST gateway's vindex is dynamically assigned with DMA id */ + node_id.vindex = mod->owner->owner->dma_id; + break; + + case AVS_DMA_HDA_LINK_OUTPUT: + case AVS_DMA_HDA_LINK_INPUT: + node_id.vindex = t->cfg_ext->copier.vindex.val | + mod->owner->owner->dma_id; + break; + + case INVALID_OBJECT_ID: + default: + node_id = INVALID_NODE_ID; + break; + } + + cfg_size = sizeof(*cfg) + data_size; + /* Every config-BLOB contains gateway attributes. */ + if (data_size) + cfg_size -= sizeof(cfg->gtw_cfg.config.attrs); + + cfg = kzalloc(cfg_size, GFP_KERNEL); + if (!cfg) + return -ENOMEM; + + cfg->base.cpc = t->cfg_base->cpc; + cfg->base.ibs = t->cfg_base->ibs; + cfg->base.obs = t->cfg_base->obs; + cfg->base.is_pages = t->cfg_base->is_pages; + cfg->base.audio_fmt = *t->in_fmt; + cfg->out_fmt = *t->cfg_ext->copier.out_fmt; + cfg->feature_mask = t->cfg_ext->copier.feature_mask; + cfg->gtw_cfg.node_id = node_id; + cfg->gtw_cfg.dma_buffer_size = t->cfg_ext->copier.dma_buffer_size; + /* config_length in DWORDs */ + cfg->gtw_cfg.config_length = DIV_ROUND_UP(data_size, 4); + if (data) + memcpy(&cfg->gtw_cfg.config, data, data_size); + + mod->gtw_attrs = cfg->gtw_cfg.config.attrs; + + ret = avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, cfg, cfg_size, + &mod->instance_id); + kfree(cfg); + return ret; +} + +static int avs_updown_mix_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_updown_mixer_cfg cfg; + int i; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.out_channel_config = t->cfg_ext->updown_mix.out_channel_config; + cfg.coefficients_select = t->cfg_ext->updown_mix.coefficients_select; + for (i = 0; i < AVS_CHANNELS_MAX; i++) + cfg.coefficients[i] = t->cfg_ext->updown_mix.coefficients[i]; + cfg.channel_map = t->cfg_ext->updown_mix.channel_map; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_src_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_src_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.out_freq = t->cfg_ext->src.out_freq; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_asrc_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_asrc_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.out_freq = t->cfg_ext->asrc.out_freq; + cfg.mode = t->cfg_ext->asrc.mode; + cfg.disable_jitter_buffer = t->cfg_ext->asrc.disable_jitter_buffer; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_aec_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_aec_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.ref_fmt = *t->cfg_ext->aec.ref_fmt; + cfg.out_fmt = *t->cfg_ext->aec.out_fmt; + cfg.cpc_lp_mode = t->cfg_ext->aec.cpc_lp_mode; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_mux_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_mux_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.ref_fmt = *t->cfg_ext->mux.ref_fmt; + cfg.out_fmt = *t->cfg_ext->mux.out_fmt; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_wov_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_wov_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.cpc_lp_mode = t->cfg_ext->wov.cpc_lp_mode; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_micsel_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_micsel_cfg cfg; + + cfg.base.cpc = t->cfg_base->cpc; + cfg.base.ibs = t->cfg_base->ibs; + cfg.base.obs = t->cfg_base->obs; + cfg.base.is_pages = t->cfg_base->is_pages; + cfg.base.audio_fmt = *t->in_fmt; + cfg.out_fmt = *t->cfg_ext->micsel.out_fmt; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_modbase_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_modcfg_base cfg; + + cfg.cpc = t->cfg_base->cpc; + cfg.ibs = t->cfg_base->ibs; + cfg.obs = t->cfg_base->obs; + cfg.is_pages = t->cfg_base->is_pages; + cfg.audio_fmt = *t->in_fmt; + + return avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, &cfg, sizeof(cfg), + &mod->instance_id); +} + +static int avs_modext_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + struct avs_tplg_module *t = mod->template; + struct avs_tplg_modcfg_ext *tcfg = t->cfg_ext; + struct avs_modcfg_ext *cfg; + size_t cfg_size, num_pins; + int ret, i; + + num_pins = tcfg->generic.num_input_pins + tcfg->generic.num_output_pins; + cfg_size = sizeof(*cfg) + sizeof(*cfg->pin_fmts) * num_pins; + + cfg = kzalloc(cfg_size, GFP_KERNEL); + if (!cfg) + return -ENOMEM; + + cfg->base.cpc = t->cfg_base->cpc; + cfg->base.ibs = t->cfg_base->ibs; + cfg->base.obs = t->cfg_base->obs; + cfg->base.is_pages = t->cfg_base->is_pages; + cfg->base.audio_fmt = *t->in_fmt; + cfg->num_input_pins = tcfg->generic.num_input_pins; + cfg->num_output_pins = tcfg->generic.num_output_pins; + + /* configure pin formats */ + for (i = 0; i < num_pins; i++) { + struct avs_tplg_pin_format *tpin = &tcfg->generic.pin_fmts[i]; + struct avs_pin_format *pin = &cfg->pin_fmts[i]; + + pin->pin_index = tpin->pin_index; + pin->iobs = tpin->iobs; + pin->audio_fmt = *tpin->fmt; + } + + ret = avs_dsp_init_module(adev, mod->module_id, mod->owner->instance_id, + t->core_id, t->domain, cfg, cfg_size, + &mod->instance_id); + kfree(cfg); + return ret; +} + +static int avs_path_module_type_create(struct avs_dev *adev, struct avs_path_module *mod) +{ + const guid_t *type = &mod->template->cfg_ext->type; + + if (guid_equal(type, &AVS_MIXIN_MOD_UUID) || + guid_equal(type, &AVS_MIXOUT_MOD_UUID) || + guid_equal(type, &AVS_KPBUFF_MOD_UUID)) + return avs_modbase_create(adev, mod); + if (guid_equal(type, &AVS_COPIER_MOD_UUID)) + return avs_copier_create(adev, mod); + if (guid_equal(type, &AVS_MICSEL_MOD_UUID)) + return avs_micsel_create(adev, mod); + if (guid_equal(type, &AVS_MUX_MOD_UUID)) + return avs_mux_create(adev, mod); + if (guid_equal(type, &AVS_UPDWMIX_MOD_UUID)) + return avs_updown_mix_create(adev, mod); + if (guid_equal(type, &AVS_SRCINTC_MOD_UUID)) + return avs_src_create(adev, mod); + if (guid_equal(type, &AVS_AEC_MOD_UUID)) + return avs_aec_create(adev, mod); + if (guid_equal(type, &AVS_ASRC_MOD_UUID)) + return avs_asrc_create(adev, mod); + if (guid_equal(type, &AVS_INTELWOV_MOD_UUID)) + return avs_wov_create(adev, mod); + + if (guid_equal(type, &AVS_PROBE_MOD_UUID)) { + dev_err(adev->dev, "Probe module can't be instantiated by topology"); + return -EINVAL; + } + + return avs_modext_create(adev, mod); +} + static void avs_path_module_free(struct avs_dev *adev, struct avs_path_module *mod) { kfree(mod); @@ -124,7 +493,7 @@ avs_path_module_create(struct avs_dev *adev, struct avs_tplg_module *template) { struct avs_path_module *mod; - int module_id; + int module_id, ret;
module_id = avs_get_module_id(adev, &template->cfg_ext->type); if (module_id < 0) @@ -139,6 +508,13 @@ avs_path_module_create(struct avs_dev *adev, mod->owner = owner; INIT_LIST_HEAD(&mod->node);
+ ret = avs_path_module_type_create(adev, mod); + if (ret) { + dev_err(adev->dev, "module-type create failed: %d\n", ret); + kfree(mod); + return ERR_PTR(ret); + } + return mod; }
diff --git a/sound/soc/intel/avs/path.h b/sound/soc/intel/avs/path.h index 04a06473f04b..197222c5e008 100644 --- a/sound/soc/intel/avs/path.h +++ b/sound/soc/intel/avs/path.h @@ -38,6 +38,7 @@ struct avs_path_pipeline { struct avs_path_module { u16 module_id; u16 instance_id; + union avs_gtw_attributes gtw_attrs;
struct avs_tplg_module *template; struct avs_path_pipeline *owner;
A continuation of avs-driver initial series [1]. This chapter covers path management and topology parsing part which was ealier path of the main series. The two patches that represented these two subjects in the initial series, have been split into many to allow for easier review and discussion.
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 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.
A 'path' represents a DSP side of audio stream in runtime - is a logical container for pipelines which are themselves composed of modules - processing units. Due to high range of possible audio format combinations, there can be more variants of given path (and thus, its pipelines and modules) than total number of pipelines and module instances which firmware supports concurrently, all the instance IDs are allocated dynamically with help of IDA interface. 'Path templates' come from topology file and describe a pattern which is later used to actually create runtime 'path'.
This is one of the most 'dense' patchsets I've reviewed in a long time. While the code looks mostly good, I am afraid the directions and scope are rather unclear. Now that you've split the basic parts out, ironically it makes the intent of this patchset really difficult to grasp.
The first order of business is really to clarify the concepts:
What is a 'stream'? what is a 'path'? what is a 'path template'? What is a 'module template'? What topologies are supported? what is the link between a path and FE? how does this interoperate or duplicate DPAM? why does a path rely on a single DMA? What would happen if there is no host PCM, e.g. for a beep tone generated in firmware? How would this work for a firmware capture pipeline that only sends notification on acoustic events and no PCM data?
I have reviewed this set of patches three times already and this set of concepts are still nebulous to me, please refer to my detailed comments.
You really need to describe in layman's terms what the problem statement is, what your solution tries to fix, what other options you considered, what cases you didn't handle, etc. Put yourself if the shoes of someone that is not part of your team and has no prior exposure to the cAVS firmware design.
I would recommend that you use the Windows documentation [1] to provide ascii-art examples with hierarchical mixing, multiple outputs and loopbacks on what a 'path' really is and how the concept of template helps.
But at a more fundamental level, the main concern I have is with the BE use: this patchset assumes the BE configuration is fixed, and that's a very very strong limitation. It's clearly not right for e.g. Bluetooth offload where multiple profiles need to be supported. It's also not right when the codec or receiver can adjust to multiple hw_params, which could lead to simplifications such as removal of unnecessary SRC/downmixers, etc.
We absolutely want the capability to reconfigure the BE by using constraint matching between FE hw_params requested by applications and what the hardware can support. If your solution solved that problem you would have my full support. But if we're adding a rather complicated framework on top of a known limitation, then that's really a missed opportunity to do things better collectively.
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/audio/audio-proces...
On Fri, Feb 25, 2022 at 03:35:12PM -0600, Pierre-Louis Bossart wrote:
But at a more fundamental level, the main concern I have is with the BE use: this patchset assumes the BE configuration is fixed, and that's a very very strong limitation. It's clearly not right for e.g. Bluetooth offload where multiple profiles need to be supported. It's also not right when the codec or receiver can adjust to multiple hw_params, which could lead to simplifications such as removal of unnecessary SRC/downmixers, etc.
We absolutely want the capability to reconfigure the BE by using constraint matching between FE hw_params requested by applications and what the hardware can support. If your solution solved that problem you would have my full support. But if we're adding a rather complicated framework on top of a known limitation, then that's really a missed opportunity to do things better collectively.
On the one hand everything you say here is true. On the other hand I *did* suggest starting off with something with stripped down functionality and then building up from that to make things more digestable so some of this could be the result of that approach (I've not got enough recollection of previous serieses to confirm), obviously fixing the output is also quite a common thing for DSP based systems to do just in general.
participants (3)
-
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart