Skip to content

soc: ace: fix IPC D3 power state entry #6440

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.

Sign up for GitHub

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

Jump to bottom
Merged
kv2019i merged 1 commit into from
Dec 22, 2022
Merged

soc: ace: fix IPC D3 power state entry #6440

kv2019i merged 1 commit into from
Dec 22, 2022

Conversation

@aborisovich
Copy link
Contributor

@aborisovich aborisovich commented Oct 18, 2022 •
edited
Loading

Fist commit
Patch fixes D3 issue - now platform enters and wakes up from D3 correctly.
It had been achieved using Zephyr cpu function call that redirects D3 flow to power down assembly code
sending IPC response to host just before powering down.

Second commit
Adds pm notifiers to Zephyr power manager to allocate IMR memory inside SOF code in order to save LPSRAM and HPSRAM memory before powering down.
Edit: dropped for now due to missing implementation of devices context restore.
zephyrproject-rtos/zephyr#53105 will be used instead to skip context restore for now.
It will be implemented soontm.

Requires:

Signed-off-by: Andrey Borisovich andrey.borisovich@intel.com
Signed-off-by: Marcin Szkudlinski marcin.szkudlinski@intel.com
Signed-off-by: Tomasz Leman tomasz.m.leman@intel.com

lyakh
lyakh reviewed Oct 19, 2022
View reviewed changes
@aborisovich aborisovich force-pushed the ace-ipc-d3-fix branch 2 times, most recently from cac13e6 to 115cddb Compare October 19, 2022 09:23
@aborisovich aborisovich requested a review from lyakh October 19, 2022 09:23
lyakh
lyakh reviewed Oct 20, 2022
View reviewed changes
@lgirdwood
Copy link
Member

lgirdwood commented Oct 26, 2022

@aborisovich fyi - #6041 is now merged

@aborisovich aborisovich marked this pull request as ready for review October 26, 2022 23:23
@aborisovich aborisovich changed the title soc: ace: fix IPC D3 power state entry [DO NOT MERGE] soc: ace: fix IPC D3 power state entry Oct 27, 2022
@aborisovich
Copy link
Contributor Author

aborisovich commented Oct 27, 2022

Awaits full scope results please do not merge for now.

@lgirdwood
Copy link
Member

lgirdwood commented Oct 27, 2022

Awaits full scope results please do not merge for now.

@aborisovich Looks like CI is good, but pls ping when ready.

@aborisovich
Copy link
Contributor Author

aborisovich commented Nov 2, 2022

Awaits full scope results please do not merge for now.

@aborisovich Looks like CI is good, but pls ping when ready.

It fails all MTL tests as Zephyr Power Manager is not enabled. Waits for #6489 .

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 2, 2022

@aborisovich Please also check #6515

@kv2019i kv2019i mentioned this pull request Nov 2, 2022
Merged
@lgirdwood
Copy link
Member

lgirdwood commented Nov 2, 2022

@aborisovich @kv2019i we are blocking on the CI update for the dependencies ...

tmleman
tmleman requested changes Nov 3, 2022
View reviewed changes
@aborisovich aborisovich requested a review from tmleman November 5, 2022 10:51
@aborisovich
Copy link
Contributor Author

aborisovich commented Nov 5, 2022

Internal Intel CI System/merge/build broken?
Opens empty page on details click.

@aborisovich
Copy link
Contributor Author

aborisovich commented Nov 5, 2022 •
edited
Loading

Executed tests for MTL manually. Test load_fw_gracefull_shutdown passes now, but tests D3_master_core keep failing (however SetDx response is received and lack of it was the cause of this bug).
@marcinszkudlinski I assume this may be caused by missing IMR logic for context_save_restore?

@lgirdwood
Copy link
Member

lgirdwood commented Nov 7, 2022

@aborisovich both CI's are impacted by the 01.org server move. I hope to have it resolved today.

@aborisovich aborisovich marked this pull request as draft November 10, 2022 09:52
@kv2019i kv2019i changed the title soc: ace: fix IPC D3 power state entry [DNM] soc: ace: fix IPC D3 power state entry Dec 14, 2022
@kv2019i
Copy link
Collaborator

kv2019i commented Dec 14, 2022

Do not merge please, looks like we have randomly failing HDA tests...

Ack, I added DNM to PR subject, please remove when resolved.

@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 15, 2022

Improvement - ipc->task_mask is now zeroed before entering D3 for MTL.
Before - flag value had been restored after wake from D3 what caused FW to enter D3 again on the first incoming IPC.
Still IPC timeout after wake from D3, work in progress.

@lgirdwood
Copy link
Member

lgirdwood commented Dec 15, 2022

@aborisovich can you check CI, not sure if we are testing this case today on MTL.

@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 15, 2022

@aborisovich can you check CI, not sure if we are testing this case today on MTL.

I've asked validation team yesterday to add two more tests related to D3 - enter D3 with and without power gating.
So they are added now and the one with power gating is still failing.

ranj063
ranj063 reviewed Dec 15, 2022
View reviewed changes
@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 16, 2022 •
edited
Loading

Discovered bug cause - we do not have implemented device reinitialization (context restore) after boot from IMR what causes FW to idle after waking up from D3 state and not responding to IPCs.
We had discussed that we will use work-around for now to implement D3 state skipping context restore.
@tmleman prepared a patch to Zephyr zephyrproject-rtos/zephyr#53105 which is a dependency to this PR now (added to list in description). @marcinszkudlinski commit 65f829e34b39af6ad0e0675ab2e017955a2b92e8 will be dropped from this PR as it is too early to use pm_notifiers.

@aborisovich aborisovich force-pushed the ace-ipc-d3-fix branch 2 times, most recently from 0882fa0 to 2cf6b34 Compare December 16, 2022 15:51
@marc-hb
Copy link
Collaborator

marc-hb commented Dec 16, 2022

Switching this to a "draft" to make sure it does not get merged by accident while west.yml still points at https://github.com/tmleman/zephyr

@marc-hb marc-hb marked this pull request as draft December 16, 2022 21:54
@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 18, 2022

What happened to Zephyr workflow? It looks like it can not west clone the zephyr project?
image

@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 19, 2022

Fixed checkpatch.

soc: ace: fix IPC D3 power state entry
e8ce842 
This patch fixes D3 issue - now platform enters and wakes up from
D3 correctly on first iteration. This had been achieved using Zephyr
cpu function call that redirects D3 flow to power down assembly code
sending IPC response to host just before powering down.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@aborisovich aborisovich marked this pull request as ready for review December 21, 2022 16:23
@aborisovich aborisovich changed the title [DNM] soc: ace: fix IPC D3 power state entry soc: ace: fix IPC D3 power state entry Dec 21, 2022
@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 21, 2022

Still waiting for test results.

@aborisovich aborisovich changed the title soc: ace: fix IPC D3 power state entry [DNM[ soc: ace: fix IPC D3 power state entry Dec 21, 2022
@aborisovich aborisovich changed the title [DNM[ soc: ace: fix IPC D3 power state entry [DNM] soc: ace: fix IPC D3 power state entry Dec 21, 2022
@aborisovich aborisovich changed the title [DNM] soc: ace: fix IPC D3 power state entry soc: ace: fix IPC D3 power state entry Dec 21, 2022
@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 21, 2022 •
edited
Loading

PR can be merged now.
Last run of Internal CI had run additional D3 tests on MTL:

  • enter D3 without power gating ✅
  • enter D3 with power gating ✅
  • start new stream after D3 ✅
    All passed, good to go.

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 21, 2022

https://sof-ci.01.org/sofpr/PR6440/build3143/devicetest/index.html looks pretty bad.

@marc-hb
Copy link
Collaborator

marc-hb commented Dec 21, 2022 •
edited
Loading

SOFCI TEST

EDIT: https://sof-ci.01.org/sofpr/PR6440/build3152/devicetest/index.html

@aborisovich
Copy link
Contributor Author

aborisovich commented Dec 22, 2022

Well, rerun fixed the issue...
I guess the only one failing now: check-alsabat-headset-playback.sh for TGLU_UP_HDA_IPC4ZPH is a known issue yes?

@ranj063
Copy link
Collaborator

ranj063 commented Dec 22, 2022

Well, rerun fixed the issue... I guess the only one failing now: check-alsabat-headset-playback.sh for TGLU_UP_HDA_IPC4ZPH is a known issue yes?

yes, good to go!

@kv2019i kv2019i merged commit 80af480 into thesofproject:main Dec 22, 2022
@aborisovich aborisovich deleted the ace-ipc-d3-fix branch June 11, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ranj063 ranj063 ranj063 left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@abonislawski abonislawski abonislawski approved these changes

@kv2019i kv2019i kv2019i approved these changes

@marcinszkudlinski marcinszkudlinski marcinszkudlinski approved these changes

@tmleman tmleman tmleman approved these changes

@bardliao bardliao Awaiting requested review from bardliao bardliao is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@juimonen juimonen Awaiting requested review from juimonen

@lyakh lyakh Awaiting requested review from lyakh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@aborisovich @lgirdwood @kv2019i @lyakh @ranj063 @marc-hb @juimonen @abonislawski @marcinszkudlinski @tmleman