Re: [PATCH v1 1/1] ASoC: SOF: ipc4-topology: Allocate ref_params on stack

[Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>]

On 12/03/2025 18:05, Andy Shevchenko wrote:
> Currently the compiler (clang 19.1.7) is not happy about the size of
> the stack frame in sof_ipc4_prepare_copier_module:
> 
> sound/soc/sof/ipc4-topology.c:1800:1: error: stack frame size (1288) exceeds limit (1024) in 'sof_ipc4_prepare_copier_module' [-Werror,-Wframe-larger-than]
>  1800 | sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>       | ^
> 
> Work around this by allocating ref_params on stack, as it looks the biggest
> variable on stack right now.
> 
> Note, this only happens when compile for 32-bit machines (x86_32 in my case).

I don't have better idea for fix, apart from somehow refactor this huge
function.
Thank you for the fix,

Acked-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  sound/soc/sof/ipc4-topology.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
> index 6d5cda813e48..41110ee6edc7 100644
> --- a/sound/soc/sof/ipc4-topology.c
> +++ b/sound/soc/sof/ipc4-topology.c
> @@ -7,6 +7,7 @@
>  //
>  //
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <uapi/sound/sof/tokens.h>
>  #include <sound/pcm_params.h>
>  #include <sound/sof/ext_manifest4.h>
> @@ -1807,8 +1808,8 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
>  	struct sof_ipc4_copier_data *copier_data;
>  	int input_fmt_index, output_fmt_index;
> -	struct snd_pcm_hw_params ref_params;
>  	struct sof_ipc4_copier *ipc4_copier;
> +	struct snd_pcm_hw_params *ref_params __free(kfree) = NULL;
>  	struct snd_sof_dai *dai;
>  	u32 gtw_cfg_config_length;
>  	u32 dma_config_tlv_size = 0;
> @@ -1884,9 +1885,11 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  		 * for capture.
>  		 */
>  		if (dir == SNDRV_PCM_STREAM_PLAYBACK)
> -			ref_params = *fe_params;
> +			ref_params = kmemdup(fe_params, sizeof(*ref_params), GFP_KERNEL);
>  		else
> -			ref_params = *pipeline_params;
> +			ref_params = kmemdup(pipeline_params, sizeof(*ref_params), GFP_KERNEL);
> +		if (!ref_params)
> +			return -ENOMEM;
>  
>  		copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK;
>  		copier_data->gtw_cfg.node_id |=
> @@ -1919,8 +1922,11 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  		 * In case of capture the ref_params returned will be used to
>  		 * find the input configuration of the copier.
>  		 */
> -		ref_params = *fe_params;
> -		ret = sof_ipc4_prepare_dai_copier(sdev, dai, &ref_params, dir);
> +		ref_params = kmemdup(fe_params, sizeof(*ref_params), GFP_KERNEL);
> +		if (!ref_params)
> +			return -ENOMEM;
> +
> +		ret = sof_ipc4_prepare_dai_copier(sdev, dai, ref_params, dir);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -1929,7 +1935,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  		 * input configuration of the copier.
>  		 */
>  		if (dir == SNDRV_PCM_STREAM_PLAYBACK)
> -			ref_params = *pipeline_params;
> +			memcpy(ref_params, pipeline_params, sizeof(*ref_params));
>  
>  		break;
>  	}
> @@ -1938,7 +1944,10 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  		ipc4_copier = (struct sof_ipc4_copier *)swidget->private;
>  		copier_data = &ipc4_copier->data;
>  		available_fmt = &ipc4_copier->available_fmt;
> -		ref_params = *pipeline_params;
> +
> +		ref_params = kmemdup(pipeline_params, sizeof(*ref_params), GFP_KERNEL);
> +		if (!ref_params)
> +			return -ENOMEM;
>  
>  		break;
>  	}
> @@ -1951,7 +1960,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>  	/* set input and output audio formats */
>  	input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget,
>  							&copier_data->base_config,
> -							&ref_params, available_fmt);
> +							ref_params, available_fmt);
>  	if (input_fmt_index < 0)
>  		return input_fmt_index;
>  

-- 
Péter