* ASoC dailink capture/playback flags
@ 2020-05-20 2:25 Pierre-Louis Bossart
2020-05-20 12:14 ` Mark Brown
2020-05-21 3:01 ` Kuninori Morimoto
0 siblings, 2 replies; 3+ messages in thread
From: Pierre-Louis Bossart @ 2020-05-20 2:25 UTC (permalink / raw)
To: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...
Cc: Stephan Gerhold, Kuninori Morimoto, Kai Vehmanen, Takashi Iwai,
Ranjani Sridharan, Wang, Rander, Mark Brown, Bard liao
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: ASoC dailink capture/playback flags
2020-05-20 2:25 ASoC dailink capture/playback flags Pierre-Louis Bossart
@ 2020-05-20 12:14 ` Mark Brown
2020-05-21 3:01 ` Kuninori Morimoto
1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2020-05-20 12:14 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Stephan Gerhold, Kuninori Morimoto, Kai Vehmanen, Takashi Iwai,
Ranjani Sridharan, Wang, Rander, Bard liao
[-- Attachment #1: Type: text/plain, Size: 784 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: ASoC dailink capture/playback flags
2020-05-20 2:25 ASoC dailink capture/playback flags Pierre-Louis Bossart
2020-05-20 12:14 ` Mark Brown
@ 2020-05-21 3:01 ` Kuninori Morimoto
1 sibling, 0 replies; 3+ messages in thread
From: Kuninori Morimoto @ 2020-05-21 3:01 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Stephan Gerhold, Kai Vehmanen, Takashi Iwai, Ranjani Sridharan,
Wang, Rander, Mark Brown, Bard liao
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-21 3:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 2:25 ASoC dailink capture/playback flags Pierre-Louis Bossart
2020-05-20 12:14 ` Mark Brown
2020-05-21 3:01 ` Kuninori Morimoto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).