From b7a5fe03746f1c444a1f9a3a41b45e9b34d6e254 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 13 Aug 2021 12:03:59 -0500 Subject: [PATCH] soundwire: stream: make enable/disable/deprepare idempotent The stream management currently flags an 'inconsistent state' error when a change is requested multiple times. This was added on purpose to identify programming mistakes. This is however imcompatible with the current ASoC-DPCM implementation. There is currently no mutual exclusion in dpcm_be_dai_trigger(), and if two FEs connected to the same BE are started concurrently the state management the BE may be triggered twice. The following code shows the state can be tested before being updated: case SNDRV_PCM_TRIGGER_START: 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)) continue; ret = soc_pcm_trigger(be_substream, cmd); if (ret) goto end; be->dpcm[stream].state = SND_SOC_DPCM_STATE_START; This patch suggests allowing the stream functions to be idempotent, i.e. they can be called multiple times. Note that the prepare case was already handling multiple calls, this was added in commit c32464c9393d ("soundwire: stream: only prepare stream when it is configured.") FIXME: is this the right fix? Seem like DPCM should not trigger the same BE twice! BugLink: https://github.com/thesofproject/sof/pull/4611 Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 5d4f6b308ef731..901683a78dad2e 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1679,6 +1679,11 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_ENABLED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_PREPARED && stream->state != SDW_STREAM_DISABLED) { pr_err("%s: %s: inconsistent state state %d\n", @@ -1762,6 +1767,11 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_DISABLED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_ENABLED) { pr_err("%s: %s: inconsistent state state %d\n", __func__, stream->name, stream->state); @@ -1837,6 +1847,11 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_DEPREPARED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_PREPARED && stream->state != SDW_STREAM_DISABLED) { pr_err("%s: %s: inconsistent state state %d\n",