>-----Original Message----- >From: Takashi Iwai <tiwai@xxxxxxx> >Sent: Thursday, November 21, 2019 1:13 PM >To: Sridharan, Ranjani <ranjani.sridharan@xxxxxxxxx> >Cc: Jie, Yang <yang.jie@xxxxxxxxx>; Ranjani Sridharan ><ranjani.sridharan@xxxxxxxxxxxxxxx>; Linux-ALSA <alsa-devel@xxxxxxxxxxxxxxxx>; >Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> >Subject: Re: [PATCH 7/8] ALSA: pcm: Add card sync_irq field > >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. Yes, this was actually the method @Uimonen, Jaska suggested on April, since we have sync_stop() to flush the potential un-finished async trigger_stop task now, it's time to switch to use this solution now. Thanks, ~Keyon > > >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
