Commit c8ede14a authored by Takashi Iwai's avatar Takashi Iwai Committed by Luis Henriques

ASoC: dpcm: Fix race between FE/BE updates and trigger

commit ea9d0d77 upstream.

DPCM can update the FE/BE connection states totally asynchronously
from the FE's PCM state.  Most of FE/BE state changes are protected by
mutex, so that they won't race, but there are still some actions that
are uncovered.  For example, suppose to switch a BE while a FE's
stream is running.  This would call soc_dpcm_runtime_update(), which
sets FE's runtime_update flag, then sets up and starts BEs, and clears
FE's runtime_update flag again.

When a device emits XRUN during this operation, the PCM core triggers
snd_pcm_stop(XRUN).  Since the trigger action is an atomic ops, this
isn't blocked by the mutex, thus it kicks off DPCM's trigger action.
It eventually updates and clears FE's runtime_update flag while
soc_dpcm_runtime_update() is running concurrently, and it results in
confusion.

Usually, for avoiding such a race, we take a lock.  There is a PCM
stream lock for that purpose.  However, as already mentioned, the
trigger action is atomic, and we can't take the lock for the whole
soc_dpcm_runtime_update() or other operations that include the lengthy
jobs like hw_params or prepare.

This patch provides an alternative solution.  This adds a way to defer
the conflicting trigger callback to be executed at the end of FE/BE
state changes.  For doing it, two things are introduced:

- Each runtime_update state change of FEs is protected via PCM stream
  lock.
- The FE's trigger callback checks the runtime_update flag.  If it's
  not set, the trigger action is executed there.  If set, mark the
  pending trigger action and returns immediately.
- At the exit of runtime_update state change, it checks whether the
  pending trigger is present.  If yes, it executes the trigger action
  at this point.
Reported-and-tested-by: default avatarQiao Zhou <zhouqiao@marvell.com>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Acked-by: default avatarLiam Girdwood <liam.r.girdwood@linux.intel.com>
Signed-off-by: default avatarMark Brown <broonie@kernel.org>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 05002bd8
...@@ -102,6 +102,8 @@ struct snd_soc_dpcm_runtime { ...@@ -102,6 +102,8 @@ struct snd_soc_dpcm_runtime {
/* state and update */ /* state and update */
enum snd_soc_dpcm_update runtime_update; enum snd_soc_dpcm_update runtime_update;
enum snd_soc_dpcm_state state; enum snd_soc_dpcm_state state;
int trigger_pending; /* trigger cmd + 1 if pending, 0 if not */
}; };
/* can this BE stop and free */ /* can this BE stop and free */
......
...@@ -1316,13 +1316,36 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream) ...@@ -1316,13 +1316,36 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture); dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture);
} }
static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd);
/* Set FE's runtime_update state; the state is protected via PCM stream lock
* for avoiding the race with trigger callback.
* If the state is unset and a trigger is pending while the previous operation,
* process the pending trigger action here.
*/
static void dpcm_set_fe_update_state(struct snd_soc_pcm_runtime *fe,
int stream, enum snd_soc_dpcm_update state)
{
struct snd_pcm_substream *substream =
snd_soc_dpcm_get_substream(fe, stream);
snd_pcm_stream_lock_irq(substream);
if (state == SND_SOC_DPCM_UPDATE_NO && fe->dpcm[stream].trigger_pending) {
dpcm_fe_dai_do_trigger(substream,
fe->dpcm[stream].trigger_pending - 1);
fe->dpcm[stream].trigger_pending = 0;
}
fe->dpcm[stream].runtime_update = state;
snd_pcm_stream_unlock_irq(substream);
}
static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
{ {
struct snd_soc_pcm_runtime *fe = fe_substream->private_data; struct snd_soc_pcm_runtime *fe = fe_substream->private_data;
struct snd_pcm_runtime *runtime = fe_substream->runtime; struct snd_pcm_runtime *runtime = fe_substream->runtime;
int stream = fe_substream->stream, ret = 0; int stream = fe_substream->stream, ret = 0;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
ret = dpcm_be_dai_startup(fe, fe_substream->stream); ret = dpcm_be_dai_startup(fe, fe_substream->stream);
if (ret < 0) { if (ret < 0) {
...@@ -1344,13 +1367,13 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) ...@@ -1344,13 +1367,13 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
dpcm_set_fe_runtime(fe_substream); dpcm_set_fe_runtime(fe_substream);
snd_pcm_limit_hw_rates(runtime); snd_pcm_limit_hw_rates(runtime);
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
return 0; return 0;
unwind: unwind:
dpcm_be_dai_startup_unwind(fe, fe_substream->stream); dpcm_be_dai_startup_unwind(fe, fe_substream->stream);
be_err: be_err:
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
return ret; return ret;
} }
...@@ -1397,7 +1420,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) ...@@ -1397,7 +1420,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *fe = substream->private_data; struct snd_soc_pcm_runtime *fe = substream->private_data;
int stream = substream->stream; int stream = substream->stream;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
/* shutdown the BEs */ /* shutdown the BEs */
dpcm_be_dai_shutdown(fe, substream->stream); dpcm_be_dai_shutdown(fe, substream->stream);
...@@ -1411,7 +1434,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) ...@@ -1411,7 +1434,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream)
dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP); dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; fe->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
return 0; return 0;
} }
...@@ -1459,7 +1482,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream) ...@@ -1459,7 +1482,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
int err, stream = substream->stream; int err, stream = substream->stream;
mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
dev_dbg(fe->dev, "ASoC: hw_free FE %s\n", fe->dai_link->name); dev_dbg(fe->dev, "ASoC: hw_free FE %s\n", fe->dai_link->name);
...@@ -1474,7 +1497,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream) ...@@ -1474,7 +1497,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
err = dpcm_be_dai_hw_free(fe, stream); err = dpcm_be_dai_hw_free(fe, stream);
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE; fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
mutex_unlock(&fe->card->mutex); mutex_unlock(&fe->card->mutex);
return 0; return 0;
...@@ -1567,7 +1590,7 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream, ...@@ -1567,7 +1590,7 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
int ret, stream = substream->stream; int ret, stream = substream->stream;
mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
memcpy(&fe->dpcm[substream->stream].hw_params, params, memcpy(&fe->dpcm[substream->stream].hw_params, params,
sizeof(struct snd_pcm_hw_params)); sizeof(struct snd_pcm_hw_params));
...@@ -1590,7 +1613,7 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream, ...@@ -1590,7 +1613,7 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS; fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS;
out: out:
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
mutex_unlock(&fe->card->mutex); mutex_unlock(&fe->card->mutex);
return ret; return ret;
} }
...@@ -1704,7 +1727,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, ...@@ -1704,7 +1727,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
} }
EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger); EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger);
static int dpcm_fe_dai_trigger(struct snd_pcm_substream *substream, int cmd) static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
{ {
struct snd_soc_pcm_runtime *fe = substream->private_data; struct snd_soc_pcm_runtime *fe = substream->private_data;
int stream = substream->stream, ret; int stream = substream->stream, ret;
...@@ -1778,6 +1801,23 @@ static int dpcm_fe_dai_trigger(struct snd_pcm_substream *substream, int cmd) ...@@ -1778,6 +1801,23 @@ static int dpcm_fe_dai_trigger(struct snd_pcm_substream *substream, int cmd)
return ret; return ret;
} }
static int dpcm_fe_dai_trigger(struct snd_pcm_substream *substream, int cmd)
{
struct snd_soc_pcm_runtime *fe = substream->private_data;
int stream = substream->stream;
/* if FE's runtime_update is already set, we're in race;
* process this trigger later at exit
*/
if (fe->dpcm[stream].runtime_update != SND_SOC_DPCM_UPDATE_NO) {
fe->dpcm[stream].trigger_pending = cmd + 1;
return 0; /* delayed, assuming it's successful */
}
/* we're alone, let's trigger */
return dpcm_fe_dai_do_trigger(substream, cmd);
}
int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream)
{ {
struct snd_soc_dpcm *dpcm; struct snd_soc_dpcm *dpcm;
...@@ -1821,7 +1861,7 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) ...@@ -1821,7 +1861,7 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream)
dev_dbg(fe->dev, "ASoC: prepare FE %s\n", fe->dai_link->name); dev_dbg(fe->dev, "ASoC: prepare FE %s\n", fe->dai_link->name);
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
/* there is no point preparing this FE if there are no BEs */ /* there is no point preparing this FE if there are no BEs */
if (list_empty(&fe->dpcm[stream].be_clients)) { if (list_empty(&fe->dpcm[stream].be_clients)) {
...@@ -1848,7 +1888,7 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) ...@@ -1848,7 +1888,7 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream)
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE; fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
out: out:
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
mutex_unlock(&fe->card->mutex); mutex_unlock(&fe->card->mutex);
return ret; return ret;
...@@ -1995,11 +2035,11 @@ static int dpcm_run_new_update(struct snd_soc_pcm_runtime *fe, int stream) ...@@ -1995,11 +2035,11 @@ static int dpcm_run_new_update(struct snd_soc_pcm_runtime *fe, int stream)
{ {
int ret; int ret;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_BE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_BE);
ret = dpcm_run_update_startup(fe, stream); ret = dpcm_run_update_startup(fe, stream);
if (ret < 0) if (ret < 0)
dev_err(fe->dev, "ASoC: failed to startup some BEs\n"); dev_err(fe->dev, "ASoC: failed to startup some BEs\n");
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
return ret; return ret;
} }
...@@ -2008,11 +2048,11 @@ static int dpcm_run_old_update(struct snd_soc_pcm_runtime *fe, int stream) ...@@ -2008,11 +2048,11 @@ static int dpcm_run_old_update(struct snd_soc_pcm_runtime *fe, int stream)
{ {
int ret; int ret;
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_BE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_BE);
ret = dpcm_run_update_shutdown(fe, stream); ret = dpcm_run_update_shutdown(fe, stream);
if (ret < 0) if (ret < 0)
dev_err(fe->dev, "ASoC: failed to shutdown some BEs\n"); dev_err(fe->dev, "ASoC: failed to shutdown some BEs\n");
fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment