This series provides a solution to solve a corner case issue where the
manager device may become pm_runtime active, but without ALSA/ASoC
requesting any functionality from the peripherals. In this case, the
hardware peripheral device will report as ATTACHED and its initialization
routine will be executed. If this initialization routine initiates any
sort of deferred processing, there is a possibility that the manager could
suspend without the peripheral suspend sequence being invoked: from the
pm_runtime framework perspective, the peripheral is *already* suspended.
Pierre-Louis Bossart (3):
soundwire: intel: prevent pm_runtime resume prior to system suspend
soundwire: intel: disable WAKEEN in pm_runtime resume
soundwire: bus: pm_runtime_request_resume on peripheral attachment
drivers/soundwire/bus.c | 12 ++++++++++++
drivers/soundwire/intel.c | 6 ++++++
2 files changed, 18 insertions(+)
--
2.17.1
From: Pierre-Louis Bossart <[email protected]>
When the manager device is pm_runtime resumed, we see a series of
spurious wakes and attempts to resume the same device:
soundwire_intel.link.0: intel_resume_runtime: start
soundwire_intel.link.0: intel_link_power_up: powering up all links
soundwire_intel.link.0: intel_link_power_up: first link up, programming SYNCPRD
soundwire_intel.link.0: intel_shim_wake: WAKEEN disabled for link 0
soundwire_intel.link.0: intel_link_process_wakeen_event: pm_request_resume start
soundwire_intel.link.0: intel_link_process_wakeen_event: pm_request_resume done
soundwire_intel.link.0: intel_shim_wake: WAKEEN disabled for link 0
soundwire_intel.link.0: intel_link_process_wakeen_event: pm_request_resume start
soundwire_intel.link.0: intel_link_process_wakeen_event: pm_request_resume done
This sequence does not break anything but is totally unnecessary.
Currently the wakes are only disabled after the peripheral generates a
wake, e.g. for jack detection.
If the resume is initiated by the host drivers as a result of
userspace actions (play/record typically), we need to disable wake
detection as well. Doing so prevents the spurious wakes and calls to
pm_request_resume().
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 32e5fdb823c4..4b458e5e7336 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1831,6 +1831,9 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
return 0;
}
+ /* unconditionally disable WAKEEN interrupt */
+ intel_shim_wake(sdw, false);
+
link_flags = md_flags >> (bus->link_id * 8);
multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
--
2.17.1
From: Pierre-Louis Bossart <[email protected]>
In typical use cases, the peripheral becomes pm_runtime active as a
result of the ALSA/ASoC framework starting up a DAI. The parent/child
hierarchy guarantees that the manager device will be fully resumed
beforehand.
There is however a corner case where the manager device may become
pm_runtime active, but without ALSA/ASoC requesting any functionality
from the peripherals. In this case, the hardware peripheral device
will report as ATTACHED and its initialization routine will be
executed. If this initialization routine initiates any sort of
deferred processing, there is a possibility that the manager could
suspend without the peripheral suspend sequence being invoked: from
the pm_runtime framework perspective, the peripheral is *already*
suspended.
To avoid such disconnects between hardware state and pm_runtime state,
this patch adds an asynchronous pm_request_resume() upon successful
attach/initialization which will result in the proper resume/suspend
sequence to be followed on the peripheral side.
BugLink: https://github.com/thesofproject/linux/issues/3459
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/bus.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 354d3f89366f..8b7a680f388e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1838,6 +1838,18 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
__func__, slave->dev_num);
complete(&slave->initialization_complete);
+
+ /*
+ * If the manager became pm_runtime active, the peripherals will be
+ * restarted and attach, but their pm_runtime status may remain
+ * suspended. If the 'update_slave_status' callback initiates
+ * any sort of deferred processing, this processing would not be
+ * cancelled on pm_runtime suspend.
+ * To avoid such zombie states, we queue a request to resume.
+ * This would be a no-op in case the peripheral was being resumed
+ * by e.g. the ALSA/ASoC framework.
+ */
+ pm_request_resume(&slave->dev);
}
}
--
2.17.1
On 20-04-22, 10:32, Bard Liao wrote:
> This series provides a solution to solve a corner case issue where the
> manager device may become pm_runtime active, but without ALSA/ASoC
> requesting any functionality from the peripherals. In this case, the
> hardware peripheral device will report as ATTACHED and its initialization
> routine will be executed. If this initialization routine initiates any
> sort of deferred processing, there is a possibility that the manager could
> suspend without the peripheral suspend sequence being invoked: from the
> pm_runtime framework perspective, the peripheral is *already* suspended.
Applied, thanks
--
~Vinod