[alsa-devel] [Question about DPCM] dpcm_run_new_update races again xrun

[Qiao Zhou]

Hi Mark, Liam

I met one BE not-function issue when developing DPCM feature, and found
dpcm_run_new_update is not protected well against xrun(interrupt context).
Could you help to give some suggestions?

The scenario is like this, taking audio playback for example:
1, FE1 <-> BE1, working well for some time.
2, disconnect BE1 and connect BE2(FE1 <-> BE2)
3, during FE1 connecting BE2, FE1 is still streaming data normally. Then an
under-run happens. Below are the code sequence.
    
soc_dpcm_runtime_update() {
    ...
    dpcm_connect_be() // connect FE1 & BE2
    dpcm_run_new_update(fe, stream) {
      fe->dpcm.runtime_update = SND_SOC_DPCM_UPDATE_BE
      dpcm_run_update_startup(fe, stream) {
        dpcm_be_dai_startup(fe, stream)
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
   |    // under-run happens (interrupt context)                       |
   |    snd_pcm_stop(substream) {                                      |
   |        dpcm_fe_dai_trigger(STOP) {                                |
   |            fe->dpcm.runtime_update = SND_SOC_DPCM_UPDATE_FE       |
   |            // trigger stop FE1, BE2                               |
   |            fe->dpcm.runtime_update = SND_SOC_DPCM_UPDATE_NO       |
   |        }                                                          |
   |    }                                                              |
   |_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _|
        dpcm_be_dai_hw_params(fe, stream)
        dpcm_be_dai_prepare(fe, stream)
        if(fe state == STOP)
            return 0;
        dpcm_be_dai_trigger(fe, ....)
      fe->dpcm.runtime_update = SND_SOC_DPCM_UPDATE_NO
    ...
    }
}

After xrun handler finishes, the FE runtime update staus is UPDATE_NO. Then
the following dpcm_be_dai_hw_params & dpcm_be_dai_prepare skip related driver
API excuting with this FE runtime status, and return 0 to end runtime-startup.

When user APP(ALSA lib/TinyALSA) detects xrun, usually it will do the substream
prepare and write data again. Due to BE dai has not been ready for hw_params,
the BE dai can't work properly after substream prepare and trigger start. After
that system has no sound and can't be recovered, maybe due to FE1 doesn't know
what's going on inside BE2.

The under-run is random, and under-run handler does what's necessary to be done,
though trigger-stop a BE which has not been started yet is not good in my opinion.

I did an experiment to avoid be udpate checking in dpcm_be_dai_hw_params by
Commenting out "if(!snd_soc_dpcm_be_can_update()}". This change is verified OK
since APP will do the prepare & trigger start to resume the under-run. The patch
is also attached. 

>From current code, dpcm_be_dai_hw_params is only callded by fe_hw_params & this
runtime startup. This be update checking might be redundant or unnecessary in
current code context.

Could you help to give some suggestions? Please help to correct me if anything
wrong. Thanks in advance.

---
 sound/soc/soc-pcm.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 002311a..dfeb1e7 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1697,10 +1697,6 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
 		struct snd_pcm_substream *be_substream =
 			snd_soc_dpcm_get_substream(be, stream);
 
-		/* is this op for this BE ? */
-		if (!snd_soc_dpcm_be_can_update(fe, be, stream))
-			continue;
-
 		/* only allow hw_params() if no connected FEs are running */
 		if (!snd_soc_dpcm_can_be_params(fe, be, stream))
 			continue;
-- 
1.7.9.5

Best Regards
Qiao