On Tue, 27 Jul 2021 07:32:56 +0200, Bard Liao wrote:
From: Ranjani Sridharan ranjani.sridharan@linux.intel.com
This patch provides both a simplification of the suspend flows and a better balanced operation during suspend/resume transition.
The exiting code relies on a convoluted way of dealing with suspend signals. Since there is no .suspend DAI callback, we used the component .suspend and marked all the component DAI dmas as 'suspended'. The information was used in the .prepare stage to differentiate resume operations from xrun handling, and only reinitialize SHIM registers and DMA in the former case.
While this solution has been working reliably for about 2 years, there is a much better solution consisting in trapping the TRIGGER_SUSPEND in the .trigger DAI ops. The DMA is still marked in the same way for the .prepare op to run, but in addition the callbacks sent to DSP firmware are now balanced.
Normal operation: hw_params -> intel_params_stream hw_free -> intel_free_stream
suspend -> intel_free_stream prepare -> intel_params_stream
This balanced operation was not required with existing SOF firmware relying on static pipelines instantiated at every boot. With the on-going transition to dynamic pipelines, it's however a requirement to keep the use count for the DAI widget balanced across all transitions.
The trigger callback is handled in the stream lock atomically, and are you sure that you want to operate a possibly heavy task there?
If it's just for releasing before re-acquiring a stream, you can do it in sync_stop PCM ops instead, too. This is called in prior to prepare and hw_params ops when a stream has been stopped or suspended beforehand.
thanks,
Takashi
Co-developed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
drivers/soundwire/intel.c | 53 +++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index fb9c23e13206..a0178779a5ba 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1005,29 +1005,6 @@ static void intel_shutdown(struct snd_pcm_substream *substream, pm_runtime_put_autosuspend(cdns->dev); }
-static int intel_component_dais_suspend(struct snd_soc_component *component) -{
- struct sdw_cdns_dma_data *dma;
- struct snd_soc_dai *dai;
- for_each_component_dais(component, dai) {
/** we don't have a .suspend dai_ops, and we don't have access* to the substream, so let's mark both capture and playback* DMA contexts as suspended*/dma = dai->playback_dma_data;if (dma)dma->suspended = true;dma = dai->capture_dma_data;if (dma)dma->suspended = true;- }
- return 0;
-}
static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction) { @@ -1056,11 +1033,39 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai, return dma->stream; }
+static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) +{
- struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
- struct sdw_intel *sdw = cdns_to_intel(cdns);
- struct sdw_cdns_dma_data *dma;
- /*
* The .prepare callback is used to deal with xruns and resume operations. In the case* of xruns, the DMAs and SHIM registers cannot be touched, but for resume operations the* DMAs and SHIM registers need to be initialized.* the .trigger callback is used to track the suspend case only.*/- if (cmd != SNDRV_PCM_TRIGGER_SUSPEND)
return 0;- dma = snd_soc_dai_get_dma_data(dai, substream);
- if (!dma) {
dev_err(dai->dev, "failed to get dma data in %s\n",__func__);return -EIO;- }
- dma->suspended = true;
- return intel_free_stream(sdw, substream, dai, sdw->instance);
+}
static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free,
- .trigger = intel_trigger, .shutdown = intel_shutdown, .set_sdw_stream = intel_pcm_set_sdw_stream, .get_sdw_stream = intel_get_sdw_stream,
@@ -1071,6 +1076,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, .prepare = intel_prepare, .hw_free = intel_hw_free,
- .trigger = intel_trigger, .shutdown = intel_shutdown, .set_sdw_stream = intel_pdm_set_sdw_stream, .get_sdw_stream = intel_get_sdw_stream,
@@ -1078,7 +1084,6 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
static const struct snd_soc_component_driver dai_component = { .name = "soundwire",
- .suspend = intel_component_dais_suspend
};
static int intel_create_dai(struct sdw_cdns *cdns,
2.17.1