Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Nov 25, 2019

When entering runtime suspend or system suspend, the host
driver sends a CTX_SAVE IPC. When this IPC is received,
the FW should call wait_for_interrupt from run level 0.
The IPC task is not at run level 0, therefore this
should be done in the main audio task, which runs at level 0.

Additionally, in the case of baytrail, the DSP doesn't
enter D3 in the case of runtime suspend. So, when the host
tried to resume the DSP, the pm_prepare_D3 flag is not reset
to 0. So, we need to explicitly reset it to 0 when the
host sends the CTX_RESTORE IPC.

Signed-off-by: Ranjani Sridharan [email protected]

@ranj063
Copy link
Collaborator Author

ranj063 commented Nov 25, 2019

Fixes #2123

When entering runtime suspend or system suspend, the host
driver sends a CTX_SAVE IPC. When this IPC is received,
the FW should call wait_for_interrupt from run level 0.
The IPC task is not at run level 0, therefore this
should be done in the main audio task, which runs at level 0.

Additionally, in the case of baytrail, the DSP doesn't
enter D3 in the case of runtime suspend. So, when the host
tries to resume the DSP, the pm_prepare_D3 flag is not reset
to 0. So, we need to explicitly reset it to 0 when the
host sends the CTX_RESTORE IPC.

Signed-off-by: Ranjani Sridharan <[email protected]>
@fredoh9
Copy link
Contributor

fredoh9 commented Nov 25, 2019

SOFCI TEST

@keyonjie
Copy link
Contributor

This looks good, but does IPC_CTX_RESTORE really work with this simple change? I think at least we need re-enable interrupts and timers there?

@ranj063
Copy link
Collaborator Author

ranj063 commented Nov 26, 2019

This looks good, but does IPC_CTX_RESTORE really work with this simple change? I think at least we need re-enable interrupts and timers there?

@keyonjie its possible we're missing something more which is whats causing the IPC timeouts now. But whats unclear to me is if we are WFI(0) when we receive the CTX_SAVE IPC and the host resets the DSP, loads the FW and boots the FW again, do the interrupts and timers get re-enabled?

@keyonjie
Copy link
Contributor

This looks good, but does IPC_CTX_RESTORE really work with this simple change? I think at least we need re-enable interrupts and timers there?

@keyonjie its possible we're missing something more which is whats causing the IPC timeouts now. But whats unclear to me is if we are WFI(0) when we receive the CTX_SAVE IPC and the host resets the DSP, loads the FW and boots the FW again, do the interrupts and timers get re-enabled?

Yes, the wording "context save" here is somewhat confusing, with general understanding, what we do in "context save" should be symmetric with "context restore".

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@ranj063 same change could be done for BDW too.

@lyakh
Copy link
Collaborator

lyakh commented Dec 4, 2019

This looks good, but does IPC_CTX_RESTORE really work with this simple change? I think at least we need re-enable interrupts and timers there?

@keyonjie its possible we're missing something more which is whats causing the IPC timeouts now. But whats unclear to me is if we are WFI(0) when we receive the CTX_SAVE IPC and the host resets the DSP, loads the FW and boots the FW again, do the interrupts and timers get re-enabled?

Yes, the wording "context save" here is somewhat confusing, with general understanding, what we do in "context save" should be symmetric with "context restore".

@ranj063 does a DSP reset happen always after a "resume?" If so, that doesn't sound like a real resume to me, that's more like a power off / power on cycle? But in some cases we do proper suspending / resuming, e.g. for WoV?

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

For what it does, the PR LGTM, but it would be good to get some understanding of the resume process - in which cases we do and don't reset the DSP as a part of a "resume" process? As far as I understand this PR isn't a complete fix and we still need to fix sporadic boot failures, mentioned in #2123?

@lbetlej
Copy link
Collaborator

lbetlej commented Dec 4, 2019

SOFCI TEST

@lgirdwood
Copy link
Member

CI usual false positives.

@lgirdwood lgirdwood merged commit 051a1f8 into thesofproject:master Dec 16, 2019
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.

7 participants