From 06bc14341e37d136fe63bf96b4a5122712804467 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 19 Jul 2023 10:18:17 +0200 Subject: [PATCH 1/3] ipc4: helper: storing size of received configuration This patch temporary extends comp_ipc_config struct by the value of input configuration size. Each module receive its own configuration at creation (MODULE_INIT IPC). In case of a gateway those configuration can contain additional value (aux_conf). The only way to check if such config was received is to compare size of the data received via ipc and standard configuration size. This solution is temporary because ultimately it would be necessary to transfer to each module the size of the received configuration along with a pointer to the configuration itself. Just like the test is in the case of a module adapter. Signed-off-by: Tomasz Leman --- src/include/sof/audio/component.h | 3 +++ src/ipc/ipc4/helper.c | 1 + 2 files changed, 4 insertions(+) diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 746ae1333a24..c6f0ef4851cf 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -553,6 +553,9 @@ struct comp_ipc_config { uint32_t periods_source;/**< 0 means variable */ uint32_t frame_fmt; /**< SOF_IPC_FRAME_ */ uint32_t xrun_action; /**< action we should take on XRUN */ +#if CONFIG_IPC_MAJOR_4 + uint32_t ipc_config_size; /**< size of a config received by ipc */ +#endif }; /** diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 3d889a294235..182fb81a1c2f 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -103,6 +103,7 @@ struct comp_dev *comp_new_ipc4(struct ipc4_module_init_instance *module_init) ipc_config.id = comp_id; ipc_config.pipeline_id = module_init->extension.r.ppl_instance_id; ipc_config.core = module_init->extension.r.core_id; + ipc_config.ipc_config_size = module_init->extension.r.param_block_size * sizeof(uint32_t); #if CONFIG_ZEPHYR_DP_SCHEDULER if (module_init->extension.r.proc_domain) From 8272c4af73929b04dc126888987bfc0b59f6c409 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 19 Jul 2023 10:46:39 +0200 Subject: [PATCH 2/3] component: comment code style fix Comment style correction after adding a new field in the structure. Signed-off-by: Tomasz Leman --- src/include/sof/audio/component.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index c6f0ef4851cf..7db12168615a 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -544,15 +544,15 @@ struct comp_driver_info { * Audio component base configuration from IPC at creation. */ struct comp_ipc_config { - uint32_t core; /**< core we run on */ - uint32_t id; /**< component id */ - uint32_t pipeline_id; /**< component pipeline id */ - uint32_t proc_domain; /**< processing domain - LL or DP */ + uint32_t core; /**< core we run on */ + uint32_t id; /**< component id */ + uint32_t pipeline_id; /**< component pipeline id */ + uint32_t proc_domain; /**< processing domain - LL or DP */ enum sof_comp_type type; /**< component type */ - uint32_t periods_sink; /**< 0 means variable */ - uint32_t periods_source;/**< 0 means variable */ - uint32_t frame_fmt; /**< SOF_IPC_FRAME_ */ - uint32_t xrun_action; /**< action we should take on XRUN */ + uint32_t periods_sink; /**< 0 means variable */ + uint32_t periods_source; /**< 0 means variable */ + uint32_t frame_fmt; /**< SOF_IPC_FRAME_ */ + uint32_t xrun_action; /**< action we should take on XRUN */ #if CONFIG_IPC_MAJOR_4 uint32_t ipc_config_size; /**< size of a config received by ipc */ #endif From ecbd00a23a0268763294621ee3f302492d3da789 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 5 Jul 2023 20:49:02 +0200 Subject: [PATCH 3/3] copier: synchronized fpi updates of hd-a gateways This patch add synchronized FPI updates of HD-A gateways. Driver can define group of such gateways via module init IPC. The driver may also specify an update period for each group, different than the default one determined by the system tick frequency. Signed-off-by: Tomasz Leman --- src/audio/Kconfig | 10 +++ src/audio/copier/copier_host.c | 113 ++++++++++++++++++++++++++++ src/audio/host-zephyr.c | 34 ++++++++- src/include/ipc4/copier.h | 8 ++ src/include/sof/audio/host_copier.h | 6 ++ 5 files changed, 167 insertions(+), 4 deletions(-) diff --git a/src/audio/Kconfig b/src/audio/Kconfig index 3b00624188e1..bdc26973a3d1 100644 --- a/src/audio/Kconfig +++ b/src/audio/Kconfig @@ -32,6 +32,16 @@ config HOST_DMA_RELOAD_THRESHOLD The DMA buffer threshold in milliseconds to trigger host DMA reloading. +config HOST_DMA_STREAM_SYNCHRONIZATION + bool "Stream DMA Transfers Synchronization" + default y if ACE + default n + help + Enable synchronized Firmware Pointer Increment (FPI) register updates of HD-A gateways + belonging to a group defined by the driver. The driver may also specify an update period + for each group, different than the default one determined by the system tick frequency. + This feature will allow host lower power consumption in scenarios with deep buffering. + config COMP_DAI bool "DAI component" default y diff --git a/src/audio/copier/copier_host.c b/src/audio/copier/copier_host.c index 3e9f40139e26..3db69686e405 100644 --- a/src/audio/copier/copier_host.c +++ b/src/audio/copier/copier_host.c @@ -6,12 +6,91 @@ #include #include +#include #include #include #include LOG_MODULE_DECLARE(copier, CONFIG_SOF_LOG_LEVEL); +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION + +/* NOTE: The code that modifies and checks the contents of the list can be executed on different + * cores. But since adding and removing modules is done exclusively via IPC, and we never add or + * remove more than one module, these functions do not require synchronization. + */ +struct fpi_sync_group { + uint32_t id; + uint32_t period; + uint32_t ref_count; + struct list_item item; +}; + +static struct list_item group_list_head = LIST_INIT(group_list_head); + +static struct fpi_sync_group *find_group_by_id(uint32_t id) +{ + struct list_item *item; + + list_for_item(item, &group_list_head) { + struct fpi_sync_group *group = list_item(item, struct fpi_sync_group, item); + + if (group->id == id) + return group; + } + + return NULL; +} + +int add_to_fpi_sync_group(struct comp_dev *parent_dev, + struct host_data *hd, + struct ipc4_copier_sync_group *sync_group) +{ + struct fpi_sync_group *group = find_group_by_id(sync_group->group_id); + + if (group) { + if (group->period != sync_group->fpi_update_period_usec) { + comp_err(parent_dev, "incorrect period %u for group %u (currently %u)", + group->id, group->period, sync_group->fpi_update_period_usec); + return -EINVAL; + } + + group->ref_count++; + } else { + group = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*group)); + if (!group) { + comp_err(parent_dev, "Failed to alloc memory for new group"); + return -ENOMEM; + } + + group->id = sync_group->group_id; + group->period = sync_group->fpi_update_period_usec; + group->ref_count = 1; + list_item_append(&group->item, &group_list_head); + } + + hd->is_grouped = true; + hd->group_id = group->id; + hd->period_in_cycles = k_us_to_cyc_ceil64(group->period); + comp_dbg(parent_dev, "gtw added to group %u with period %u", group->id, group->period); + return 0; +} + +void delete_from_fpi_sync_group(struct host_data *hd) +{ + struct fpi_sync_group *group = find_group_by_id(hd->group_id); + + if (!group) + return; + + group->ref_count--; + if (group->ref_count == 0) { + list_item_del(&group->item); + rfree(group); + } +} +#endif + /* Playback only */ static int init_pipeline_reg(struct comp_dev *dev) { @@ -81,6 +160,36 @@ int copier_host_create(struct comp_dev *parent_dev, struct copier_data *cd, comp_err(parent_dev, "copier: host new failed with exit"); goto e_data; } +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION + /* Size of a configuration without optional parameters. */ + const uint32_t basic_size = sizeof(*copier_cfg) + + (copier_cfg->gtw_cfg.config_length - 1) * sizeof(uint32_t); + /* Additional data size */ + const uint32_t tlv_buff_size = config->ipc_config_size - basic_size; + const uint32_t min_tlv_size = sizeof(struct ipc4_copier_sync_group) + 2 * sizeof(uint32_t); + + if (tlv_buff_size >= min_tlv_size) { + const uint32_t tlv_addr = (uint32_t)copier_cfg + basic_size; + uint32_t value_size = 0; + void *value_ptr = NULL; + + tlv_value_get((void *)tlv_addr, tlv_buff_size, HDA_SYNC_FPI_UPDATE_GROUP, + &value_ptr, &value_size); + + if (value_ptr) { + struct ipc4_copier_sync_group *sync_group; + + if (value_size != sizeof(struct ipc4_copier_sync_group)) + return -EINVAL; + + sync_group = (struct ipc4_copier_sync_group *)((void *)value_ptr); + + ret = add_to_fpi_sync_group(parent_dev, hd, sync_group); + if (ret < 0) + return ret; + } + } +#endif cd->converter[IPC4_COPIER_GATEWAY_PIN] = get_converter_func(&copier_cfg->base.audio_fmt, @@ -122,6 +231,10 @@ int copier_host_create(struct comp_dev *parent_dev, struct copier_data *cd, void copier_host_free(struct copier_data *cd) { +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION + if (cd->hd->is_grouped) + delete_from_fpi_sync_group(cd->hd); +#endif host_common_free(cd->hd); rfree(cd->hd); } diff --git a/src/audio/host-zephyr.c b/src/audio/host-zephyr.c index 725e982bc0e9..a1faeea8e152 100644 --- a/src/audio/host-zephyr.c +++ b/src/audio/host-zephyr.c @@ -433,6 +433,29 @@ static uint32_t host_get_copy_bytes_normal(struct host_data *hd, struct comp_dev return ALIGN_DOWN(dma_copy_bytes, hd->dma_copy_align); } +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION +static inline bool stream_sync(struct host_data *hd, struct comp_dev *dev) +{ + if (!hd->is_grouped) + return true; + + uint64_t current_time = k_cycle_get_64(); + + if (current_time >= hd->next_sync) { + hd->next_sync = current_time + hd->period_in_cycles; + comp_dbg(dev, "hd(%p) next sync = %llu", hd, hd->next_sync); + return true; + } + + return false; +} +#else +static inline bool stream_sync(struct host_data *hd, struct comp_dev *dev) +{ + return true; +} +#endif + /** * Performs copy operation for host component working in normal mode. * It means DMA works continuously and doesn't need reconfiguration. @@ -471,11 +494,14 @@ static int host_copy_normal(struct host_data *hd, struct comp_dev *dev, copy_cal audio_stream_get_size(&buffer_c->stream) < hd->period_bytes << 3 || audio_stream_get_size(&buffer_c->stream) - hd->partial_size <= (2 + threshold) * hd->period_bytes) { - ret = dma_reload(hd->chan->dma->z_dev, hd->chan->index, 0, 0, hd->partial_size); - if (ret < 0) - comp_err(dev, "host_copy_normal(): dma_copy() failed, ret = %u", ret); + if (stream_sync(hd, dev)) { + ret = dma_reload(hd->chan->dma->z_dev, hd->chan->index, 0, 0, + hd->partial_size); + if (ret < 0) + comp_err(dev, "dma_reload() failed, ret = %u", ret); - hd->partial_size = 0; + hd->partial_size = 0; + } } buffer_release(buffer_c); diff --git a/src/include/ipc4/copier.h b/src/include/ipc4/copier.h index d6965f1c3c22..b91b58172086 100644 --- a/src/include/ipc4/copier.h +++ b/src/include/ipc4/copier.h @@ -119,6 +119,14 @@ enum ipc4_copier_features { IPC4_COPIER_FAST_MODE = 0 }; +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION +#define HDA_SYNC_FPI_UPDATE_GROUP 0 +struct ipc4_copier_sync_group { + uint32_t group_id; + uint32_t fpi_update_period_usec; +} __packed __aligned(4); +#endif + struct ipc4_copier_gateway_cfg { /* ID of Gateway Node. If node_id is valid, i.e. != -1, copier instance is connected to the * specified gateway using either input pin 0 or output pin 0 depending on diff --git a/src/include/sof/audio/host_copier.h b/src/include/sof/audio/host_copier.h index 6abff822fe53..62bc00b0232d 100644 --- a/src/include/sof/audio/host_copier.h +++ b/src/include/sof/audio/host_copier.h @@ -96,6 +96,12 @@ struct host_data { /* stream info */ struct sof_ipc_stream_posn posn; /* TODO: update this */ struct ipc_msg *msg; /**< host notification */ +#if CONFIG_HOST_DMA_STREAM_SYNCHRONIZATION + bool is_grouped; + uint8_t group_id; + uint64_t next_sync; + uint64_t period_in_cycles; +#endif }; int host_common_new(struct host_data *hd, struct comp_dev *dev,