Message-ID: <875yb5frnl.wl-kuninori.morimoto.gx@renesas.com>
From:   Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To:     Shengjiu Wang <shengjiu.wang@nxp.com>
Cc:     shengjiu.wang@gmail.com, lgirdwood@gmail.com, broonie@kernel.org,
        perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC: soc-pcm: fix hw->formats cleared by soc_pcm_hw_init() for dpcm
In-Reply-To: <1678346017-3660-1-git-send-email-shengjiu.wang@nxp.com>
References: <1678346017-3660-1-git-send-email-shengjiu.wang@nxp.com>
User-Agent: Wanderlust/2.15.9 Emacs/26.3 Mule/6.0
Content-Type: text/plain; charset=US-ASCII
Date:   Sun, 12 Mar 2023 23:58:59 +0000
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?sOskYpvIwHIl1b6HYDWBxEF3U6hXDyZ5qqj/vyr9MKCdnO4eZdqEBDXrWyKV?=
 =?us-ascii?Q?BkEfsKZNDhh4Qf5L0Xz1sa94NGWbyF8CLQz7gPWK2C3Ul0Jz9OCZ2DEgtGPA?=
 =?us-ascii?Q?0AFsMaJAfQT/6qNhD+eoNIt7skEFay014HF/YlqAnAQYza7NgP8FDcPhipt/?=
 =?us-ascii?Q?khCl8LDT5ZIk8Fpe6wkW0knTu80SNnSNj4D5mazzUVSzzpuxAJe+opn5mLv9?=
 =?us-ascii?Q?+T1XMhi7c8ZTBtiy8HQAcbz0S1gX/vhZHySZEq5twKef54Eo+5RpUJ7FA1EQ?=
 =?us-ascii?Q?uS7g7hTpnzVTelPIgdq/RAiHCPzrEVEISo9BEOsXcXXHT2OkRPhxYovRut6H?=
 =?us-ascii?Q?pWmCxk/157/C+CPsU9nn09V1duCgdsvC2nyArL5EPRZjisYR56rXUiOFS13Q?=
 =?us-ascii?Q?qPXkYlR8XpIygH8HBd2IFqJ+XGc6Yta/B/HLKWIPznPw6RUyVlavkvkvacxp?=
 =?us-ascii?Q?bIBYOkHx8NQy8IHNSvCDqsbL1U3bIIsQ3rj/dum80TkQVr6RGEwziARZunRJ?=
 =?us-ascii?Q?6nAQ8d/vD7eA/EwJFCPeY6ovTNszFSTH6i5UPQL881psBOh5sob7vW02KZT4?=
 =?us-ascii?Q?le6nZ6vAJzyFWRvEGHXxVfk8F650G9hA5BRRlUuJ+a2iH/QlAXzNcrrz0Sjl?=
 =?us-ascii?Q?51v2KZYKkcJTUhDniNnRwtOoQIjw/i/7YcB69jebLHOmxqEWrtvR6PI50IZf?=
 =?us-ascii?Q?nz+SlxbWbXFZERCS9HXIdAQelNBQg5qtbl5TT1nGkFCo7oL2RusGJlNTtCXx?=
 =?us-ascii?Q?8+NWoWhJ1hLGLze0w+Pc1e7z7pfWwfgKT1p7U3sIlXTRonoP+l1MKLUNJryc?=
 =?us-ascii?Q?upnDmyYfS4kc5TIthWFOID92WhhlZKgpRrQmCbHgcos4CqhhP4+1uDJu6Bme?=
 =?us-ascii?Q?n2TYofmkIWZMW+QC847UuAIDhwZmm3/z5+w4LtcPYQxJQUAXfm8FxJztucrp?=
 =?us-ascii?Q?VIoZRjz+Xd7VWyc/2FHfYkuGW5zMeFP681VJwQ8RDObtYqQn1yenppwaY09S?=
 =?us-ascii?Q?ilivgHAqav8+7rtpVgix4huPte4Gc02f/rfa6Psm0u/gMdYxwnQO06AA5qW0?=
 =?us-ascii?Q?xRDnz+HtTLeAu+aUAqo9IGd9IuVQB2kLrEGp1UaoL6sJspD/1SpKGY59D/kf?=
 =?us-ascii?Q?lnswKyEtoIBCRpjdnkOLRghSKnqGM2zdYyehwaxGqjRPWGGaYOE1yHu3a3ov?=
 =?us-ascii?Q?c46O7CwC9iNrNKtbpSfBUnx1r95kWhRIV6vEpP4mWCTOp2d+MCsnOhHH1av3?=
 =?us-ascii?Q?pelAK79TBvzIpNdSuGVTafTc/708S1j9Ot5eaj7xnHpODt+zPSjMtth8/616?=
 =?us-ascii?Q?uNkjiLe2ovh+/6LI0ifvA2R+VTMC+NZpmgh5uW/0gAs8yN2JODIspaAI7cOF?=
 =?us-ascii?Q?ZRVMaJcPPoqpik3G2eoAYG03Cew5c0MwZaZ9PsZscRhgSW7LWNPRYCMRqpbl?=
 =?us-ascii?Q?mXF28k234gkQ3zWu/M6EPhyVgDI7FkmPRxNKYM/FbhII/HiBtAR/ekKW1gjS?=
 =?us-ascii?Q?4tB2whB32WVYILy0QThsr9vcNHDhZ3O3EKYAehtWN6V7J3GEz7FrzoDs3sew?=
 =?us-ascii?Q?cvFMsrXpW+aaUbjD+kSpYOSwX4X6qCJ0WpQxWfcbmri+jwq+7hANBUyAQ4vU?=
 =?us-ascii?Q?M5rAFMFrKNRXmbC6zdhS6PM=3D?=
X-OriginatorOrg: renesas.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ce9b2378-74c4-4a84-4c94-08db2355bfd0
X-MS-Exchange-CrossTenant-AuthSource: OS3PR01MB8426.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Mar 2023 23:59:00.9097
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zAIC+ptzk7eerZL5Ka/+kbLuRBclTMvC9k/pIt4bJpnHUlAATQNVHFG4RlMmKjf4x/qWJQ7D3Vp0gr+noQhyw8PtXHM12qJcnIaB20rX9Yp4R47O5ZmE2sj+4U8Xcfl+
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYBPR01MB5503
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Hi Shengjiu

> The hw->formats may be set by snd_dmaengine_pcm_refine_runtime_hwparams()
> in component's startup()/open(), but soc_pcm_hw_init() will init
> hw->formats in dpcm_runtime_setup_fe() after component's startup()/open(),
> which causes the valuable hw->formats to be cleared.
> 
> So need to store the hw->formats before initialization, then restore
> it after initialization.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/soc-pcm.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 5eb056b942ce..7958c9defd49 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1661,10 +1661,14 @@ static void dpcm_runtime_setup_fe(struct snd_pcm_substream *substream)
>  	struct snd_pcm_hardware *hw = &runtime->hw;
>  	struct snd_soc_dai *dai;
>  	int stream = substream->stream;
> +	u64 formats = hw->formats;
>  	int i;
>  
>  	soc_pcm_hw_init(hw);
>  
> +	if (formats)
> +		hw->formats &= formats;
> +

If my understanding was correct, dpcm_runtime_setup_fe() (B) is called
after __soc_pcm_open() (A), and you updated (B) part.

	static int dpcm_fe_dai_startup(...) {
		...
(A)		ret = __soc_pcm_open(fe, fe_substream);
		...
(B)		dpcm_runtime_setup_fe(fe_substream);
		...
	}

But, it is doing same things under (A), too.
Do we need to initialize hw many times ? I'm not sure.
Can we simply remove soc_pcm_hw_init() from dpcm_runtime_setup_fe() ?

(A)	static int __soc_pcm_open()
	{
		...
(X)		soc_pcm_init_runtime_hw(substream);
		...
	}

(X)	static void soc_pcm_init_runtime_hw(...)
	{
=>		u64 formats = hw->formats;

(Y)		snd_soc_runtime_calc_hw(rtd, hw, substream->stream);

=>		if (formats)
=>			hw->formats &= formats;
	}

(Y)	int snd_soc_runtime_calc_hw(...)
	{
		...
=>		soc_pcm_hw_init(hw);
		...
	}

Thank you for your help !!

Best regards
---
Kuninori Morimoto