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

[Depends on 62522] zephyr: cavs: add secondary core context save support #8185

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

RanderWang
Copy link
Collaborator

@RanderWang RanderWang commented Sep 12, 2023

Register pm_state_notifier to set ready_flag for secondary core when it is powered up for second time after first fw boot. We can remove CONFIG_ADSP_IMR_CONTEXT_SAVE check for cavs platform for this feature.

zephyr side pr: zephyrproject-rtos/zephyr#62522

It has been discussed in #8156 and #8170

The IMR_CONTEXT_SAVE check was first introduced by #8027

@kv2019i kv2019i changed the title zephyr: cavs: add secondary core context save support [Depends on 62522] zephyr: cavs: add secondary core context save support Sep 12, 2023
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @RanderWang ! Have you made a test PR that combines this PR with a Zephyr update pointing to your Zephyr patch (like I did #8136 ). This would good step to make to ensure the combination passes all SOF CI tests.

The patch itself looks good. I won't mark approve yet to ensure this isn't accidentally merged too soon, but yes, looks good.

@RanderWang
Copy link
Collaborator Author

Thanks @RanderWang ! Have you made a test PR that combines this PR with a Zephyr update pointing to your Zephyr patch (like I did #8136 ). This would good step to make to ensure the combination passes all SOF CI tests.

The patch itself looks good. I won't mark approve yet to ensure this isn't accidentally merged too soon, but yes, looks good.

@kv2019i I tested it in local environment. Current Github CI test doesn't use multi-core topology so we can't depend on Github CI.
My next plan is to add multi-core support in cavs-nocodec and remove cavs-nocodec-multicore topology as we have discussed

@lgirdwood
Copy link
Member

@andyross need your re-review on the Zephyr dependecy to unblock this target for 3rd party SDV usage. Thanks !
@cujomalainey fyi.

@lgirdwood lgirdwood added this to the v2.8 milestone Oct 20, 2023
@RanderWang
Copy link
Collaborator Author

RanderWang commented Nov 6, 2023

@lgirdwood @kv2019i I tested this PR with multi-core tplg #8240 (include recent zephyr cavs multicore changes). All tests passed. Next I will update the west for zephyr and try to merge the topology PR. The dependent PR: zephyrproject-rtos/zephyr#62522 was merged.
Thanks!

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Looks good! I guess we can merge this already now, and merge the Zephyr part independently @RanderWang ?

@RanderWang
Copy link
Collaborator Author

Looks good! I guess we can merge this already now, and merge the Zephyr part independently @RanderWang ?

sure. Zephyr part has been merged. Thanks!

Register pm_state_notifier to set ready_flag for secondary core when it
is powered up for second time after first fw boot. We can remove
CONFIG_ADSP_IMR_CONTEXT_SAVE check for cavs platform for this feature.

Signed-off-by: Rander Wang <[email protected]>
@RanderWang RanderWang force-pushed the cavs-dsp-context-save branch from c7fafed to 96b4d94 Compare November 8, 2023 08:05
@kv2019i
Copy link
Collaborator

kv2019i commented Nov 8, 2023

@RanderWang I now pushed #8463 . The Zephyr change does not become visible to SOF until we bring it in via west.yml. Not sure whether we can merged this and #8463 independently, do we need to merge in particular order, or even at the same time...?

@RanderWang
Copy link
Collaborator Author

RanderWang commented Nov 9, 2023

@RanderWang I now pushed #8463 . The Zephyr change does not become visible to SOF until we bring it in via west.yml. Not sure whether we can merged this and #8463 independently, do we need to merge in particular order, or even at the same time...?

@kv2019i This PR is for multi-core feature and depends on topology to enable multi-core path. Now our topology is not merged and waiting for this PR so CI doesn't get any error. (topology PR: #8240)

@RanderWang RanderWang force-pushed the cavs-dsp-context-save branch from 96b4d94 to b818ec2 Compare November 9, 2023 02:24
@kv2019i kv2019i merged commit b7858e8 into thesofproject:main Nov 9, 2023
43 of 44 checks passed
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.

5 participants