Skip to content

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

Merged
kv2019i merged 1 commit into
thesofproject:mainfrom
aborisovich:ace-ipc-d3-fix
Dec 22, 2022
Merged

soc: ace: fix IPC D3 power state entry#6440
kv2019i merged 1 commit into
thesofproject:mainfrom
aborisovich:ace-ipc-d3-fix

Conversation

@aborisovich

@aborisovichaborisovich commented Oct 18, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

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
Comment threadsrc/ipc/ipc-zephyr.c Outdated
Comment threadsrc/ipc/ipc-zephyr.c Outdated
Comment threadsrc/ipc/ipc4/handler.c Outdated
Comment threadsrc/platform/intel/ace/platform.c Outdated
@aborisovich
aborisovichforce-pushed the ace-ipc-d3-fix branch 2 times, most recently from cac13e6 to 115cddbCompareOctober 19, 2022 09:23
lyakh
lyakh reviewed Oct 20, 2022
View reviewed changes
Comment threadsrc/ipc/ipc-zephyr.c Outdated
@lgirdwood

lgirdwood commented Oct 26, 2022

Copy link
Copy Markdown
Member

@aborisovich fyi - #6041 is now merged

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

aborisovich commented Oct 27, 2022

Copy link
Copy Markdown
ContributorAuthor

Awaits full scope results please do not merge for now.

@lgirdwood

lgirdwood commented Oct 27, 2022

Copy link
Copy Markdown
Member

Awaits full scope results please do not merge for now.

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

@aborisovich

aborisovich commented Nov 2, 2022

Copy link
Copy Markdown
ContributorAuthor

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

kv2019i commented Nov 2, 2022

Copy link
Copy Markdown
Collaborator

@aborisovich Please also check #6515

@kv2019ikv2019i mentioned this pull request Nov 2, 2022
Merged
@lgirdwood

lgirdwood commented Nov 2, 2022

Copy link
Copy Markdown
Member

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

tmleman
tmleman requested changes Nov 3, 2022
View reviewed changes
Comment threadsrc/platform/intel/ace/platform.c Outdated
@aborisovich

aborisovich commented Nov 5, 2022

Copy link
Copy Markdown
ContributorAuthor

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

@aborisovich

aborisovich commented Nov 5, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

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

lgirdwood commented Nov 7, 2022

Copy link
Copy Markdown
Member

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

aborisovich commented Dec 14, 2022

Copy link
Copy Markdown
ContributorAuthor

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

@kv2019ikv2019i changed the title soc: ace: fix IPC D3 power state entry[DNM] soc: ace: fix IPC D3 power state entryDec 14, 2022
@kv2019i

kv2019i commented Dec 14, 2022

Copy link
Copy Markdown
Collaborator

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

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

@aborisovich

aborisovich commented Dec 15, 2022

Copy link
Copy Markdown
ContributorAuthor

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

lgirdwood commented Dec 15, 2022

Copy link
Copy Markdown
Member

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

@aborisovich

aborisovich commented Dec 15, 2022

Copy link
Copy Markdown
ContributorAuthor

@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
Comment threadsrc/platform/intel/ace/platform.c Outdated
@aborisovich

aborisovich commented Dec 16, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

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
aborisovichforce-pushed the ace-ipc-d3-fix branch 2 times, most recently from 0882fa0 to 2cf6b34CompareDecember 16, 2022 15:51
@marc-hb

marc-hb commented Dec 16, 2022

Copy link
Copy Markdown
Collaborator

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

aborisovich commented Dec 18, 2022

Copy link
Copy Markdown
ContributorAuthor

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

@aborisovich

aborisovich commented Dec 19, 2022

Copy link
Copy Markdown
ContributorAuthor

Fixed checkpatch.

Andrey Borisovich
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
@aborisovichaborisovich changed the title [DNM] soc: ace: fix IPC D3 power state entrysoc: ace: fix IPC D3 power state entryDec 21, 2022
@aborisovich

aborisovich commented Dec 21, 2022

Copy link
Copy Markdown
ContributorAuthor

Still waiting for test results.

@aborisovichaborisovich changed the title soc: ace: fix IPC D3 power state entry[DNM[ soc: ace: fix IPC D3 power state entryDec 21, 2022
@aborisovichaborisovich changed the title [DNM[ soc: ace: fix IPC D3 power state entry[DNM] soc: ace: fix IPC D3 power state entryDec 21, 2022
@aborisovichaborisovich changed the title [DNM] soc: ace: fix IPC D3 power state entrysoc: ace: fix IPC D3 power state entryDec 21, 2022
@aborisovich

aborisovich commented Dec 21, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

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

marc-hb commented Dec 21, 2022

Copy link
Copy Markdown
Collaborator

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

@marc-hb

marc-hb commented Dec 21, 2022•
edited
Loading

Copy link
Copy Markdown
Collaborator

SOFCI TEST

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

@aborisovich

aborisovich commented Dec 22, 2022

Copy link
Copy Markdown
ContributorAuthor

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

ranj063 commented Dec 22, 2022

Copy link
Copy Markdown
Collaborator

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!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ranj063ranj063ranj063 left review comments
@lgirdwoodlgirdwoodlgirdwood approved these changes
@abonislawskiabonislawskiabonislawski approved these changes
@kv2019ikv2019ikv2019i approved these changes
@marcinszkudlinskimarcinszkudlinskimarcinszkudlinski approved these changes
@tmlemantmlemantmleman approved these changes
@bardliaobardliaoAwaiting requested review from bardliaobardliao is a code owner
@plbossartplbossartAwaiting requested review from plbossartplbossart is a code owner
@mmaka1mmaka1Awaiting requested review from mmaka1mmaka1 is a code owner
@lbetlejlbetlejAwaiting requested review from lbetlejlbetlej is a code owner
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner
@juimonenjuimonenAwaiting requested review from juimonen
@lyakhlyakhAwaiting 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