Skip to content

Commit

Permalink
ASoC: soc-pcm: protect BE dailink state changes in trigger
Browse files Browse the repository at this point in the history 
When more than one FE is connected to a BE, e.g. in a mixing use case,
the BE can be triggered multiple times when the FE are opened/started
concurrently. This race condition is problematic in the case of
SoundWire BE dailinks, and this is not desirable in a general
case. The code carefully checks when the BE can be stopped or
hw_free'ed, but the trigger code does not use any mutual exclusion.

Fix by using the same spinlock already used to check FE states, and
set the state before the trigger. In case of errors,  the initial
state will be restored.

This patch does not change how the triggers are handled, it only makes
sure the states are handled in critical sections.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-Id: <20210817164054.250028-2-pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
  • Loading branch information
@plbossart @broonie
plbossart authored and broonie committed Aug 26, 2021
1 parent 515b436 commit 0c75fc7
Showing 1 changed file with 85 additions and 18 deletions.
103 changes: 85 additions & 18 deletions sound/soc/soc-pcm.c
View file
Expand Up @@ -1999,6 +1999,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
struct snd_soc_pcm_runtime *be;
struct snd_soc_dpcm *dpcm;
int ret = 0;
unsigned long flags;
enum snd_soc_dpcm_state state;

for_each_dpcm_be(fe, stream, dpcm) {
struct snd_pcm_substream *be_substream;
Expand All @@ -2015,76 +2017,141 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,

switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}
state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
break;
case SNDRV_PCM_TRIGGER_RESUME:
if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND))
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}

state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}

state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
break;
case SNDRV_PCM_TRIGGER_STOP:
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) &&
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
(be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;

spin_lock_irqsave(&fe->card->dpcm_lock, flags);
state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;

spin_lock_irqsave(&fe->card->dpcm_lock, flags);
state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_SUSPEND;
break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) {
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
}
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;

spin_lock_irqsave(&fe->card->dpcm_lock, flags);
state = be->dpcm[stream].state;
be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

ret = soc_pcm_trigger(be_substream, cmd);
if (ret)
if (ret) {
spin_lock_irqsave(&fe->card->dpcm_lock, flags);
be->dpcm[stream].state = state;
spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
}

be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
break;
}
}
Expand Down

0 comments on commit 0c75fc7

Please sign in to comment.