-
Notifications
You must be signed in to change notification settings - Fork 80
Pipeline 2,0 #497
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.
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
base: master
Are you sure you want to change the base?
Pipeline 2,0 #497
Conversation
40b384e
to
16b730d
Compare
Please include fast mode task in implementation Pipeline 2.0 |
sink/src APIs should have states. Based on a state, module would know if the sink/source has already been configured and if it is allowed to process data from if / put result to it |
@marcinszkudlinski I think pipeline 2.0 has made some recent progress upstream, do you think we will be able to target this PR in Q4 for merge or do you still have many code PRs pending ? |
@lgirdwood currently the modules don't use any API for interaction with buffers, that makes any modifications extremely hard. The modules are bound hard to comp_buffer and some internal pipeline structures |
TODO: add probe support to generic sink/src - this should allow flawless work of probes at any point of pipeline |
This commit contains a description of a pipeline2.0 design Signed-off-by: Marcin Szkudlinski <[email protected]>
ea3b11f
to
d4515f4
Compare
RFC changed to a commit - please proceed with merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcinszkudlinski LGTM. I've spotted one spelling, will see if I can add co-pilot for review to pick up other spellings.
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data recievers than buffers. That generates a requirement: sink/source API must | ||
provide all required data/functions for a module to configure and use data source. For performance reasons there may be neccessary keeping a shortcuts to certains strucures, like audio data parameters, in all APIs. | ||
|
||
Also the pipeline code must maitain buffers using audio_buffer API only, there must not be any "side calls" to buffers code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces Pipeline 2.0 design documentation for the SOF (Sound Open Firmware) project. The proposal aims to make the audio pipeline more flexible, optimal, multi-core friendly, and better suited for IPC4 while maintaining IPC3 backward compatibility.
- Introduces new scheduling types (LL and DP) with cross-core capabilities
- Proposes a new sink/source API for flexible data flow between modules
- Defines multiple buffer types optimized for different use cases
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
File | Description |
---|---|
architectures/firmware/sof-common/pipeline_2_0/pipeline2_0.rst |
Main document describing Pipeline 2.0 architecture, scheduling, APIs, and buffer management |
architectures/firmware/sof-common/pipeline_2_0/index.rst |
Index file for Pipeline 2.0 documentation |
architectures/firmware/sof-common/index.rst |
Updated to include Pipeline 2.0 documentation |
Multiple .pu files |
PlantUML diagram files illustrating various pipeline connection patterns |
Comments suppressed due to low confidence (1)
architectures/firmware/sof-common/pipeline_2_0/images/shaerd_buffer_1.pu:1
- The filename contains a typo: 'shaerd_buffer_1.pu' should be 'shared_buffer_1.pu' to match the intended 'shared buffer' terminology used in the document.
left to right direction
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The purpose of the changes is to make the pipeline: | ||
|
||
- more flexible | ||
- more optimal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing space after 'optimal'.
- more optimal | |
- more optimal |
Copilot uses AI. Check for mistakes.
|
||
LL5, LL6, LL1, LL2, LL3, LL4 | ||
|
||
Agin, LL3 is able to work having data on both inputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'Agin' should be 'Again'.
Agin, LL3 is able to work having data on both inputs | |
Again, LL3 is able to work having data on both inputs |
Copilot uses AI. Check for mistakes.
|
||
LL5, LL6, LL1, LL2, LL3, LL4 | ||
|
||
Agin, LL3 is able to work having data on both inputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'Agin' should be 'Again'.
Agin, LL3 is able to work having data on both inputs | |
Again, LL3 is able to work having data on both inputs |
Copilot uses AI. Check for mistakes.
|
||
Backward compatibility: legacy recurrency based algorithm should be kept in case of IPC3 and be used for creating module iteration list. | ||
|
||
DP modules are also a part of pipelies, but they are not a part of LL scheduling. That means DP modules data processing is excluded from module iteration list. DP, however, is still a part of a pipeline, so it should be included in pipeline iteration list for all other operations, like pipeline start/stop etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'pipelies' should be 'pipelines'.
DP modules are also a part of pipelies, but they are not a part of LL scheduling. That means DP modules data processing is excluded from module iteration list. DP, however, is still a part of a pipeline, so it should be included in pipeline iteration list for all other operations, like pipeline start/stop etc. | |
DP modules are also a part of pipelines, but they are not a part of LL scheduling. That means DP modules data processing is excluded from module iteration list. DP, however, is still a part of a pipeline, so it should be included in pipeline iteration list for all other operations, like pipeline start/stop etc. |
Copilot uses AI. Check for mistakes.
======================= | ||
|
||
*Although SOF is written in C, not C++, lets use C++ naming conventions for classes and objects* | ||
Current pipeline processing is based on comp_buffer structure, wchich is providing much more than just buffering. It is a base for module binding, iteration etc. This is a source of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'wchich' should be 'which'.
Current pipeline processing is based on comp_buffer structure, wchich is providing much more than just buffering. It is a base for module binding, iteration etc. This is a source of | |
Current pipeline processing is based on comp_buffer structure, which is providing much more than just buffering. It is a base for module binding, iteration etc. This is a source of |
Copilot uses AI. Check for mistakes.
|
||
(1.0 compatibility) before 2.0 is implemented, some features for module binding (ilke "is_walked" flag, etc.) must be also put into audio_buffer API. | ||
|
||
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data recievers than buffers. That generates a requirement: sink/source API must |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'recievers' should be 'receivers'.
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data recievers than buffers. That generates a requirement: sink/source API must | |
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data receivers than buffers. That generates a requirement: sink/source API must |
Copilot uses AI. Check for mistakes.
(1.0 compatibility) before 2.0 is implemented, some features for module binding (ilke "is_walked" flag, etc.) must be also put into audio_buffer API. | ||
|
||
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data recievers than buffers. That generates a requirement: sink/source API must | ||
provide all required data/functions for a module to configure and use data source. For performance reasons there may be neccessary keeping a shortcuts to certains strucures, like audio data parameters, in all APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple typos: 'neccessary' should be 'necessary', 'a shortcuts' should be 'shortcuts', 'certains strucures' should be 'certain structures'.
provide all required data/functions for a module to configure and use data source. For performance reasons there may be neccessary keeping a shortcuts to certains strucures, like audio data parameters, in all APIs. | |
provide all required data/functions for a module to configure and use data source. For performance reasons there may be necessary keeping shortcuts to certain structures, like audio data parameters, in all APIs. |
Copilot uses AI. Check for mistakes.
Note that modules have access to sink or source API only, modules cannot use audio_buffer API because (as described) there are more types of data sources / data recievers than buffers. That generates a requirement: sink/source API must | ||
provide all required data/functions for a module to configure and use data source. For performance reasons there may be neccessary keeping a shortcuts to certains strucures, like audio data parameters, in all APIs. | ||
|
||
Also the pipeline code must maitain buffers using audio_buffer API only, there must not be any "side calls" to buffers code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'maitain' should be 'maintain'.
Also the pipeline code must maitain buffers using audio_buffer API only, there must not be any "side calls" to buffers code. | |
Also the pipeline code must maintain buffers using audio_buffer API only, there must not be any "side calls" to buffers code. |
Copilot uses AI. Check for mistakes.
In case of a typical LL pipeline, each of the modules is processing a complete set of data on its input and produce a complete set of data on output. That typically means 16 - 48 audio frames per LL cycle. The requirement is that the input buffer(s) is always drained completely (unless explicitely requested). | ||
In case of LL chain of modules within a single pipeline: | ||
|
||
.. uml:: images/shaerd_buffer_1.pu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in filename: 'shaerd_buffer_1.pu' should be 'shared_buffer_1.pu'.
.. uml:: images/shaerd_buffer_1.pu | |
.. uml:: images/shared_buffer_1.pu |
Copilot uses AI. Check for mistakes.
|
||
*implentation details - TODO* | ||
|
||
The price of course will be some additional cycles for data copying, befefits - data linearity and less memory usage. Of course if a module request for renention is close to the buffer size, it makes no sense. In this case probably a "cross core lockless data buffer" or "cross core linearization data buffer" will be more optimal. It is up to the buffer factory to decide each time what kind of buffer is more optimal and should be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: 'befefits' should be 'benefits'.
The price of course will be some additional cycles for data copying, befefits - data linearity and less memory usage. Of course if a module request for renention is close to the buffer size, it makes no sense. In this case probably a "cross core lockless data buffer" or "cross core linearization data buffer" will be more optimal. It is up to the buffer factory to decide each time what kind of buffer is more optimal and should be created. | |
The price of course will be some additional cycles for data copying, benefits - data linearity and less memory usage. Of course if a module request for renention is close to the buffer size, it makes no sense. In this case probably a "cross core lockless data buffer" or "cross core linearization data buffer" will be more optimal. It is up to the buffer factory to decide each time what kind of buffer is more optimal and should be created. |
Copilot uses AI. Check for mistakes.
DO NOT MERGE - DISCUSSION ONLY
After long time I could proceed with Pipeline 2.0 design
To make tracking changes and discussions easier, I translated the initial discussion to .rst format an put it on my private branch
Initial headsup:
thesofproject/sof#7261
discussion and 1st version of the document
https://github.com/orgs/thesofproject/discussions/8645
current version of the document:
https://marcinszkudlinski.github.io/sof-docs/PAGES/architectures/firmware/sof-common/pipeline_2_0/pipeline2_0_discussion.html
the above HTML is a compilation of this PR
please - read it and comment in this PR. I will add all changes as commits to this branch so we'll be able to track changes