-
Notifications
You must be signed in to change notification settings - Fork 349
manifest: rimage: Update manifest module type with reference FW state #10219
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
Conversation
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 updates the sof_man_module_type structure in the rimage manifest to align with the reference firmware layout by adding new bit fields and reordering existing ones.
- Adds new bit fields:
domain_rtos,core_type,user_mode,large_param, andstack_on_bss - Moves the previously added
init_configfield to the end of the structure - Adjusts reserved bits to maintain proper structure alignment
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
319d4fc to
48fffe0
Compare
| uint32_t large_param:1; | ||
| uint32_t stack_on_bss:1; | ||
| uint32_t rsvd_:9; | ||
| uint32_t init_config:4; /* SOF_MAN_MOD_INIT_CONFIG_ */ |
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.
@kv2019i could you please check if we can move init_config? Looks like huge ABI change
kv2019i
left a comment
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.
The interface cannot be changed without breaking existing systems. Please see inline
| uint32_t large_param:1; | ||
| uint32_t stack_on_bss:1; | ||
| uint32_t rsvd_:9; | ||
| uint32_t init_config:4; /* SOF_MAN_MOD_INIT_CONFIG_ */ |
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.
I'm afraid this is too late. We have already upstreamed the module type definition to Linux upstream and we cannot change this now without breaking end-user systems. FYI @abonislawski @lgirdwood @ujfalusi
Options, 1) change non-SOF implementation, or 2) sync up in a later generation (i.e. branch the definition for a future generation).
In Linux, the old definition is used in linux/sound/soc/sof/ipc4-topology.h
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.
@softwarecki we've got the 20 reserved bits which come after the init_config in upstream version, we need to add the new fields into reserved only (and not move the init_config). This should keep us ABI compatible for a long time.
lgirdwood
left a comment
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.
Needs to extend the ABI only and not break compatibility.
| uint32_t large_param:1; | ||
| uint32_t stack_on_bss:1; | ||
| uint32_t rsvd_:9; | ||
| uint32_t init_config:4; /* SOF_MAN_MOD_INIT_CONFIG_ */ |
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.
@softwarecki we've got the 20 reserved bits which come after the init_config in upstream version, we need to add the new fields into reserved only (and not move the init_config). This should keep us ABI compatible for a long time.
48fffe0 to
fc91bbf
Compare
The reference FW added some new definitions to manifest man_module_type structure. The SOF version must be updated to synchronize with reference FW definitions. Added fields: - domain_rtos - indicates whether a module can be executed in RTOS scheduling domain; - core_type - LX7 HiFi4, Fusion etc. - user_mode - module executed in non-priviledged mode - large_param - IPC supports parameters above 4kB - stack_on_bss - allocate DP task stack in module bss section Signed-off-by: Jaroslaw Stelter <[email protected]>
fc91bbf to
58d8df7
Compare
kv2019i
left a comment
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.
Thanks, this will work with current Linux kernel driver.
Update
sof_man_module_typeto match the layout defined in the reference firmware. This includesreordering andextending the structure with new fields such asdomain_rtos,core_type,user_mode,large_param, andstack_on_bss.The previously addedinit_configfield (15ea481 thesofproject/rimage#130), which had been added earlier using reserved bits has been moved to the end of the structure to avoid layout conflicts and maintain compatibility with the reference FW.This change is required for ABI alignment but will cause CI failures due to the need for a software rebuild.