[alsa-devel] [PATCH v2 0/5] topology: Enhance support for private data
From: Mengdong Lin mengdong.lin@linux.intel.com
There is no ABI change in this series.
We finished verification on latest Intel platforms. There will be 4 or 5 series. Later series will come with some ABI update for topology. The size check on ABI objects in current ASoC kernel will detect possible ABI version mismatch between user space and kernel
History: v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c that are not caused by this series and will be fixed in later patches.
Mengdong Lin (5): topology: An element can refer to multipe data sections in text conf file topology: Merge lookup for data reference into tplg_copy_data() topology: Change uuid value to 16 separate characters in text conf file topology: Parse vendor private data for manifest topology: Tuple type can have an extenstion
include/topology.h | 69 ++++++++++- src/topology/ctl.c | 52 +++----- src/topology/dapm.c | 17 ++- src/topology/data.c | 307 ++++++++++++++++++++++++++++++++++++++-------- src/topology/elem.c | 4 + src/topology/parser.c | 28 ++++- src/topology/tplg_local.h | 15 ++- 7 files changed, 395 insertions(+), 97 deletions(-)
From: Mengdong Lin mengdong.lin@linux.intel.com
Previously in text conf file, an element can only refer to one data section (SectionData). Now it can also refer to a list of data sections. Thus users can split groups of firmware parameters to multiple data sections, and put them all in the reference list.
Finally, data of these data sections will be merged, in the same order as they are in the reference list, as the element's private data for kernel.
We still support the original syntax of reference to a single data section. The doc is updated for the syntax extension.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index 9d57ce3..d666505 100644 --- a/include/topology.h +++ b/include/topology.h @@ -213,6 +213,34 @@ extern "C" { * The keyword tuples is to define vendor specific tuples. Please refer to * section Vendor Tokens and Vendor tuples. * + * <h5>How to define an element with private data</h5> + * An element can refer to a single data section or multiple data + * sections. + * + * <h6>To refer to a single data section:</h6> + * <pre> + * Sectionxxx."element name" { + * ... + * data "name of data section" # optional private data + * } + * </pre> + * + * <h6>To refer to multiple data sections:</h6> + * <pre> + * Sectionxxx."element name" { + * ... + * data [ # optional private data + * "name of 1st data section" + * "name of 2nd data section" + * ... + * ] + * } + * </pre> + * And data of these sections will be merged in the same order as they are + * in the list, as the element's private data for kernel. + * + * </pre> + * * <h6>Vendor Tokens</h6> * A vendor token list is defined as a new section. Each token element is * a pair of string ID and integer value. And both the ID and value are diff --git a/src/topology/ctl.c b/src/topology/ctl.c index b948ac0..7ded0a4 100644 --- a/src/topology/ctl.c +++ b/src/topology/ctl.c @@ -447,11 +447,9 @@ int tplg_parse_control_bytes(snd_tplg_t *tplg, }
if (strcmp(id, "data") == 0) { - if (snd_config_get_string(n, &val) < 0) - return -EINVAL; - - tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); - tplg_dbg("\t%s: %s\n", id, val); + err = tplg_parse_data_refs(n, elem); + if (err < 0) + return err; continue; }
@@ -587,11 +585,9 @@ int tplg_parse_control_enum(snd_tplg_t *tplg, snd_config_t *cfg, }
if (strcmp(id, "data") == 0) { - if (snd_config_get_string(n, &val) < 0) - return -EINVAL; - - tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); - tplg_dbg("\t%s: %s\n", id, val); + err = tplg_parse_data_refs(n, elem); + if (err < 0) + return err; continue; }
@@ -725,11 +721,9 @@ int tplg_parse_control_mixer(snd_tplg_t *tplg, }
if (strcmp(id, "data") == 0) { - if (snd_config_get_string(n, &val) < 0) - return -EINVAL; - - tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); - tplg_dbg("\t%s: %s\n", id, val); + err = tplg_parse_data_refs(n, elem); + if (err < 0) + return err; continue; }
diff --git a/src/topology/dapm.c b/src/topology/dapm.c index 278d605..d8eb10c 100644 --- a/src/topology/dapm.c +++ b/src/topology/dapm.c @@ -598,11 +598,9 @@ int tplg_parse_dapm_widget(snd_tplg_t *tplg, }
if (strcmp(id, "data") == 0) { - if (snd_config_get_string(n, &val) < 0) - return -EINVAL; - - tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); - tplg_dbg("\t%s: %s\n", id, val); + err = tplg_parse_data_refs(n, elem); + if (err < 0) + return err; continue; } } diff --git a/src/topology/data.c b/src/topology/data.c index c0a098c..0c5469a 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -20,6 +20,36 @@ #include "tplg_local.h" #include <ctype.h>
+/* Get private data buffer of an element */ +struct snd_soc_tplg_private *get_priv_data(struct tplg_elem *elem) +{ + struct snd_soc_tplg_private *priv = NULL; + + switch (elem->type) { + case SND_TPLG_TYPE_MIXER: + priv = &elem->mixer_ctrl->priv; + break; + + case SND_TPLG_TYPE_ENUM: + priv = &elem->enum_ctrl->priv; + break; + + case SND_TPLG_TYPE_BYTES: + priv = &elem->bytes_ext->priv; + break; + + case SND_TPLG_TYPE_DAPM_WIDGET: + priv = &elem->widget->priv; + break; + + default: + SNDERR("error: '%s': no support for private data for type %d\n", + elem->id, elem->type); + } + + return priv; +} + /* Get Private data from a file. */ static int tplg_parse_data_file(snd_config_t *cfg, struct tplg_elem *elem) { @@ -614,6 +644,48 @@ static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg, return 0; }
+/* Parse private data references for the element, either a single data section + * or a list of data sections. + */ +int tplg_parse_data_refs(snd_config_t *cfg, + struct tplg_elem *elem) +{ + snd_config_type_t type; + snd_config_iterator_t i, next; + snd_config_t *n; + const char *val = NULL; + + type = snd_config_get_type(cfg); + + /* refer to a single data section */ + if (type == SND_CONFIG_TYPE_STRING) { + if (snd_config_get_string(cfg, &val) < 0) + return -EINVAL; + + tplg_dbg("\tdata: %s\n", val); + return tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); + } + + if (type != SND_CONFIG_TYPE_COMPOUND) { + SNDERR("error: compound type expected for %s", elem->id); + return -EINVAL; + } + + /* refer to a list of data sections */ + snd_config_for_each(i, next, cfg) { + const char *val; + + n = snd_config_iterator_entry(i); + if (snd_config_get_string(n, &val) < 0) + continue; + + tplg_dbg("\tdata: %s\n", val); + tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val); + } + + return 0; +} + /* Parse vendor tokens */ int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg, @@ -817,11 +889,15 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg, return err; }
-/* copy private data into the bytes extended control */ +/* Merge data from a referenced data element to the parent element's + * private data buffer. + * An element can refer to multiple data sections. Data of these sections + * will be merged in the their reference order. + */ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) { - struct snd_soc_tplg_private *priv; - int priv_data_size; + struct snd_soc_tplg_private *priv, *old_priv; + int priv_data_size, old_priv_data_size; void *obj;
if (!ref) @@ -831,6 +907,11 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) if (!ref->data || !ref->data->size) /* overlook empty private data */ return 0;
+ old_priv = get_priv_data(elem); + if (!old_priv) + return -EINVAL; + old_priv_data_size = old_priv->size; + priv_data_size = ref->data->size; obj = realloc(elem->obj, elem->size + priv_data_size); @@ -838,33 +919,16 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) return -ENOMEM; elem->obj = obj;
- switch (elem->type) { - case SND_TPLG_TYPE_MIXER: - priv = &elem->mixer_ctrl->priv; - break; - - case SND_TPLG_TYPE_ENUM: - priv = &elem->enum_ctrl->priv; - break; - - case SND_TPLG_TYPE_BYTES: - priv = &elem->bytes_ext->priv; - break; - - case SND_TPLG_TYPE_DAPM_WIDGET: - priv = &elem->widget->priv; - break; - - default: - SNDERR("error: elem '%s': type %d private data not supported \n", - elem->id, elem->type); + priv = get_priv_data(elem); + if (!priv) return -EINVAL; - }
+ /* merge the new data block */ elem->size += priv_data_size; - priv->size = priv_data_size; + priv->size = priv_data_size + old_priv_data_size; ref->compound_elem = 1; - memcpy(priv->data, ref->data->data, priv_data_size); + memcpy(priv->data + old_priv_data_size, + ref->data->data, priv_data_size); return 0; }
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h index 9239aef..4daa540 100644 --- a/src/topology/tplg_local.h +++ b/src/topology/tplg_local.h @@ -229,6 +229,7 @@ int tplg_build_routes(snd_tplg_t *tplg); int tplg_build_pcm_dai(snd_tplg_t *tplg, unsigned int type);
int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref); +int tplg_parse_data_refs(snd_config_t *cfg, struct tplg_elem *elem);
int tplg_ref_add(struct tplg_elem *elem, int type, const char* id); int tplg_ref_add_elem(struct tplg_elem *elem, struct tplg_elem *elem_ref);
From: Mengdong Lin mengdong.lin@linux.intel.com
Code refactor to reduce function calls. Now tplg_copy_data() can look up a referenced data element and merge its data.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/src/topology/ctl.c b/src/topology/ctl.c index 7ded0a4..592dded 100644 --- a/src/topology/ctl.c +++ b/src/topology/ctl.c @@ -140,9 +140,9 @@ static int tplg_build_mixer_control(snd_tplg_t *tplg, err = copy_tlv(elem, ref->elem);
} else if (ref->type == SND_TPLG_TYPE_DATA) { - ref->elem = tplg_elem_lookup(&tplg->pdata_list, - ref->id, SND_TPLG_TYPE_DATA); - err = tplg_copy_data(elem, ref->elem); + err = tplg_copy_data(tplg, elem, ref); + if (err < 0) + return err; }
if (!ref->elem) { @@ -188,9 +188,9 @@ static int tplg_build_enum_control(snd_tplg_t *tplg, copy_enum_texts(elem, ref->elem);
} else if (ref->type == SND_TPLG_TYPE_DATA) { - ref->elem = tplg_elem_lookup(&tplg->pdata_list, - ref->id, SND_TPLG_TYPE_DATA); - err = tplg_copy_data(elem, ref->elem); + err = tplg_copy_data(tplg, elem, ref); + if (err < 0) + return err; } if (!ref->elem) { SNDERR("error: cannot find '%s' referenced by" @@ -208,6 +208,7 @@ static int tplg_build_bytes_control(snd_tplg_t *tplg, struct tplg_elem *elem) { struct tplg_ref *ref; struct list_head *base, *pos; + int err;
base = &elem->ref_list;
@@ -217,18 +218,11 @@ static int tplg_build_bytes_control(snd_tplg_t *tplg, struct tplg_elem *elem) if (ref->id == NULL || ref->elem) continue;
- /* bytes control only reference one private data section */ - ref->elem = tplg_elem_lookup(&tplg->pdata_list, - ref->id, SND_TPLG_TYPE_DATA); - if (!ref->elem) { - SNDERR("error: cannot find data '%s'" - " referenced by control '%s'\n", - ref->id, elem->id); - return -EINVAL; + if (ref->type == SND_TPLG_TYPE_DATA) { + err = tplg_copy_data(tplg, elem, ref); + if (err < 0) + return err; } - - /* copy texts to enum elem */ - return tplg_copy_data(elem, ref->elem); }
return 0; diff --git a/src/topology/dapm.c b/src/topology/dapm.c index d8eb10c..4d343b2 100644 --- a/src/topology/dapm.c +++ b/src/topology/dapm.c @@ -191,12 +191,11 @@ static int tplg_build_widget(snd_tplg_t *tplg, break;
case SND_TPLG_TYPE_DATA: - if (!ref->elem) - ref->elem = tplg_elem_lookup(&tplg->pdata_list, - ref->id, SND_TPLG_TYPE_DATA); - if (ref->elem) - err = tplg_copy_data(elem, ref->elem); + err = tplg_copy_data(tplg, elem, ref); + if (err < 0) + return err; break; + default: break; } diff --git a/src/topology/data.c b/src/topology/data.c index 0c5469a..e60114e 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -889,22 +889,30 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg, return err; }
-/* Merge data from a referenced data element to the parent element's - * private data buffer. +/* Find a referenced data element and copy its data to the parent + * element's private data buffer. * An element can refer to multiple data sections. Data of these sections * will be merged in the their reference order. */ -int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) +int tplg_copy_data(snd_tplg_t *tplg, struct tplg_elem *elem, + struct tplg_ref *ref) { + struct tplg_elem *ref_elem; struct snd_soc_tplg_private *priv, *old_priv; int priv_data_size, old_priv_data_size; void *obj;
- if (!ref) + ref_elem = tplg_elem_lookup(&tplg->pdata_list, + ref->id, SND_TPLG_TYPE_DATA); + if (!ref_elem) { + SNDERR("error: cannot find data '%s' referenced by" + " element '%s'\n", ref->id, elem->id); return -EINVAL; + }
tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id); - if (!ref->data || !ref->data->size) /* overlook empty private data */ + /* overlook empty private data */ + if (!ref_elem->data || !ref_elem->data->size) return 0;
old_priv = get_priv_data(elem); @@ -912,7 +920,7 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) return -EINVAL; old_priv_data_size = old_priv->size;
- priv_data_size = ref->data->size; + priv_data_size = ref_elem->data->size; obj = realloc(elem->obj, elem->size + priv_data_size); if (!obj) @@ -926,9 +934,9 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) /* merge the new data block */ elem->size += priv_data_size; priv->size = priv_data_size + old_priv_data_size; - ref->compound_elem = 1; + ref_elem->compound_elem = 1; memcpy(priv->data + old_priv_data_size, - ref->data->data, priv_data_size); + ref_elem->data->data, priv_data_size); return 0; }
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h index 4daa540..518b81e 100644 --- a/src/topology/tplg_local.h +++ b/src/topology/tplg_local.h @@ -228,7 +228,9 @@ int tplg_build_widgets(snd_tplg_t *tplg); int tplg_build_routes(snd_tplg_t *tplg); int tplg_build_pcm_dai(snd_tplg_t *tplg, unsigned int type);
-int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref); +int tplg_copy_data(snd_tplg_t *tplg, struct tplg_elem *elem, + struct tplg_ref *ref); + int tplg_parse_data_refs(snd_config_t *cfg, struct tplg_elem *elem);
int tplg_ref_add(struct tplg_elem *elem, int type, const char* id);
From: Mengdong Lin mengdong.lin@linux.intel.com
Previously in text conf file, the uuid value of vendor tuples is a 16-characer string. Now change it to 16 characters separated by commas, easier for users to edit it manually.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index d666505..89bed6c 100644 --- a/include/topology.h +++ b/include/topology.h @@ -273,8 +273,8 @@ extern "C" { * ... * } * - * tuples."uuid" { - * VENDOR_TOKEN_ID2 "16 character uuid" + * tuples."uuid" { # 16 characters separated by commas + * VENDOR_TOKEN_ID2 "0x01,0x02,...,0x0f" * ... * } * diff --git a/src/topology/data.c b/src/topology/data.c index e60114e..a0c5ea2 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -176,6 +176,49 @@ static int get_hex_num(const char *str) return values; }
+/* get uuid from a string made by 16 characters separated by commas */ +static int get_uuid(const char *str, unsigned char *uuid_le) +{ + unsigned long int val; + char *tmp, *s = NULL; + int values = 0, ret = 0; + + tmp = strdup(str); + if (tmp == NULL) + return -ENOMEM; + + s = strtok(tmp, ","); + + while (s != NULL) { + errno = 0; + val = strtoul(s, NULL, 0); + if ((errno == ERANGE && val == ULONG_MAX) + || (errno != 0 && val == 0) + || (val > UCHAR_MAX)) { + SNDERR("error: invalid value for uuid\n"); + ret = -EINVAL; + goto out; + } + + *(uuid_le + values) = (unsigned char)val; + + values++; + if (values >= 16) + break; + + s = strtok(NULL, ","); + } + + if (values < 16) { + SNDERR("error: less than 16 integers for uuid\n"); + ret = -EINVAL; + } + +out: + free(tmp); + return ret; +} + static int write_hex(char *buf, char *str, int width) { long val; @@ -474,7 +517,7 @@ static int build_tuples(snd_tplg_t *tplg, struct tplg_elem *elem) return 0; }
-static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg, +static int parse_tuple_set(snd_config_t *cfg, struct tplg_tuple_set **s) { snd_config_iterator_t i, next; @@ -484,7 +527,6 @@ static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg, unsigned int type, num_tuples = 0; struct tplg_tuple *tuple; unsigned long int tuple_val; - int len;
snd_config_get_id(cfg, &id);
@@ -535,14 +577,8 @@ static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
switch (type) { case SND_SOC_TPLG_TUPLE_TYPE_UUID: - len = strlen(value); - if (len > 16 || len == 0) { - SNDERR("error: tuple %s: invalid uuid\n", id); + if (get_uuid(value, tuple->uuid) < 0) goto err; - } - - memcpy(tuple->uuid, value, len); - tplg_dbg("\t\t%s = %s\n", tuple->token, tuple->uuid); break;
case SND_SOC_TPLG_TUPLE_TYPE_STRING: @@ -598,7 +634,7 @@ err: return -EINVAL; }
-static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg, +static int parse_tuple_sets(snd_config_t *cfg, struct tplg_vendor_tuples *tuples) { snd_config_iterator_t i, next; @@ -632,7 +668,7 @@ static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg, return -EINVAL; }
- err = parse_tuple_set(tplg, n, &tuples->set[tuples->num_sets]); + err = parse_tuple_set(n, &tuples->set[tuples->num_sets]); if (err < 0) return err;
@@ -774,7 +810,7 @@ int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg, }
if (strcmp(id, "tuples") == 0) { - err = parse_tuple_sets(tplg, n, tuples); + err = parse_tuple_sets(n, tuples); if (err < 0) return err; } diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h index 518b81e..48db813 100644 --- a/src/topology/tplg_local.h +++ b/src/topology/tplg_local.h @@ -104,7 +104,7 @@ struct tplg_tuple { char token[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; union { char string[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; - char uuid[16]; + unsigned char uuid[16]; unsigned int value; }; };
From: Mengdong Lin mengdong.lin@linux.intel.com
In text conf file, user can define a manifest section and let it refer to private data sections, in the same syntax as other element types.
The text conf file can have at most 1 manifest section.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index 89bed6c..644e548 100644 --- a/include/topology.h +++ b/include/topology.h @@ -586,6 +586,20 @@ extern "C" { * } * </pre> * + * <h4>Manifest Private Data</h4> + * Manfiest may have private data. Users need to define a manifest section + * and add the references to 1 or multiple data sections. Please refer to + * section 'How to define an element with private data'. <br> + * And the text conf file can have at most 1 manifest section. <br><br> + * + * Manifest section is defined as follows :- + * + * <pre> + * SectionManifest"name" { + * + * data "name" # optional private data + * } + * </pre> */
/** Maximum number of channels supported in one control */ diff --git a/src/topology/data.c b/src/topology/data.c index a0c5ea2..245a834 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -26,6 +26,10 @@ struct snd_soc_tplg_private *get_priv_data(struct tplg_elem *elem) struct snd_soc_tplg_private *priv = NULL;
switch (elem->type) { + case SND_TPLG_TYPE_MANIFEST: + priv = &elem->manifest->priv; + break; + case SND_TPLG_TYPE_MIXER: priv = &elem->mixer_ctrl->priv; break; @@ -834,6 +838,103 @@ void tplg_free_tuples(void *obj) free(tuples->set); }
+/* Parse manifest's data references + */ +int tplg_parse_manifest_data(snd_tplg_t *tplg, snd_config_t *cfg, + void *private ATTRIBUTE_UNUSED) +{ + struct snd_soc_tplg_manifest *manifest; + struct tplg_elem *elem; + snd_config_iterator_t i, next; + snd_config_t *n; + const char *id; + int err; + + if (!list_empty(&tplg->manifest_list)) { + SNDERR("error: already has manifest data\n"); + return -EINVAL; + } + + elem = tplg_elem_new_common(tplg, cfg, NULL, SND_TPLG_TYPE_MANIFEST); + if (!elem) + return -ENOMEM; + + manifest = elem->manifest; + manifest->size = elem->size; + + tplg_dbg(" Manifest: %s\n", elem->id); + + snd_config_for_each(i, next, cfg) { + n = snd_config_iterator_entry(i); + if (snd_config_get_id(n, &id) < 0) + continue; + + /* skip comments */ + if (strcmp(id, "comment") == 0) + continue; + if (id[0] == '#') + continue; + + + if (strcmp(id, "data") == 0) { + err = tplg_parse_data_refs(n, elem); + if (err < 0) + return err; + continue; + } + } + + return 0; +} + +/* merge private data of manifest */ +int tplg_build_manifest_data(snd_tplg_t *tplg) +{ + struct list_head *base, *pos; + struct tplg_elem *elem = NULL; + struct tplg_ref *ref; + struct snd_soc_tplg_manifest *manifest; + int err = 0; + + base = &tplg->manifest_list; + list_for_each(pos, base) { + + elem = list_entry(pos, struct tplg_elem, list); + break; + } + + if (!elem) /* no manifest data */ + return 0; + + base = &elem->ref_list; + + /* for each ref in this manifest elem */ + list_for_each(pos, base) { + + ref = list_entry(pos, struct tplg_ref, list); + if (ref->id == NULL || ref->elem) + continue; + + if (ref->type == SND_TPLG_TYPE_DATA) { + err = tplg_copy_data(tplg, elem, ref); + if (err < 0) + return err; + } + } + + manifest = elem->manifest; + if (!manifest->priv.size) /* no manifest data */ + return 0; + + tplg->manifest_pdata = malloc(manifest->priv.size); + if (!tplg->manifest_pdata) + return -ENOMEM; + + tplg->manifest.priv.size = manifest->priv.size; + memcpy(tplg->manifest_pdata, manifest->priv.data, manifest->priv.size); + return 0; +} + /* Parse Private data. * * Object private data can either be from file or defined as bytes, shorts, diff --git a/src/topology/elem.c b/src/topology/elem.c index 50414f0..029c9ab 100644 --- a/src/topology/elem.c +++ b/src/topology/elem.c @@ -150,6 +150,10 @@ struct tplg_elem* tplg_elem_new_common(snd_tplg_t *tplg, case SND_TPLG_TYPE_DATA: list_add_tail(&elem->list, &tplg->pdata_list); break; + case SND_TPLG_TYPE_MANIFEST: + list_add_tail(&elem->list, &tplg->manifest_list); + obj_size = sizeof(struct snd_soc_tplg_manifest); + break; case SND_TPLG_TYPE_TEXT: list_add_tail(&elem->list, &tplg->text_list); break; diff --git a/src/topology/parser.c b/src/topology/parser.c index f6fc944..3ab64f4 100644 --- a/src/topology/parser.c +++ b/src/topology/parser.c @@ -189,6 +189,15 @@ static int tplg_parse_config(snd_tplg_t *tplg, snd_config_t *cfg) continue; }
+ if (strcmp(id, "SectionManifest") == 0) { + err = tplg_parse_compound(tplg, n, + tplg_parse_manifest_data, + NULL); + if (err < 0) + return err; + continue; + } + SNDERR("error: unknown section %s\n", id); } return 0; @@ -246,6 +255,10 @@ static int tplg_build_integ(snd_tplg_t *tplg) if (err < 0) return err;
+ err = tplg_build_manifest_data(tplg); + if (err < 0) + return err; + err = tplg_build_controls(tplg); if (err < 0) return err; @@ -374,8 +387,16 @@ out:
int snd_tplg_set_manifest_data(snd_tplg_t *tplg, const void *data, int len) { + if (len <= 0) + return 0; + tplg->manifest.priv.size = len; - tplg->manifest_pdata = data; + + tplg->manifest_pdata = malloc(len); + if (!tplg->manifest_pdata) + return -ENOMEM; + + memcpy(tplg->manifest_pdata, data, len); return 0; }
@@ -423,6 +444,7 @@ snd_tplg_t *snd_tplg_new(void) INIT_LIST_HEAD(&tplg->cc_list); INIT_LIST_HEAD(&tplg->route_list); INIT_LIST_HEAD(&tplg->pdata_list); + INIT_LIST_HEAD(&tplg->manifest_list); INIT_LIST_HEAD(&tplg->text_list); INIT_LIST_HEAD(&tplg->pcm_config_list); INIT_LIST_HEAD(&tplg->pcm_caps_list); @@ -437,6 +459,9 @@ snd_tplg_t *snd_tplg_new(void)
void snd_tplg_free(snd_tplg_t *tplg) { + if (tplg->manifest_pdata) + free(tplg->manifest_pdata); + tplg_elem_free_list(&tplg->tlv_list); tplg_elem_free_list(&tplg->widget_list); tplg_elem_free_list(&tplg->pcm_list); @@ -444,6 +469,7 @@ void snd_tplg_free(snd_tplg_t *tplg) tplg_elem_free_list(&tplg->cc_list); tplg_elem_free_list(&tplg->route_list); tplg_elem_free_list(&tplg->pdata_list); + tplg_elem_free_list(&tplg->manifest_list); tplg_elem_free_list(&tplg->text_list); tplg_elem_free_list(&tplg->pcm_config_list); tplg_elem_free_list(&tplg->pcm_caps_list); diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h index 48db813..4d79aa7 100644 --- a/src/topology/tplg_local.h +++ b/src/topology/tplg_local.h @@ -58,7 +58,7 @@ struct snd_tplg {
/* manifest */ struct snd_soc_tplg_manifest manifest; - const void *manifest_pdata; /* copied by builder at file write */ + void *manifest_pdata; /* copied by builder at file write */
/* list of each element type */ struct list_head tlv_list; @@ -71,6 +71,7 @@ struct snd_tplg { struct list_head pdata_list; struct list_head token_list; struct list_head tuple_list; + struct list_head manifest_list; struct list_head pcm_config_list; struct list_head pcm_caps_list;
@@ -154,6 +155,7 @@ struct tplg_elem { struct snd_soc_tplg_private *data; struct tplg_vendor_tokens *tokens; struct tplg_vendor_tuples *tuples; + struct snd_soc_tplg_manifest *manifest; };
/* an element may refer to other elements: @@ -195,6 +197,9 @@ int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg,
void tplg_free_tuples(void *obj);
+int tplg_parse_manifest_data(snd_tplg_t *tplg, snd_config_t *cfg, + void *private ATTRIBUTE_UNUSED); + int tplg_parse_control_bytes(snd_tplg_t *tplg, snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
@@ -223,6 +228,7 @@ int tplg_parse_cc(snd_tplg_t *tplg, snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
int tplg_build_data(snd_tplg_t *tplg); +int tplg_build_manifest_data(snd_tplg_t *tplg); int tplg_build_controls(snd_tplg_t *tplg); int tplg_build_widgets(snd_tplg_t *tplg); int tplg_build_routes(snd_tplg_t *tplg);
From: Mengdong Lin mengdong.lin@linux.intel.com
After the type specific string ("uuid", "string", "byte", "short" and "word"), users may append a string, like "uuidxxx". The topology parser will check the first few characters to get the tuple type.
This can allow users to put multiple tuples of the same type into one vendor tuple section (SectionVendorTuples), e.g. parameters of multiple firmware modules.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index 644e548..0675b52 100644 --- a/include/topology.h +++ b/include/topology.h @@ -302,6 +302,29 @@ extern "C" { * } * } * </pre> + * To define multiple vendor tuples of same type, please append some + * characters after the type string ("string", "uuid", "bool", "byte", "short" + * or "word"), to avoid ID duplication in the SectionVendorTuples.<br> + * The parser will check the first few characters in ID to get the tuple type. + * Here is an example: + * <pre> + * SectionVendorTuples."id of the vendor tuples" { + * ... + * tuples."word.module0" { + * VENDOR_TOKEN_PARAM_ID1 "0x00112233" + * VENDOR_TOKEN_PARAM_ID2 "0x44556677" + * ... + * } + * + * tuples."word.module2" { + * VENDOR_TOKEN_PARAM_ID1 "0x11223344" + * VENDOR_TOKEN_PARAM_ID2 "0x55667788" + * ... + * } + * ... + * } + * + * </pre> * * <h5>Mixer Controls</h5> * A mixer control is defined as a new section that can include channel mapping, diff --git a/src/topology/data.c b/src/topology/data.c index 245a834..e81b7f1 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -534,17 +534,17 @@ static int parse_tuple_set(snd_config_t *cfg,
snd_config_get_id(cfg, &id);
- if (strcmp(id, "uuid") == 0) + if (strncmp(id, "uuid", 4) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_UUID; - else if (strcmp(id, "string") == 0) + else if (strncmp(id, "string", 5) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_STRING; - else if (strcmp(id, "bool") == 0) + else if (strncmp(id, "bool", 4) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_BOOL; - else if (strcmp(id, "byte") == 0) + else if (strncmp(id, "byte", 4) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_BYTE; - else if (strcmp(id, "short") == 0) + else if (strncmp(id, "short", 5) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_SHORT; - else if (strcmp(id, "word") == 0) + else if (strncmp(id, "word", 4) == 0) type = SND_SOC_TPLG_TUPLE_TYPE_WORD; else { SNDERR("error: invalid tuple type '%s'\n", id);
On Jul 15 2016 21:17, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
There is no ABI change in this series.
We finished verification on latest Intel platforms. There will be 4 or 5 series. Later series will come with some ABI update for topology. The size check on ABI objects in current ASoC kernel will detect possible ABI version mismatch between user space and kernel
History: v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c that are not caused by this series and will be fixed in later patches.
Mengdong Lin (5): topology: An element can refer to multipe data sections in text conf file topology: Merge lookup for data reference into tplg_copy_data() topology: Change uuid value to 16 separate characters in text conf file topology: Parse vendor private data for manifest topology: Tuple type can have an extenstion
include/topology.h | 69 ++++++++++- src/topology/ctl.c | 52 +++----- src/topology/dapm.c | 17 ++- src/topology/data.c | 307 ++++++++++++++++++++++++++++++++++++++-------- src/topology/elem.c | 4 + src/topology/parser.c | 28 ++++- src/topology/tplg_local.h | 15 ++- 7 files changed, 395 insertions(+), 97 deletions(-)
Reviewed-by: Takashi Sakamoto o-takashi@sakamocchi.jp
Current implementation of ALSA SoC part in kernel has no call corresponding to sys_ioctl(SNDRV_CTL_IOCTL_ELEM_ADD) or something like that, thus I recommend you to drop this line by an additional patch: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b9...
Regards
Takashi Sakamoto
On 07/16/2016 08:58 PM, Takashi Sakamoto wrote:
Current implementation of ALSA SoC part in kernel has no call corresponding to sys_ioctl(SNDRV_CTL_IOCTL_ELEM_ADD) or something like that, thus I recommend you to drop this line by an additional patch: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b9...
Thanks for your suggestions. But this tool is not for directly adding controls from the user space.
It's for ADSP firmware vendors to describe the controls/widgets/pcms for a specific version of firmware. Previously, these components are hard coded in ASoC platforms drivers and not so convenient to share a common driver for different firmwares. The topology binary generated by this tool will be loaded by ASoC platform driver with a specific firmware binary, and so ASoC will create the controls/widgets/pcms that match the firmware automatically.
Thanks Mengdong
On Jul 18 2016 11:47, Mengdong Lin wrote:
Thanks for your suggestions. But this tool is not for directly adding controls from the user space.
It's for ADSP firmware vendors to describe the controls/widgets/pcms for a specific version of firmware. Previously, these components are hard coded in ASoC platforms drivers and not so convenient to share a common driver for different firmwares. The topology binary generated by this tool will be loaded by ASoC platform driver with a specific firmware binary, and so ASoC will create the controls/widgets/pcms that match the firmware automatically.
I know all of what you explained. Therefore, You misunderstand about my suggestion.
What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER flag from current topology implementation in userspace library, because current implementation of ALSA SoC part doesn't support it.
In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a result of parsing binary blob generated by topology implementation.
In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these lines: http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/...
Totally, the flag cannot be supported by current implementation of topology in kernel. Thus, in userspace, we should drop this line to prevent users from confusions: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b9...
That's all what I mentioned. If something unclear, please tell it to me.
Regards
Takashi Sakamoto
-----Original Message----- From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp] Sent: Monday, July 18, 2016 11:43 AM
I know all of what you explained. Therefore, You misunderstand about my suggestion.
What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER flag from current topology implementation in userspace library, because current implementation of ALSA SoC part doesn't support it.
In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a result of parsing binary blob generated by topology implementation.
In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these lines: http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/... ontrol.c#n263
Totally, the flag cannot be supported by current implementation of topology in kernel. Thus, in userspace, we should drop this line to prevent users from confusions: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b9... 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
That's all what I mentioned. If something unclear, please tell it to me.
Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag. I'll remove this from the topology user space tool to avoid confusion.
Thanks again Mengdong
On Mon, Jul 18, 2016 at 03:06:24PM +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp] Sent: Monday, July 18, 2016 11:43 AM
I know all of what you explained. Therefore, You misunderstand about my suggestion.
What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER flag from current topology implementation in userspace library, because current implementation of ALSA SoC part doesn't support it.
In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a result of parsing binary blob generated by topology implementation.
In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these lines: http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/... ontrol.c#n263
Totally, the flag cannot be supported by current implementation of topology in kernel. Thus, in userspace, we should drop this line to prevent users from confusions: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b9... 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
That's all what I mentioned. If something unclear, please tell it to me.
Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag. I'll remove this from the topology user space tool to avoid confusion.
Oh no, that won't be a good idea.
We would like to specify the access for controls from topology. Some controls can be read only and some write only :)
For example, any VU-meter controls should be read-only. Similarly if we have some user data being sent to some modules which can do do all fancy audio detection then these controls should be write-only.
Yes it seems to be broken by this, but we should fix it rather than remove.
On Jul 19 2016 12:59, Vinod Koul wrote:
Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag. I'll remove this from the topology user space tool to avoid confusion.
Oh no, that won't be a good idea.
We would like to specify the access for controls from topology. Some controls can be read only and some write only :)
For example, any VU-meter controls should be read-only. Similarly if we have some user data being sent to some modules which can do do all fancy audio detection then these controls should be write-only.
?
I just suggested to remove 'user' from topology implementation in userspace library. You can still control access rights of control elements by adding enough flags; 'read' or 'write' (SNDRV_CTL_ELEM_ACCESS_READ or SNDRV_CTL_ELEM_ACCESS_WRITE). SNDRV_CTL_ELEM_ACCESS_USER has no relationship to your purposes.
When a control element has SNDRV_CTL_ELEM_ACCESS_USER flag, a control element set including the element has below operations; - struct snd_kcontrol.info = snd_ctl_elem_user_enum_info or snd_ctl_elem_user_info - struct snd_kcontrol.get = snd_ctl_elem_user_get - struct snd_kcontrol.put = snd_ctl_elem_user_put - struct snd_kcontrol.tlv.c = snd_ctl_elem_user_tlv
When userland applications call ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD, then ALSA ctl core adds new control element set. The 'USER' flag is used only for this case.
I think it better for you to read 'sound/core/control.c' or my test program in alsa-lib source. http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=test/user-ctl-element-s...
Regards
Takashi Sakamoto
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Vinod Koul Sent: Tuesday, July 19, 2016 12:00 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org; Mengdong Lin; tiwai@suse.de; Takashi Sakamoto; broonie@kernel.org; Girdwood, Liam R; Nc, Shreyas Subject: Re: [alsa-devel] [PATCH v2 0/5] topology: Enhance support for private data
On Mon, Jul 18, 2016 at 03:06:24PM +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp] Sent: Monday, July 18, 2016 11:43 AM
I know all of what you explained. Therefore, You misunderstand about my suggestion.
What I suggested is to drop a support of
SNDRV_CTL_ELEM_ACCESS_USER
flag from current topology implementation in userspace library, because current implementation of ALSA SoC part doesn't support it.
In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a result of parsing binary blob generated by topology
implementation.
In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these lines: http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sou nd/core/c ontrol.c#n263
Totally, the flag cannot be supported by current implementation of topology in kernel. Thus, in userspace, we should drop this line to prevent users from confusions: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ct l.c;h=b94 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
That's all what I mentioned. If something unclear, please tell it to me.
Yes, you're right. snd_ctl_new1() dropped
SNDRV_CTL_ELEM_ACCESS_USER flag.
I'll remove this from the topology user space tool to avoid confusion.
Oh no, that won't be a good idea.
Hi Vinod,
I think topology will drop support for only one flag "SNDRV_CTL_ELEM_ACCESS_USER". All other access flags are still supported.
Thanks Mengdong
We would like to specify the access for controls from topology. Some controls can be read only and some write only :)
For example, any VU-meter controls should be read-only. Similarly if we have some user data being sent to some modules which can do do all fancy audio detection then these controls should be write-only.
Yes it seems to be broken by this, but we should fix it rather than remove.
-- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, 15 Jul 2016 14:17:21 +0200, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
There is no ABI change in this series.
We finished verification on latest Intel platforms. There will be 4 or 5 series. Later series will come with some ABI update for topology. The size check on ABI objects in current ASoC kernel will detect possible ABI version mismatch between user space and kernel
History: v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c that are not caused by this series and will be fixed in later patches.
Mengdong Lin (5): topology: An element can refer to multipe data sections in text conf file topology: Merge lookup for data reference into tplg_copy_data() topology: Change uuid value to 16 separate characters in text conf file topology: Parse vendor private data for manifest topology: Tuple type can have an extenstion
Applied, thanks.
Takashi
participants (6)
-
Lin, Mengdong -
Mengdong Lin -
mengdong.lin＠linux.intel.com
-
Takashi Iwai -
Takashi Sakamoto -
Vinod Koul