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 
[ Upstream commit 0c75fc7 ]

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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@plbossart @gregkh
plbossart authored and gregkh committed Sep 18, 2021
1 parent 46e8be7 commit acc8abf
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 @@ -2001,6 +2001,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 @@ -2017,76 +2019,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 acc8abf

Please sign in to comment.