From 43e97ee9e5e6389a03cd9bd592088e03794aa46c Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Wed, 16 Oct 2024 15:23:00 +0200 Subject: [PATCH 1/5] audio: spelling correction: audo to audio sof_audo_buffer_from_* => sof_audio_buffer_from_* Signed-off-by: Marcin Szkudlinski --- src/audio/buffers/ring_buffer.c | 4 ++-- src/include/sof/audio/audio_buffer.h | 4 ++-- src/include/sof/audio/buffer.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/audio/buffers/ring_buffer.c b/src/audio/buffers/ring_buffer.c index 45849b0b0669..0fef054e0f13 100644 --- a/src/audio/buffers/ring_buffer.c +++ b/src/audio/buffers/ring_buffer.c @@ -19,14 +19,14 @@ DECLARE_TR_CTX(ring_buffer_tr, SOF_UUID(ring_buffer_uuid), LOG_LEVEL_INFO); static inline struct ring_buffer *ring_buffer_from_sink(struct sof_sink *sink) { - struct sof_audio_buffer *audio_buffer = sof_audo_buffer_from_sink(sink); + struct sof_audio_buffer *audio_buffer = sof_audio_buffer_from_sink(sink); return container_of(audio_buffer, struct ring_buffer, audio_buffer); } static inline struct ring_buffer *ring_buffer_from_source(struct sof_source *source) { - struct sof_audio_buffer *audio_buffer = sof_audo_buffer_from_source(source); + struct sof_audio_buffer *audio_buffer = sof_audio_buffer_from_source(source); return container_of(audio_buffer, struct ring_buffer, audio_buffer); } diff --git a/src/include/sof/audio/audio_buffer.h b/src/include/sof/audio/audio_buffer.h index 3b8aedd30fb4..d61c32a654d0 100644 --- a/src/include/sof/audio/audio_buffer.h +++ b/src/include/sof/audio/audio_buffer.h @@ -219,7 +219,7 @@ struct sof_audio_stream_params *audio_buffer_get_stream_params(struct sof_audio_ * NOTE! ensure that sink is really provided by sof_audio_buffer * otherwise a random value will be returned */ -static inline struct sof_audio_buffer *sof_audo_buffer_from_sink(struct sof_sink *sink) +static inline struct sof_audio_buffer *sof_audio_buffer_from_sink(struct sof_sink *sink) { return container_of(sink, struct sof_audio_buffer, _sink_api); } @@ -229,7 +229,7 @@ static inline struct sof_audio_buffer *sof_audo_buffer_from_sink(struct sof_sink * NOTE! ensure that source is really provided by sof_audio_buffer * otherwise a random value will be returned */ -static inline struct sof_audio_buffer *sof_audo_buffer_from_source(struct sof_source *source) +static inline struct sof_audio_buffer *sof_audio_buffer_from_source(struct sof_source *source) { return container_of(source, struct sof_audio_buffer, _source_api); } diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 55e01b624ff9..402ef084fc10 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -200,14 +200,14 @@ int buffer_set_size_range(struct comp_buffer *buffer, size_t preferred_size, siz /* legacy wrappers, to be removed. Don't use them if possible */ static inline struct comp_buffer *comp_buffer_get_from_source(struct sof_source *source) { - struct sof_audio_buffer *audio_buffer = sof_audo_buffer_from_source(source); + struct sof_audio_buffer *audio_buffer = sof_audio_buffer_from_source(source); return container_of(audio_buffer, struct comp_buffer, audio_buffer); } static inline struct comp_buffer *comp_buffer_get_from_sink(struct sof_sink *sink) { - struct sof_audio_buffer *audio_buffer = sof_audo_buffer_from_sink(sink); + struct sof_audio_buffer *audio_buffer = sof_audio_buffer_from_sink(sink); return container_of(audio_buffer, struct comp_buffer, audio_buffer); } From c01fd3ccd1331513f5e1b7ef3841328e11fdc413 Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Tue, 15 Oct 2024 15:13:31 +0200 Subject: [PATCH 2/5] buf: add .clean method to audio_buffer API clean method does clean all buffer data leaving config as is Need to be implemented as virtual method as is implemented difrently in each buffer type Signed-off-by: Marcin Szkudlinski --- src/audio/buffers/comp_buffer.c | 11 ++++++ src/audio/buffers/ring_buffer.c | 41 +++++++++++++++++------ src/audio/module_adapter/module_adapter.c | 4 +-- src/audio/pipeline/pipeline-graph.c | 5 +++ src/audio/pipeline/pipeline-params.c | 5 +++ src/include/sof/audio/audio_buffer.h | 18 ++++++++++ src/include/sof/audio/buffer.h | 14 -------- tools/plugin/modules/alsa.c | 4 +-- 8 files changed, 74 insertions(+), 28 deletions(-) diff --git a/src/audio/buffers/comp_buffer.c b/src/audio/buffers/comp_buffer.c index ff32f3942244..952375969bf3 100644 --- a/src/audio/buffers/comp_buffer.c +++ b/src/audio/buffers/comp_buffer.c @@ -152,6 +152,16 @@ static int comp_buffer_sink_set_alignment_constants(struct sof_sink *sink, return 0; } +static void comp_buffer_reset(struct sof_audio_buffer *audio_buffer) +{ + struct comp_buffer *buffer = container_of(audio_buffer, struct comp_buffer, audio_buffer); + + /* reset rw pointers and avail/free bytes counters */ + audio_stream_reset(&buffer->stream); + /* clear buffer contents */ + buffer_zero(buffer); +} + /* free component in the pipeline */ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer) { @@ -197,6 +207,7 @@ static const struct sink_ops comp_buffer_sink_ops = { static const struct audio_buffer_ops audio_buffer_ops = { .free = comp_buffer_free, + .reset = comp_buffer_reset, }; static struct comp_buffer *buffer_alloc_struct(void *stream_addr, size_t size, uint32_t caps, diff --git a/src/audio/buffers/ring_buffer.c b/src/audio/buffers/ring_buffer.c index 0fef054e0f13..1027d9500fad 100644 --- a/src/audio/buffers/ring_buffer.c +++ b/src/audio/buffers/ring_buffer.c @@ -31,16 +31,6 @@ static inline struct ring_buffer *ring_buffer_from_source(struct sof_source *sou return container_of(audio_buffer, struct ring_buffer, audio_buffer); } -/** - * @brief remove the queue from the list, free memory - */ -static void ring_buffer_free(struct sof_audio_buffer *buffer) -{ - struct ring_buffer *ring_buffer = (struct ring_buffer *)buffer; - - rfree((__sparse_force void *)ring_buffer->_data_buffer); -} - /** * @brief return true if the ring buffer is shared between 2 cores */ @@ -93,6 +83,36 @@ static inline void ring_buffer_writeback_shared(struct ring_buffer *ring_buffer, dcache_writeback_region(ptr, size); } + +/** + * @brief remove the queue from the list, free memory + */ +static void ring_buffer_free(struct sof_audio_buffer *audio_buffer) +{ + if (!audio_buffer) + return; + + struct ring_buffer *ring_buffer = + container_of(audio_buffer, struct ring_buffer, audio_buffer); + + rfree((__sparse_force void *)ring_buffer->_data_buffer); +} + +static void ring_buffer_reset(struct sof_audio_buffer *audio_buffer) +{ + struct ring_buffer *ring_buffer = + container_of(audio_buffer, struct ring_buffer, audio_buffer); + + ring_buffer->_write_offset = 0; + ring_buffer->_read_offset = 0; + + ring_buffer_invalidate_shared(ring_buffer, ring_buffer->_data_buffer, + ring_buffer->data_buffer_size); + bzero((__sparse_force void *)ring_buffer->_data_buffer, ring_buffer->data_buffer_size); + ring_buffer_writeback_shared(ring_buffer, ring_buffer->_data_buffer, + ring_buffer->data_buffer_size); +} + static inline uint8_t __sparse_cache *ring_buffer_get_pointer(struct ring_buffer *ring_buffer, size_t offset) { @@ -277,6 +297,7 @@ static const struct sink_ops ring_buffer_sink_ops = { static const struct audio_buffer_ops audio_buffer_ops = { .free = ring_buffer_free, + .reset = ring_buffer_reset }; struct ring_buffer *ring_buffer_create(size_t min_available, size_t min_free_space, bool is_shared, diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 33049f69f20e..116a744c4113 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -385,7 +385,7 @@ int module_adapter_prepare(struct comp_dev *dev) irq_local_enable(flags); buffer_set_params(buffer, mod->stream_params, BUFFER_UPDATE_FORCE); - buffer_reset_pos(buffer, NULL); + audio_buffer_reset(&buffer->audio_buffer); } } else { list_for_item(blist, &mod->sink_buffer_list) { @@ -400,7 +400,7 @@ int module_adapter_prepare(struct comp_dev *dev) } buffer_set_params(buffer, mod->stream_params, BUFFER_UPDATE_FORCE); - buffer_reset_pos(buffer, NULL); + audio_buffer_reset(&buffer->audio_buffer); } } diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 8b21774c6d19..bb865cc83229 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -360,6 +360,11 @@ static int pipeline_comp_reset(struct comp_dev *current, return pipeline_for_each_comp(current, ctx, dir); } +static inline void buffer_reset_params(struct comp_buffer *buffer, void *data) +{ + audio_buffer_reset_params(&buffer->audio_buffer); +} + /* reset the whole pipeline */ int pipeline_reset(struct pipeline *p, struct comp_dev *host) { diff --git a/src/audio/pipeline/pipeline-params.c b/src/audio/pipeline/pipeline-params.c index c9b4f132c9e4..2fa3d19e62bc 100644 --- a/src/audio/pipeline/pipeline-params.c +++ b/src/audio/pipeline/pipeline-params.c @@ -289,6 +289,11 @@ static int pipeline_comp_prepare(struct comp_dev *current, return pipeline_for_each_comp(current, ctx, dir); } +static void buffer_reset_pos(struct comp_buffer *buffer, void *data) +{ + audio_buffer_reset(&buffer->audio_buffer); +} + /* prepare the pipeline for usage */ int pipeline_prepare(struct pipeline *p, struct comp_dev *dev) { diff --git a/src/include/sof/audio/audio_buffer.h b/src/include/sof/audio/audio_buffer.h index d61c32a654d0..a84d6abcfaac 100644 --- a/src/include/sof/audio/audio_buffer.h +++ b/src/include/sof/audio/audio_buffer.h @@ -25,6 +25,13 @@ struct audio_buffer_ops { * OPTIONAL */ void (*free)(struct sof_audio_buffer *buffer); + + /** + * @brief clean all buffer data, set buffer positions to initial, leaving config as is + * the procedure is to be called only when buffer is not in use + * OPTIONAL + */ + void (*reset)(struct sof_audio_buffer *buffer); }; /* base class for all buffers, all buffers must inherit from it */ @@ -267,4 +274,15 @@ void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bo */ void audio_buffer_free(struct sof_audio_buffer *buffer); +/** + * @brief clean all buffer data, set buffer positions to initial, leaving config as is + * the procedure is to be called only when buffer is not in use + */ +static inline +void audio_buffer_reset(struct sof_audio_buffer *buffer) +{ + if (buffer->ops->reset) + buffer->ops->reset(buffer); +} + #endif /* __SOF_AUDIO_BUFFER__ */ diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 402ef084fc10..eafe9d1cd190 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -279,15 +279,6 @@ static inline uint32_t buffer_pipeline_id(const struct comp_buffer *buffer) return buffer->stream.runtime_stream_params.pipeline_id; } -static inline void buffer_reset_pos(struct comp_buffer *buffer, void *data) -{ - /* reset rw pointers and avail/free bytes counters */ - audio_stream_reset(&buffer->stream); - - /* clear buffer contents */ - buffer_zero(buffer); -} - /* Run-time buffer re-configuration calls this too, so it must use cached access */ static inline void buffer_init_stream(struct comp_buffer *buffer, size_t size) { @@ -295,9 +286,4 @@ static inline void buffer_init_stream(struct comp_buffer *buffer, size_t size) audio_stream_init(&buffer->stream, buffer->stream.addr, size); } -static inline void buffer_reset_params(struct comp_buffer *buffer, void *data) -{ - audio_buffer_reset_params(&buffer->audio_buffer); -} - #endif /* __SOF_AUDIO_BUFFER_H__ */ diff --git a/tools/plugin/modules/alsa.c b/tools/plugin/modules/alsa.c index 2d946529d05f..b261de40ee80 100644 --- a/tools/plugin/modules/alsa.c +++ b/tools/plugin/modules/alsa.c @@ -429,7 +429,7 @@ static int arecord_params(struct comp_dev *dev, struct sof_ipc_stream_params *pa /* file component sink/source buffer period count */ buffer = comp_dev_get_first_data_consumer(dev); - buffer_reset_pos(buffer, NULL); + audio_buffer_reset(&buffer->audio_buffer); comp_dbg(dev, "prepare done ret = %d", ret); @@ -461,7 +461,7 @@ static int aplay_params(struct comp_dev *dev, struct sof_ipc_stream_params *para /* file component sink/source buffer period count */ buffer = comp_dev_get_first_data_producer(dev); - buffer_reset_pos(buffer, NULL); + audio_buffer_reset(&buffer->audio_buffer); comp_dbg(dev, "prepare done ret = %d", ret); return 0; From c7c52c41676f2df48f1d46c0298d3a5e0eca2bed Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Wed, 16 Oct 2024 15:31:58 +0200 Subject: [PATCH 3/5] buf: extend audio_buf api and add default sink/src handlers There are 3 APIs each buffer must implement (Sink/src/audio_buffer) for data producers/consumers/maintenance each of them need to have methods doing same operations - like settings stream parameters this commit provides simplification for buffer implementation - it is enough to implement those methods for audio_buffer API, default handlers for sink/src will propagate calls accordingly Specific handlers for sink and source may still be provided i.e. in case when sink or source API is not provided by a buffer but some other data producer (i.e. DAI) Also a default implementation of .audio_set_ipc_params is provided, that probably will fit needs of all buffers' implementation, except of currently used legacy comp_buffer Signed-off-by: Marcin Szkudlinski --- src/audio/buffers/audio_buffer.c | 126 +++++++++++++++++++++++++++ src/audio/buffers/comp_buffer.c | 4 +- src/audio/buffers/ring_buffer.c | 4 +- src/include/sof/audio/audio_buffer.h | 50 ++++++++--- 4 files changed, 166 insertions(+), 18 deletions(-) diff --git a/src/audio/buffers/audio_buffer.c b/src/audio/buffers/audio_buffer.c index 9cbd587196b9..af882accaae8 100644 --- a/src/audio/buffers/audio_buffer.c +++ b/src/audio/buffers/audio_buffer.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -95,3 +96,128 @@ void audio_buffer_free(struct sof_audio_buffer *buffer) buffer->ops->free(buffer); rfree(buffer); } + +static +int audio_buffer_source_set_ipc_params_default(struct sof_audio_buffer *buffer, + struct sof_ipc_stream_params *params, + bool force_update) +{ + CORE_CHECK_STRUCT(buffer); + + if (audio_buffer_hw_params_configured(buffer) && !force_update) + return 0; + + struct sof_audio_stream_params *audio_stream_params = + audio_buffer_get_stream_params(buffer); + + audio_stream_params->frame_fmt = params->frame_fmt; + audio_stream_params->rate = params->rate; + audio_stream_params->channels = params->channels; + audio_stream_params->buffer_fmt = params->buffer_fmt; + + audio_buffer_set_hw_params_configured(buffer); + + if (buffer->ops->on_audio_format_set) + return buffer->ops->on_audio_format_set(buffer); + return 0; +} + +static +int audio_buffer_sink_set_ipc_params(struct sof_sink *sink, struct sof_ipc_stream_params *params, + bool force_update) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_sink(sink); + + if (buffer->ops->audio_set_ipc_params) + return buffer->ops->audio_set_ipc_params(buffer, params, force_update); + return audio_buffer_source_set_ipc_params_default(buffer, params, force_update); +} + +static +int audio_buffer_sink_on_audio_format_set(struct sof_sink *sink) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_sink(sink); + + if (buffer->ops->on_audio_format_set) + return buffer->ops->on_audio_format_set(buffer); + return 0; +} + +static +int audio_buffer_sink_set_alignment_constants(struct sof_sink *sink, + const uint32_t byte_align, + const uint32_t frame_align_req) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_sink(sink); + + if (buffer->ops->set_alignment_constants) + return buffer->ops->set_alignment_constants(buffer, byte_align, frame_align_req); + return 0; +} + +static +int audio_buffer_source_set_ipc_params(struct sof_source *source, + struct sof_ipc_stream_params *params, bool force_update) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_source(source); + + if (buffer->ops->audio_set_ipc_params) + return buffer->ops->audio_set_ipc_params(buffer, params, force_update); + return audio_buffer_source_set_ipc_params_default(buffer, params, force_update); +} + +static +int audio_buffer_source_on_audio_format_set(struct sof_source *source) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_source(source); + + if (buffer->ops->on_audio_format_set) + return buffer->ops->on_audio_format_set(buffer); + return 0; +} + +static +int audio_buffer_source_set_alignment_constants(struct sof_source *source, + const uint32_t byte_align, + const uint32_t frame_align_req) +{ + struct sof_audio_buffer *buffer = sof_audio_buffer_from_source(source); + + if (buffer->ops->set_alignment_constants) + return buffer->ops->set_alignment_constants(buffer, byte_align, frame_align_req); + return 0; +} + +void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bool is_shared, + struct source_ops *source_ops, struct sink_ops *sink_ops, + const struct audio_buffer_ops *audio_buffer_ops, + struct sof_audio_stream_params *audio_stream_params) +{ + CORE_CHECK_STRUCT_INIT(&buffer, is_shared); + buffer->buffer_type = buffer_type; + buffer->ops = audio_buffer_ops; + buffer->audio_stream_params = audio_stream_params; + buffer->is_shared = is_shared; + + /* set default implementations of sink/source methods, if there's no + * specific implementation provided + */ + if (!sink_ops->audio_set_ipc_params) + sink_ops->audio_set_ipc_params = audio_buffer_sink_set_ipc_params; + if (!sink_ops->on_audio_format_set && buffer->ops->on_audio_format_set) + sink_ops->on_audio_format_set = audio_buffer_sink_on_audio_format_set; + if (!sink_ops->set_alignment_constants && buffer->ops->set_alignment_constants) + sink_ops->set_alignment_constants = audio_buffer_sink_set_alignment_constants; + + if (!source_ops->audio_set_ipc_params) + source_ops->audio_set_ipc_params = audio_buffer_source_set_ipc_params; + if (!source_ops->on_audio_format_set && buffer->ops->on_audio_format_set) + source_ops->on_audio_format_set = audio_buffer_source_on_audio_format_set; + if (!source_ops->set_alignment_constants && buffer->ops->set_alignment_constants) + source_ops->set_alignment_constants = audio_buffer_source_set_alignment_constants; + + source_init(audio_buffer_get_source(buffer), source_ops, + audio_buffer_get_stream_params(buffer)); + sink_init(audio_buffer_get_sink(buffer), sink_ops, + audio_buffer_get_stream_params(buffer)); +} diff --git a/src/audio/buffers/comp_buffer.c b/src/audio/buffers/comp_buffer.c index 952375969bf3..76cd73ba98f3 100644 --- a/src/audio/buffers/comp_buffer.c +++ b/src/audio/buffers/comp_buffer.c @@ -187,7 +187,7 @@ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer) rfree(buffer->stream.addr); } -static const struct source_ops comp_buffer_source_ops = { +static struct source_ops comp_buffer_source_ops = { .get_data_available = comp_buffer_get_data_available, .get_data = comp_buffer_get_data, .release_data = comp_buffer_release_data, @@ -196,7 +196,7 @@ static const struct source_ops comp_buffer_source_ops = { .set_alignment_constants = comp_buffer_source_set_alignment_constants }; -static const struct sink_ops comp_buffer_sink_ops = { +static struct sink_ops comp_buffer_sink_ops = { .get_free_size = comp_buffer_get_free_size, .get_buffer = comp_buffer_get_buffer, .commit_buffer = comp_buffer_commit_buffer, diff --git a/src/audio/buffers/ring_buffer.c b/src/audio/buffers/ring_buffer.c index 1027d9500fad..b05ba1fec2a0 100644 --- a/src/audio/buffers/ring_buffer.c +++ b/src/audio/buffers/ring_buffer.c @@ -281,14 +281,14 @@ static int ring_buffer_set_ipc_params_sink(struct sof_sink *sink, return ring_buffer_set_ipc_params(ring_buffer, params, force_update); } -static const struct source_ops ring_buffer_source_ops = { +static struct source_ops ring_buffer_source_ops = { .get_data_available = ring_buffer_get_data_available, .get_data = ring_buffer_get_data, .release_data = ring_buffer_release_data, .audio_set_ipc_params = ring_buffer_set_ipc_params_source, }; -static const struct sink_ops ring_buffer_sink_ops = { +static struct sink_ops ring_buffer_sink_ops = { .get_free_size = ring_buffer_get_free_size, .get_buffer = ring_buffer_get_buffer, .commit_buffer = ring_buffer_commit_buffer, diff --git a/src/include/sof/audio/audio_buffer.h b/src/include/sof/audio/audio_buffer.h index a84d6abcfaac..4984c3dee99a 100644 --- a/src/include/sof/audio/audio_buffer.h +++ b/src/include/sof/audio/audio_buffer.h @@ -32,6 +32,32 @@ struct audio_buffer_ops { * OPTIONAL */ void (*reset)(struct sof_audio_buffer *buffer); + + /** + * OPTIONAL: Notification to the sink implementation about changes in audio format + * + * Once any of *audio_stream_params elements changes, the implementation of + * sink may need to perform some extra operations. + * This callback will be called immediately after any change + * + * @retval 0 if success, negative if new parameters are not supported + */ + int (*on_audio_format_set)(struct sof_audio_buffer *buffer); + + /** + * OPTIONAL + * see sink_set_params comments + */ + int (*audio_set_ipc_params)(struct sof_audio_buffer *buffer, + struct sof_ipc_stream_params *params, bool force_update); + + /** + * OPTIONAL + * see comment for sink_set_alignment_constants + */ + int (*set_alignment_constants)(struct sof_audio_buffer *buffer, + const uint32_t byte_align, + const uint32_t frame_align_req); }; /* base class for all buffers, all buffers must inherit from it */ @@ -251,23 +277,19 @@ static inline struct sof_audio_buffer *sof_audio_buffer_from_source(struct sof_s * @param sink_ops pointer to virtual methods table for sink API * @param audio_buffer_ops pointer to required buffer virtual methods implementation * @param audio_stream_params pointer to audio stream (currently kept in buffer implementation) + * + * in sink_ops and source_ops API if there's no implemented of following methods: + * on_audio_format_set + * audio_set_ipc_params + * set_alignment_constants + * + * default implementation will be used instead, redirecting those calls to proper + * audio_buffer_ops operations */ -static inline void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bool is_shared, - const struct source_ops *source_ops, const struct sink_ops *sink_ops, + struct source_ops *source_ops, struct sink_ops *sink_ops, const struct audio_buffer_ops *audio_buffer_ops, - struct sof_audio_stream_params *audio_stream_params) -{ - CORE_CHECK_STRUCT_INIT(&buffer, is_shared); - buffer->buffer_type = buffer_type; - buffer->ops = audio_buffer_ops; - buffer->audio_stream_params = audio_stream_params; - buffer->is_shared = is_shared; - source_init(audio_buffer_get_source(buffer), source_ops, - audio_buffer_get_stream_params(buffer)); - sink_init(audio_buffer_get_sink(buffer), sink_ops, - audio_buffer_get_stream_params(buffer)); -} + struct sof_audio_stream_params *audio_stream_params); /** * @brief free buffer and all allocated resources From fe44ce4f1117eac444ec6555401e070cc0e1d331 Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Thu, 17 Oct 2024 14:51:18 +0200 Subject: [PATCH 4/5] buf: ring_buffer and comp_buffer use default methods This commit modifies both existing buffers implementations to use simplified API for sink/source/audio_buffer Signed-off-by: Marcin Szkudlinski --- src/audio/buffers/comp_buffer.c | 56 ++++++++------------------------- src/audio/buffers/ring_buffer.c | 44 -------------------------- 2 files changed, 13 insertions(+), 87 deletions(-) diff --git a/src/audio/buffers/comp_buffer.c b/src/audio/buffers/comp_buffer.c index 76cd73ba98f3..62f90bd2b5c9 100644 --- a/src/audio/buffers/comp_buffer.c +++ b/src/audio/buffers/comp_buffer.c @@ -64,33 +64,6 @@ static int comp_buffer_release_data(struct sof_source *source, size_t free_size) return 0; } -static int comp_buffer_set_ipc_params_source(struct sof_source *source, - struct sof_ipc_stream_params *params, - bool force_update) -{ - struct comp_buffer *buffer = comp_buffer_get_from_source(source); - - return buffer_set_params(buffer, params, force_update); -} - -static int comp_buffer_source_format_set(struct sof_source *source) -{ - struct comp_buffer *buffer = comp_buffer_get_from_source(source); - - audio_stream_recalc_align(&buffer->stream); - return 0; -} - -static int comp_buffer_source_set_alignment_constants(struct sof_source *source, - const uint32_t byte_align, - const uint32_t frame_align_req) -{ - struct comp_buffer *buffer = comp_buffer_get_from_source(source); - - audio_stream_set_align(byte_align, frame_align_req, &buffer->stream); - return 0; -} - static size_t comp_buffer_get_free_size(struct sof_sink *sink) { struct comp_buffer *buffer = comp_buffer_get_from_sink(sink); @@ -125,28 +98,28 @@ static int comp_buffer_commit_buffer(struct sof_sink *sink, size_t commit_size) return 0; } -static int comp_buffer_set_ipc_params_sink(struct sof_sink *sink, - struct sof_ipc_stream_params *params, - bool force_update) +static int comp_buffer_set_ipc_params(struct sof_audio_buffer *audio_buffer, + struct sof_ipc_stream_params *params, + bool force_update) { - struct comp_buffer *buffer = comp_buffer_get_from_sink(sink); + struct comp_buffer *buffer = container_of(audio_buffer, struct comp_buffer, audio_buffer); return buffer_set_params(buffer, params, force_update); } -static int comp_buffer_sink_format_set(struct sof_sink *sink) +static int comp_buffer_format_set(struct sof_audio_buffer *audio_buffer) { - struct comp_buffer *buffer = comp_buffer_get_from_sink(sink); + struct comp_buffer *buffer = container_of(audio_buffer, struct comp_buffer, audio_buffer); audio_stream_recalc_align(&buffer->stream); return 0; } -static int comp_buffer_sink_set_alignment_constants(struct sof_sink *sink, - const uint32_t byte_align, - const uint32_t frame_align_req) +static int comp_buffer_set_alignment_constants(struct sof_audio_buffer *audio_buffer, + const uint32_t byte_align, + const uint32_t frame_align_req) { - struct comp_buffer *buffer = comp_buffer_get_from_sink(sink); + struct comp_buffer *buffer = container_of(audio_buffer, struct comp_buffer, audio_buffer); audio_stream_set_align(byte_align, frame_align_req, &buffer->stream); return 0; @@ -191,23 +164,20 @@ static struct source_ops comp_buffer_source_ops = { .get_data_available = comp_buffer_get_data_available, .get_data = comp_buffer_get_data, .release_data = comp_buffer_release_data, - .audio_set_ipc_params = comp_buffer_set_ipc_params_source, - .on_audio_format_set = comp_buffer_source_format_set, - .set_alignment_constants = comp_buffer_source_set_alignment_constants }; static struct sink_ops comp_buffer_sink_ops = { .get_free_size = comp_buffer_get_free_size, .get_buffer = comp_buffer_get_buffer, .commit_buffer = comp_buffer_commit_buffer, - .audio_set_ipc_params = comp_buffer_set_ipc_params_sink, - .on_audio_format_set = comp_buffer_sink_format_set, - .set_alignment_constants = comp_buffer_sink_set_alignment_constants }; static const struct audio_buffer_ops audio_buffer_ops = { .free = comp_buffer_free, .reset = comp_buffer_reset, + .audio_set_ipc_params = comp_buffer_set_ipc_params, + .on_audio_format_set = comp_buffer_format_set, + .set_alignment_constants = comp_buffer_set_alignment_constants }; static struct comp_buffer *buffer_alloc_struct(void *stream_addr, size_t size, uint32_t caps, diff --git a/src/audio/buffers/ring_buffer.c b/src/audio/buffers/ring_buffer.c index b05ba1fec2a0..8111f2adc46d 100644 --- a/src/audio/buffers/ring_buffer.c +++ b/src/audio/buffers/ring_buffer.c @@ -239,60 +239,16 @@ static int ring_buffer_release_data(struct sof_source *source, size_t free_size) return 0; } -static int ring_buffer_set_ipc_params(struct ring_buffer *ring_buffer, - struct sof_ipc_stream_params *params, - bool force_update) -{ - CORE_CHECK_STRUCT(&ring_buffer->audio_buffer); - - if (audio_buffer_hw_params_configured(&ring_buffer->audio_buffer) && !force_update) - return 0; - - struct sof_audio_stream_params *audio_stream_params = - audio_buffer_get_stream_params(&ring_buffer->audio_buffer); - - audio_stream_params->frame_fmt = params->frame_fmt; - audio_stream_params->rate = params->rate; - audio_stream_params->channels = params->channels; - audio_stream_params->buffer_fmt = params->buffer_fmt; - - audio_buffer_set_hw_params_configured(&ring_buffer->audio_buffer); - - return 0; -} - -static int ring_buffer_set_ipc_params_source(struct sof_source *source, - struct sof_ipc_stream_params *params, - bool force_update) -{ - struct ring_buffer *ring_buffer = ring_buffer_from_source(source); - - CORE_CHECK_STRUCT(&ring_buffer->audio_buffer); - return ring_buffer_set_ipc_params(ring_buffer, params, force_update); -} - -static int ring_buffer_set_ipc_params_sink(struct sof_sink *sink, - struct sof_ipc_stream_params *params, - bool force_update) -{ - struct ring_buffer *ring_buffer = ring_buffer_from_sink(sink); - - CORE_CHECK_STRUCT(&ring_buffer->audio_buffer); - return ring_buffer_set_ipc_params(ring_buffer, params, force_update); -} - static struct source_ops ring_buffer_source_ops = { .get_data_available = ring_buffer_get_data_available, .get_data = ring_buffer_get_data, .release_data = ring_buffer_release_data, - .audio_set_ipc_params = ring_buffer_set_ipc_params_source, }; static struct sink_ops ring_buffer_sink_ops = { .get_free_size = ring_buffer_get_free_size, .get_buffer = ring_buffer_get_buffer, .commit_buffer = ring_buffer_commit_buffer, - .audio_set_ipc_params = ring_buffer_set_ipc_params_sink, }; static const struct audio_buffer_ops audio_buffer_ops = { From 0fc1d22585c843add9587427248e4d3938e53834 Mon Sep 17 00:00:00 2001 From: Marcin Szkudlinski Date: Thu, 17 Oct 2024 17:44:46 +0200 Subject: [PATCH 5/5] buf: remove unnecessary cache operation All control structures of buffers shared between cores are now stored in a non-cached memory alias. Cache writeback is no longer needed here Signed-off-by: Marcin Szkudlinski --- src/audio/pipeline/pipeline-graph.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index bb865cc83229..218ab422490b 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -409,17 +409,7 @@ int pipeline_for_each_comp(struct comp_dev *current, continue; /* don't go back to the buffer which already walked */ - /* - * Note, that this access must be performed unlocked via - * uncached address. Trying to lock before checking the flag - * understandably leads to a deadlock when this function is - * called recursively from .comp_func() below. We do it in a - * safe way: this flag must *only* be accessed in this function - * only in these three cases: testing, setting and clearing. - * Note, that it is also assumed that buffers aren't shared - * across CPUs. See further comment below. - */ - dcache_writeback_invalidate_region(uncache_to_cache(buffer), sizeof(*buffer)); + if (buffer->audio_buffer.walking) continue;