core: set timeout to 400ms for secondary core disabling #8149
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RanderWang
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
RanderWang
requested review from
abonislawski,
kv2019i,
lgirdwood,
lyakh,
mwasko,
plbossart and
tmleman
and removed request for
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
RanderWang
force-pushed
the
disable_secondary_core
branch
from
98ce87c to
cc5c9a5
Compare
btian1
approved these changes
Sep 5, 2023
Contributor
btian1
left a comment
btian1
left a comment
There was a problem hiding this comment.
assumed 100ms used to do communication and disabling is enough?
kv2019i
requested changes
Sep 5, 2023
Collaborator
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Let's just change the default.
src/platform/Kconfig
Outdated
| config SECONDARY_CORE_DISABLING_TIMEOUT | ||
| int | ||
| int "Secondary core disabling timeout" | ||
| default 5000 |
Collaborator
There was a problem hiding this comment.
@RanderWang Can you make a patch that changes the default to 400ms instead? SOF driver and firmware are hosted in same public project, so it makes sense to adapt any defaults between the two main entities. Special configs (like for FPGA) should be the exception, not the norm. Otherwise we need to keep doing the same change for all new platforms..
Contributor
There was a problem hiding this comment.
FPGA does need 5000ms, in future, if SOF need run on FPGA again, we'd better to have a mechanism to change it back.
The CONFIG_SECONDARY_CORE_DISABLING_TIMEOUT is for FW to do secondary core disabling check and set to 5000ms by default. Linux driver set ipc timeout duration to 500ms, but FW wait 5000ms in cpu_disable_core for SET_DX ipc message. This makes driver stop but fw is still waiting. Actually the disabling can be successful in far less than 5000ms and it was first designed for FPGA case. Now change it to 400ms to make debugging easier. Signed-off-by: Rander Wang <rander.wang@intel.com>
RanderWang
force-pushed
the
disable_secondary_core
branch
from
cc5c9a5 to
5d3c504
Compare
tmleman
approved these changes
Sep 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Linux driver set ipc timeout duration to 500ms, but FW wait 5000ms in cpu_disable_core for SET_DX ipc message. This makes driver stop but fw is still waiting. Actually the disabling can be successful in far less than 5000ms and it was first designed for FPGA case. Now change it to 400ms to make debugging easier.
This patch changes cavs and ace15 platforms and doesn't change ace20 for its current state.
fix #8115