Re: [PATCH v2 00/16] ASoC: Replace dpcm_playback/capture to playback/capture_only

[Jerome Brunet <jbrunet@baylibre.com>]

On Mon 01 Apr 2024 at 00:27, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote:

> Hi Mark
>
> This is v2 patch-set
>
> When we use DPCM, we need to set dpcm_playback/capture flag.
> If these flag are set, soc_get_playback_capture() will check its
> availability, but non DPCM doesn't need such special flags.
>
> OTOH, it cares playback/capture_only flag. It is needed.
>
> This patch remove DPCM special flag, and replace it playback/capture_only
> flag if needed.

Hi Kuninori-san,

Thanks for taking the time to clean the dpcm flags.
While at it, I wonder if it would be worth taking it a step further.

playback_only and capture_only have implication on each other. If one is
set, the other can/must not be set. This leads to conditions which can
be fairly hard to read and possibly bugs.

I had to re-read the meson patch a few times to make sure it still had the
same meaning, TBH

Wouldn't it be better to replace those 2 flags with a single bitfield ?

something like:

unsigned int directions;
#define PLAYBACK_VALID	BIT(0)
#define CAPTURE_VALID BIT(1)

... or something similar.

I think conditions on the enabled stream would become much clearer like
this. The only invalid configuation would be '!directions', which again
is easier to read, instead of checking if both flags are set.

It would be easy to keep playback_only/capture_only tests, where
necessary, with an helper function.

What do you think ?

Sorry if it is a bit late to discuss this.

>
> v1 -> v2
> 	- based on latest ASoC branch
> 	- keep comment on Intel
> 	- tidyup patch title
> 	- tidyup DPCM BE warning output condition
> 	- Add new patch for Document
>
> Link: https://lore.kernel.org/r/87o7b353of.wl-kuninori.morimoto.gx@renesas.com
>
> Kuninori Morimoto (16):
>   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-core: Replace dpcm_playback/capture to playback/capture_only
>   ASoC: soc-topology: Replace dpcm_playback/capture to
>     playback/capture_only
>   ASoC: soc-compress: Replace dpcm_playback/capture to
>     playback/capture_only
>   ASoC: Intel: avs: boards: Replace dpcm_playback/capture to
>     playback/capture_only
>   ASoC: remove snd_soc_dai_link_set_capabilities()
>   ASoC: soc-pcm: remove dpcm_playback/capture
>   ASoC: soc-pcm: indicate warning if DPCM BE Codec has no settings
>   ASoC: doc: remove .dpcm_playback/capture flags
>
>  Documentation/sound/soc/dpcm.rst              | 14 ++-
>  include/sound/soc-dai.h                       |  1 -
>  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           | 24 ++---
>  sound/soc/amd/acp3x-rt5682-max9836.c          |  6 +-
>  sound/soc/amd/vangogh/acp5x-mach.c            |  6 --
>  sound/soc/fsl/fsl-asoc-card.c                 | 16 ++--
>  sound/soc/fsl/imx-audmix.c                    |  6 +-
>  sound/soc/fsl/imx-card.c                      |  7 +-
>  sound/soc/generic/audio-graph-card.c          |  2 -
>  sound/soc/generic/audio-graph-card2.c         |  2 -
>  sound/soc/generic/simple-card.c               |  2 -
>  sound/soc/intel/avs/boards/da7219.c           |  2 -
>  sound/soc/intel/avs/boards/dmic.c             |  4 +-
>  sound/soc/intel/avs/boards/es8336.c           |  2 -
>  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/rt5514.c           |  2 +-
>  sound/soc/intel/avs/boards/rt5663.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    | 15 ++--
>  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_board_helpers.c    | 13 +--
>  sound/soc/intel/boards/sof_es8336.c           |  8 +-
>  sound/soc/intel/boards/sof_pcm512x.c          |  8 +-
>  sound/soc/intel/boards/sof_sdw.c              |  4 +-
>  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/mt7986/mt7986-wm8960.c     |  6 +-
>  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     | 58 ++++++-------
>  .../mt8192/mt8192-mt6359-rt1015-rt5682.c      | 78 ++++++++---------
>  sound/soc/mediatek/mt8195/mt8195-mt6359.c     | 60 +++++++------
>  sound/soc/meson/axg-card.c                    |  9 +-
>  sound/soc/meson/gx-card.c                     |  1 -
>  sound/soc/meson/meson-card-utils.c            |  4 +-
>  sound/soc/qcom/common.c                       |  1 -
>  sound/soc/samsung/odroid.c                    | 11 ++-
>  sound/soc/soc-compress.c                      | 10 ++-
>  sound/soc/soc-core.c                          | 20 +----
>  sound/soc/soc-dai.c                           | 38 --------
>  sound/soc/soc-pcm.c                           | 87 ++++++++-----------
>  sound/soc/soc-topology-test.c                 |  2 -
>  sound/soc/soc-topology.c                      |  4 +-
>  sound/soc/sof/nocodec.c                       |  4 -
>  91 files changed, 502 insertions(+), 863 deletions(-)


-- 
Jerome