Received-SPF: pass (google.com: domain of linux-kernel+bounces-171432-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <89cf75d8-1f85-43d8-9c33-377a04b36121@linaro.org>
Date: Tue, 7 May 2024 14:25:31 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping
 in Audioreach
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 James Schulman <james.schulman@cirrus.com>,
 David Rhodes <david.rhodes@cirrus.com>,
 Richard Fitzgerald <rf@opensource.cirrus.com>,
 Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
 Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
 Lars-Peter Clausen <lars@metafoo.de>, =?UTF-8?Q?Nuno_S=C3=A1?=
 <nuno.sa@analog.com>, Banajit Goswami <bgoswami@quicinc.com>
Cc: alsa-devel@alsa-project.org, patches@opensource.cirrus.com,
 linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240507-asoc-x1e80100-4-channel-mapping-v1-0-b12c13e0a55d@linaro.org>
 <20240507-asoc-x1e80100-4-channel-mapping-v1-2-b12c13e0a55d@linaro.org>
Content-Language: en-US
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20240507-asoc-x1e80100-4-channel-mapping-v1-2-b12c13e0a55d@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 07/05/2024 11:27, Krzysztof Kozlowski wrote:
> Instead of relying on default channel mapping in all Audioreach
> platforms, implement set_channel_map() callback to allow sound cards
> customize the mapping depending on needs.
> 
> The channel mapping is set on frontend DAIs coming from the topology,
> not DTS, thus need to add DAI ops in topology dai_load() callback.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   sound/soc/qcom/qdsp6/audioreach.c |  2 +-
>   sound/soc/qcom/qdsp6/audioreach.h |  1 +
>   sound/soc/qcom/qdsp6/q6apm.c      | 28 +++++++++++++++++++++++++++-
>   sound/soc/qcom/qdsp6/q6apm.h      |  8 ++++++++
>   sound/soc/qcom/qdsp6/topology.c   | 12 ++++++++++++
>   5 files changed, 49 insertions(+), 2 deletions(-)
Please use the existing q6dma_set_channel_map() and set the channel map 
for the backend dai from machine driver, this should work.

setting channels on FE is not a scalable one.

Please take a look at some of the patches that I shared privately.

--srini
> 
> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
> index c655f0213723..8175678d8843 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.c
> +++ b/sound/soc/qcom/qdsp6/audioreach.c
> @@ -267,7 +267,7 @@ void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t token
>   }
>   EXPORT_SYMBOL_GPL(audioreach_alloc_apm_cmd_pkt);
>   
> -static void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
> +void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
>   {
>   	if (num_channels == 1) {
>   		ch_map[0] =  PCM_CHANNEL_FL;
> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
> index 2c82917b7162..cef9a9015dcc 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.h
> +++ b/sound/soc/qcom/qdsp6/audioreach.h
> @@ -767,6 +767,7 @@ struct audioreach_module_config {
>   /* Packet Allocation routines */
>   void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t
>   				    token);
> +void audioreach_set_channel_mapping(u8 *ch_map, int num_channels);
>   void *audioreach_alloc_cmd_pkt(int payload_size, uint32_t opcode,
>   			       uint32_t token, uint32_t src_port,
>   			       uint32_t dest_port);
> diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
> index 2a2a5bd98110..c29a2dd36992 100644
> --- a/sound/soc/qcom/qdsp6/q6apm.c
> +++ b/sound/soc/qcom/qdsp6/q6apm.c
> @@ -13,6 +13,7 @@
>   #include <linux/soc/qcom/apr.h>
>   #include <linux/wait.h>
>   #include <sound/soc.h>
> +#include <sound/soc-dai.h>
>   #include <sound/soc-dapm.h>
>   #include <sound/pcm.h>
>   #include "audioreach.h"
> @@ -29,6 +30,29 @@ struct apm_graph_mgmt_cmd {
>   
>   static struct q6apm *g_apm;
>   
> +static int q6apm_dai_set_channel_map(struct snd_soc_dai *dai,
> +				     unsigned int tx_num,
> +				     const unsigned int *tx_ch_mask,
> +				     unsigned int rx_num,
> +				     const unsigned int *rx_ch_mask)
> +{
> +	struct q6apm *apm = dev_get_drvdata(dai->dev);
> +	int i;
> +
> +	if (dai->id >= ARRAY_SIZE(apm->dai_config))
> +		return -EINVAL;
> +
> +	apm->dai_config[dai->id].num_channels = rx_num;
> +	for (i = 0; i < rx_num; i++)
> +		apm->dai_config[dai->id].channel_map[i] = rx_ch_mask[i];
> +
> +	return 0;
> +}
> +
> +const struct snd_soc_dai_ops q6apm_dai_ops = {
> +	.set_channel_map	= q6apm_dai_set_channel_map,
> +};
> +
>   int q6apm_send_cmd_sync(struct q6apm *apm, struct gpr_pkt *pkt, uint32_t rsp_opcode)
>   {
>   	gpr_device_t *gdev = apm->gdev;
> @@ -722,7 +746,7 @@ static int apm_probe(gpr_device_t *gdev)
>   {
>   	struct device *dev = &gdev->dev;
>   	struct q6apm *apm;
> -	int ret;
> +	int ret, i;
>   
>   	apm = devm_kzalloc(dev, sizeof(*apm), GFP_KERNEL);
>   	if (!apm)
> @@ -733,6 +757,8 @@ static int apm_probe(gpr_device_t *gdev)
>   	mutex_init(&apm->lock);
>   	apm->dev = dev;
>   	apm->gdev = gdev;
> +	for (i = 0; i < ARRAY_SIZE(apm->dai_config); i++)
> +		audioreach_set_channel_mapping(apm->dai_config[i].channel_map, 4);
>   	init_waitqueue_head(&apm->wait);
>   
>   	INIT_LIST_HEAD(&apm->widget_list);
> diff --git a/sound/soc/qcom/qdsp6/q6apm.h b/sound/soc/qcom/qdsp6/q6apm.h
> index c248c8d2b1ab..0e2e7b6cd6c1 100644
> --- a/sound/soc/qcom/qdsp6/q6apm.h
> +++ b/sound/soc/qcom/qdsp6/q6apm.h
> @@ -47,6 +47,11 @@
>   #define APM_LAST_BUFFER_FLAG			BIT(30)
>   #define NO_TIMESTAMP				0xFF00
>   
> +struct q6apm_dai_config {
> +	unsigned int num_channels;
> +	u8 channel_map[AR_PCM_MAX_NUM_CHANNEL];
> +};
> +
>   struct q6apm {
>   	struct device *dev;
>   	gpr_port_t *port;
> @@ -65,6 +70,7 @@ struct q6apm {
>   	struct idr sub_graphs_idr;
>   	struct idr containers_idr;
>   	struct idr modules_idr;
> +	struct q6apm_dai_config dai_config[4];
>   };
>   
>   struct audio_buffer {
> @@ -108,6 +114,8 @@ struct q6apm_graph {
>   	struct audioreach_graph_info *info;
>   };
>   
> +extern const struct snd_soc_dai_ops q6apm_dai_ops;
> +
>   /* Graph Operations */
>   struct q6apm_graph *q6apm_graph_open(struct device *dev, q6apm_cb cb,
>   				     void *priv, int graph_id);
> diff --git a/sound/soc/qcom/qdsp6/topology.c b/sound/soc/qcom/qdsp6/topology.c
> index 70572c83e101..9708d200568d 100644
> --- a/sound/soc/qcom/qdsp6/topology.c
> +++ b/sound/soc/qcom/qdsp6/topology.c
> @@ -1034,6 +1034,17 @@ static int audioreach_tplg_complete(struct snd_soc_component *component)
>   	return 0;
>   }
>   
> +static int audioreach_dai_load(struct snd_soc_component *cmp, int index,
> +			       struct snd_soc_dai_driver *dai_drv,
> +			       struct snd_soc_tplg_pcm *pcm,
> +			       struct snd_soc_dai *dai)
> +{
> +	if (pcm)
> +		dai_drv->ops = &q6apm_dai_ops;
> +
> +	return 0;
> +}
> +
>   /* DAI link - used for any driver specific init */
>   static int audioreach_link_load(struct snd_soc_component *component, int index,
>   				struct snd_soc_dai_link *link,
> @@ -1251,6 +1262,7 @@ static struct snd_soc_tplg_ops audioreach_tplg_ops  = {
>   	.widget_unload = audioreach_widget_unload,
>   
>   	.complete = audioreach_tplg_complete,
> +	.dai_load = audioreach_dai_load,
>   	.link_load = audioreach_link_load,
>   
>   	.dapm_route_load	= audioreach_route_load,
>