From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 362FDC4338F for ; Tue, 27 Jul 2021 06:31:20 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A510C60F58 for ; Tue, 27 Jul 2021 06:31:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A510C60F58 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 5D62C1AD0; Tue, 27 Jul 2021 08:30:26 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 5D62C1AD0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1627367476; bh=ZsbivaInfUgF0KiBjZSvUA4FEw5pv/MYfnyJjhWhh1w=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=l3U+GirKopbh/r8rKmXD8QvJQGCkAE/O4Ijp0LiMOIlP77tcv3/fCxuBMmBsJYuE0 b3Zx7Q2nvt5broao/Br4mlpzr+lBFp50Zg0kT1R4rF1w5B2SnfkVoRzMuXnE7CMFly Va5laDZ8xH95DLwmngkXeWny8AhBIiVpE5rTjffA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A7B88F80258; Tue, 27 Jul 2021 08:30:25 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 38B0EF8026C; Tue, 27 Jul 2021 08:30:23 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id AF4BEF80159 for ; Tue, 27 Jul 2021 08:30:12 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz AF4BEF80159 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="qcaB5gaN"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="ydfyZsG0" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B0B9D200BC; Tue, 27 Jul 2021 06:30:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1627367411; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9U+ad6d6diRLHDBw2iNS4zAU/sgxY3O4O6inqf0KXOk=; b=qcaB5gaN4j8jMDHYdVnpeQmpG0nPGB1JR2GvbQ/+0BlXcSPpRSN+3Re1ZH0Q0w503Vx1v8 lfCuJAD4K92CFIvQkk3SAkl4zR8ia0A9MWkqKCKf1HO5hCkfB2+I4VcmielCkXbuh7OPAO 6TKtwq08OOeX/tpnhoxAmhxVfi8xr/4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1627367411; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9U+ad6d6diRLHDBw2iNS4zAU/sgxY3O4O6inqf0KXOk=; b=ydfyZsG0jFYbFusjX5pzLE4topu90NSjKg8FajPa7vTP1rXrlKRZkXNjU3PWD18w0lZtKO nJAq7pImn52OoGDQ== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 9DFE8A3B87; Tue, 27 Jul 2021 06:30:11 +0000 (UTC) Date: Tue, 27 Jul 2021 08:30:11 +0200 Message-ID: From: Takashi Iwai To: Bard Liao Subject: Re: [PATCH] soundwire: intel: trap TRIGGER_SUSPEND in .trigger callback In-Reply-To: <20210727053256.29949-1-yung-chuan.liao@linux.intel.com> References: <20210727053256.29949-1-yung-chuan.liao@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: vkoul@kernel.org, alsa-devel@alsa-project.org, broonie@kernel.org, pierre-louis.bossart@linux.intel.com, bard.liao@intel.com X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Tue, 27 Jul 2021 07:32:56 +0200, Bard Liao wrote: > > From: Ranjani Sridharan > > 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? If it's just for releasing before re-acquiring a stream, you can do it in sync_stop PCM ops instead, too. This is called in prior to prepare and hw_params ops when a stream has been stopped or suspended beforehand. thanks, Takashi > > Co-developed-by: Pierre-Louis Bossart > Signed-off-by: Pierre-Louis Bossart > Signed-off-by: Ranjani Sridharan > Reviewed-by: Péter Ujfalusi > Reviewed-by: Rander Wang > Signed-off-by: Bard Liao > --- > drivers/soundwire/intel.c | 53 +++++++++++++++++++++------------------ > 1 file changed, 29 insertions(+), 24 deletions(-) > > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c > index fb9c23e13206..a0178779a5ba 100644 > --- a/drivers/soundwire/intel.c > +++ b/drivers/soundwire/intel.c > @@ -1005,29 +1005,6 @@ static void intel_shutdown(struct snd_pcm_substream *substream, > pm_runtime_put_autosuspend(cdns->dev); > } > > -static int intel_component_dais_suspend(struct snd_soc_component *component) > -{ > - struct sdw_cdns_dma_data *dma; > - struct snd_soc_dai *dai; > - > - for_each_component_dais(component, dai) { > - /* > - * we don't have a .suspend dai_ops, and we don't have access > - * to the substream, so let's mark both capture and playback > - * DMA contexts as suspended > - */ > - dma = dai->playback_dma_data; > - if (dma) > - dma->suspended = true; > - > - dma = dai->capture_dma_data; > - if (dma) > - dma->suspended = true; > - } > - > - return 0; > -} > - > static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, > void *stream, int direction) > { > @@ -1056,11 +1033,39 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai, > return dma->stream; > } > > +static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) > +{ > + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); > + struct sdw_intel *sdw = cdns_to_intel(cdns); > + struct sdw_cdns_dma_data *dma; > + > + /* > + * The .prepare callback is used to deal with xruns and resume operations. In the case > + * of xruns, the DMAs and SHIM registers cannot be touched, but for resume operations the > + * DMAs and SHIM registers need to be initialized. > + * the .trigger callback is used to track the suspend case only. > + */ > + if (cmd != SNDRV_PCM_TRIGGER_SUSPEND) > + return 0; > + > + dma = snd_soc_dai_get_dma_data(dai, substream); > + if (!dma) { > + dev_err(dai->dev, "failed to get dma data in %s\n", > + __func__); > + return -EIO; > + } > + > + dma->suspended = true; > + > + return intel_free_stream(sdw, substream, dai, sdw->instance); > +} > + > static const struct snd_soc_dai_ops intel_pcm_dai_ops = { > .startup = intel_startup, > .hw_params = intel_hw_params, > .prepare = intel_prepare, > .hw_free = intel_hw_free, > + .trigger = intel_trigger, > .shutdown = intel_shutdown, > .set_sdw_stream = intel_pcm_set_sdw_stream, > .get_sdw_stream = intel_get_sdw_stream, > @@ -1071,6 +1076,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { > .hw_params = intel_hw_params, > .prepare = intel_prepare, > .hw_free = intel_hw_free, > + .trigger = intel_trigger, > .shutdown = intel_shutdown, > .set_sdw_stream = intel_pdm_set_sdw_stream, > .get_sdw_stream = intel_get_sdw_stream, > @@ -1078,7 +1084,6 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { > > static const struct snd_soc_component_driver dai_component = { > .name = "soundwire", > - .suspend = intel_component_dais_suspend > }; > > static int intel_create_dai(struct sdw_cdns *cdns, > -- > 2.17.1 >