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

reload library based on fw_image_flag in fw manifest #4669

Closed

Conversation

RanderWang
Copy link

@RanderWang RanderWang commented Oct 27, 2023

Discussed in converged fw sync meeting and got a conclusion that use a manifest flag
We will set fw_image_flag for library reload in fw config file so library will be reloaded by default. This will not affect ref fw.

Validated on mtl device, with mtl-006 and rimage PR thesofproject/rimage#191

@RanderWang RanderWang force-pushed the library_reload branch 3 times, most recently from fa0ecc0 to 2c20825 Compare October 27, 2023 07:51
@RanderWang RanderWang changed the title reload library based on feature_mask in fw manifest reload library based on fw_image_flag in fw manifest Oct 27, 2023
@RanderWang RanderWang force-pushed the library_reload branch 3 times, most recently from 5cadd53 to abb78cc Compare October 30, 2023 03:08
Copy link
Collaborator

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

@RanderWang, I did not even knew that the fw image have existing flag, this makes it clear. Great!
Few nitpicks from my side.

#define SOF_IPC4_IMAGE_TYPE GENMASK(2, 1) /* 0 - ROM, 1 - Base FW, 2 -library */
#define SOF_IPC4_GET_IMAGE_TYPE(x) (((x) & SOF_IPC4_IMAGE_TYPE) >> 1)
#define SOF_IPC4_RELOCATABLE_LIB BIT(3)
#define SOF_IPC4_LIBRARY_RELOAD BIT(4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please align the lines

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mwasko Are we aligned on this?

Copy link
Author

Choose a reason for hiding this comment

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

thanks, updated

if (reload && hda->booted_from_imr)
/* if IMR booting is enabled and library reload is not defined, skip the loading */
if (reload && hda->booted_from_imr &&
!(ipc4_data->fw_image_flags & SOF_IPC4_LIBRARY_RELOAD))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would opt for a more generic name still, like SOF_IPC4_CONTEXT_NOT_PRESERVED or something (probably toss in D3 to IMR? or just use comment to clarify it's meaning and keeping it generic).
If we ever want or need to experiment with the context save/restore at large in other parts of the code.

Copy link
Author

Choose a reason for hiding this comment

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

@ujfalusi I use the name SOF_IPC4_FW_CONTEXT_LOST, how about it ? Thanks!

@kv2019i kv2019i requested a review from mwasko October 30, 2023 08:39
Driver gets fw_image_flags fw manifest and adopts features based on
this flag.

Signed-off-by: Rander Wang <[email protected]>
If library reload is not defined by fw, driver can skip library reload on
d3 exit or reload library.

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

updated flag name

Copy link
Collaborator

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

@RanderWang, @mwasko, assuming we are aligned with SOF and reference firmware.

@RanderWang
Copy link
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.

3 participants