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

pipeline: disable xrun recovery #1439

Closed

Conversation

slawblauciak
Copy link
Collaborator

added a flag to FW ready message to inform
whether the internal xrun recovery is enabled or not

Signed-off-by: Slawomir Blauciak [email protected]

@slawblauciak slawblauciak added the ABI ABI change involved label May 15, 2019
@slawblauciak
Copy link
Collaborator Author

@keyonjie this flag will have to be handled accordingly on the kernel side now

@keyonjie
Copy link
Contributor

@keyonjie this flag will have to be handled accordingly on the kernel side now

LGTM, thanks.

@jajanusz
Copy link
Contributor

SOFCI TEST

Kconfig Outdated Show resolved Hide resolved
added a flag to FW ready message to inform
whether the internal xrun recovery is enabled or not

Signed-off-by: Slawomir Blauciak <[email protected]>
@lgirdwood
Copy link
Member

@slawblauciak is there a kernel PR for ABI addition ?

@lgirdwood
Copy link
Member

@slawblauciak MINOR 8 atm, if you can get kernel PR created soon (as it can merge with other PRs on the dashboard), otherwise it will be MINOR 9 if kernel PR cant be done until late next week. Let me know and I'll update as required.

@plbossart
Copy link
Member

@lgirdwood what would the kernel do with this anyways? If it's not set, then the firmware recovers on its own? Why isn't the ALSA prepare/restart not good enough ? Didn't we have this conversation already?

@jajanusz
Copy link
Contributor

@lgirdwood @plbossart Please let us know what you decided. Almost week passed and we don't know if it's good to merge for you. Thanks.

@lgirdwood
Copy link
Member

@plbossart user here is not ALSA, but an RTOS or stand alone mode.

@slawblauciak
Copy link
Collaborator Author

slawblauciak commented May 23, 2019

@lgirdwood no, there's no change ready yet. I believe it might require a functional change in the kernel, not just an ABI change.
@keyonjie mind shedding some light on the situation on your side?

@keyonjie
Copy link
Contributor

@lgirdwood no, there's no change ready yet. I believe it might require a functional change in the kernel, not just an ABI change.
@keyonjie mind shedding some light on the situation on your side?

Ah, get back to this, yes, no ABI change needed in Linux driver side, we will need to enable CONFIG_SND_SOC_SOF_DEBUG_XRUN_STOP to handle Xrun IPC come from DSP.
This PR LGTM.

@plbossart
Copy link
Member

plbossart commented May 24, 2019 via email

@lgirdwood
Copy link
Member

That doesn't look quite right to me. If the kernel behavior needs to depend on how the firmware is compiled, then the kernel needs to discover this capability and not use a Kconfig at all.

Yep, @slawblauciak @keyonjie we need a similar PR for the kernel that has the new flag and @keyonjie that will stop ALSA attempting to recover XRUNs (this would be core ALSA).

Alternatively, we could postpone this PR until we have IPC2.0 and a user ? Do we have a user for this PR today ?

@slawblauciak
Copy link
Collaborator Author

@lgirdwood frankly, this is such an old discussed issue that I'm no longer sure whether it's really necessary to have it merged ASAP. There was another problem that's been gated by this, but it's no longer relevant since it's been solved.

@lgirdwood
Copy link
Member

@slawblauciak ok, please close if you want too and nobody objects.

@slawblauciak
Copy link
Collaborator Author

Closing for now, will revisit later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI ABI change involved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants