| Message ID | 87wn0skkuw.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: replace dpcm_playback/capture to playback/capture_only | expand |
On 5/29/2023 3:05 AM, Kuninori Morimoto wrote: > soc_get_playback_capture() is now handling DPCM and normal comprehensively > for playback/capture stream. We can use playback/capture_only flag > instead of using dpcm_playback/capture. This patch replace these. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > sound/soc/soc-topology-test.c | 2 -- > sound/soc/soc-topology.c | 4 ++-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c > index 2cd3540cec04..703a366e0abe 100644 > --- a/sound/soc/soc-topology-test.c > +++ b/sound/soc/soc-topology-test.c > @@ -94,8 +94,6 @@ static struct snd_soc_dai_link kunit_dai_links[] = { > .nonatomic = 1, > .dynamic = 1, > .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, > - .dpcm_playback = 1, > - .dpcm_capture = 1, > SND_SOC_DAILINK_REG(dummy, dummy, platform), > }, > }; > diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c > index 47ab5cf99497..cc1f08f2f17b 100644 > --- a/sound/soc/soc-topology.c > +++ b/sound/soc/soc-topology.c > @@ -1735,8 +1735,8 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, > /* enable DPCM */ > link->dynamic = 1; > link->ignore_pmdown_time = 1; > - link->dpcm_playback = le32_to_cpu(pcm->playback); > - link->dpcm_capture = le32_to_cpu(pcm->capture); > + link->playback_only = le32_to_cpu(pcm->playback) && !le32_to_cpu(pcm->capture); > + link->capture_only = le32_to_cpu(pcm->capture) && !le32_to_cpu(pcm->playback); > if (pcm->flag_mask) > set_link_flags(link, > le32_to_cpu(pcm->flag_mask), Hi, patches look ok - I haven't run tests yet on v3, will try to get results tomorrow. However looking at those assignments again, I wonder if we really need them to be "negative" ones? Can't we just have some simple fields like playback and capture (similar to dpcm_playback & dpcm_capture from before). My concern is that having fields with "_only" in name requires them to be handled properly - like in above code, while having just "playback" and "capture" would be just simple assignment and in the end a lot easier to understand. Is there any reason you chose to use the *_only fields? Thanks, Amadeusz
Hi Amadeusz Thank you for your review and feedback > patches look ok - I haven't run tests yet on v3, will try to get results > tomorrow. However looking at those assignments again, I wonder if we > really need them to be "negative" ones? Can't we just have some simple > fields like playback and capture (similar to dpcm_playback & > dpcm_capture from before). My concern is that having fields with "_only" > in name requires them to be handled properly - like in above code, while > having just "playback" and "capture" would be just simple assignment and > in the end a lot easier to understand. Is there any reason you chose to > use the *_only fields? Not a super big reason, but want to keep compatibility as much as possible. The driver which get effect from this patch is only DPCM user if it uses playback/capture_only. I think DPCM user < normal user. If we want to switch to use playback/capture flag from _only flag, we want have such additional patch-set instead of "change everything" patch-set ? And my understand is "playback/capture_only" are "option" flag, "playback/capture" are "necessary" flag. I like less "necessary settings" driver :) Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/soc-topology-test.c b/sound/soc/soc-topology-test.c index 2cd3540cec04..703a366e0abe 100644 --- a/sound/soc/soc-topology-test.c +++ b/sound/soc/soc-topology-test.c @@ -94,8 +94,6 @@ static struct snd_soc_dai_link kunit_dai_links[] = { .nonatomic = 1, .dynamic = 1, .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, - .dpcm_playback = 1, - .dpcm_capture = 1, SND_SOC_DAILINK_REG(dummy, dummy, platform), }, }; diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 47ab5cf99497..cc1f08f2f17b 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1735,8 +1735,8 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg, /* enable DPCM */ link->dynamic = 1; link->ignore_pmdown_time = 1; - link->dpcm_playback = le32_to_cpu(pcm->playback); - link->dpcm_capture = le32_to_cpu(pcm->capture); + link->playback_only = le32_to_cpu(pcm->playback) && !le32_to_cpu(pcm->capture); + link->capture_only = le32_to_cpu(pcm->capture) && !le32_to_cpu(pcm->playback); if (pcm->flag_mask) set_link_flags(link, le32_to_cpu(pcm->flag_mask),
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream. We can use playback/capture_only flag instead of using dpcm_playback/capture. This patch replace these. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- sound/soc/soc-topology-test.c | 2 -- sound/soc/soc-topology.c | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-)