Baytrail/Cherrytrail suspend-resume + module reload support #2138
Conversation
plbossart
requested review from
dbaluta,
juimonen,
keyonjie,
kv2019i,
lgirdwood,
lyakh and
ranj063
as code owners
sound/soc/sof/ipc.c
Outdated
There was a problem hiding this comment.
@ranj063 Does this ever trigger? I see that the return value of this function is never checked, so this patch's only (potential) run-time effect is if the error message disappears. But isn't it really an error? Shouldn't we at least warn?
There was a problem hiding this comment.
I saw this error a couple of times, though less often after applying @keyonjie 's idea of only enabling the DONE interrupt when sending a command.
|
After picking these patches, this PR seems to fix all the issues we saw. Tested-by: Enric Balletbo i Serra enric.balletbo@collabora.com Thanks, @plbossart for work on this. |
Add the PM callbacks for BYT/CHT platforms. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add new case when set_power_state() is not supported, e.g. for Intel Baytrail/Cherrytrail legacy platforms. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Low-power playback was never enabled on Baytrail devices, remove what looks like copy/paste from other machine drivers which were never submitted upstream. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add required .pm_ops to support suspend/resume on baytrail/cherrytrail machines. This .pm_ops is conditionally-added to avoid impacting the legacy driver where power management is handled differently. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
We currently ignore the reply messages from the DSP when they are not expected but call it out as an error. Change the error message to a debug message. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add remove op that disables interrupts and reset the DSP for BYT and CHT platforms. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DSP may send the same interrupt multiple times before it's handled in the interrupt thread. Rather than masking it in the thread, mask it in the handler directly. This patch also removes useless checks that cannot happen, and masks that are set don't need to be re-tested. BugLink: thesofproject#1492 Suggested-by: Keyon Jie <yang.jie@linux.intel.com> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
On probe and reset, we should not touch the SHIM_IMRD register since it is configured by firmware. The driver only configures SHIM_IMRX with the BUSY interrupt enabled by default and DONE interrupt disabled. When sending an IPC message, the DONE interrupt is enabled until the DSP response is provided. This sequence hardens the IPC communication and avoid interrupt-related issues when adding/removing modules or during system suspend-resume transitions. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Keyon Jie <yang.jie@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
force-pushed
the
fix/bytcht-suspend-resume-module-reload
branch
from
35871c0 to
351f531
Compare
Thanks @eballetbo for testing, added your tag in commit messages |
ranj063
left a comment
ranj063
left a comment
There was a problem hiding this comment.
@plbossart LGTM. Thanks for pulling everything together!
|
I left my Cyan in the office so I can't test easily :( but I am looking forward to trying this code out. |
|
I am going to merge this, I'd like all Baytrail to be handled in the next kernel cycle. |
|
test patches, sometimes no sound on Bay trail. |
|
@eballetbo side question on Cyan: do you see "Headphone Jack" and "Headset Mic Jack" controls on your device ('amixer controls' will give you the list)? I did an upgrade to Ubuntu 20.04 and of course the jack detection doesn't work, not sure if I am missing something (ACPI or .config setup)? Thanks! I added UCM support but that stupid jack is not functional for some reason. |
what's the mean of "legacy driver are available"? ( 30.692| 0.006) I: [pulseaudio] (alsa-lib)main.c: error: failed to import hw:0 use case configuration -2 |
|
@plbossart how to make ucm2 work with sofbytcrrt5640? |
You need the latest alsa-lib, alsa-ucm-conf and the latest topic/sof-dev kernel branch (all code submitted upstream) |
7 participants
Adding all fixes in one PR to help with testing. This works fine for me on Cyan Chromebook and RT5640 device.