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

[v2.7] Disable IMR context save for MTL [alternative comment wording] #8170

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Sep 6, 2023

We really need this for v2.7 and rc1 cut-off is planned on Friday, so let me propose an alternative comment wording for
#8156

We can merge either that PR or this, but we really need to get this forward (as we agree on the code and this is now about the comment).

Intel ACE platform has implemented runtime-idle based management of
secondary cores. On these systems, z_init_cpu() must be only run when
pm_state_next_get() returns PM_STATE_ACTIVE, or otherwise the kernel
structs and idle thread stack get overwritten.

On other SOF platforms with Zephyr support, z_init_cpu()
must be always run at cpu_enable_core().

Signed-off-by: Mengdong Lin <[email protected]>
Signed-off-by: Kai Vehmanen <[email protected]>
Disable IMR context save on MTL by default, as it isn't a mandatory
feature for MTL.

The feature implementation is kept. So users can try this feature by
setting CONFIG_ADSP_IMR_CONTEXT_SAVE=y

Signed-off-by: Mengdong Lin <[email protected]>
@marc-hb
Copy link
Collaborator

marc-hb commented Sep 6, 2023

The only two MTL tests that failed in https://sof-ci.01.org/sof-pr-viewer/#/build/PR8170/build12739722 are called test_00_08_start_paused_stream_after_d3 and test_02_09_memory_with_modules_d0_d3_state so I'm afraid this code change is guilty. See error messages below.

Same test results, failures and error messages in earlier version of this PR at https://sof-ci.01.org/sof-pr-viewer/#/build/PR8156/build12735262 so these failures are deterministic.

Other PRs do not fail like this, for instance this very recent one is all green:
https://sof-ci.01.org/sof-pr-viewer/#/build/PR8163/build12738280

10:36:23,866 INFO  - cavs_scripts-py\utilities\ipc_driver.py:175: in raise_or_save
10:36:23,866 INFO  - raise error
10:36:23,866 INFO  - E utilities.ipc_driver_exceptions.AvsIpcErrorResponseException: FW responded with error
10:36:23,866 INFO  - E FW status: 0x00000005
10:36:23,866 INFO  - E IPC error: 9 (ADSP_IPC_INVALID_RESOURCE_ID)
10:36:23,866 INFO  - E ADSP mailbox error: 108 (ADSP_FREE_SHARED_VAR_FAILED_BAD_ALIGNMENT)
10:36:32,097 INFO  - cavs_scripts-py\utilities\ipc_driver.py:175: in raise_or_save
10:36:32,097 INFO  - raise error
10:36:32,097 INFO  - E utilities.ipc_driver_exceptions.AvsIpcErrorResponseException: FW responded with error
10:36:32,097 INFO  - E FW status: 0x00000005
10:36:32,097 INFO  - E IPC error: 9 (ADSP_IPC_INVALID_RESOURCE_ID)
10:36:32,097 INFO  - E ADSP mailbox error: 1 (ADSP_ERROR_INVALID_PARAM)

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 6, 2023

I read #7994 (two layers of indirections...) and realized this is only for the 2.7 branch and will never be submitted for main. So maybe the test failures are expected and OK.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 7, 2023

@marc-hb ack, so tests "test_00_08_start_paused_stream_after_d3 and test_02_09_memory_with_modules_d0_d3_state" test exactly the feature that is disabled in this PR, so the tests are expected to fail. Thus this is ok (for SOF2.7 as this feature is not used by Linux SOF driver).

Copy link
Collaborator

@RanderWang RanderWang left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tmleman tmleman left a comment

Choose a reason for hiding this comment

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

test_00_08_start_paused_stream_after_d3 and test_02_09_memory_with_modules_d0_d3_state use disabled feature so fail is expected. The first commit should be cherry-pick to main branch.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 7, 2023

Thanks all, proceeding with merge!

@kv2019i kv2019i merged commit d829c10 into thesofproject:stable-v2.7 Sep 7, 2023
40 of 41 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.

8 participants