mbox series

[v3,00/21] ASoC: replace dpcm_playback/capture to playback/capture_only

Message ID 87o7m4lzl3.wl-kuninori.morimoto.gx@renesas.com
Headers show
Series ASoC: replace dpcm_playback/capture to playback/capture_only | expand

Message

Kuninori Morimoto May 29, 2023, 1:02 a.m. UTC 
Hi Mark

This is v3 patch-set of dpcm_playback/capture flag cleanup.

Current ASoC DPCM need dpcm_playback/capture flags to use it.
But we are using playback/capture_only flag on Normal/Codec2Codec case.
I think these are duplicated, we can share same flags for all cases.

On v1 patch-set, we have noticed that it has been don't check
DPCM BE Codec valid before.
Therefore, it breaks Intel sound card probing if it strictly check
all cases validation. To solve this issue, we need to update Intel driver
first, but it is very complex and I can't test it.
Thus, this patch-set keep not to check DPCM BE Codec to keep compatibility.
But it should be updated in the future.

v2 patch-set had error handling mistake. CPU:Codec = 1:N has been possible to
handling, but v2 patch-set handled it as error. v3 solved it.

v2 -> v3
	- fixup error handling

v1 -> v2
	- Add Reviewed-by
	- Separate Intel patch
	- tidyup playback/capture_only flags conversion

Link: https://lore.kernel.org/r/87ttw1gqgn.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/87353uqjiu.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/ab3f0c0a-62fd-a468-b3cf-0e4b59bac6ae@linux.intel.com

Kuninori Morimoto (21):
  ASoC: soc-pcm.c: indicate error if stream has no playback no capture
  ASoC: soc-pcm.c: use dai_link on soc_get_playback_capture()
  ASoC: soc-pcm.c: cleanup soc_get_playback_capture() error
  ASoC: soc-pcm.c: use temporary variable at soc_get_playback_capture()
  ASoC: soc-pcm.c: tidyup playback/capture_only at soc_get_playback_capture()
  ASoC: soc-pcm.c: cleanup normal connection loop at soc_get_playback_capture() part1
  ASoC: soc-pcm.c: cleanup normal connection loop at soc_get_playback_capture() part2
  ASoC: soc-pcm.c: cleanup soc_get_playback_capture()
  ASoC: amd: replace dpcm_playback/capture to playback/capture_only
  ASoC: fsl: replace dpcm_playback/capture to playback/capture_only
  ASoC: sof: replace dpcm_playback/capture to playback/capture_only
  ASoC: meson: replace dpcm_playback/capture to playback/capture_only
  ASoC: Intel: replace dpcm_playback/capture to playback/capture_only
  ASoC: samsung: replace dpcm_playback/capture to playback/capture_only
  ASoC: mediatek: replace dpcm_playback/capture to playback/capture_only
  ASoC: soc-dai.c: replace dpcm_playback/capture to playback/capture_only
  ASoC: Intel/avs: replace dpcm_playback/capture to playback/capture_only
  ASoC: soc-core.c: replace dpcm_playback/capture to playback/capture_only
  ASoC: soc-topology.c: replace dpcm_playback/capture to playback/capture_only
  ASoC: soc-compress.c: replace dpcm_playback/capture to playback/capture_only
  ASoC: soc-pcm.c: remove dpcm_playback/capture

 include/sound/soc.h                           |   4 -
 sound/soc/amd/acp-da7219-max98357a.c          |  20 +--
 sound/soc/amd/acp-es8336.c                    |   2 -
 sound/soc/amd/acp/acp-mach-common.c           |  20 +--
 sound/soc/amd/acp3x-rt5682-max9836.c          |   6 +-
 sound/soc/amd/vangogh/acp5x-mach.c            |   3 -
 sound/soc/fsl/fsl-asoc-card.c                 |  16 +--
 sound/soc/fsl/imx-audmix.c                    |   6 +-
 sound/soc/fsl/imx-card.c                      |   4 +-
 sound/soc/intel/avs/boards/da7219.c           |   2 -
 sound/soc/intel/avs/boards/dmic.c             |   4 +-
 sound/soc/intel/avs/boards/hdaudio.c          |   4 -
 sound/soc/intel/avs/boards/i2s_test.c         |   2 -
 sound/soc/intel/avs/boards/max98357a.c        |   2 +-
 sound/soc/intel/avs/boards/max98373.c         |   2 -
 sound/soc/intel/avs/boards/max98927.c         |   2 -
 sound/soc/intel/avs/boards/nau8825.c          |   2 -
 sound/soc/intel/avs/boards/rt274.c            |   2 -
 sound/soc/intel/avs/boards/rt286.c            |   2 -
 sound/soc/intel/avs/boards/rt298.c            |   2 -
 sound/soc/intel/avs/boards/rt5682.c           |   2 -
 sound/soc/intel/avs/boards/ssm4567.c          |   2 -
 sound/soc/intel/boards/bdw-rt5650.c           |   4 -
 sound/soc/intel/boards/bdw-rt5677.c           |   4 -
 sound/soc/intel/boards/bdw_rt286.c            |  10 +-
 sound/soc/intel/boards/bxt_da7219_max98357a.c |  32 +++--
 sound/soc/intel/boards/bxt_rt298.c            |  26 ++--
 sound/soc/intel/boards/bytcht_cx2072x.c       |   6 +-
 sound/soc/intel/boards/bytcht_da7213.c        |   6 +-
 sound/soc/intel/boards/bytcht_es8316.c        |   6 +-
 sound/soc/intel/boards/bytcht_nocodec.c       |   6 +-
 sound/soc/intel/boards/bytcr_rt5640.c         |   6 +-
 sound/soc/intel/boards/bytcr_rt5651.c         |   6 +-
 sound/soc/intel/boards/bytcr_wm5102.c         |   6 +-
 sound/soc/intel/boards/cht_bsw_max98090_ti.c  |   6 +-
 sound/soc/intel/boards/cht_bsw_nau8824.c      |   6 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c       |   6 +-
 sound/soc/intel/boards/cht_bsw_rt5672.c       |   6 +-
 sound/soc/intel/boards/cml_rt1011_rt5682.c    |  14 +--
 sound/soc/intel/boards/ehl_rt5660.c           |  14 +--
 sound/soc/intel/boards/glk_rt5682_max98357a.c |  30 +++--
 sound/soc/intel/boards/hsw_rt5640.c           |  10 +-
 sound/soc/intel/boards/kbl_da7219_max98357a.c |  26 ++--
 sound/soc/intel/boards/kbl_da7219_max98927.c  |  54 ++++----
 sound/soc/intel/boards/kbl_rt5660.c           |  18 ++-
 sound/soc/intel/boards/kbl_rt5663_max98927.c  |  44 +++----
 .../intel/boards/kbl_rt5663_rt5514_max98927.c |  22 ++--
 sound/soc/intel/boards/skl_hda_dsp_common.c   |  14 +--
 .../soc/intel/boards/skl_nau88l25_max98357a.c |  26 ++--
 sound/soc/intel/boards/skl_nau88l25_ssm4567.c |  26 ++--
 sound/soc/intel/boards/skl_rt286.c            |  26 ++--
 sound/soc/intel/boards/sof_cs42l42.c          |  12 +-
 sound/soc/intel/boards/sof_da7219_max98373.c  |  16 +--
 sound/soc/intel/boards/sof_es8336.c           |   8 +-
 sound/soc/intel/boards/sof_nau8825.c          |  12 +-
 sound/soc/intel/boards/sof_pcm512x.c          |   8 +-
 sound/soc/intel/boards/sof_rt5682.c           |  12 +-
 sound/soc/intel/boards/sof_sdw.c              |   4 +-
 sound/soc/intel/boards/sof_ssp_amp.c          |  11 +-
 sound/soc/intel/boards/sof_wm8804.c           |   2 -
 sound/soc/mediatek/mt2701/mt2701-cs42448.c    |  20 +--
 sound/soc/mediatek/mt2701/mt2701-wm8960.c     |   6 +-
 sound/soc/mediatek/mt6797/mt6797-mt6351.c     |  24 ++--
 sound/soc/mediatek/mt8173/mt8173-max98090.c   |   6 +-
 .../mediatek/mt8173/mt8173-rt5650-rt5514.c    |   6 +-
 .../mediatek/mt8173/mt8173-rt5650-rt5676.c    |  10 +-
 sound/soc/mediatek/mt8173/mt8173-rt5650.c     |  10 +-
 .../mediatek/mt8183/mt8183-da7219-max98357.c  |  34 +++---
 .../mt8183/mt8183-mt6358-ts3a227-max98357.c   |  34 +++---
 .../mt8186/mt8186-mt6366-da7219-max98357.c    |  86 +++++--------
 .../mt8186/mt8186-mt6366-rt1019-rt5682s.c     |  86 +++++--------
 sound/soc/mediatek/mt8188/mt8188-mt6359.c     |  48 ++++----
 .../mt8192/mt8192-mt6359-rt1015-rt5682.c      |  78 ++++++------
 sound/soc/mediatek/mt8195/mt8195-mt6359.c     |  60 +++++----
 sound/soc/meson/axg-card.c                    |   8 +-
 sound/soc/meson/meson-card-utils.c            |   4 +-
 sound/soc/samsung/odroid.c                    |  10 +-
 sound/soc/soc-compress.c                      |  11 +-
 sound/soc/soc-core.c                          |  20 +--
 sound/soc/soc-dai.c                           |   6 +-
 sound/soc/soc-pcm.c                           | 115 +++++++-----------
 sound/soc/soc-topology-test.c                 |   2 -
 sound/soc/soc-topology.c                      |   4 +-
 sound/soc/sof/nocodec.c                       |   4 -
 84 files changed, 512 insertions(+), 842 deletions(-)

Comments

Kuninori Morimoto May 29, 2023, 11:49 p.m. UTC | #1 
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
Kuninori Morimoto May 30, 2023, 12:42 a.m. UTC | #2 
Hi Mark, again

> > @@ -2795,11 +2795,7 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
> >  			SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
> >  
> >  		for_each_rtd_codec_dais(rtd, i, codec_dai) {
> > -			if (dai_link->num_cpus == 1) {
> > -				cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> > -			} else if (dai_link->num_cpus == dai_link->num_codecs) {
> > -				cpu_dai = asoc_rtd_to_cpu(rtd, i);
> > -			}
> > +			cpu_dai = asoc_rtd_to_cpu(rtd, i);
> 
> Grr
> I noticed that this patch is also wrong.
> It doesn't care CPU:Codec = 1:N case.
> Need v4 patch

The playable/capturable calculation are similar operation,
thus I had been thinking that it is possible to merge.
But there are many patterns and has difference, moreover complex
(therefor we noticed that lack of DPCM BE Codec check,
but not sure it is lack or intentional).

It seems that my posted patch-set has a high possibility to
contain some new bugs which is difficult to notice.

I'm still thinking that we can merge it and have more simple
and understandable code, but I'm starting to think that it
should more caution.

So, I will post first few small cleanup patches only instead
of v4 patch-set for this time.

Thank you for your help !!

Best regards
---
Kuninori Morimoto