Pipeline2.0 step 7: Make all the code use API to access comp_buffer lists#9802
Merged
lgirdwood merged 8 commits intothesofproject:mainfrom Feb 26, 2025
Merged
Conversation
marcinszkudlinski
requested review from
bardliao,
dbaluta,
jxstelter,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
pblaszko,
plbossart,
ranj063 and
singalsu
as code owners
marcinszkudlinski
changed the title
singalsu
approved these changes
Jan 29, 2025
lyakh
reviewed
Jan 30, 2025
| 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)); |
Collaborator
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); |
Collaborator
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
Collaborator
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Last commit is hard to review, but looks fine. Guennadi spotted some issues, especially the attach/detach needs address, with those this seems good to go.
marcinszkudlinski
force-pushed
the
pipeline2_0_7
branch
2 times, most recently
from
91a562f to
4b5fc69
Compare
lgirdwood
reviewed
Feb 10, 2025
Member
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM, but can we add some more comments/context regarding "raw" processing mode.We can merge after that.
lgirdwood
approved these changes
Feb 11, 2025
Member
|
@marcinszkudlinski can you check CI testbench, looks like we maybe reading next if it does not exist ? |
marcinszkudlinski
force-pushed
the
pipeline2_0_7
branch
2 times, most recently
from
1c28401 to
822e3cd
Compare
Contributor
Author
|
null pointer dereference fixed, still some problems in internal CI, checking... EDIT: re-run has passed. |
Member
This showed up on LNL in jenkins: |
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>
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>
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
822e3cd to
5c76f6b
Compare
Contributor
Author
|
rebase only |
lgirdwood
approved these changes
Feb 24, 2025
Member
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Now lets just wait for CI results
Member
|
CI ran out of disk. Rerun to be sure. |
Member
|
SOFCI TEST |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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