From ec7c2156f54f94403cc279a10c3fdfe380c9c64a Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Fri, 17 Jul 2020 20:04:44 +0800 Subject: [PATCH 1/6] testbench: topology: initial the sof_ipc_comp_file with 0s Initial the struct sof_ipc_comp_file to make sure all no updated flags are 0s. Signed-off-by: Keyon Jie --- tools/testbench/topology.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testbench/topology.c b/tools/testbench/topology.c index b216e6ab98f6..5cb4425c4c34 100644 --- a/tools/testbench/topology.c +++ b/tools/testbench/topology.c @@ -272,7 +272,7 @@ static int load_fileread(void *dev, int comp_id, int pipeline_id, struct testbench_prm *tp) { struct sof *sof = (struct sof *)dev; - struct sof_ipc_comp_file fileread; + struct sof_ipc_comp_file fileread = {0}; int size = widget->priv.size; int ret; @@ -319,7 +319,7 @@ static int load_filewrite(struct sof *sof, int comp_id, int pipeline_id, struct snd_soc_tplg_dapm_widget *widget, int dir, struct testbench_prm *tp) { - struct sof_ipc_comp_file filewrite; + struct sof_ipc_comp_file filewrite = {0}; int size = widget->priv.size; int ret; From 272a32c3a9ecf1c77cd688665dd360e500ef1436 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Wed, 17 Jun 2020 11:57:52 +0800 Subject: [PATCH 2/6] ipc: topology: add definition of the extended data Initial the definition of struct sof_ipc_comp_new_ext, which will be used to align the extended data between FW and the SW, add UUID array to it. Signed-off-by: Keyon Jie Signed-off-by: Marc Herbert --- src/include/ipc/topology.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/include/ipc/topology.h b/src/include/ipc/topology.h index 58c9074ab0ab..19f09f3c2210 100644 --- a/src/include/ipc/topology.h +++ b/src/include/ipc/topology.h @@ -94,6 +94,9 @@ struct sof_ipc_comp { */ #define SOF_BUF_UNDERRUN_PERMITTED BIT(1) +/* the UUID size in bytes, shared between FW and host */ +#define SOF_UUID_SIZE 16 + /* create new component buffer - SOF_IPC_TPLG_BUFFER_NEW */ struct sof_ipc_buffer { struct sof_ipc_comp comp; @@ -287,4 +290,9 @@ struct sof_ipc_pipe_comp_connect { uint32_t sink_id; } __attribute__((packed)); +/* extended data struct for UUID components */ +struct sof_ipc_comp_ext { + uint8_t uuid[SOF_UUID_SIZE]; +} __attribute__((packed)); + #endif /* __IPC_TOPOLOGY_H__ */ From 192e8e81f29577a39c5fe3294b3ffd34023e780c Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Wed, 17 Jun 2020 11:42:32 +0800 Subject: [PATCH 3/6] ipc: topology: use the sof_ipc_comp reserved bytes for extended data Use the 32bit reserved member of the struct sof_ipc_comp as the extended data length, this will help to minimize the ABI change for adding new extended data to the struct sof_ipc_comp, usually only minor ABI version bump needed for every update with this new solution. Signed-off-by: Keyon Jie --- src/include/ipc/topology.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/ipc/topology.h b/src/include/ipc/topology.h index 19f09f3c2210..b98e596b4bdd 100644 --- a/src/include/ipc/topology.h +++ b/src/include/ipc/topology.h @@ -65,8 +65,8 @@ struct sof_ipc_comp { uint32_t pipeline_id; uint32_t core; - /* reserved for future use */ - uint32_t reserved[1]; + /* extended data length, 0 if no extended data */ + uint32_t ext_data_length; } __attribute__((packed)); /* From 2f34d771a8226190bb9c8c5bdf7a9e045f6c4c8e Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Fri, 29 May 2020 14:15:43 +0800 Subject: [PATCH 4/6] lib: add memcmp for memory comparing Add function memcmp for comparing two length-limited memory. Signed-off-by: Keyon Jie --- src/include/sof/string.h | 1 + src/lib/lib.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/include/sof/string.h b/src/include/sof/string.h index 787f73ffef9a..d76e5b45d298 100644 --- a/src/include/sof/string.h +++ b/src/include/sof/string.h @@ -13,6 +13,7 @@ /* C memcpy for arch that don't have arch_memcpy() */ void cmemcpy(void *dest, void *src, size_t size); +int memcmp(const void *p, const void *q, size_t count); int rstrlen(const char *s); int rstrcmp(const char *s1, const char *s2); diff --git a/src/lib/lib.c b/src/lib/lib.c index c75aab889d58..c1ef3fa46222 100644 --- a/src/lib/lib.c +++ b/src/lib/lib.c @@ -43,6 +43,21 @@ int memset_s(void *dest, size_t dest_size, int data, size_t count) return arch_memset_s(dest, dest_size, data, count); } +int memcmp(const void *p, const void *q, size_t count) +{ + uint8_t *s1 = (uint8_t *)p; + uint8_t *s2 = (uint8_t *)q; + + while (count) { + if (*s1 != *s2) + return *s1 < *s2 ? -1 : 1; + s1++; + s2++; + count--; + } + return 0; +} + /* generic strlen - TODO: can be optimsed for ARCH ? */ int rstrlen(const char *s) { From 11a4a22042c6d83fa76e3e346da6129f622798fd Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Sun, 28 Jun 2020 16:59:23 +0800 Subject: [PATCH 5/6] component: switch to use UUID for component creation Switch to use UUID for component creation, if it is provided from the host, otherwise, use component type for the component driver matching. Signed-off-by: Keyon Jie Signed-off-by: Marc Herbert --- src/audio/component.c | 56 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/src/audio/component.c b/src/audio/component.c index ea72ed6fc198..d28a689ce534 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -29,20 +29,70 @@ DECLARE_SOF_UUID("component", comp_uuid, 0x7c42ce8b, 0x0108, 0x43d0, DECLARE_TR_CTX(comp_tr, SOF_UUID(comp_uuid), LOG_LEVEL_INFO); -static const struct comp_driver *get_drv(uint32_t type) +static const struct comp_driver *get_drv(struct sof_ipc_comp *comp) { struct comp_driver_list *drivers = comp_drivers_get(); struct list_item *clist; const struct comp_driver *drv = NULL; struct comp_driver_info *info; + struct sof_ipc_comp_ext *comp_ext; uint32_t flags; irq_local_disable(flags); + /* do we have extended data ? */ + if (!comp->ext_data_length) + goto comp_type_match; + + /* Basic sanity check of the total size and extended data + * length. A bit lax because in this generic code we don't know + * which derived comp we have and how much its specific members + * add. + */ + if (comp->hdr.size < sizeof(*comp) + comp->ext_data_length) { + tr_err(&comp_tr, "Invalid size, hdr.size=0x%x, ext_data_length=0x%x\n", + comp->hdr.size, comp->ext_data_length); + goto out; + } + + comp_ext = (struct sof_ipc_comp_ext *) + ((uint8_t *)comp + comp->hdr.size - + comp->ext_data_length); + + /* UUID is first item in extended data - check its big enough */ + if (comp->ext_data_length < UUID_SIZE) { + tr_err(&comp_tr, "UUID is invalid!\n"); + goto out; + } + + /* search driver list with UUID */ + list_for_item(clist, &drivers->list) { + info = container_of(clist, struct comp_driver_info, + list); + if (!memcmp(info->drv->uid, comp_ext->uuid, + UUID_SIZE)) { + tr_dbg(&comp_tr, + "get_drv_from_uuid(), found driver type %d, uuid %s", + info->drv->type, + info->drv->tctx->uuid_p); + drv = info->drv; + goto out; + } + } + + tr_err(&comp_tr, "get_drv(): the provided UUID (%8x%8x%8x%8x) doesn't match to any driver!", + *(uint32_t *)(&comp_ext->uuid[0]), + *(uint32_t *)(&comp_ext->uuid[4]), + *(uint32_t *)(&comp_ext->uuid[8]), + *(uint32_t *)(&comp_ext->uuid[12])); + + goto out; + +comp_type_match: /* search driver list for driver type */ list_for_item(clist, &drivers->list) { info = container_of(clist, struct comp_driver_info, list); - if (info->drv->type == type) { + if (info->drv->type == comp->type) { drv = info->drv; platform_shared_commit(info, sizeof(*info)); goto out; @@ -63,7 +113,7 @@ struct comp_dev *comp_new(struct sof_ipc_comp *comp) const struct comp_driver *drv; /* find the driver for our new component */ - drv = get_drv(comp->type); + drv = get_drv(comp); if (!drv) { tr_err(&comp_tr, "comp_new(): driver not found, comp->type = %u", comp->type); From fa32944f8e37704022cca7eb40e5b782835723c9 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 14 Jul 2020 23:07:47 +0800 Subject: [PATCH 6/6] ipc: update documentation to include extended data Update the IPC protocol documentation to include the extended data. Signed-off-by: Marc Herbert Signed-off-by: Keyon Jie --- src/include/ipc/header.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/include/ipc/header.h b/src/include/ipc/header.h index 63a291b1e673..2c01f22490cc 100644 --- a/src/include/ipc/header.h +++ b/src/include/ipc/header.h @@ -74,11 +74,11 @@ * 0 1 2 3 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | Size | + * | Size (Total size including Extended Data) | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * | Command (G=TPLG_MSG, C=COMP_NEW) | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | Component Id | + * | Component ID | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * | Type | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ @@ -86,9 +86,12 @@ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * | Core | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | Reserved | + * | Extended Data size | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | Type specific payload, size varies based on Component ID. | + * | Ends at: Total size - Extended Data size | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | (Type specific payload, size varies based on Component ID) | + * | extended data, including e.g. UUID | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * * \see sof_ipc_comp