Skip to content

Commit 1b0f957

Browse files
ranj063plbossart
authored andcommitted
soundwire: intel: improve suspend flows
This patch provides both a simplification of the suspend flows and a better balanced operation during suspend/resume transition, as part of the transition of Sound Open Firmware (SOF) to dynamic pipelines: the DSP resources are only enabled when required instead of enabled on startup. 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 component suspend is not removed but instead modified to deal with a corner case: when a substream is PAUSED, the ALSA core does not throw the TRIGGER_SUSPEND. This is problematic since the refcount for all pipelines and widgets is not balanced, leading to issues on resume. The trigger callback keeps track of the 'paused' state with a new flag, which is tested during the component suspend called later to release the remaining DSP resources. These resources will be re-enabled in the .prepare step. The IPC used in the TRIGGER_SUSPEND to release DSP resources is not a problem since the BE dailink is already marked as non-atomic. 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>
1 parent c786258 commit 1b0f957

2 files changed

Lines changed: 89 additions & 23 deletions

File tree

drivers/soundwire/cadence_master.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ struct sdw_cdns_stream_config {
8080
* @link_id: Master link id
8181
* @hw_params: hw_params to be applied in .prepare step
8282
* @suspended: status set when suspended, to be used in .prepare
83+
* @paused: status set in .trigger, to be used in suspend
8384
*/
8485
struct sdw_cdns_dma_data {
8586
char *name;
@@ -90,6 +91,7 @@ struct sdw_cdns_dma_data {
9091
int link_id;
9192
struct snd_pcm_hw_params *hw_params;
9293
bool suspended;
94+
bool paused;
9395
};
9496

9597
/**

drivers/soundwire/intel.c

Lines changed: 87 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
846846
sdw_cdns_config_stream(cdns, ch, dir, pdi);
847847

848848
/* store pdi and hw_params, may be needed in prepare step */
849+
dma->paused = false;
849850
dma->suspended = false;
850851
dma->pdi = pdi;
851852
dma->hw_params = params;
@@ -978,29 +979,6 @@ static void intel_shutdown(struct snd_pcm_substream *substream,
978979
pm_runtime_put_autosuspend(cdns->dev);
979980
}
980981

981-
static int intel_component_dais_suspend(struct snd_soc_component *component)
982-
{
983-
struct sdw_cdns_dma_data *dma;
984-
struct snd_soc_dai *dai;
985-
986-
for_each_component_dais(component, dai) {
987-
/*
988-
* we don't have a .suspend dai_ops, and we don't have access
989-
* to the substream, so let's mark both capture and playback
990-
* DMA contexts as suspended
991-
*/
992-
dma = dai->playback_dma_data;
993-
if (dma)
994-
dma->suspended = true;
995-
996-
dma = dai->capture_dma_data;
997-
if (dma)
998-
dma->suspended = true;
999-
}
1000-
1001-
return 0;
1002-
}
1003-
1004982
static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
1005983
void *stream, int direction)
1006984
{
@@ -1023,11 +1001,97 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai,
10231001
return dma->stream;
10241002
}
10251003

1004+
static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai)
1005+
{
1006+
struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
1007+
struct sdw_intel *sdw = cdns_to_intel(cdns);
1008+
struct sdw_cdns_dma_data *dma;
1009+
int ret = 0;
1010+
1011+
dma = snd_soc_dai_get_dma_data(dai, substream);
1012+
if (!dma) {
1013+
dev_err(dai->dev, "failed to get dma data in %s\n",
1014+
__func__);
1015+
return -EIO;
1016+
}
1017+
1018+
switch (cmd) {
1019+
case SNDRV_PCM_TRIGGER_SUSPEND:
1020+
1021+
/*
1022+
* The .prepare callback is used to deal with xruns and resume operations.
1023+
* In the case of xruns, the DMAs and SHIM registers cannot be touched,
1024+
* but for resume operations the DMAs and SHIM registers need to be initialized.
1025+
* the .trigger callback is used to track the suspend case only.
1026+
*/
1027+
1028+
dma->suspended = true;
1029+
1030+
ret = intel_free_stream(sdw, substream->stream, dai, sdw->instance);
1031+
break;
1032+
1033+
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
1034+
dma->paused = true;
1035+
break;
1036+
case SNDRV_PCM_TRIGGER_STOP:
1037+
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
1038+
dma->paused = false;
1039+
break;
1040+
default:
1041+
break;
1042+
}
1043+
1044+
return ret;
1045+
}
1046+
1047+
static int intel_component_dais_suspend(struct snd_soc_component *component)
1048+
{
1049+
struct snd_soc_dai *dai;
1050+
1051+
/*
1052+
* In the corner case where a SUSPEND happens during a PAUSE, the ALSA core
1053+
* does not throw the TRIGGER_SUSPEND. This leaves the DAIs in an unbalanced state.
1054+
* Since the component suspend is called last, we can trap this corner case
1055+
* and force the DAIs to release their resources.
1056+
*/
1057+
for_each_component_dais(component, dai) {
1058+
struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
1059+
struct sdw_intel *sdw = cdns_to_intel(cdns);
1060+
struct sdw_cdns_dma_data *dma;
1061+
int stream;
1062+
int ret;
1063+
1064+
dma = dai->playback_dma_data;
1065+
stream = SNDRV_PCM_STREAM_PLAYBACK;
1066+
if (!dma) {
1067+
dma = dai->capture_dma_data;
1068+
stream = SNDRV_PCM_STREAM_CAPTURE;
1069+
}
1070+
1071+
if (!dma)
1072+
continue;
1073+
1074+
if (dma->suspended)
1075+
continue;
1076+
1077+
if (dma->paused) {
1078+
dma->suspended = true;
1079+
1080+
ret = intel_free_stream(sdw, stream, dai, sdw->instance);
1081+
if (ret < 0)
1082+
return ret;
1083+
}
1084+
}
1085+
1086+
return 0;
1087+
}
1088+
10261089
static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
10271090
.startup = intel_startup,
10281091
.hw_params = intel_hw_params,
10291092
.prepare = intel_prepare,
10301093
.hw_free = intel_hw_free,
1094+
.trigger = intel_trigger,
10311095
.shutdown = intel_shutdown,
10321096
.set_sdw_stream = intel_pcm_set_sdw_stream,
10331097
.get_sdw_stream = intel_get_sdw_stream,

0 commit comments

Comments
 (0)