Re: [PATCH v3] ASoC: qcom: audioreach: support multichannel PCM formats

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: qcom: audioreach: 1-channel mapping changed from FL to FC by q6dsp_map_channels()
--

commit ec2c026fe429a34883e53d19ea1d90d34c838cea
Author: Zhangfei Gao <zhangfei.gao@oss.qualcomm.com>

ASoC: qcom: audioreach: support multichannel PCM formats

This commit removes the 4-channel limit in the AudioReach PCM and shared
memory media format paths and replaces audioreach_set_default_channel_mapping()
with q6dsp_map_channels() and PCM_MAX_NUM_CHANNEL.

> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
> index d2dc5d37bccf8..e6f077a5b750f 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.c
> +++ b/sound/soc/qcom/qdsp6/audioreach.c
> @@ -306,22 +306,6 @@ void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t token
>  }
>  EXPORT_SYMBOL_GPL(audioreach_alloc_apm_cmd_pkt);
>  
> -void audioreach_set_default_channel_mapping(u8 *ch_map, int num_channels)
> -{
> -	if (num_channels == 1) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -	} else if (num_channels == 2) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -		ch_map[1] =  PCM_CHANNEL_FR;
> -	} else if (num_channels == 4) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -		ch_map[1] =  PCM_CHANNEL_FR;
> -		ch_map[2] =  PCM_CHANNEL_LS;
> -		ch_map[3] =  PCM_CHANNEL_RS;
> -	}
> -}
> -EXPORT_SYMBOL_GPL(audioreach_set_default_channel_mapping);
> -

[ ... ]

> @@ -1436,7 +1410,7 @@ static int audioreach_speaker_protection_vi(struct q6apm_graph *graph,
>  
>  	cm_cfg->cfg.num_channels = num_channels * 2;
>  	/* Convert the physical mapping to a logical mapping of the channels */
> -	for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j < num_channels; i++) {
> +	for (i = 0, j = 0; i < PCM_MAX_NUM_CHANNEL && j < num_channels; i++) {
>  		if (!mcfg->channel_map[i])
>  			continue;
>  		/*

[Severity: Medium]
Does replacing audioreach_set_default_channel_mapping() with
q6dsp_map_channels() cause a regression for 1-channel streams?

Previously, audioreach_set_default_channel_mapping() mapped num_channels == 1
to PCM_CHANNEL_FL (1). In sound/soc/qcom/qdsp6/q6dsp-common.c,
q6dsp_map_channels() instead maps 1 channel to PCM_CHANNEL_FC (3):

	switch (ch) {
	case 1:
		ch_map[0] = PCM_CHANNEL_FC;
		break;

When a 1-channel stream is configured through q6dma_hw_params() in
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c or q6apm_dai_prepare() in
sound/soc/qcom/qdsp6/q6apm-dai.c, cfg->channel_map[0] is now set to
PCM_CHANNEL_FC (3) instead of PCM_CHANNEL_FL (1) for
audioreach_mfc_set_media_format(), audioreach_pcm_set_media_format(), and
audioreach_shmem_set_media_format().

In addition, audioreach_speaker_protection_vi() in
sound/soc/qcom/qdsp6/audioreach.c computes the Vsense and Isense channel
mapping as:

		/*
		 * Map speakers into Vsense and then Isense of each channel.
		 * E.g. for PCM_CHANNEL_FL and PCM_CHANNEL_FR to:
		 * [1, 2, 3, 4]
		 */
		cm_cfg->cfg.channel_mapping[2 * j] = (mcfg->channel_map[i] - 1) * 2 + 1;
		cm_cfg->cfg.channel_mapping[2 * j + 1] = (mcfg->channel_map[i] - 1) * 2 + 2;

With mcfg->channel_map[0] set to PCM_CHANNEL_FC (3) on a 1-channel stream,
this produces [5, 6] instead of [1, 2]. Is PCM_CHANNEL_FC intentional for
mono AudioReach streams and speaker protection VI?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028024720.981971-1-zhangfei.gao@oss.qualcomm.com?part=1