Skip to content

Pipeline2.0 step 7: Make all the code use API to access comp_buffer lists #9802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 8 commits into from
Feb 26, 2025
Merged

Pipeline2.0 step 7: Make all the code use API to access comp_buffer lists #9802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2f9b3fc
ipc: cosmetic name change status => state
marcinszkudlinski Nov 7, 2024
4760320
buf: split sink_list connector to 2 fields
marcinszkudlinski Nov 18, 2024
60d496e
buf: remove unnecessary cache operation
marcinszkudlinski Nov 18, 2024
6bbebc7
buf: use comp_module API in smartamp and tone components
marcinszkudlinski Jan 28, 2025
4e2807b
buf: extend buffer API to satisfy pipeline management needs
marcinszkudlinski Jan 28, 2025
05b42c2
comp: add macros for buffers iteration for comp deletions
marcinszkudlinski Feb 4, 2025
d84930c
buf: make unit test using newly added buffer API
marcinszkudlinski Jan 28, 2025
5c76f6b
buf: make pipeline management code using comp_buffer API
marcinszkudlinski Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/audio/buffers/comp_buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ static struct comp_buffer *buffer_alloc_struct(void *stream_addr, size_t size, u
audio_stream_set_underrun(&buffer->stream, !!(flags & SOF_BUF_UNDERRUN_PERMITTED));
audio_stream_set_overrun(&buffer->stream, !!(flags & SOF_BUF_OVERRUN_PERMITTED));

list_init(&buffer->source_list);
list_init(&buffer->sink_list);
comp_buffer_reset_source_list(buffer);
comp_buffer_reset_sink_list(buffer);
Copy link
Collaborator

@lyakh lyakh Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each change in this commit is simple, but there are so many of them... Maybe this could be split in 1 commit per accessor?


return buffer;
}
Expand Down Expand Up @@ -534,6 +534,13 @@ void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes)
#endif
}

static inline struct list_item *buffer_comp_list(struct comp_buffer *buffer,
int dir)
{
return dir == PPL_DIR_DOWNSTREAM ?
&buffer->source_list : &buffer->sink_list;
}

/*
* Locking: must be called with interrupts disabled! Serialized IPCs protect us
* from racing attach / detach calls, but the scheduler can interrupt the IPC
Expand Down
2 changes: 1 addition & 1 deletion src/audio/module_adapter/module/cadence.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ cadence_codec_process(struct processing_module *mod,
}

/* do not proceed with processing if not enough free space left in the local buffer */
local_buff = list_first_item(&mod->sink_buffer_list, struct comp_buffer, sink_list);
local_buff = list_first_item(&mod->raw_data_buffers_list, struct comp_buffer, buffers_list);
free_bytes = audio_stream_get_free(&local_buff->stream);
if (free_bytes < output_bytes)
return -ENOSPC;
Expand Down
90 changes: 36 additions & 54 deletions src/audio/module_adapter/module_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct comp_dev *module_adapter_new(const struct comp_driver *drv,
mod->dev = dev;
dev->mod = mod;

list_init(&mod->sink_buffer_list);
list_init(&mod->raw_data_buffers_list);

ret = module_adapter_init_data(dev, dst, config, spec);
if (ret) {
Expand Down Expand Up @@ -230,7 +230,7 @@ int module_adapter_prepare(struct comp_dev *dev)
/* Get period_bytes first on prepare(). At this point it is guaranteed that the stream
* parameter from sink buffer is settled, and still prior to all references to period_bytes.
*/
sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
sink = comp_dev_get_first_data_consumer(dev);

mod->period_bytes = audio_stream_period_bytes(&sink->stream, dev->frames);
comp_dbg(dev, "module_adapter_prepare(): got period_bytes = %u", mod->period_bytes);
Expand Down Expand Up @@ -367,7 +367,7 @@ int module_adapter_prepare(struct comp_dev *dev)
}

/* allocate buffer for all sinks */
if (list_is_empty(&mod->sink_buffer_list)) {
if (list_is_empty(&mod->raw_data_buffers_list)) {
for (i = 0; i < mod->num_of_sinks; i++) {
/* allocate not shared buffer */
struct comp_buffer *buffer = buffer_alloc(buff_size, SOF_MEM_CAPS_RAM,
Expand All @@ -381,16 +381,16 @@ int module_adapter_prepare(struct comp_dev *dev)
}

irq_local_disable(flags);
buffer_attach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
list_item_prepend(&buffer->buffers_list, &mod->raw_data_buffers_list);
irq_local_enable(flags);

buffer_set_params(buffer, mod->stream_params, BUFFER_UPDATE_FORCE);
audio_buffer_reset(&buffer->audio_buffer);
}
} else {
list_for_item(blist, &mod->sink_buffer_list) {
list_for_item(blist, &mod->raw_data_buffers_list) {
struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
sink_list);
buffers_list);

ret = buffer_set_size(buffer, buff_size, 0);
if (ret < 0) {
Expand All @@ -409,13 +409,13 @@ int module_adapter_prepare(struct comp_dev *dev)
return 0;

free:
list_for_item_safe(blist, _blist, &mod->sink_buffer_list) {
list_for_item_safe(blist, _blist, &mod->raw_data_buffers_list) {
struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
sink_list);
buffers_list);
uint32_t flags;

irq_local_disable(flags);
buffer_detach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
list_item_del(&buffer->buffers_list);
irq_local_enable(flags);
buffer_free(buffer);
}
Expand Down Expand Up @@ -606,11 +606,11 @@ static void module_adapter_process_output(struct comp_dev *dev)
* copy all produced output samples to output buffers. This loop will do nothing when
* there are no samples produced.
*/
list_for_item(blist, &mod->sink_buffer_list) {
list_for_item(blist, &mod->raw_data_buffers_list) {
if (mod->output_buffers[i].size > 0) {
struct comp_buffer *buffer;

buffer = container_of(blist, struct comp_buffer, sink_list);
buffer = container_of(blist, struct comp_buffer, buffers_list);

ca_copy_from_module_to_sink(&buffer->stream, mod->output_buffers[i].data,
mod->output_buffers[i].size);
Expand All @@ -621,17 +621,14 @@ static void module_adapter_process_output(struct comp_dev *dev)

/* copy from all output local buffers to sink buffers */
i = 0;
list_for_item(blist, &dev->bsink_list) {
struct list_item *_blist;
comp_dev_for_each_consumer(dev, sink) {
int j = 0;

list_for_item(_blist, &mod->sink_buffer_list) {
list_for_item(blist, &mod->raw_data_buffers_list) {
if (i == j) {
struct comp_buffer *source;

sink = container_of(blist, struct comp_buffer, source_list);
source = container_of(_blist, struct comp_buffer, sink_list);

source = container_of(blist, struct comp_buffer, buffers_list);
module_copy_samples(dev, source, sink,
mod->output_buffers[i].size);

Expand Down Expand Up @@ -756,7 +753,7 @@ static int module_adapter_audio_stream_copy_1to1(struct comp_dev *dev)
/* Note: Source buffer state is not checked to enable mixout to generate zero
* PCM codes when source is not active.
*/
if (mod->sink_comp_buffer->sink->state == dev->state)
if (comp_buffer_get_sink_state(mod->sink_comp_buffer) == dev->state)
num_output_buffers = 1;

ret = module_process_legacy(mod, mod->input_buffers, 1,
Expand All @@ -783,10 +780,11 @@ static int module_adapter_audio_stream_type_copy(struct comp_dev *dev)
{
struct comp_buffer *sources[PLATFORM_MAX_STREAMS];
struct comp_buffer *sinks[PLATFORM_MAX_STREAMS];
struct comp_buffer *sink;
struct comp_buffer *source;
struct processing_module *mod = comp_mod(dev);
struct list_item *blist;
uint32_t num_input_buffers, num_output_buffers;
int ret, i = 0;
int ret, i;

/* handle special case of HOST/DAI type components */
if (dev->ipc_config.type == SOF_COMP_HOST || dev->ipc_config.type == SOF_COMP_DAI)
Expand All @@ -796,25 +794,18 @@ static int module_adapter_audio_stream_type_copy(struct comp_dev *dev)
return module_adapter_audio_stream_copy_1to1(dev);

/* acquire all sink and source buffers */
list_for_item(blist, &dev->bsink_list) {
struct comp_buffer *sink;

sink = container_of(blist, struct comp_buffer, source_list);
i = 0;
comp_dev_for_each_consumer(dev, sink)
sinks[i++] = sink;
}
num_output_buffers = i;
if (num_output_buffers > mod->max_sinks) {
comp_err(dev, "Invalid number of sinks %d\n", num_output_buffers);
return -EINVAL;
}

i = 0;
list_for_item(blist, &dev->bsource_list) {
struct comp_buffer *source;

source = container_of(blist, struct comp_buffer, sink_list);
comp_dev_for_each_producer(dev, source)
sources[i++] = source;
}
num_input_buffers = i;
if (num_input_buffers > mod->max_sources) {
comp_err(dev, "Invalid number of sources %d\n", num_input_buffers);
Expand All @@ -824,11 +815,11 @@ static int module_adapter_audio_stream_type_copy(struct comp_dev *dev)
/* setup active input/output buffers for processing */
if (num_output_buffers == 1) {
module_single_sink_setup(dev, sources, sinks);
if (sinks[0]->sink->state != dev->state)
if (comp_buffer_get_sink_state(sinks[0]) != dev->state)
num_output_buffers = 0;
} else if (num_input_buffers == 1) {
module_single_source_setup(dev, sources, sinks);
if (sources[0]->source->state != dev->state) {
if (comp_buffer_get_source_state(sources[0]) != dev->state) {
num_input_buffers = 0;
}
} else {
Expand Down Expand Up @@ -910,15 +901,13 @@ static int module_adapter_copy_ring_buffers(struct comp_dev *dev)
* This is an adapter, to be removed when pipeline2.0 is ready
*/
struct processing_module *mod = comp_mod(dev);
struct list_item *blist;
struct comp_buffer *buffer;
int err;

list_for_item(blist, &dev->bsource_list) {
comp_dev_for_each_producer(dev, buffer) {
/* input - we need to copy data from audio_stream (as source)
* to ring_buffer (as sink)
*/
struct comp_buffer *buffer =
container_of(blist, struct comp_buffer, sink_list);
err = audio_buffer_sync_secondary_buffer(&buffer->audio_buffer, UINT_MAX);

if (err) {
Expand All @@ -930,7 +919,7 @@ static int module_adapter_copy_ring_buffers(struct comp_dev *dev)
if (mod->dp_startup_delay)
return 0;

list_for_item(blist, &dev->bsink_list) {
comp_dev_for_each_consumer(dev, buffer) {
/* output - we need to copy data from ring_buffer (as source)
* to audio_stream (as sink)
*
Expand All @@ -943,8 +932,6 @@ static int module_adapter_copy_ring_buffers(struct comp_dev *dev)
*
* FIX: copy only the following module's IBS in each LL cycle
*/
struct comp_buffer *buffer =
container_of(blist, struct comp_buffer, source_list);
struct sof_source *following_mod_data_source =
audio_buffer_get_source(&buffer->audio_buffer);

Expand Down Expand Up @@ -1013,22 +1000,20 @@ static int module_adapter_raw_data_type_copy(struct comp_dev *dev)

comp_dbg(dev, "module_adapter_raw_data_type_copy(): start");

list_for_item(blist, &mod->sink_buffer_list) {
sink = container_of(blist, struct comp_buffer, sink_list);
list_for_item(blist, &mod->raw_data_buffers_list) {
sink = container_of(blist, struct comp_buffer, buffers_list);

min_free_frames = MIN(min_free_frames,
audio_stream_get_free_frames(&sink->stream));
}

/* copy source samples into input buffer */
list_for_item(blist, &dev->bsource_list) {
comp_dev_for_each_producer(dev, source) {
uint32_t bytes_to_process;
int frames, source_frame_bytes;

source = container_of(blist, struct comp_buffer, sink_list);

/* check if the source dev is in the same state as the dev */
if (!source->source || source->source->state != dev->state)
if (comp_buffer_get_source_state(source) != dev->state)
continue;

frames = MIN(min_free_frames,
Expand Down Expand Up @@ -1061,10 +1046,7 @@ static int module_adapter_raw_data_type_copy(struct comp_dev *dev)

i = 0;
/* consume from all input buffers */
list_for_item(blist, &dev->bsource_list) {

source = container_of(blist, struct comp_buffer, sink_list);

comp_dev_for_each_producer(dev, source) {
comp_update_buffer_consume(source, mod->input_buffers[i].consumed);

bzero((__sparse_force void *)mod->input_buffers[i].data, size);
Expand Down Expand Up @@ -1179,9 +1161,9 @@ int module_adapter_reset(struct comp_dev *dev)
mod->total_data_consumed = 0;
mod->total_data_produced = 0;

list_for_item(blist, &mod->sink_buffer_list) {
list_for_item(blist, &mod->raw_data_buffers_list) {
struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
sink_list);
buffers_list);
buffer_zero(buffer);
}

Expand All @@ -1206,13 +1188,13 @@ void module_adapter_free(struct comp_dev *dev)
if (ret)
comp_err(dev, "module_adapter_free(): failed with error: %d", ret);

list_for_item_safe(blist, _blist, &mod->sink_buffer_list) {
list_for_item_safe(blist, _blist, &mod->raw_data_buffers_list) {
struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
sink_list);
buffers_list);
uint32_t flags;

irq_local_disable(flags);
buffer_detach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
list_item_del(&buffer->buffers_list);
irq_local_enable(flags);
buffer_free(buffer);
}
Expand Down
31 changes: 11 additions & 20 deletions src/audio/module_adapter/module_adapter_ipc3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ void module_adapter_check_data(struct processing_module *mod, struct comp_dev *d
*/
if (IS_PROCESSING_MODE_AUDIO_STREAM(mod) && mod->num_of_sources == 1 &&
mod->num_of_sinks == 1) {
mod->source_comp_buffer = list_first_item(&dev->bsource_list,
struct comp_buffer, sink_list);
mod->source_comp_buffer = comp_dev_get_first_data_producer(dev);
mod->sink_comp_buffer = sink;
mod->stream_copy_single_to_single = true;
}
Expand All @@ -132,26 +131,22 @@ void module_adapter_set_params(struct processing_module *mod, struct sof_ipc_str
{
}

static int module_source_status_count(struct comp_dev *dev, uint32_t status)
static int module_source_state_count(struct comp_dev *dev, uint32_t state)
{
struct list_item *blist;
int count = 0;
struct comp_buffer *source;

/* count source with state == status */
list_for_item(blist, &dev->bsource_list) {
comp_dev_for_each_producer(dev, source)
/*
* FIXME: this is racy, state can be changed by another core.
* This is implicitly protected by serialised IPCs. Even when
* IPCs are processed in the pipeline thread, the next IPC will
* not be sent until the thread has processed and replied to the
* current one.
*/
struct comp_buffer *source = container_of(blist, struct comp_buffer,
sink_list);

if (source->source && source->source->state == status)
if (comp_buffer_get_source_state(source) == state)
count++;
}

return count;
}
Expand All @@ -163,8 +158,8 @@ int module_adapter_set_state(struct processing_module *mod, struct comp_dev *dev
bool sources_active;
int ret;

sources_active = module_source_status_count(dev, COMP_STATE_ACTIVE) ||
module_source_status_count(dev, COMP_STATE_PAUSED);
sources_active = module_source_state_count(dev, COMP_STATE_ACTIVE) ||
module_source_state_count(dev, COMP_STATE_PAUSED);

/* don't stop/start module if one of the sources is active/paused */
if ((cmd == COMP_TRIGGER_STOP || cmd == COMP_TRIGGER_PRE_START) && sources_active) {
Expand Down Expand Up @@ -324,25 +319,21 @@ int module_adapter_cmd(struct comp_dev *dev, int cmd, void *data, int max_data_s
int module_adapter_sink_src_prepare(struct comp_dev *dev)
{
struct processing_module *mod = comp_mod(dev);
struct list_item *blist;
struct comp_buffer *sink_buffer;
struct comp_buffer *source_buffer;
int ret;
int i;

/* acquire all sink and source buffers, get handlers to sink/source API */
i = 0;
list_for_item(blist, &dev->bsink_list) {
struct comp_buffer *sink_buffer =
container_of(blist, struct comp_buffer, source_list);
comp_dev_for_each_consumer(dev, sink_buffer) {
mod->sinks[i] = audio_buffer_get_sink(&sink_buffer->audio_buffer);
i++;
}
mod->num_of_sinks = i;

i = 0;
list_for_item(blist, &dev->bsource_list) {
struct comp_buffer *source_buffer =
container_of(blist, struct comp_buffer, sink_list);

comp_dev_for_each_producer(dev, source_buffer) {
mod->sources[i] = audio_buffer_get_source(&source_buffer->audio_buffer);
i++;
}
Expand Down
Loading
Loading