On 02-08-21, 07:49, Takashi Iwai wrote:
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?
Yes I think init_dai_link() should set it. Maybe Pierre/Bard would know why.