Skip to content
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

loadable module: define a new bit in fw_image_flags #191

Conversation

RanderWang
Copy link
Contributor

@RanderWang RanderWang commented Oct 30, 2023

The bit 4 is set to force driver reload library on d3 exit.

It is a prototype for validation with mtl-006 branch so I use hard core. We need to add a new feature that toml can decode macro definition like
fw_image_flag = BASE_FW | RELOAD_LIBRARY

Kernel PR thesofproject/linux#4669 depends on this rimage update.

src/manifest.c Show resolved Hide resolved
config/mtl.toml Outdated
@@ -55,6 +55,7 @@ base_offset = "0x2000"
[fw_desc.header]
name = "ADSPFW"
load_offset = "0x40000"
fw_image_flags = "0x12"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment that 0x12 is BASE_FW+CONTEXT_IS_NOT_SAVED or something like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, updated

The bit 4 is set to force driver reload library on d3 exit

Signed-off-by: Rander Wang <[email protected]>
@RanderWang
Copy link
Contributor Author

add more comments

@mengdonglin mengdonglin requested a review from mmaka1 October 30, 2023 11:41
@lgirdwood
Copy link
Member

lgirdwood commented Oct 30, 2023

Adding @mmaka1 and @mwasko for review

@mmaka1
Copy link

mmaka1 commented Oct 30, 2023

The ability to survive D3 is the property of FW, not the FW image binary (a file). It is known at the FW build time, therefore adding it to the binaries making this information instantly available to the driver looks like a good idea. But then it should be added consistently to all the binaries, otherwise it might be interpreted like a different behavior of individual binaries (base_fw/libs) while this defines a behavior of the FW related to all loaded binaries. So, it requires extra maintenance and looking from this perspective, adding this property to the IPC4 FW_CONFIG parameter of the Base FW looks like a simpler solution to maintain (adding more overhead at the run-time though) unless we agree to (a) develop an asymmetric Extended Manifest containing properties valid for the Base FW only or (b) agree that extra maintenance costs are OK or (c) agree that the flag is valid in case of the base_fw binary only and extra "if" in the driver's code is OK.

@marc-hb
Copy link
Contributor

marc-hb commented Oct 30, 2023

Sorry for adding another twist to an already complex topic but... why is this submitted directly to mtl-006-drop-stable? Shouldn't complex and long term design questions like this be debated and more importantly tested on the main development branch first before going to a maintenance branch?

Also, there is now a single git repo on the main branch which makes everything more direct, faster and easier to test (then later cherry-pick code already tested well to separate git repos)

@RanderWang
Copy link
Contributor Author

no need

@RanderWang RanderWang closed this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants