| Message ID | 875zft97i4.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-pcm cleanup step5 | expand |
On 2/26/20 12:40 AM, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > DAI is counting its activity, and both playback/capture directional > activity. When considering mute, DAI's activity is enough instead of > caring both playback/capture. > This patch makes mute check simply. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > sound/soc/soc-pcm.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > index 4a14f10b8c90..7e0464ec802e 100644 > --- a/sound/soc/soc-pcm.c > +++ b/sound/soc/soc-pcm.c > @@ -1202,7 +1202,6 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) > struct snd_soc_pcm_runtime *rtd = substream->private_data; > struct snd_soc_dai *cpu_dai; > struct snd_soc_dai *codec_dai; > - bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; > int i; > > mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass); > @@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) > > /* apply codec digital mute */ > for_each_rtd_codec_dai(rtd, i, codec_dai) { > - int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK]; > - int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE]; > - > - if ((playback && playback_active == 1) || > - (!playback && capture_active == 1)) > + if (codec_dai->active == 1) nit-pick: we have two tests in soc-pcm.c if (codec_dai->active) if (codec_dai->active == 1) The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean. > snd_soc_dai_digital_mute(codec_dai, 1, > substream->stream); > } >
Hi Pierre-Louis > > @@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) > > /* apply codec digital mute */ > > for_each_rtd_codec_dai(rtd, i, codec_dai) { > > - int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK]; > > - int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE]; > > - > > - if ((playback && playback_active == 1) || > > - (!playback && capture_active == 1)) > > + if (codec_dai->active == 1) > > nit-pick: we have two tests in soc-pcm.c > > if (codec_dai->active) > if (codec_dai->active == 1) > > The two are functionality equivalent but it'd be good to choose one > version - or possibly use 'active' as a boolean. In my understanding, dai->active can be 0/1/2. But yes, we want to call mute if codec_dai is working /* Call Mute if Codec DAI is working */ if (codec_dai->active) /* Call Mute if Codec DAI is used only for Playback or Capture */ if (codec_dai->active == 1) Thank you for your help !! Best regards --- Kuninori Morimoto
>>> - if ((playback && playback_active == 1) || >>> - (!playback && capture_active == 1)) >>> + if (codec_dai->active == 1) >> >> nit-pick: we have two tests in soc-pcm.c >> >> if (codec_dai->active) >> if (codec_dai->active == 1) >> >> The two are functionality equivalent but it'd be good to choose one >> version - or possibly use 'active' as a boolean. > > In my understanding, dai->active can be 0/1/2. I see, I guess I missed this completely. Thanks Morimoto-san for the precision.
Hi Pierre-Louis > >>> - if ((playback && playback_active == 1) || > >>> - (!playback && capture_active == 1)) > >>> + if (codec_dai->active == 1) > >> > >> nit-pick: we have two tests in soc-pcm.c > >> > >> if (codec_dai->active) > >> if (codec_dai->active == 1) > >> > >> The two are functionality equivalent but it'd be good to choose one > >> version - or possibly use 'active' as a boolean. > > > > In my understanding, dai->active can be 0/1/2. > > I see, I guess I missed this completely. Thanks Morimoto-san for the > precision. But, we want to use "if (codec_dai->active)" anyway. Your review indicated my mistake. Thank you for your help !! Best regards --- Kuninori Morimoto
>>>>> - if ((playback && playback_active == 1) || >>>>> - (!playback && capture_active == 1)) >>>>> + if (codec_dai->active == 1) >>>> >>>> nit-pick: we have two tests in soc-pcm.c >>>> >>>> if (codec_dai->active) >>>> if (codec_dai->active == 1) >>>> >>>> The two are functionality equivalent but it'd be good to choose one >>>> version - or possibly use 'active' as a boolean. >>> >>> In my understanding, dai->active can be 0/1/2. >> >> I see, I guess I missed this completely. Thanks Morimoto-san for the >> precision. > > But, we want to use "if (codec_dai->active)" anyway. > Your review indicated my mistake. Not in this case though, the initial idea was to do the mute when only playback or capture were enabled? If you mute when both are enabled then that's a real change.
Hi Pierre-Louis > Not in this case though, the initial idea was to do the mute when only > playback or capture were enabled? If you mute when both are enabled > then that's a real change. Let's check original if ((playback && playback_active == 1) || (!playback && capture_active == 1)) snd_soc_dai_digital_mute(...) It calls mute if Playback or Capture is working. and my patch (v2) is if (codec_dai->activity) snd_soc_dai_digital_mute(...) It calls mute if Codec DAI is working (= Playback or Capture). I think it doesn't change behavior. Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Pierre-Louis, again > > Not in this case though, the initial idea was to do the mute when only > > playback or capture were enabled? If you mute when both are enabled > > then that's a real change. > > Let's check original > > if ((playback && playback_active == 1) || > (!playback && capture_active == 1)) > snd_soc_dai_digital_mute(...) > > It calls mute if Playback or Capture is working. > and my patch (v2) is > > if (codec_dai->activity) > snd_soc_dai_digital_mute(...) > > It calls mute if Codec DAI is working (= Playback or Capture). > I think it doesn't change behavior. Oops ! I was wrong. I will post mail to v2 patch for detail Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4a14f10b8c90..7e0464ec802e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1202,7 +1202,6 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *cpu_dai; struct snd_soc_dai *codec_dai; - bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; int i; mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass); @@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) /* apply codec digital mute */ for_each_rtd_codec_dai(rtd, i, codec_dai) { - int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK]; - int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE]; - - if ((playback && playback_active == 1) || - (!playback && capture_active == 1)) + if (codec_dai->active == 1) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); }