Re: [PATCH] ASoC: soc-pcm: fix hw->formats cleared by soc_pcm_hw_init() for dpcm

13 Mar 2023

Hi
On Mon, Mar 13, 2023 at 7:59 AM Kuninori Morimoto <
kuninori.morimoto.gx@renesas.com> wrote:
...
Hi Shengjiu
...
The hw->formats may be set by snd_dmaengine_pcm_refine_runtime_hwparams()
in component's startup()/open(), but soc_pcm_hw_init() will init
hw->formats in dpcm_runtime_setup_fe() after component's
startup()/open(),
...
which causes the valuable hw->formats to be cleared.
So need to store the hw->formats before initialization, then restore
it after initialization.
Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com
sound/soc/soc-pcm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 5eb056b942ce..7958c9defd49 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1661,10 +1661,14 @@ static void dpcm_runtime_setup_fe(struct
snd_pcm_substream *substream)
...
  struct snd_pcm_hardware *hw = &runtime->hw;
  struct snd_soc_dai *dai;
  int stream = substream->stream;


u64 formats = hw->formats;
int i;

soc_pcm_hw_init(hw);



if (formats)


        hw->formats &= formats;




If my understanding was correct, dpcm_runtime_setup_fe() (B) is called
after __soc_pcm_open() (A), and you updated (B) part.
    static int dpcm_fe_dai_startup(...) {
            ...

(A)             ret = __soc_pcm_open(fe, fe_substream);
                ...
(B)             dpcm_runtime_setup_fe(fe_substream);
                ...
        }
But, it is doing same things under (A), too.
Do we need to initialize hw many times ? I'm not sure.
Can we simply remove soc_pcm_hw_init() from dpcm_runtime_setup_fe() ?
(A)     static int __soc_pcm_open()
        {
                ...
(X)             soc_pcm_init_runtime_hw(substream);
                ...
        }
Here there is condition:
/* Dynamic PCM DAI links compat checks use dynamic capabilities */
        if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm)
                goto dynamic;
/* Check that the codec and cpu DAIs are compatible */
        soc_pcm_init_runtime_hw(substream);
which only calls soc_pcm_init_runtime_hw() for non dpcm case.
So the initialization of hw is not many times.
For dpcm the code here will be skipped. The initialization happens
only in dpcm_runtime_setup_fe().
Best regards
Wang shengjiu
...
(X)     static void soc_pcm_init_runtime_hw(...)
        {
=>              u64 formats = hw->formats;
(Y)             snd_soc_runtime_calc_hw(rtd, hw, substream->stream);
=>              if (formats)
=>                      hw->formats &= formats;
        }
(Y)     int snd_soc_runtime_calc_hw(...)
        {
                ...
=>              soc_pcm_hw_init(hw);
                ...
        }
Thank you for your help !!
Best regards
Kuninori Morimoto

    