On Mon, 02 Aug 2021 06:35:16 +0200, Vinod Koul wrote: > > On 27-07-21, 09:12, Pierre-Louis Bossart wrote: > > Thanks Takashi for the review. > > > > > > >> This patch provides both a simplification of the suspend flows and a > > >> better balanced operation during suspend/resume transition. > > >> > > >> The exiting code relies on a convoluted way of dealing with suspend > > >> signals. Since there is no .suspend DAI callback, we used the > > >> component .suspend and marked all the component DAI dmas as > > >> 'suspended'. The information was used in the .prepare stage to > > >> differentiate resume operations from xrun handling, and only > > >> reinitialize SHIM registers and DMA in the former case. > > >> > > >> While this solution has been working reliably for about 2 years, there > > >> is a much better solution consisting in trapping the TRIGGER_SUSPEND > > >> in the .trigger DAI ops. The DMA is still marked in the same way for > > >> the .prepare op to run, but in addition the callbacks sent to DSP > > >> firmware are now balanced. > > >> > > >> Normal operation: > > >> hw_params -> intel_params_stream > > >> hw_free -> intel_free_stream > > >> > > >> suspend -> intel_free_stream > > >> prepare -> intel_params_stream > > >> > > >> This balanced operation was not required with existing SOF firmware > > >> relying on static pipelines instantiated at every boot. With the > > >> on-going transition to dynamic pipelines, it's however a requirement > > >> to keep the use count for the DAI widget balanced across all > > >> transitions. > > > > > > The trigger callback is handled in the stream lock atomically, and are > > > you sure that you want to operate a possibly heavy task there? > > > > It's a good objection that we didn't think of. > > Doesn't Intel use non atomic trigger to send IPCs which anyway involve > code which can sleep..? sof_sdw.c doesn't seem setting it? Takashi