Re: [PATCH 3/7] soundwire: intel: improve suspend flows

[Vinod Koul]

On 13-12-21, 13:46, Bard Liao wrote:
> From: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
> 
> This patch provides both a simplification of the suspend flows and a
> better balanced operation during suspend/resume transition, as part of
> the transition of Sound Open Firmware (SOF) to dynamic pipelines: the
> DSP resources are only enabled when required instead of enabled on
> startup.
> 
> 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 component suspend is not removed but instead modified to deal with
> a corner case: when a substream is PAUSED, the ALSA core does not
> throw the TRIGGER_SUSPEND. This is problematic since the refcount for
> all pipelines and widgets is not balanced, leading to issues on
> resume. The trigger callback keeps track of the 'paused' state with a
> new flag, which is tested during the component suspend called later to
> release the remaining DSP resources. These resources will be
> re-enabled in the .prepare step.
> 
> The IPC used in the TRIGGER_SUSPEND to release DSP resources is not a
> problem since the BE dailink is already marked as non-atomic.

Acked-By: Vinod Koul <vkoul@xxxxxxxxxx>

-- 
~Vinod