Skip to content

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

Merged
kv2019i merged 1 commit into
thesofproject:mainfrom
RanderWang:cavs-dsp-context-save
Nov 9, 2023
Merged

[Depends on 62522] zephyr: cavs: add secondary core context save support#8185
kv2019i merged 1 commit into
thesofproject:mainfrom
RanderWang:cavs-dsp-context-save

Conversation

@RanderWang

@RanderWangRanderWang commented Sep 12, 2023

Copy link
Copy Markdown
Collaborator

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

@kv2019ikv2019i changed the title zephyr: cavs: add secondary core context save support[Depends on 62522] zephyr: cavs: add secondary core context save supportSep 12, 2023

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
CollaboratorAuthor

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

Comment threadzephyr/lib/cpu.c
@lgirdwood

Copy link
Copy Markdown
Member

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

@lgirdwoodlgirdwood added this to the v2.8 milestone Oct 20, 2023
@RanderWang

RanderWang commented Nov 6, 2023

Copy link
Copy Markdown
CollaboratorAuthor

@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!

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
CollaboratorAuthor

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 <rander.wang@intel.com>
@kv2019i

Copy link
Copy Markdown
Collaborator

@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

RanderWang commented Nov 9, 2023

Copy link
Copy Markdown
CollaboratorAuthor

@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)

@kv2019i
kv2019i merged commit b7858e8 into thesofproject:mainNov 9, 2023
Sign up for freeto 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

@RanderWang@lgirdwood@kv2019i@lyakh@tmleman