Date:   Thu, 23 Dec 2021 12:26:41 +0530
From:   Vinod Koul <vkoul@kernel.org>
To:     Bard Liao <yung-chuan.liao@linux.intel.com>
Cc:     alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        tiwai@suse.de, broonie@kernel.org, gregkh@linuxfoundation.org,
        srinivas.kandagatla@linaro.org,
        pierre-louis.bossart@linux.intel.com, sanyog.r.kale@intel.com,
        bard.liao@intel.com
Subject: Re: [PATCH 3/7] soundwire: intel: improve suspend flows
Message-ID: <YcQdqT6M+W9du8wy@matsya>
References: <20211213054634.30088-1-yung-chuan.liao@linux.intel.com>
 <20211213054634.30088-4-yung-chuan.liao@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211213054634.30088-4-yung-chuan.liao@linux.intel.com>
Precedence: bulk

On 13-12-21, 13:46, Bard Liao wrote:
> From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> 
> 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@kernel.org>

-- 
~Vinod