Re: [PATCH] soundwire: intel: trap TRIGGER_SUSPEND in .trigger callback

[Takashi Iwai <tiwai@suse.de>]

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