On Thu, Nov 21, 2019 at 1:13 PM Takashi Iwai <tiwai@xxxxxxx> wrote: > On Thu, 21 Nov 2019 21:46:17 +0100, > Sridharan, Ranjani wrote: > > > > > > > > Hi Takashi, > > > > > > Sorry the stress tests took a while. > > > As we discussed earlier, adding the sync_stop() op didnt quite > help the > > SOF > > > driver in removing the delayed work for snd_pcm_period_elapsed(). > > > > Yeah, that's understandable. If the stop operation itself needs some > > serialization, sync_stop() won't influence at all. > > > > However, now after these discussions, I have some concerns in the > > current code: > > > > - The async work started by schedule_work() may be executed > > (literally) immediately. So if the timing or the serialization > > matters, it doesn't guarantee at all. The same level of > concurrency > > can happen at any time. > > > > - The period_elapsed work might be pending at prepare or other > > operation; > > the async work means also that it doesn't guarantee its execution > in > > time, and it might be delayed much, and the PCM core might go to > > prepare or other state even before the work is executed. > > > > The second point can be fixed easily now with sync_stop. You can > just > > put flush_work() in sync_stop in addition to synchronize_irq(). > > > > But the first point is still unclear. More exactly, which operation > > does it conflict? Does it the playback drain? Then it might take > > very long (up to seconds) to block the next operation? > > > > Hi Takashi, > > > > As I understand the original intention for adding the > period_elapsed_work() > > was that snd_pcm_period_elapsed() could cause a STOP trigger while the > > current IPC interrupt is still being handled. > > In this case, the STOP trigger generates an IPC to the DSP but the host > never > > misses the IPC response from the DSP because it is still handling the > previous > > interrupt. > > OK, that makes sense. So the issue is that the trigger stop itself > requires the ack via the interrupt and it can't be caught because it's > being called from the irq handler itself. > > In that case, though, another solution would be to make the trigger- > stop an async work (but conditionally) while processing the normal > period_elapsed in the irq handler. That is, set some flag before > calling snd_pcm_period_elapsed(), and in the trigger-stop, check the > flag. If the flag is set, schedule the work and return. And, you'll > sync this async work with sync_stop(). In that way, the period > handling is processed without any delay more lightly. > OK, that makes sense. Thanks for the suggestion. Regarding your previous comment about adding flush_work() to the sync_stop() op, would that still be required? Thanks, Ranjani > > > thanks, > > Takashi > > > Adding Keyon who added this change to add more and clarify your concerns. > > > > Thanks, > > Ranjani > > > > thanks, > > > > Takashi > > > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
