Pipeline2.0 step 7: Make all the code use API to access comp_buffer lists #9802
+203
−211
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
module_source_status_count in fact counts "state" field of all components. Name changed to module_source_state_count Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
marcinszkudlinski
requested review from
bardliao,
pblaszko,
ranj063,
jxstelter,
singalsu,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
marcinszkudlinski
changed the title
singalsu
approved these changes
Jan 29, 2025
lyakh
reviewed
Jan 30, 2025
| @@ -309,9 +309,6 @@ int ipc_comp_free(struct ipc *ipc, uint32_t comp_id) | |||
| struct comp_buffer *buffer = container_of(clist, struct comp_buffer, sink_list); | |||
|
|
|||
| buffer->sink = NULL; | |||
| /* Also if it isn't shared - we are about to modify uncached data */ | |||
| dcache_writeback_invalidate_region(uncache_to_cache(buffer), | |||
| sizeof(*buffer)); | |||
There was a problem hiding this comment.
The comment about modifying uncached data is a left-over from the time, when we only accessed buffer list headers uncached, even when the buffer object itself was cached. We don't do that any more - cached buffer objects are added to the lists via cached pointers and uncached ones - via uncached. So, yes, this cache synchronisation doesn't seem to be needed any more.
| list_init(&buffer->source_list); | ||
| list_init(&buffer->sink_list); | ||
| comp_buffer_reset_source_list(buffer); | ||
| comp_buffer_reset_sink_list(buffer); |
There was a problem hiding this comment.
each change in this commit is simple, but there are so many of them... Maybe this could be split in 1 commit per accessor?
kv2019i
reviewed
Feb 3, 2025
comp_buffer::sink_list field used to have 2 meanings:
- a list connector of all data receivers from the module
in this case list is based on
comp_dev::bsource_list
- a list connector for data receivers for extra modules
allocated when raw data processing is in use
in this case list is based on
processing_module::sink_buffer_list
This commit separate both cases
case 1 - no changes
case 2 -
list is now based on processing_module::raw_data_buffers_list
list is connected by a new field comp_buffer::buffers_list
Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
All control structures of buffers shared between cores are now stored in a non-cached memory alias. Cache writeback_invalidate is no longer needed here Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
All componetns had been modified to use buffer's API some time ago, those 2 were either missed or have been modified later Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
this commit adds 4 methods to comp_buffer API and refactors one macro to a static inline function in .c file the reason - macro was used once only and provides no type control Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
marcinszkudlinski
force-pushed
the
pipeline2_0_7
branch
2 times, most recently
from
91a562f to
4b5fc69
Compare
lgirdwood
reviewed
Feb 10, 2025
| @@ -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); | |||
There was a problem hiding this comment.
What do we mean by "raw" data processing here, how is this different from source/sink ?
There was a problem hiding this comment.
"raw data" is used at many places
/*
* helpers to determine processing type
* Needed till all the modules use PROCESSING_MODE_SINK_SOURCE
*/
#define IS_PROCESSING_MODE_RAW_DATA(mod) ((mod)->proc_type == MODULE_PROCESS_TYPE_RAW)
/**
* process_raw_data (depreciated)
* - sources are input_stream_buffer[]
* - sources[].data is a pointer to raw audio data
* - sinks are output_stream_buffer[]
* - sinks[].data is a pointer to raw audio data
*/
int (*process_raw_data)(struct processing_module *mod,
struct input_stream_buffer *input_buffers,
int num_input_buffers,
struct output_stream_buffer *output_buffers,
int num_output_buffers);
lgirdwood
approved these changes
Feb 11, 2025
|
@marcinszkudlinski can you check CI testbench, looks like we maybe reading next if it does not exist ? |
add comp_dev_for_each_consumer_safe and comp_dev_for_each_producer_safe Those macros allow iteration through all buffers allowing buffers deletion Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
Unit test should also - if possible - use buffer API There are, however, some parts of testing that still use direct access to comp_buffer internals. Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
This commit make all pipeline management code using comp_buffer API. Since now, no access to comp_buffer internals should be performed by the SOF code (with exception of uint test) Signed-off-by: Marcin Szkudlinski <marcin.szkudlinski@intel.com>
marcinszkudlinski
force-pushed
the
pipeline2_0_7
branch
from
4b5fc69 to
1c28401
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the pipeline management code use API to access comp_buffer
it had been done similarly in the modules some time ago
Since now all access to comp_buffer lists should be performed through an API
There's one exception - unit testing that perform an unusual operations, that should not be available in other cases