From patchwork Tue Sep 29 04:31:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kuninori Morimoto X-Patchwork-Id: 291532 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=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY,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 3A5DEC4346E for ; Tue, 29 Sep 2020 04:34:24 +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 7BEC6208FE for ; Tue, 29 Sep 2020 04:34:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="nLDkHrN/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7BEC6208FE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=renesas.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@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 ED5C01883; Tue, 29 Sep 2020 06:33:31 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz ED5C01883 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1601354062; bh=/ssUSE+1gLDbJ3bN9nFokZFCrYziGeUyELKFwpUzs/I=; h=Date:From:Subject:To:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=nLDkHrN/Izo7juNS/JmBXNsgc5xrYsMfxDhFlGxfGI/h2Tdl5Vr8DRey3Djjr3cOf oBcovQty7yTn194e1uexNpwGq4K6LJKWmNjaKH38GzkKGRJ+OpF93dXEvFAldSfxu1 x6uYdCtZUULaxq2lvyR2p7wq4AufiILfUVhXdcOU= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id EEB9AF802E0; Tue, 29 Sep 2020 06:31:47 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 7B9FFF802E1; Tue, 29 Sep 2020 06:31:46 +0200 (CEST) Received: from relmlie6.idc.renesas.com (relmlor2.renesas.com [210.160.252.172]) by alsa1.perex.cz (Postfix) with ESMTP id 96020F802DF for ; Tue, 29 Sep 2020 06:31:42 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 96020F802DF Date: 29 Sep 2020 13:31:41 +0900 X-IronPort-AV: E=Sophos;i="5.77,316,1596466800"; d="scan'208";a="58205668" Received: from unknown (HELO relmlir5.idc.renesas.com) ([10.200.68.151]) by relmlie6.idc.renesas.com with ESMTP; 29 Sep 2020 13:31:41 +0900 Received: from mercury.renesas.com (unknown [10.166.252.133]) by relmlir5.idc.renesas.com (Postfix) with ESMTP id 2339E40061AD; Tue, 29 Sep 2020 13:31:41 +0900 (JST) Message-ID: <87k0wdgqav.wl-kuninori.morimoto.gx@renesas.com> From: Kuninori Morimoto Subject: [PATCH 4/6] ASoC: soc-component: add mark for snd_soc_pcm_component_hw_params/free() User-Agent: Wanderlust/2.15.9 Emacs/26.3 Mule/6.0 To: Mark Brown In-Reply-To: <87pn65gqcj.wl-kuninori.morimoto.gx@renesas.com> References: <87pn65gqcj.wl-kuninori.morimoto.gx@renesas.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Cc: Linux-ALSA 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" From: Kuninori Morimoto soc_pcm_hw_params() does rollback when failed (A), but, it is almost same as soc_pcm_hw_free(). static int soc_pcm_hw_params(xxx) { ... if (ret < 0) goto xxx_err; ... return ret; ^ component_err: | ... | interface_err: (A) ... | codec_err: | ... v return ret; } The difference is soc_pcm_hw_free() is for all dai/component/substream, rollback is for succeeded part only. This kind of duplicated code can be a hotbed of bugs, thus, we want to share soc_pcm_hw_free() and rollback. Now, soc_pcm_hw_params/free() are handling 1) snd_soc_link_hw_params/free() => 2) snd_soc_pcm_component_hw_params/free() 3) snd_soc_dai_hw_params/free() This patch is for 2) snd_soc_pcm_component_hw_params/free(). The idea of having bit-flag or counter is not enough for this purpose. For example if one DAI is used for 2xPlaybacks for some reasons, and if 1st Playback was succeeded but 2nd Playback was failed, 2nd Playback rollback doesn't need to call shutdown. But it has succeeded bit-flag or counter via 1st Playback, thus, 2nd Playback rollback will call unneeded shutdown. And 1st Playback's necessary shutdown will not be called, because bit-flag or counter was cleared by wrong 2nd Playback rollback. To avoid such case, this patch marks substream pointer when hw_params() was succeeded. If rollback needed, it will check rollback flag and marked substream pointer. One note here is that it cares *previous* hw_params() only now, but we might want to check *whole* marked substream in the future. This patch is using macro named "push/pop", so that it can be easily update. Signed-off-by: Kuninori Morimoto --- include/sound/soc-component.h | 6 +++--- sound/soc/soc-component.c | 19 ++++++++++--------- sound/soc/soc-pcm.c | 7 +++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 2c790ce95259..21f1d120b68e 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -220,6 +220,7 @@ struct snd_soc_component { /* function mark */ struct snd_pcm_substream *mark_module; struct snd_pcm_substream *mark_open; + struct snd_pcm_substream *mark_hw_params; void *mark_pm; #ifdef CONFIG_DEBUG_FS @@ -459,10 +460,9 @@ int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd); void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd); int snd_soc_pcm_component_prepare(struct snd_pcm_substream *substream); int snd_soc_pcm_component_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_component **last); + struct snd_pcm_hw_params *params); void snd_soc_pcm_component_hw_free(struct snd_pcm_substream *substream, - struct snd_soc_component *last); + int rollback); int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream, int cmd); int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 728e93f35ffb..6d719c2db92e 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -779,8 +779,7 @@ int snd_soc_pcm_component_prepare(struct snd_pcm_substream *substream) } int snd_soc_pcm_component_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_component **last) + struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_component *component; @@ -790,33 +789,35 @@ int snd_soc_pcm_component_hw_params(struct snd_pcm_substream *substream, if (component->driver->hw_params) { ret = component->driver->hw_params(component, substream, params); - if (ret < 0) { - *last = component; + if (ret < 0) return soc_component_ret(component, ret); - } } + /* mark substream if succeeded */ + soc_component_mark_push(component, substream, hw_params); } - *last = NULL; return 0; } void snd_soc_pcm_component_hw_free(struct snd_pcm_substream *substream, - struct snd_soc_component *last) + int rollback) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_component *component; int i, ret; for_each_rtd_components(rtd, i, component) { - if (component == last) - break; + if (rollback && !soc_component_mark_match(component, substream, hw_params)) + continue; if (component->driver->hw_free) { ret = component->driver->hw_free(component, substream); if (ret < 0) soc_component_ret(component, ret); } + + /* remove marked substream */ + soc_component_mark_pop(component, substream, hw_params); } } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 969f5774cd00..5d4d0a95891a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -888,7 +888,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) snd_soc_link_hw_free(substream, 0); /* free any component resources */ - snd_soc_pcm_component_hw_free(substream, NULL); + snd_soc_pcm_component_hw_free(substream, 0); /* now free hw params for the DAIs */ for_each_rtd_dais(rtd, i, dai) { @@ -911,7 +911,6 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_soc_component *component; struct snd_soc_dai *cpu_dai; struct snd_soc_dai *codec_dai; int i, ret = 0; @@ -994,7 +993,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, snd_soc_dapm_update_dai(substream, params, cpu_dai); } - ret = snd_soc_pcm_component_hw_params(substream, params, &component); + ret = snd_soc_pcm_component_hw_params(substream, params); if (ret < 0) goto component_err; @@ -1003,7 +1002,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, return ret; component_err: - snd_soc_pcm_component_hw_free(substream, component); + snd_soc_pcm_component_hw_free(substream, 1); i = rtd->num_cpus;