Skip to content

Conversation

@plbossart
Copy link
Member

@plbossart plbossart commented Aug 13, 2021

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 c32464c ("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: thesofproject/sof#4611
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

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 c32464c ("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: thesofproject/sof#4611
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member Author

This patch makes the "sdw_enable_stream: subdevice #0-Playback: inconsistent state state 3" error go away with thesofproject/sof#4611

I did however see a major kernel oops on multiple-pipeline.sh -f p, so this deep-buffer thing is far from straightforward to enable.

@plbossart
Copy link
Member Author

This doesn't work, it's problematic with multiple pipelines on stop, I see this message now

[ 314.939714] kernel: sdw_deprepare_stream: subdevice #0-Playback: inconsistent state state 3

That means a stream is still ENABLED, this cannot really happen to deprepare without stopping first.

Looks like the DPCM states really need to be consistent and avoid race conditions.

@plbossart plbossart closed this Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant