From 6b584dcce49ec464d6224e87a39f6ae1d9de261a Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 21 Feb 2025 16:35:41 +0100 Subject: [PATCH 1/7] init: move several functions to DRAM Move several initialisation functions to run from DRAM directly. Note, that we cannot use assert_can_be_cold() in these functions yet because the schedulers haven't been initialised at that point yet, but we're sufficiently confident, that these functions never run in LL-scheduler context. Signed-off-by: Guennadi Liakhovetski --- src/init/init.c | 6 +++--- src/platform/intel/ace/platform.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/init/init.c b/src/init/init.c index 9ff13c5d8b12..486ce49a2147 100644 --- a/src/init/init.c +++ b/src/init/init.c @@ -164,7 +164,7 @@ static int secondary_core_restore(void) #endif -int secondary_core_init(struct sof *sof) +__cold int secondary_core_init(struct sof *sof) { int err; struct ll_schedule_domain *dma_domain; @@ -237,7 +237,7 @@ int secondary_core_init(struct sof *sof) #endif -static void print_version_banner(void) +__cold static void print_version_banner(void) { /* * Non-Zephyr builds emit the version banner in DMA-trace @@ -267,7 +267,7 @@ static log_timestamp_t default_get_timestamp(void) } #endif -static int primary_core_init(int argc, char *argv[], struct sof *sof) +__cold static int primary_core_init(int argc, char *argv[], struct sof *sof) { /* setup context */ sof->argc = argc; diff --git a/src/platform/intel/ace/platform.c b/src/platform/intel/ace/platform.c index cc79e92ca9c9..f6dd464b6e6d 100644 --- a/src/platform/intel/ace/platform.c +++ b/src/platform/intel/ace/platform.c @@ -59,7 +59,7 @@ static const struct sof_ipc_fw_ready ready .flags = DEBUG_SET_FW_READY_FLAGS, }; -int platform_boot_complete(uint32_t boot_message) +__cold int platform_boot_complete(uint32_t boot_message) { struct ipc_cmd_hdr header; @@ -85,7 +85,7 @@ static struct pm_notifier pm_state_notifier = { #endif /* Runs on the primary core only */ -int platform_init(struct sof *sof) +__cold int platform_init(struct sof *sof) { int ret; From 1bad38b0565d6a85784384e6b13d760d38f15f04 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 21 Feb 2025 16:38:08 +0100 Subject: [PATCH 2/7] ipc: move most functions to run from DRAM Mark most IPC functions as "cold" to run them directly in DRAM. Explicitly avoid making exported functions "cold," also those that are either known to or can potentially be called from hot code paths. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/ipc/topology.h | 2 - src/ipc/ipc-common.c | 9 ++- src/ipc/ipc-helper.c | 20 +++++-- src/ipc/ipc4/handler.c | 104 ++++++++++++++++++++++++--------- src/ipc/ipc4/helper.c | 83 ++++++++++++++++++++------ 5 files changed, 165 insertions(+), 53 deletions(-) diff --git a/src/include/sof/ipc/topology.h b/src/include/sof/ipc/topology.h index 5605c6ef9837..3503c7a407d8 100644 --- a/src/include/sof/ipc/topology.h +++ b/src/include/sof/ipc/topology.h @@ -49,8 +49,6 @@ typedef uint32_t ipc_comp; struct ipc_comp_dev; const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id); struct comp_dev *ipc4_get_comp_dev(uint32_t comp_id); -int ipc4_add_comp_dev(struct comp_dev *dev); -const struct comp_driver *ipc4_get_drv(const void *uuid); int ipc4_chain_manager_create(struct ipc4_chain_dma *cdma); int ipc4_chain_dma_state(struct comp_dev *dev, struct ipc4_chain_dma *cdma); int ipc4_create_chain_dma(struct ipc *ipc, struct ipc4_chain_dma *cdma); diff --git a/src/ipc/ipc-common.c b/src/ipc/ipc-common.c index 8f72ac1ba91b..3c81064cb7cd 100644 --- a/src/ipc/ipc-common.c +++ b/src/ipc/ipc-common.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -188,12 +189,14 @@ static void schedule_ipc_worker(void) #endif } -void ipc_msg_send_direct(struct ipc_msg *msg, void *data) +__cold void ipc_msg_send_direct(struct ipc_msg *msg, void *data) { struct ipc *ipc = ipc_get(); k_spinlock_key_t key; int ret; + assert_can_be_cold(); + key = k_spin_lock(&ipc->lock); /* copy mailbox data to message if not already copied */ @@ -280,8 +283,10 @@ void ipc_schedule_process(struct ipc *ipc) #endif } -int ipc_init(struct sof *sof) +__cold int ipc_init(struct sof *sof) { + assert_can_be_cold(); + tr_dbg(&ipc_tr, "ipc_init()"); /* init ipc data */ diff --git a/src/ipc/ipc-helper.c b/src/ipc/ipc-helper.c index 1d1b651d1c49..65910767d95e 100644 --- a/src/ipc/ipc-helper.c +++ b/src/ipc/ipc-helper.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -37,8 +38,10 @@ LOG_MODULE_DECLARE(ipc, CONFIG_SOF_LOG_LEVEL); -static bool valid_ipc_buffer_desc(const struct sof_ipc_buffer *desc) +__cold static bool valid_ipc_buffer_desc(const struct sof_ipc_buffer *desc) { + assert_can_be_cold(); + if (desc->caps >= SOF_MEM_CAPS_LOWEST_INVALID) return false; @@ -47,10 +50,12 @@ static bool valid_ipc_buffer_desc(const struct sof_ipc_buffer *desc) } /* create a new component in the pipeline */ -struct comp_buffer *buffer_new(const struct sof_ipc_buffer *desc, bool is_shared) +__cold struct comp_buffer *buffer_new(const struct sof_ipc_buffer *desc, bool is_shared) { struct comp_buffer *buffer; + assert_can_be_cold(); + tr_info(&buffer_tr, "buffer new size 0x%x id %d.%d flags 0x%x", desc->size, desc->comp.pipeline_id, desc->comp.id, desc->flags); @@ -75,6 +80,7 @@ struct comp_buffer *buffer_new(const struct sof_ipc_buffer *desc, bool is_shared return buffer; } +/* Called from multiple locations, including ipc_get_comp_by_ppl_id(), so cannot be cold */ int32_t ipc_comp_pipe_id(const struct ipc_comp_dev *icd) { switch (icd->type) { @@ -177,9 +183,11 @@ int comp_verify_params(struct comp_dev *dev, uint32_t flag, } EXPORT_SYMBOL(comp_verify_params); -int comp_buffer_connect(struct comp_dev *comp, uint32_t comp_core, - struct comp_buffer *buffer, uint32_t dir) +__cold int comp_buffer_connect(struct comp_dev *comp, uint32_t comp_core, + struct comp_buffer *buffer, uint32_t dir) { + assert_can_be_cold(); + /* check if it's a connection between cores */ if (buffer->core != comp_core) { #if CONFIG_INCOHERENT @@ -258,13 +266,15 @@ int ipc_pipeline_complete(struct ipc *ipc, uint32_t comp_id) ipc_ppl_sink->cd); } -int ipc_comp_free(struct ipc *ipc, uint32_t comp_id) +__cold int ipc_comp_free(struct ipc *ipc, uint32_t comp_id) { struct ipc_comp_dev *icd; struct comp_buffer *buffer; struct comp_buffer *safe; uint32_t flags; + assert_can_be_cold(); + /* check whether component exists */ icd = ipc_get_comp_by_id(ipc, comp_id); if (!icd) { diff --git a/src/ipc/ipc4/handler.c b/src/ipc/ipc4/handler.c index 5ea024430ec7..7fb32a7112bd 100644 --- a/src/ipc/ipc4/handler.c +++ b/src/ipc/ipc4/handler.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -131,18 +132,22 @@ static inline const struct ipc4_pipeline_set_state_data *ipc4_get_pipeline_data( /* * Global IPC Operations. */ -static int ipc4_new_pipeline(struct ipc4_message_request *ipc4) +__cold static int ipc4_new_pipeline(struct ipc4_message_request *ipc4) { struct ipc *ipc = ipc_get(); + assert_can_be_cold(); + return ipc_pipeline_new(ipc, (ipc_pipe_new *)ipc4); } -static int ipc4_delete_pipeline(struct ipc4_message_request *ipc4) +__cold static int ipc4_delete_pipeline(struct ipc4_message_request *ipc4) { struct ipc4_pipeline_delete *pipe; struct ipc *ipc = ipc_get(); + assert_can_be_cold(); + pipe = (struct ipc4_pipeline_delete *)ipc4; tr_dbg(&ipc_tr, "ipc4 delete pipeline %x:", (uint32_t)pipe->primary.r.instance_id); @@ -229,11 +234,13 @@ static int ipc4_pcm_params(struct ipc_comp_dev *pcm_dev) return err; } -static bool is_any_ppl_active(void) +__cold static bool is_any_ppl_active(void) { struct ipc_comp_dev *icd; struct list_item *clist; + assert_can_be_cold(); + list_for_item(clist, &ipc_get()->comp_list) { icd = container_of(clist, struct ipc_comp_dev, list); if (icd->type != COMP_TYPE_PIPELINE) @@ -562,15 +569,14 @@ static int ipc_wait_for_compound_msg(void) } #endif -const struct ipc4_pipeline_set_state_data *ipc4_get_pipeline_data_wrapper(void) +__cold const struct ipc4_pipeline_set_state_data *ipc4_get_pipeline_data_wrapper(void) { - const struct ipc4_pipeline_set_state_data *ppl_data; - - ppl_data = ipc4_get_pipeline_data(); + assert_can_be_cold(); - return ppl_data; + return ipc4_get_pipeline_data(); } +/* Entry point for ipc4_pipeline_trigger(), therefore cannot be cold */ static int ipc4_set_pipeline_state(struct ipc4_message_request *ipc4) { const struct ipc4_pipeline_set_state_data *ppl_data; @@ -700,11 +706,13 @@ static int ipc4_set_pipeline_state(struct ipc4_message_request *ipc4) } #if CONFIG_LIBRARY_MANAGER -static int ipc4_load_library(struct ipc4_message_request *ipc4) +__cold static int ipc4_load_library(struct ipc4_message_request *ipc4) { struct ipc4_module_load_library library; int ret; + assert_can_be_cold(); + library.header.dat = ipc4->primary.dat; ret = lib_manager_load_library(library.header.r.dma_id, library.header.r.lib_id, @@ -716,8 +724,10 @@ static int ipc4_load_library(struct ipc4_message_request *ipc4) } #endif -static int ipc4_process_chain_dma(struct ipc4_message_request *ipc4) +__cold static int ipc4_process_chain_dma(struct ipc4_message_request *ipc4) { + assert_can_be_cold(); + #if CONFIG_COMP_CHAIN_DMA struct ipc_comp_dev *cdma_comp; struct ipc *ipc = ipc_get(); @@ -771,8 +781,10 @@ static int ipc4_process_chain_dma(struct ipc4_message_request *ipc4) #endif } -static int ipc4_process_ipcgtw_cmd(struct ipc4_message_request *ipc4) +__cold static int ipc4_process_ipcgtw_cmd(struct ipc4_message_request *ipc4) { + assert_can_be_cold(); + #if CONFIG_IPC4_GATEWAY struct ipc *ipc = ipc_get(); uint32_t reply_size = 0; @@ -880,14 +892,15 @@ __cold static int ipc4_init_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_init_instance module_init; struct comp_dev *dev; + + assert_can_be_cold(); + /* we only need the common header here, all we have from the IPC */ int ret = memcpy_s(&module_init, sizeof(module_init), ipc4, sizeof(*ipc4)); if (ret < 0) return IPC4_FAILURE; - assert_can_be_cold(); - tr_dbg(&ipc_tr, "ipc4_init_module_instance %x : %x", (uint32_t)module_init.primary.r.module_id, @@ -908,10 +921,13 @@ __cold static int ipc4_init_module_instance(struct ipc4_message_request *ipc4) return 0; } -static int ipc4_bind_module_instance(struct ipc4_message_request *ipc4) +__cold static int ipc4_bind_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_bind_unbind bu; struct ipc *ipc = ipc_get(); + + assert_can_be_cold(); + int ret = memcpy_s(&bu, sizeof(bu), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -924,10 +940,13 @@ static int ipc4_bind_module_instance(struct ipc4_message_request *ipc4) return ipc_comp_connect(ipc, (ipc_pipe_comp_connect *)&bu); } -static int ipc4_unbind_module_instance(struct ipc4_message_request *ipc4) +__cold static int ipc4_unbind_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_bind_unbind bu; struct ipc *ipc = ipc_get(); + + assert_can_be_cold(); + int ret = memcpy_s(&bu, sizeof(bu), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -940,7 +959,7 @@ static int ipc4_unbind_module_instance(struct ipc4_message_request *ipc4) return ipc_comp_disconnect(ipc, (ipc_pipe_comp_connect *)&bu); } -static int ipc4_get_vendor_config_module_instance(struct comp_dev *dev, +__cold static int ipc4_get_vendor_config_module_instance(struct comp_dev *dev, const struct comp_driver *drv, bool init_block, bool final_block, @@ -952,6 +971,8 @@ static int ipc4_get_vendor_config_module_instance(struct comp_dev *dev, int ret; struct ipc4_vendor_error *error; + assert_can_be_cold(); + if (init_block && final_block) { /* we use data_off_size as in/out, * save value to new variable so it can be used as out size @@ -1033,7 +1054,7 @@ static int ipc4_get_vendor_config_module_instance(struct comp_dev *dev, return IPC4_SUCCESS; } -static int ipc4_get_large_config_module_instance(struct ipc4_message_request *ipc4) +__cold static int ipc4_get_large_config_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_large_config_reply reply; struct ipc4_module_large_config config; @@ -1041,6 +1062,9 @@ static int ipc4_get_large_config_module_instance(struct ipc4_message_request *ip const struct comp_driver *drv; struct comp_dev *dev = NULL; uint32_t data_offset; + + assert_can_be_cold(); + int ret = memcpy_s(&config, sizeof(config), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -1125,7 +1149,7 @@ static int ipc4_get_large_config_module_instance(struct ipc4_message_request *ip return ret; } -static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, +__cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, const struct comp_driver *drv, uint32_t module_id, uint32_t instance_id, @@ -1136,6 +1160,8 @@ static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, { int ret; + assert_can_be_cold(); + /* Old FW comment: bursted configs */ if (init_block && final_block) { const struct sof_tlv *tlv = (struct sof_tlv *)data; @@ -1188,11 +1214,14 @@ static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, data_off_size, data); } -static int ipc4_set_large_config_module_instance(struct ipc4_message_request *ipc4) +__cold static int ipc4_set_large_config_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_large_config config; struct comp_dev *dev = NULL; const struct comp_driver *drv; + + assert_can_be_cold(); + int ret = memcpy_s(&config, sizeof(config), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -1256,11 +1285,14 @@ static int ipc4_set_large_config_module_instance(struct ipc4_message_request *ip return ret; } -static int ipc4_delete_module_instance(struct ipc4_message_request *ipc4) +__cold static int ipc4_delete_module_instance(struct ipc4_message_request *ipc4) { struct ipc4_module_delete_instance module; struct ipc *ipc = ipc_get(); uint32_t comp_id; + + assert_can_be_cold(); + int ret = memcpy_s(&module, sizeof(module), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -1282,10 +1314,13 @@ static int ipc4_delete_module_instance(struct ipc4_message_request *ipc4) } /* disable power gating on core 0 */ -static int ipc4_module_process_d0ix(struct ipc4_message_request *ipc4) +__cold static int ipc4_module_process_d0ix(struct ipc4_message_request *ipc4) { struct ipc4_module_set_d0ix d0ix; uint32_t module_id, instance_id; + + assert_can_be_cold(); + int ret = memcpy_s(&d0ix, sizeof(d0ix), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -1311,12 +1346,15 @@ static int ipc4_module_process_d0ix(struct ipc4_message_request *ipc4) } /* enable/disable cores according to the state mask */ -static int ipc4_module_process_dx(struct ipc4_message_request *ipc4) +__cold static int ipc4_module_process_dx(struct ipc4_message_request *ipc4) { struct ipc4_module_set_dx dx; struct ipc4_dx_state_info dx_info; uint32_t module_id, instance_id; uint32_t core_id; + + assert_can_be_cold(); + int ret = memcpy_s(&dx, sizeof(dx), ipc4, sizeof(*ipc4)); if (ret < 0) @@ -1402,11 +1440,13 @@ static int ipc4_module_process_dx(struct ipc4_message_request *ipc4) return IPC4_SUCCESS; } -static int ipc4_process_module_message(struct ipc4_message_request *ipc4) +__cold static int ipc4_process_module_message(struct ipc4_message_request *ipc4) { uint32_t type; int ret; + assert_can_be_cold(); + type = ipc4->primary.r.type; switch (type) { @@ -1451,9 +1491,12 @@ static int ipc4_process_module_message(struct ipc4_message_request *ipc4) return ret; } -struct ipc_cmd_hdr *mailbox_validate(void) +__cold struct ipc_cmd_hdr *mailbox_validate(void) { struct ipc_cmd_hdr *hdr = ipc_get()->comp_data; + + assert_can_be_cold(); + return hdr; } @@ -1479,18 +1522,22 @@ struct ipc_cmd_hdr *ipc_prepare_to_send(const struct ipc_msg *msg) return &msg_data.msg_out; } -void ipc_boot_complete_msg(struct ipc_cmd_hdr *header, uint32_t data) +__cold void ipc_boot_complete_msg(struct ipc_cmd_hdr *header, uint32_t data) { + assert_can_be_cold(); + header->pri = SOF_IPC4_FW_READY; header->ext = 0; } #if defined(CONFIG_PM_DEVICE) && defined(CONFIG_INTEL_ADSP_IPC) -void ipc_send_failed_power_transition_response(void) +__cold void ipc_send_failed_power_transition_response(void) { struct ipc4_message_request *request = ipc_from_hdr(&msg_data.msg_in); struct ipc4_message_reply response; + assert_can_be_cold(); + response.primary.r.status = IPC4_POWER_TRANSITION_FAILED; response.primary.r.rsp = SOF_IPC4_MESSAGE_DIR_MSG_REPLY; response.primary.r.msg_tgt = request->primary.r.msg_tgt; @@ -1503,8 +1550,10 @@ void ipc_send_failed_power_transition_response(void) } #endif /* defined(CONFIG_PM_DEVICE) && defined(CONFIG_INTEL_ADSP_IPC) */ -void ipc_send_panic_notification(void) +__cold void ipc_send_panic_notification(void) { + assert_can_be_cold(); + msg_notify.header = SOF_IPC4_NOTIF_HEADER(SOF_IPC4_EXCEPTION_CAUGHT); msg_notify.extension = cpu_get_id(); msg_notify.tx_size = 0; @@ -1530,6 +1579,7 @@ static bool is_notification_queued(struct ipc_msg *msg) return queued; } +/* Called from ipc_send_buffer_status_notify(), which is currently "hot" */ void ipc_send_buffer_status_notify(void) { /* a single msg_notify object is used */ diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 8605a9e37ecd..81b3c15861dc 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,9 @@ LOG_MODULE_DECLARE(ipc, CONFIG_SOF_LOG_LEVEL); extern struct tr_ctx comp_tr; +static const struct comp_driver *ipc4_get_drv(const void *uuid); +static int ipc4_add_comp_dev(struct comp_dev *dev); + void ipc_build_stream_posn(struct sof_ipc_stream_posn *posn, uint32_t type, uint32_t id) { @@ -96,12 +100,15 @@ static const struct comp_driver *ipc4_library_get_comp_drv(char *data) return ipc4_get_drv(data); } #else -static inline char *ipc4_get_comp_new_data(void) +__cold static inline char *ipc4_get_comp_new_data(void) { + assert_can_be_cold(); + return (char *)MAILBOX_HOSTBOX_BASE; } #endif +/* Only called from ipc4_init_module_instance(), which is __cold */ __cold struct comp_dev *comp_new_ipc4(struct ipc4_module_init_instance *module_init) { struct comp_ipc_config ipc_config; @@ -192,6 +199,7 @@ __cold struct comp_dev *comp_new_ipc4(struct ipc4_module_init_instance *module_i return dev; } +/* Called from ipc4_set_pipeline_state(), so cannot be cold */ struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, uint32_t ppl_id, uint32_t ignore_remote) @@ -220,12 +228,14 @@ struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, return NULL; } -static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) +__cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) { struct ipc_comp_dev *ipc_pipe; struct pipeline *pipe; struct ipc *ipc = ipc_get(); + assert_can_be_cold(); + /* check whether pipeline id is already taken or in use */ ipc_pipe = ipc_get_pipeline_by_id(ipc, pipe_desc->primary.r.instance_id); if (ipc_pipe) { @@ -269,10 +279,13 @@ static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) return IPC4_SUCCESS; } -int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) +/* Only called from ipc4_new_pipeline(), which is __cold */ +__cold int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) { struct ipc4_pipeline_create *pipe_desc = ipc_from_pipe_new(_pipe_desc); + assert_can_be_cold(); + tr_dbg(&ipc_tr, "ipc: pipeline id = %u", (uint32_t)pipe_desc->primary.r.instance_id); /* pass IPC to target core */ @@ -282,20 +295,24 @@ int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) return ipc4_create_pipeline(pipe_desc); } -static inline int ipc_comp_free_remote(struct comp_dev *dev) +__cold static inline int ipc_comp_free_remote(struct comp_dev *dev) { struct idc_msg msg = { IDC_MSG_FREE, IDC_MSG_FREE_EXT(dev->ipc_config.id), dev->ipc_config.core,}; + assert_can_be_cold(); + return idc_send_msg(&msg, IDC_BLOCKING); } -static int ipc_pipeline_module_free(uint32_t pipeline_id) +__cold static int ipc_pipeline_module_free(uint32_t pipeline_id) { struct ipc *ipc = ipc_get(); struct ipc_comp_dev *icd; int ret; + assert_can_be_cold(); + icd = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_COMPONENT, pipeline_id, IPC_COMP_ALL); while (icd) { struct comp_buffer *buffer; @@ -334,11 +351,14 @@ static int ipc_pipeline_module_free(uint32_t pipeline_id) return IPC4_SUCCESS; } -int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) +/* Only called from ipc4_delete_pipeline(), which is __cold */ +__cold int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) { struct ipc_comp_dev *ipc_pipe; int ret; + assert_can_be_cold(); + /* check whether pipeline exists */ ipc_pipe = ipc_get_pipeline_by_id(ipc, comp_id); if (!ipc_pipe) @@ -368,12 +388,14 @@ int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) return IPC4_SUCCESS; } -static struct comp_buffer *ipc4_create_buffer(struct comp_dev *src, bool is_shared, - uint32_t buf_size, uint32_t src_queue, - uint32_t dst_queue) +__cold static struct comp_buffer *ipc4_create_buffer(struct comp_dev *src, bool is_shared, + uint32_t buf_size, uint32_t src_queue, + uint32_t dst_queue) { struct sof_ipc_buffer ipc_buf; + assert_can_be_cold(); + memset(&ipc_buf, 0, sizeof(ipc_buf)); ipc_buf.size = buf_size; ipc_buf.comp.id = IPC4_COMP_ID(src_queue, dst_queue); @@ -450,7 +472,8 @@ static int ll_wait_finished_on_core(struct comp_dev *dev) #endif -int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) +/* Only called from ipc4_bind_module_instance(), which is __cold */ +__cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) { struct ipc4_module_bind_unbind *bu; struct comp_buffer *buffer; @@ -465,6 +488,8 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) int src_id, sink_id; int ret; + assert_can_be_cold(); + bu = (struct ipc4_module_bind_unbind *)_connect; src_id = IPC4_COMP_ID(bu->primary.r.module_id, bu->primary.r.instance_id); sink_id = IPC4_COMP_ID(bu->extension.r.dst_module_id, bu->extension.r.dst_instance_id); @@ -668,7 +693,8 @@ int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) * during run-time. The only way to change pipeline topology is to delete the whole * pipeline and create it in modified form. */ -int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) +/* Only called from ipc4_unbind_module_instance(), which is __cold */ +__cold int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) { struct ipc4_module_bind_unbind *bu; struct comp_buffer *buffer = NULL; @@ -679,6 +705,8 @@ int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) int ret, ret1; bool cross_core_unbind; + assert_can_be_cold(); + bu = (struct ipc4_module_bind_unbind *)_connect; src_id = IPC4_COMP_ID(bu->primary.r.module_id, bu->primary.r.instance_id); sink_id = IPC4_COMP_ID(bu->extension.r.dst_module_id, bu->extension.r.dst_instance_id); @@ -759,12 +787,15 @@ int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) } #if CONFIG_COMP_CHAIN_DMA -int ipc4_chain_manager_create(struct ipc4_chain_dma *cdma) +/* Only called from ipc4_process_chain_dma(), which is __cold */ +__cold int ipc4_chain_manager_create(struct ipc4_chain_dma *cdma) { const struct sof_uuid uuid = SOF_REG_UUID(chain_dma); const struct comp_driver *drv; struct comp_dev *dev; + assert_can_be_cold(); + drv = ipc4_get_drv(&uuid); if (!drv) return -EINVAL; @@ -783,7 +814,8 @@ int ipc4_chain_manager_create(struct ipc4_chain_dma *cdma) return ipc4_add_comp_dev(dev); } -int ipc4_chain_dma_state(struct comp_dev *dev, struct ipc4_chain_dma *cdma) +/* Only called from ipc4_process_chain_dma(), which is __cold */ +__cold int ipc4_chain_dma_state(struct comp_dev *dev, struct ipc4_chain_dma *cdma) { const bool allocate = cdma->primary.r.allocate; const bool enable = cdma->primary.r.enable; @@ -792,6 +824,8 @@ int ipc4_chain_dma_state(struct comp_dev *dev, struct ipc4_chain_dma *cdma) struct list_item *clist, *_tmp; int ret; + assert_can_be_cold(); + if (!dev) return -EINVAL; @@ -823,12 +857,14 @@ int ipc4_chain_dma_state(struct comp_dev *dev, struct ipc4_chain_dma *cdma) } #endif -static int ipc4_update_comps_direction(struct ipc *ipc, uint32_t ppl_id) +__cold static int ipc4_update_comps_direction(struct ipc *ipc, uint32_t ppl_id) { struct ipc_comp_dev *icd; struct list_item *clist; struct comp_buffer *src_buf; + assert_can_be_cold(); + list_for_item(clist, &ipc->comp_list) { icd = container_of(clist, struct ipc_comp_dev, list); if (icd->type != COMP_TYPE_COMPONENT) @@ -904,7 +940,7 @@ int ipc4_process_on_core(uint32_t core, bool blocking) return IPC4_SUCCESS; } -const struct comp_driver *ipc4_get_drv(const void *uuid) +__cold static const struct comp_driver *ipc4_get_drv(const void *uuid) { const struct sof_uuid *const sof_uuid = (const struct sof_uuid *)uuid; struct comp_driver_list *drivers = comp_drivers_get(); @@ -913,6 +949,8 @@ const struct comp_driver *ipc4_get_drv(const void *uuid) struct comp_driver_info *info; uint32_t flags; + assert_can_be_cold(); + irq_local_disable(flags); /* search driver list with UUID */ @@ -940,13 +978,22 @@ const struct comp_driver *ipc4_get_drv(const void *uuid) return drv; } -const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) +/* + * Called from + * - ipc4_get_large_config_module_instance() + * - ipc4_set_large_config_module_instance() + * - comp_new_ipc4() + * all of which are __cold + */ +__cold const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) { const struct sof_man_fw_desc *desc = NULL; const struct comp_driver *drv; const struct sof_man_module *mod; uint32_t entry_index; + assert_can_be_cold(); + #ifdef RIMAGE_MANIFEST desc = (const struct sof_man_fw_desc *)IMR_BOOT_LDR_MANIFEST_BASE; #else @@ -1012,11 +1059,13 @@ struct comp_dev *ipc4_get_comp_dev(uint32_t comp_id) } EXPORT_SYMBOL(ipc4_get_comp_dev); -int ipc4_add_comp_dev(struct comp_dev *dev) +__cold static int ipc4_add_comp_dev(struct comp_dev *dev) { struct ipc *ipc = ipc_get(); struct ipc_comp_dev *icd; + assert_can_be_cold(); + /* check id for duplicates */ icd = ipc_get_comp_by_id(ipc, dev->ipc_config.id); if (icd) { From b389b70bc24d2d8428c65975fc43d69c224cfb7d Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 4 Mar 2025 12:08:36 +0100 Subject: [PATCH 3/7] ipc: fix a return code ipc4_pipeline_complete() returns POSIX error codes, not IPC4 ones. Signed-off-by: Guennadi Liakhovetski --- src/ipc/ipc4/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 81b3c15861dc..f727d212f179 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -898,7 +898,7 @@ int ipc4_pipeline_complete(struct ipc *ipc, uint32_t comp_id, uint32_t cmd) ipc_pipe = ipc_get_pipeline_by_id(ipc, comp_id); if (!ipc_pipe) - return -IPC4_INVALID_RESOURCE_ID; + return -EINVAL; /* Pass IPC to target core */ if (!cpu_is_me(ipc_pipe->core)) From 7ac605f208ea3516835a8e5bd8c8f844642b9401 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 4 Mar 2025 12:24:19 +0100 Subject: [PATCH 4/7] audio: module-interface: better explain .trigger method struct module_interface::trigger() is called from both hot and cold contexts, therefore it usually shouldn't be marked as __cold. Signed-off-by: Guennadi Liakhovetski --- src/include/module/module/interface.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/include/module/module/interface.h b/src/include/module/module/interface.h index 149f5659d2fc..4145bfc3d1b0 100644 --- a/src/include/module/module/interface.h +++ b/src/include/module/module/interface.h @@ -239,7 +239,9 @@ struct module_interface { /** * (optional) Module specific trigger procedure, called when modules are triggered. Usually - * can be __cold + * shouldn't be __cold. If a module implements this method, even if it only handles + * commands, running in non-LL context, it will still be called from the high priority + * LL context, which will cause a short jump to DRAM to check for supported commands. */ int (*trigger)(struct processing_module *mod, int cmd); From c1bf8abda8b0fc96c676ee7e1984cc6ec2fac604 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 7 Mar 2025 12:46:31 +0100 Subject: [PATCH 5/7] dram: improve debugging Add the function name, that was called in DRAM from LL context to simplify debugging. Signed-off-by: Guennadi Liakhovetski --- zephyr/include/sof/lib/memory.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zephyr/include/sof/lib/memory.h b/zephyr/include/sof/lib/memory.h index b7d709b5276b..bd0ebc6e61b8 100644 --- a/zephyr/include/sof/lib/memory.h +++ b/zephyr/include/sof/lib/memory.h @@ -25,10 +25,11 @@ bool ll_sch_is_current(void); #define ll_sch_is_current() false #endif -static inline void assert_can_be_cold(void) +static inline void __assert_can_be_cold(const char *fn) { - assert(!ll_sch_is_current()); + __ASSERT(!ll_sch_is_current(), "%s() called from an LL thread!", fn); } +#define assert_can_be_cold() __assert_can_be_cold(__func__) #else #define assert_can_be_cold() do {} while (0) #endif From a0ce9776b289ac741efd29db82f49a8e68bdc920 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 17 Mar 2025 15:32:06 +0100 Subject: [PATCH 6/7] dram: .reset() shouldn't be "cold" struct module_interface::reset() is called from the trigger IPC context, therefore it shouldn't be "cold." Signed-off-by: Guennadi Liakhovetski --- src/audio/multiband_drc/multiband_drc.c | 4 +--- src/audio/src/src_common.c | 4 +--- src/include/module/module/interface.h | 3 ++- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/audio/multiband_drc/multiband_drc.c b/src/audio/multiband_drc/multiband_drc.c index 3bb41ad775fb..550b19495816 100644 --- a/src/audio/multiband_drc/multiband_drc.c +++ b/src/audio/multiband_drc/multiband_drc.c @@ -407,12 +407,10 @@ __cold static int multiband_drc_prepare(struct processing_module *mod, return ret; } -__cold static int multiband_drc_reset(struct processing_module *mod) +static int multiband_drc_reset(struct processing_module *mod) { struct multiband_drc_comp_data *cd = module_get_private_data(mod); - assert_can_be_cold(); - comp_info(mod->dev, "multiband_drc_reset()"); multiband_drc_reset_state(&cd->state); diff --git a/src/audio/src/src_common.c b/src/audio/src/src_common.c index 9605a3b8bb87..1cf33749cbb4 100644 --- a/src/audio/src/src_common.c +++ b/src/audio/src/src_common.c @@ -697,12 +697,10 @@ __cold int src_get_config(struct processing_module *mod, uint32_t config_id, return -EINVAL; } -__cold int src_reset(struct processing_module *mod) +int src_reset(struct processing_module *mod) { struct comp_data *cd = module_get_private_data(mod); - assert_can_be_cold(); - comp_info(mod->dev, "src_reset()"); cd->src_func = src_fallback; diff --git a/src/include/module/module/interface.h b/src/include/module/module/interface.h index 4145bfc3d1b0..f80271400a45 100644 --- a/src/include/module/module/interface.h +++ b/src/include/module/module/interface.h @@ -214,7 +214,8 @@ struct module_interface { /** * (optional) Module specific reset procedure, called as part of module_adapter component * reset in .reset(). This should reset all parameters to their initial stage - * and free all memory allocated during prepare(). Usually can be __cold + * and free all memory allocated during prepare(). Usually shouldn't be __cold since it's + * called from pipeline_reset() from ipc4_pipeline_trigger() */ int (*reset)(struct processing_module *mod); From 28b93ee9aafb644bf6a7b3bb8a9bad21a36de5cb Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 18 Mar 2025 10:32:11 +0100 Subject: [PATCH 7/7] dram: audio: move trigger processing back to SRAM .prepare() is called as a part of the trigger processing flow, it shouldn't be "cold." Signed-off-by: Guennadi Liakhovetski --- src/audio/multiband_drc/multiband_drc.c | 8 ++--- src/audio/multiband_drc/multiband_drc_ipc4.c | 2 +- src/audio/src/src.c | 8 ++--- src/audio/src/src_common.c | 34 +++++++------------- src/audio/src/src_ipc4.c | 18 ++++------- 5 files changed, 24 insertions(+), 46 deletions(-) diff --git a/src/audio/multiband_drc/multiband_drc.c b/src/audio/multiband_drc/multiband_drc.c index 550b19495816..311894639962 100644 --- a/src/audio/multiband_drc/multiband_drc.c +++ b/src/audio/multiband_drc/multiband_drc.c @@ -359,9 +359,9 @@ static int multiband_drc_process(struct processing_module *mod, return 0; } -__cold static int multiband_drc_prepare(struct processing_module *mod, - struct sof_source **sources, int num_of_sources, - struct sof_sink **sinks, int num_of_sinks) +static int multiband_drc_prepare(struct processing_module *mod, + struct sof_source **sources, int num_of_sources, + struct sof_sink **sinks, int num_of_sinks) { struct multiband_drc_comp_data *cd = module_get_private_data(mod); struct comp_dev *dev = mod->dev; @@ -370,8 +370,6 @@ __cold static int multiband_drc_prepare(struct processing_module *mod, int rate; int ret = 0; - assert_can_be_cold(); - comp_info(dev, "multiband_drc_prepare()"); ret = multiband_drc_params(mod); diff --git a/src/audio/multiband_drc/multiband_drc_ipc4.c b/src/audio/multiband_drc/multiband_drc_ipc4.c index e0b42309d7f5..1cb6ab7aea3c 100644 --- a/src/audio/multiband_drc/multiband_drc_ipc4.c +++ b/src/audio/multiband_drc/multiband_drc_ipc4.c @@ -73,7 +73,7 @@ __cold int multiband_drc_get_ipc_config(struct processing_module *mod, return comp_data_blob_get_cmd(cd->model_handler, cdata, fragment_size); } -__cold int multiband_drc_params(struct processing_module *mod) +int multiband_drc_params(struct processing_module *mod) { struct sof_ipc_stream_params *params = mod->stream_params; struct sof_ipc_stream_params comp_params; diff --git a/src/audio/src/src.c b/src/audio/src/src.c index 9eeb93507033..4a46c2bce0bb 100644 --- a/src/audio/src/src.c +++ b/src/audio/src/src.c @@ -34,16 +34,14 @@ LOG_MODULE_DECLARE(src, CONFIG_SOF_LOG_LEVEL); -__cold static int src_prepare(struct processing_module *mod, - struct sof_source **sources, int num_of_sources, - struct sof_sink **sinks, int num_of_sinks) +static int src_prepare(struct processing_module *mod, + struct sof_source **sources, int num_of_sources, + struct sof_sink **sinks, int num_of_sinks) { struct comp_data *cd = module_get_private_data(mod); struct src_param *a = &cd->param; int ret; - assert_can_be_cold(); - comp_info(mod->dev, "src_prepare()"); if (num_of_sources != 1 || num_of_sinks != 1) diff --git a/src/audio/src/src_common.c b/src/audio/src/src_common.c index 1cf33749cbb4..625077a57e4a 100644 --- a/src/audio/src/src_common.c +++ b/src/audio/src/src_common.c @@ -45,7 +45,7 @@ LOG_MODULE_REGISTER(src, CONFIG_SOF_LOG_LEVEL); /* Calculates buffers to allocate for a SRC mode */ -__cold static int src_buffer_lengths(struct comp_dev *dev, struct comp_data *cd, int nch) +static int src_buffer_lengths(struct comp_dev *dev, struct comp_data *cd, int nch) { const struct src_stage *stage1; const struct src_stage *stage2; @@ -54,8 +54,6 @@ __cold static int src_buffer_lengths(struct comp_dev *dev, struct comp_data *cd, int source_frames; int r1, n; - assert_can_be_cold(); - a = &cd->param; fs_in = cd->source_rate; fs_out = cd->sink_rate; @@ -186,16 +184,14 @@ static int init_stages(const struct src_stage *stage1, const struct src_stage *s return 0; } -__cold static int src_polyphase_init(struct polyphase_src *src, struct src_param *p, - int32_t *delay_lines_start) +static int src_polyphase_init(struct polyphase_src *src, struct src_param *p, + int32_t *delay_lines_start) { const struct src_stage *stage1; const struct src_stage *stage2; int n_stages; int ret; - assert_can_be_cold(); - if (p->idx_in < 0 || p->idx_out < 0) return -EINVAL; @@ -397,15 +393,13 @@ void src_set_alignment(struct sof_source *source, struct sof_sink *sink) sink_set_alignment_constants(sink, byte_align, frame_align_req); } -__cold static int src_verify_params(struct processing_module *mod) +static int src_verify_params(struct processing_module *mod) { struct sof_ipc_stream_params *params = mod->stream_params; struct comp_data *cd = module_get_private_data(mod); struct comp_dev *dev = mod->dev; int ret; - assert_can_be_cold(); - comp_dbg(dev, "src_verify_params()"); /* check whether params->rate (received from driver) are equal @@ -480,9 +474,9 @@ static bool src_get_copy_limits(struct comp_data *cd, return true; } -__cold int src_params_general(struct processing_module *mod, - struct sof_source *source, - struct sof_sink *sink) +int src_params_general(struct processing_module *mod, + struct sof_source *source, + struct sof_sink *sink) { struct comp_data *cd = module_get_private_data(mod); struct comp_dev *dev = mod->dev; @@ -491,8 +485,6 @@ __cold int src_params_general(struct processing_module *mod, int n; int err; - assert_can_be_cold(); - comp_info(dev, "src_params()"); err = src_set_params(mod, sink); @@ -583,14 +575,12 @@ __cold int src_params_general(struct processing_module *mod, return 0; } -__cold int src_param_set(struct comp_dev *dev, struct comp_data *cd) +int src_param_set(struct comp_dev *dev, struct comp_data *cd) { struct src_param *a = &cd->param; int fs_in = cd->source_rate; int fs_out = cd->sink_rate; - assert_can_be_cold(); - a->idx_in = src_find_fs(a->in_fs, a->num_in_fs, fs_in); a->idx_out = src_find_fs(a->out_fs, a->num_out_fs, fs_out); @@ -604,9 +594,9 @@ __cold int src_param_set(struct comp_dev *dev, struct comp_data *cd) return 0; } -__cold int src_allocate_copy_stages(struct comp_dev *dev, struct src_param *prm, - const struct src_stage *stage_src1, - const struct src_stage *stage_src2) +int src_allocate_copy_stages(struct comp_dev *dev, struct src_param *prm, + const struct src_stage *stage_src1, + const struct src_stage *stage_src2) { #if CONFIG_FAST_GET struct src_stage *stage_dst; @@ -617,8 +607,6 @@ __cold int src_allocate_copy_stages(struct comp_dev *dev, struct src_param *prm, size_t tap_size = sizeof(int32_t); #endif - assert_can_be_cold(); - stage_dst = rmalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, 2 * sizeof(*stage_dst)); if (!stage_dst) { diff --git a/src/audio/src/src_ipc4.c b/src/audio/src/src_ipc4.c index db32bc228838..5f50c17ec537 100644 --- a/src/audio/src/src_ipc4.c +++ b/src/audio/src/src_ipc4.c @@ -77,7 +77,7 @@ int src_stream_pcm_sink_rate_check(struct ipc4_config_src cfg, * set up param then verify param. BTW for IPC3 path, the param is sent by * host driver. */ -__cold int src_set_params(struct processing_module *mod, struct sof_sink *sink) +int src_set_params(struct processing_module *mod, struct sof_sink *sink) { struct sof_ipc_stream_params src_params; struct sof_ipc_stream_params *params = mod->stream_params; @@ -86,8 +86,6 @@ __cold int src_set_params(struct processing_module *mod, struct sof_sink *sink) struct comp_dev *dev = mod->dev; int ret; - assert_can_be_cold(); - src_params = *params; src_params.channels = mod->priv.cfg.base_cfg.audio_fmt.channels_count; src_params.buffer_fmt = mod->priv.cfg.base_cfg.audio_fmt.interleaving_style; @@ -119,15 +117,13 @@ __cold int src_set_params(struct processing_module *mod, struct sof_sink *sink) return ret; } -__cold void src_get_source_sink_params(struct comp_dev *dev, struct sof_source *source, - struct sof_sink *sink) +void src_get_source_sink_params(struct comp_dev *dev, struct sof_source *source, + struct sof_sink *sink) { struct processing_module *mod = comp_mod(dev); struct comp_data *cd = module_get_private_data(mod); enum sof_ipc_frame frame_fmt, valid_fmt; - assert_can_be_cold(); - /* convert IPC4 config to format used by the module */ audio_stream_fmt_conversion(cd->ipc_config.base.audio_fmt.depth, cd->ipc_config.base.audio_fmt.valid_bit_depth, @@ -140,16 +136,14 @@ __cold void src_get_source_sink_params(struct comp_dev *dev, struct sof_source * sink_set_rate(sink, cd->ipc_config.sink_rate); } -__cold int src_prepare_general(struct processing_module *mod, - struct sof_source *source, - struct sof_sink *sink) +int src_prepare_general(struct processing_module *mod, + struct sof_source *source, + struct sof_sink *sink) { struct comp_data *cd = module_get_private_data(mod); struct comp_dev *dev = mod->dev; int ret = 0; - assert_can_be_cold(); - /* set align requirements */ src_set_alignment(source, sink);