ASoC dailink capture/playback flags
Hi,
Some of us at Intel are confused with multiple definitions of capture/playback dailink flags (credits to Rander Wang for reporting this in https://github.com/thesofproject/linux/pull/2070).
struct snd_soc_dai_link { [...]
/* For unidirectional dai links */ unsigned int playback_only:1; unsigned int capture_only:1;
[...]
/* DPCM capture and Playback support */ unsigned int dpcm_capture:1; unsigned int dpcm_playback:1; [...]
};
And of course when you start looking at some machine drivers, there are confusions, e.g. below with DPCM front-ends using one or the other set, and with copy-paste and partial diffs, this propagates without being consistent or being noticed:
[GLK_DPCM_AUDIO_HS_PB] = { .name = "Glk Audio Headset Playback", .stream_name = "Headset Audio", .dpcm_playback = 1, <<<< .nonatomic = 1, .dynamic = 1, SND_SOC_DAILINK_REG(system2, dummy, platform), }, [GLK_DPCM_AUDIO_ECHO_REF_CP] = { .name = "Glk Audio Echo Reference cap", .stream_name = "Echoreference Capture", .init = NULL, .capture_only = 1, <<<< should this be .dpcm_capture = 1? .nonatomic = 1, .dynamic = 1, SND_SOC_DAILINK_REG(echoref, dummy, platform), }, [GLK_DPCM_AUDIO_REF_CP] = { .name = "Glk Audio Reference cap", .stream_name = "Refcap", .init = NULL, .dpcm_capture = 1, <<<< .nonatomic = 1, .dynamic = 1, .ops = &geminilake_refcap_ops, SND_SOC_DAILINK_REG(reference, dummy, platform), },
So here are the questions:
- when using DPCM, is there an expectation to use dpcm_ flags only?
- should we instead use playback/capture_only when only one of the two dpcm_ flags is set?
- should we flags errors if we ever encounter cases with e.g. dpcm_playback = true and capture_only = true?
- do we actually need two sets of definitions? There are very few users of the .playback_only and .capture_only flags and only a single place where it's checked in soc-pcm.c
Thanks for your feedback
-Pierre
On Tue, May 19, 2020 at 09:25:31PM -0500, Pierre-Louis Bossart wrote:
So here are the questions:
- when using DPCM, is there an expectation to use dpcm_ flags only?
- should we instead use playback/capture_only when only one of the two dpcm_
flags is set?
- should we flags errors if we ever encounter cases with e.g. dpcm_playback
= true and capture_only = true?
TBH I'm not convinced DPCM is well enough tied together to have clear answers on this. I don't really know what those DPCM level flags are doing.
- do we actually need two sets of definitions? There are very few users of
the .playback_only and .capture_only flags and only a single place where it's checked in soc-pcm.c
We could probably do a driver only thing for the i.MX stuff if we had to.
Hi Pierre-Louis
struct snd_soc_dai_link { [...]
/* For unidirectional dai links */ unsigned int playback_only:1; unsigned int capture_only:1;
[...]
/* DPCM capture and Playback support */ unsigned int dpcm_capture:1; unsigned int dpcm_playback:1; [...]
};
I confirmed it. If my understand was correct...
dpcm_xxx were added to distinguish "dummy" DAI or not, by
1e9de42f4324b91ce2e9da0939cab8fc6ae93893 ASoC: dpcm: Explicitly set BE DAI link supported stream directions
xxx_only were added to handle unidirectional such as Freescale MX28, by
d6bead020d8f8bcaca5cdcb035250c44b21c93e7 ASoC: soc-pcm: Allow to specify unidirectional dai_link
- when using DPCM, is there an expectation to use dpcm_ flags only?
I think it is needed if it was not dummy DAI.
- should we instead use playback/capture_only when only one of the two
dpcm_ flags is set?
- should we flags errors if we ever encounter cases with
e.g. dpcm_playback = true and capture_only = true?
- do we actually need two sets of definitions? There are very few
users of the .playback_only and .capture_only flags and only a single place where it's checked in soc-pcm.c
I wonder why xxx_only were needed when it was unidirection ? I guess one of playback/capture will be (should be ?) automatically 0 in such case at soc_new_pcm() ??
...? dummy DAI was the reason ?? Do we need below or similar patch ? (not tested)
If so, and if we can handle dummy DAI correctly at soc_new_pcm() somehow, we can automatically judge dpcm_xxx and xxx_only and is able to remove them?
---------------- diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 80dd3cf6200c..dc89ead4ec62 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2811,6 +2811,9 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
for_each_rtd_codec_dais(rtd, i, codec_dai) { + if (!strcmp(codec_dai->component->name, "snd-soc-dummy")) + continue; + if (rtd->num_cpus == 1) { cpu_dai = asoc_rtd_to_cpu(rtd, 0); } else if (rtd->num_cpus == rtd->num_codecs) { ----------------
Thank you for your help !!
Best regards --- Kuninori Morimoto
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Pierre-Louis Bossart