From 303f290e760f23c7cf74825f7ddfad7cd45860ee Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Tue, 4 Jul 2023 11:34:14 +0200 Subject: [PATCH 1/3] bind/unbind support for cross core connection bind/unbind handler must be called from core on which component was created. When both source and sink components are created on same core, bind/unbind IPC is processed on that core. However, when connected source and sink are on different cores, IPC is processed on core 0. Hence comp_bind_remote() and comp_unbide_remote() is added to call bind/unbide from proper core. Signed-off-by: Serhiy Katsyuba --- src/idc/idc.c | 42 +++++++++++++++++++++++++++ src/include/sof/audio/component_ext.h | 38 ++++++++++++++++++++++++ xtos/include/rtos/idc.h | 3 ++ zephyr/include/rtos/idc.h | 3 ++ 4 files changed, 86 insertions(+) diff --git a/src/idc/idc.c b/src/idc/idc.c index 3f5ef1f57f02..b14bcb21151b 100644 --- a/src/idc/idc.c +++ b/src/idc/idc.c @@ -126,6 +126,40 @@ static void idc_ipc(void) ipc_cmd(ipc->comp_data); } +#if CONFIG_IPC_MAJOR_4 +static int idc_ipc4_bind(uint32_t comp_id) +{ + struct ipc_comp_dev *ipc_dev; + struct idc_payload *payload; + struct ipc4_module_bind_unbind *bu; + + ipc_dev = ipc_get_comp_by_id(ipc_get(), comp_id); + if (!ipc_dev) + return -ENODEV; + + payload = idc_payload_get(*idc_get(), cpu_get_id()); + bu = (struct ipc4_module_bind_unbind *)payload; + + return comp_bind(ipc_dev->cd, bu); +} + +static int idc_ipc4_unbind(uint32_t comp_id) +{ + struct ipc_comp_dev *ipc_dev; + struct idc_payload *payload; + struct ipc4_module_bind_unbind *bu; + + ipc_dev = ipc_get_comp_by_id(ipc_get(), comp_id); + if (!ipc_dev) + return -ENODEV; + + payload = idc_payload_get(*idc_get(), cpu_get_id()); + bu = (struct ipc4_module_bind_unbind *)payload; + + return comp_unbind(ipc_dev->cd, bu); +} +#endif + /** * \brief Executes IDC component params message. * \param[in] comp_id Component id to have params set. @@ -327,6 +361,14 @@ void idc_cmd(struct idc_msg *msg) case iTS(IDC_MSG_IPC): idc_ipc(); break; +#if CONFIG_IPC_MAJOR_4 + case iTS(IDC_MSG_BIND): + ret = idc_ipc4_bind(msg->extension); + break; + case iTS(IDC_MSG_UNBIND): + ret = idc_ipc4_unbind(msg->extension); + break; +#endif case iTS(IDC_MSG_PARAMS): ret = idc_params(msg->extension); break; diff --git a/src/include/sof/audio/component_ext.h b/src/include/sof/audio/component_ext.h index efa8e1b7767b..e79b39008f9e 100644 --- a/src/include/sof/audio/component_ext.h +++ b/src/include/sof/audio/component_ext.h @@ -386,24 +386,62 @@ static inline struct comp_driver_list *comp_drivers_get(void) return sof_get()->comp_drivers; } +#if CONFIG_IPC_MAJOR_4 +static inline int comp_ipc4_bind_remote(struct comp_dev *dev, void *data) +{ + struct idc_msg msg = { IDC_MSG_BIND, + IDC_EXTENSION(dev->ipc_config.id), dev->ipc_config.core, + sizeof(struct ipc4_module_bind_unbind), data}; + + return idc_send_msg(&msg, IDC_BLOCKING); +} +#endif + static inline int comp_bind(struct comp_dev *dev, void *data) { +#if CONFIG_IPC_MAJOR_4 + if (dev->drv->ops.bind) + return cpu_is_me(dev->ipc_config.core) ? + dev->drv->ops.bind(dev, data) : comp_ipc4_bind_remote(dev, data); + + return 0; +#else int ret = 0; if (dev->drv->ops.bind) ret = dev->drv->ops.bind(dev, data); return ret; +#endif } +#if CONFIG_IPC_MAJOR_4 +static inline int comp_ipc4_unbind_remote(struct comp_dev *dev, void *data) +{ + struct idc_msg msg = { IDC_MSG_UNBIND, + IDC_EXTENSION(dev->ipc_config.id), dev->ipc_config.core, + sizeof(struct ipc4_module_bind_unbind), data}; + + return idc_send_msg(&msg, IDC_BLOCKING); +} +#endif + static inline int comp_unbind(struct comp_dev *dev, void *data) { +#if CONFIG_IPC_MAJOR_4 + if (dev->drv->ops.unbind) + return cpu_is_me(dev->ipc_config.core) ? + dev->drv->ops.unbind(dev, data) : comp_ipc4_unbind_remote(dev, data); + + return 0; +#else int ret = 0; if (dev->drv->ops.unbind) ret = dev->drv->ops.unbind(dev, data); return ret; +#endif } static inline uint64_t comp_get_total_data_processed(struct comp_dev *dev, uint32_t stream_no, diff --git a/xtos/include/rtos/idc.h b/xtos/include/rtos/idc.h index 4b3ece377d53..dd684c6b97da 100644 --- a/xtos/include/rtos/idc.h +++ b/xtos/include/rtos/idc.h @@ -101,6 +101,9 @@ #define IDC_MSG_AMS IDC_TYPE(0xB) #define IDC_MSG_AMS_EXT IDC_EXTENSION(0x0) +#define IDC_MSG_BIND IDC_TYPE(0xD) +#define IDC_MSG_UNBIND IDC_TYPE(0xE) + #define IDC_HEADER_TO_AMS_SLOT_MASK(x) (x & 0xFFFF) /** \brief IDC_MSG_SECONDARY_CORE_CRASHED header fields. */ diff --git a/zephyr/include/rtos/idc.h b/zephyr/include/rtos/idc.h index e5e8f1f8a072..2f03838c370c 100644 --- a/zephyr/include/rtos/idc.h +++ b/zephyr/include/rtos/idc.h @@ -102,6 +102,9 @@ #define IDC_HEADER_TO_AMS_SLOT_MASK(x) (x & 0xFFFF) +#define IDC_MSG_BIND IDC_TYPE(0xD) +#define IDC_MSG_UNBIND IDC_TYPE(0xE) + /** \brief IDC_MSG_SECONDARY_CORE_CRASHED header fields. */ #define IDC_SCC_CORE_SHIFT 0 #define IDC_SCC_CORE_MASK 0xff From 7d7c8ec281108191f648ba333900f3ccef96aa18 Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Fri, 14 Jul 2023 17:52:19 +0200 Subject: [PATCH 2/3] Add remote get_attribute for cross core connection To support pipelines connection between cores, ipc4_create_buffer() must be able to create buffer shared between source and sink components created on different cores. ipc4_create_buffer() requires source component obs size. When both source and sink components created on same core, ipc4_create_buffer() is executed on that core, otherwise it is executed of core 0 (IPC processing core). So for the case when source and sink components are created on different cores comp_get_attribute_remote() is used to receive source obs size. Signed-off-by: Serhiy Katsyuba --- src/idc/idc.c | 21 +++++++++++- src/include/sof/audio/component_ext.h | 46 +++++++++++++++++++++++++++ xtos/include/rtos/idc.h | 1 + zephyr/include/rtos/idc.h | 1 + 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/idc/idc.c b/src/idc/idc.c index b14bcb21151b..8b6d0b9337c1 100644 --- a/src/idc/idc.c +++ b/src/idc/idc.c @@ -158,7 +158,23 @@ static int idc_ipc4_unbind(uint32_t comp_id) return comp_unbind(ipc_dev->cd, bu); } -#endif + +static int idc_get_attribute(uint32_t comp_id) +{ + struct ipc_comp_dev *ipc_dev; + struct idc_payload *idc_payload; + struct get_attribute_remote_payload *get_attr_payload; + + ipc_dev = ipc_get_comp_by_id(ipc_get(), comp_id); + if (!ipc_dev) + return -ENODEV; + + idc_payload = idc_payload_get(*idc_get(), cpu_get_id()); + get_attr_payload = (struct get_attribute_remote_payload *)idc_payload; + + return comp_get_attribute(ipc_dev->cd, get_attr_payload->type, get_attr_payload->value); +} +#endif /* CONFIG_IPC_MAJOR_4 */ /** * \brief Executes IDC component params message. @@ -368,6 +384,9 @@ void idc_cmd(struct idc_msg *msg) case iTS(IDC_MSG_UNBIND): ret = idc_ipc4_unbind(msg->extension); break; + case iTS(IDC_MSG_GET_ATTRIBUTE): + ret = idc_get_attribute(msg->extension); + break; #endif case iTS(IDC_MSG_PARAMS): ret = idc_params(msg->extension); diff --git a/src/include/sof/audio/component_ext.h b/src/include/sof/audio/component_ext.h index e79b39008f9e..4aa65bf61fcd 100644 --- a/src/include/sof/audio/component_ext.h +++ b/src/include/sof/audio/component_ext.h @@ -208,15 +208,61 @@ static inline int comp_prepare(struct comp_dev *dev) int comp_copy(struct comp_dev *dev); +#if CONFIG_IPC_MAJOR_4 +struct get_attribute_remote_payload { + uint32_t type; + void *value; +}; + +static inline int comp_ipc4_get_attribute_remote(struct comp_dev *dev, uint32_t type, + void *value) +{ + struct ipc4_base_module_cfg *base_cfg; + struct get_attribute_remote_payload payload = {}; + struct idc_msg msg = { IDC_MSG_GET_ATTRIBUTE, + IDC_EXTENSION(dev->ipc_config.id), dev->ipc_config.core, + sizeof(payload), &payload}; + int ret; + + /* Only COMP_ATTR_BASE_CONFIG is supported for remote access */ + if (type != COMP_ATTR_BASE_CONFIG) + return -EINVAL; + + base_cfg = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*base_cfg)); + if (!base_cfg) + return -ENOMEM; + + payload.type = type; + payload.value = base_cfg; + + ret = idc_send_msg(&msg, IDC_BLOCKING); + + if (ret == 0) + memcpy_s(value, sizeof(struct ipc4_base_module_cfg), + base_cfg, sizeof(struct ipc4_base_module_cfg)); + + rfree(base_cfg); + return ret; +} +#endif /* CONFIG_IPC_MAJOR_4 */ /** See comp_ops::get_attribute */ static inline int comp_get_attribute(struct comp_dev *dev, uint32_t type, void *value) { +#if CONFIG_IPC_MAJOR_4 + if (dev->drv->ops.get_attribute) + return cpu_is_me(dev->ipc_config.core) ? + dev->drv->ops.get_attribute(dev, type, value) : + comp_ipc4_get_attribute_remote(dev, type, value); + + return 0; +#else if (dev->drv->ops.get_attribute) return dev->drv->ops.get_attribute(dev, type, value); return 0; +#endif } /** See comp_ops::set_attribute */ diff --git a/xtos/include/rtos/idc.h b/xtos/include/rtos/idc.h index dd684c6b97da..bb8be2842e71 100644 --- a/xtos/include/rtos/idc.h +++ b/xtos/include/rtos/idc.h @@ -103,6 +103,7 @@ #define IDC_MSG_BIND IDC_TYPE(0xD) #define IDC_MSG_UNBIND IDC_TYPE(0xE) +#define IDC_MSG_GET_ATTRIBUTE IDC_TYPE(0xF) #define IDC_HEADER_TO_AMS_SLOT_MASK(x) (x & 0xFFFF) diff --git a/zephyr/include/rtos/idc.h b/zephyr/include/rtos/idc.h index 2f03838c370c..74c960efbc4e 100644 --- a/zephyr/include/rtos/idc.h +++ b/zephyr/include/rtos/idc.h @@ -104,6 +104,7 @@ #define IDC_MSG_BIND IDC_TYPE(0xD) #define IDC_MSG_UNBIND IDC_TYPE(0xE) +#define IDC_MSG_GET_ATTRIBUTE IDC_TYPE(0xF) /** \brief IDC_MSG_SECONDARY_CORE_CRASHED header fields. */ #define IDC_SCC_CORE_SHIFT 0 From 272880d1a037991d4c09e8d448455b355898eb6d Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Fri, 14 Jul 2023 18:13:12 +0200 Subject: [PATCH 3/3] Fix pipeline_get_dai_comp_latency() for cross core Pipeline data is allocated in cached space. This workaround prevents from accessing another core pipeline data when calculating latency. Signed-off-by: Serhiy Katsyuba --- src/audio/pipeline/pipeline-graph.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index a4360c1de618..71645c69fa84 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -558,6 +558,15 @@ struct comp_dev *pipeline_get_dai_comp_latency(uint32_t pipeline_id, uint32_t *l if (!source || !source->pipeline) return NULL; + /* As pipeline data is allocated in cached space, continue calculation for next + * connected pipeline only if that pipeline is on same core. + * This is a workaround, the real solution would be to use something like + * process_on_core() to continue calculation on required core. However, as this + * "latency feature" seems never used anyway, this workaround could be enough. + */ + if (!cpu_is_me(source->ipc_config.core)) + return NULL; + /* Get a next sink component */ ipc_sink = ipc_get_ppl_sink_comp(ipc, source->pipeline->pipeline_id); }