* [PATCH 1/4] ASoC: Constify channel mapping array arguments in set_channel_map()
2024-05-07 10:27 [PATCH 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
@ 2024-05-07 10:27 ` Krzysztof Kozlowski
2024-05-07 11:57 ` Charles Keepax
2024-05-07 10:27 ` [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 10:27 UTC (permalink / raw)
To: James Schulman, David Rhodes, Richard Fitzgerald, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Lars-Peter Clausen,
Nuno Sá, Srinivas Kandagatla, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel,
Krzysztof Kozlowski
There is no need for implementations of DAI set_channel_map() to modify
contents of passed arrays with actual channel mapping. Additionally,
the caller keeps full ownership of the array.
Constify these pointer arguments so the code will be safer and easier to
read (documenting the caller's ownership).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
include/sound/cs35l41.h | 4 ++--
include/sound/soc-dai.h | 8 ++++----
sound/soc/codecs/adau7118.c | 6 ++++--
sound/soc/codecs/cs35l41-lib.c | 4 ++--
sound/soc/codecs/cs35l41.c | 3 ++-
sound/soc/codecs/max98504.c | 6 ++++--
sound/soc/codecs/wcd9335.c | 6 ++++--
sound/soc/codecs/wcd934x.c | 6 ++++--
sound/soc/qcom/qdsp6/q6afe-dai.c | 16 ++++++++++------
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 6 ++++--
sound/soc/soc-dai.c | 4 ++--
11 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
index bb70782d15d0..43c6a9ef8d9f 100644
--- a/include/sound/cs35l41.h
+++ b/include/sound/cs35l41.h
@@ -896,8 +896,8 @@ int cs35l41_test_key_lock(struct device *dev, struct regmap *regmap);
int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap);
int cs35l41_register_errata_patch(struct device *dev, struct regmap *reg, unsigned int reg_revid);
int cs35l41_set_channels(struct device *dev, struct regmap *reg,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot);
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot);
int cs35l41_gpio_config(struct regmap *regmap, struct cs35l41_hw_cfg *hw_cfg);
void cs35l41_configure_cs_dsp(struct device *dev, struct regmap *reg, struct cs_dsp *dsp);
int cs35l41_set_cspl_mbox_cmd(struct device *dev, struct regmap *regmap,
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index adcd8719d343..15ef268c9845 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -188,8 +188,8 @@ int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width);
int snd_soc_dai_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot);
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot);
int snd_soc_dai_set_tristate(struct snd_soc_dai *dai, int tristate);
@@ -305,8 +305,8 @@ struct snd_soc_dai_ops {
unsigned int tx_mask, unsigned int rx_mask,
int slots, int slot_width);
int (*set_channel_map)(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot);
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot);
int (*get_channel_map)(struct snd_soc_dai *dai,
unsigned int *tx_num, unsigned int *tx_slot,
unsigned int *rx_num, unsigned int *rx_slot);
diff --git a/sound/soc/codecs/adau7118.c b/sound/soc/codecs/adau7118.c
index a663d37e5776..abc4764697a5 100644
--- a/sound/soc/codecs/adau7118.c
+++ b/sound/soc/codecs/adau7118.c
@@ -121,8 +121,10 @@ static const struct snd_soc_dapm_widget adau7118_widgets[] = {
};
static int adau7118_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num,
+ const unsigned int *tx_slot,
+ unsigned int rx_num,
+ const unsigned int *rx_slot)
{
struct adau7118_data *st =
snd_soc_component_get_drvdata(dai->component);
diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
index e9993a39f7d0..1702f26049d3 100644
--- a/sound/soc/codecs/cs35l41-lib.c
+++ b/sound/soc/codecs/cs35l41-lib.c
@@ -936,8 +936,8 @@ int cs35l41_register_errata_patch(struct device *dev, struct regmap *reg, unsign
EXPORT_SYMBOL_GPL(cs35l41_register_errata_patch);
int cs35l41_set_channels(struct device *dev, struct regmap *reg,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot)
{
unsigned int val, mask;
int i;
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index cb25c33cc9b9..1688c2c688f0 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -673,7 +673,8 @@ static const struct snd_soc_dapm_route cs35l41_audio_map[] = {
};
static int cs35l41_set_channel_map(struct snd_soc_dai *dai, unsigned int tx_n,
- unsigned int *tx_slot, unsigned int rx_n, unsigned int *rx_slot)
+ const unsigned int *tx_slot,
+ unsigned int rx_n, const unsigned int *rx_slot)
{
struct cs35l41_private *cs35l41 = snd_soc_component_get_drvdata(dai->component);
diff --git a/sound/soc/codecs/max98504.c b/sound/soc/codecs/max98504.c
index 93412b966b33..6b6a7ece4cec 100644
--- a/sound/soc/codecs/max98504.c
+++ b/sound/soc/codecs/max98504.c
@@ -220,8 +220,10 @@ static int max98504_set_tdm_slot(struct snd_soc_dai *dai,
return 0;
}
static int max98504_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num,
+ const unsigned int *tx_slot,
+ unsigned int rx_num,
+ const unsigned int *rx_slot)
{
struct max98504_priv *max98504 = snd_soc_dai_get_drvdata(dai);
struct regmap *map = max98504->regmap;
diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index deb15b95992d..42a99978fe5a 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -1983,8 +1983,10 @@ static int wcd9335_trigger(struct snd_pcm_substream *substream, int cmd,
}
static int wcd9335_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num,
+ const unsigned int *tx_slot,
+ unsigned int rx_num,
+ const unsigned int *rx_slot)
{
struct wcd9335_codec *wcd;
int i;
diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
index de870c7819ca..fcad2c9fba55 100644
--- a/sound/soc/codecs/wcd934x.c
+++ b/sound/soc/codecs/wcd934x.c
@@ -1923,8 +1923,10 @@ static int wcd934x_trigger(struct snd_pcm_substream *substream, int cmd,
}
static int wcd934x_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num,
+ const unsigned int *tx_slot,
+ unsigned int rx_num,
+ const unsigned int *rx_slot)
{
struct wcd934x_codec *wcd;
int i;
diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
index a9c4f896a7df..7d9628cda875 100644
--- a/sound/soc/qcom/qdsp6/q6afe-dai.c
+++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
@@ -172,8 +172,8 @@ static int q6tdm_set_tdm_slot(struct snd_soc_dai *dai,
}
static int q6tdm_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot)
{
struct q6afe_dai_data *dai_data = dev_get_drvdata(dai->dev);
@@ -250,8 +250,10 @@ static int q6tdm_hw_params(struct snd_pcm_substream *substream,
}
static int q6dma_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_ch_mask,
- unsigned int rx_num, unsigned int *rx_ch_mask)
+ unsigned int tx_num,
+ const unsigned int *tx_ch_mask,
+ unsigned int rx_num,
+ const unsigned int *rx_ch_mask)
{
struct q6afe_dai_data *dai_data = dev_get_drvdata(dai->dev);
@@ -407,8 +409,10 @@ static int q6afe_dai_prepare(struct snd_pcm_substream *substream,
}
static int q6slim_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num,
+ const unsigned int *tx_slot,
+ unsigned int rx_num,
+ const unsigned int *rx_slot)
{
struct q6afe_dai_data *dai_data = dev_get_drvdata(dai->dev);
struct q6afe_port_config *pcfg = &dai_data->port_config[dai->id];
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 68a38f63a2db..6bfbb52345e1 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -25,8 +25,10 @@ struct q6apm_lpass_dai_data {
};
static int q6dma_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_ch_mask,
- unsigned int rx_num, unsigned int *rx_ch_mask)
+ unsigned int tx_num,
+ const unsigned int *tx_ch_mask,
+ unsigned int rx_num,
+ const unsigned int *rx_ch_mask)
{
struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index fefe394dce72..03afd5efb24c 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -304,8 +304,8 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_set_tdm_slot);
* configure the relationship between channel number and TDM slot number.
*/
int snd_soc_dai_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, unsigned int *tx_slot,
- unsigned int rx_num, unsigned int *rx_slot)
+ unsigned int tx_num, const unsigned int *tx_slot,
+ unsigned int rx_num, const unsigned int *rx_slot)
{
int ret = -ENOTSUPP;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] ASoC: Constify channel mapping array arguments in set_channel_map()
2024-05-07 10:27 ` [PATCH 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
@ 2024-05-07 11:57 ` Charles Keepax
0 siblings, 0 replies; 10+ messages in thread
From: Charles Keepax @ 2024-05-07 11:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: James Schulman, David Rhodes, Richard Fitzgerald, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Lars-Peter Clausen,
Nuno Sá, Srinivas Kandagatla, Banajit Goswami, alsa-devel,
patches, linux-sound, linux-kernel
On Tue, May 07, 2024 at 12:27:30PM +0200, Krzysztof Kozlowski wrote:
> There is no need for implementations of DAI set_channel_map() to modify
> contents of passed arrays with actual channel mapping. Additionally,
> the caller keeps full ownership of the array.
>
> Constify these pointer arguments so the code will be safer and easier to
> read (documenting the caller's ownership).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> include/sound/cs35l41.h | 4 ++--
> include/sound/soc-dai.h | 8 ++++----
> sound/soc/codecs/adau7118.c | 6 ++++--
> sound/soc/codecs/cs35l41-lib.c | 4 ++--
> sound/soc/codecs/cs35l41.c | 3 ++-
For the cs35l41 bits:
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks,
Charles
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach
2024-05-07 10:27 [PATCH 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
2024-05-07 10:27 ` [PATCH 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
@ 2024-05-07 10:27 ` Krzysztof Kozlowski
2024-05-07 13:25 ` Srinivas Kandagatla
2024-05-07 10:27 ` [PATCH 3/4] ASoC: qcom: q6dsp: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
2024-05-07 10:27 ` [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
3 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 10:27 UTC (permalink / raw)
To: James Schulman, David Rhodes, Richard Fitzgerald, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Lars-Peter Clausen,
Nuno Sá, Srinivas Kandagatla, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel,
Krzysztof Kozlowski
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(-)
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,
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach
2024-05-07 10:27 ` [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach Krzysztof Kozlowski
@ 2024-05-07 13:25 ` Srinivas Kandagatla
2024-05-07 18:16 ` Krzysztof Kozlowski
0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Kandagatla @ 2024-05-07 13:25 UTC (permalink / raw)
To: Krzysztof Kozlowski, James Schulman, David Rhodes,
Richard Fitzgerald, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, Lars-Peter Clausen, Nuno Sá, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel
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,
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach
2024-05-07 13:25 ` Srinivas Kandagatla
@ 2024-05-07 18:16 ` Krzysztof Kozlowski
0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 18:16 UTC (permalink / raw)
To: Srinivas Kandagatla, James Schulman, David Rhodes,
Richard Fitzgerald, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, Lars-Peter Clausen, Nuno Sá, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel
On 07/05/2024 15:25, Srinivas Kandagatla wrote:
>
>
> 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.
>
Ack
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] ASoC: qcom: q6dsp: Set channel mapping instead of fixed defaults
2024-05-07 10:27 [PATCH 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
2024-05-07 10:27 ` [PATCH 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
2024-05-07 10:27 ` [PATCH 2/4] ASoC: qcom: q6dsp: Implement proper channel mapping in Audioreach Krzysztof Kozlowski
@ 2024-05-07 10:27 ` Krzysztof Kozlowski
2024-05-07 10:27 ` [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
3 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 10:27 UTC (permalink / raw)
To: James Schulman, David Rhodes, Richard Fitzgerald, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Lars-Peter Clausen,
Nuno Sá, Srinivas Kandagatla, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel,
Krzysztof Kozlowski
Use previously set channel mapping instead of defaults when preparing
frontent Q6APM component. This allows machine sound card drivers to
override channel mappings.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
sound/soc/qcom/qdsp6/audioreach.c | 14 +++++---------
sound/soc/qcom/qdsp6/audioreach.h | 2 +-
sound/soc/qcom/qdsp6/q6apm-dai.c | 12 ++++++++++++
sound/soc/qcom/qdsp6/q6apm.c | 2 +-
4 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index 8175678d8843..83b33e4c9de2 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);
-void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
+void audioreach_set_default_channel_mapping(u8 *ch_map, int num_channels)
{
if (num_channels == 1) {
ch_map[0] = PCM_CHANNEL_FL;
@@ -884,8 +884,8 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
mp3_cfg->endianness = PCM_LITTLE_ENDIAN;
mp3_cfg->num_channels = mcfg->num_channels;
- audioreach_set_channel_mapping(mp3_cfg->channel_mapping,
- mcfg->num_channels);
+ audioreach_set_default_channel_mapping(mp3_cfg->channel_mapping,
+ mcfg->num_channels);
break;
case SND_AUDIOCODEC_AAC:
media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
@@ -1104,9 +1104,7 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
media_cfg->num_channels = mcfg->num_channels;
media_cfg->q_factor = mcfg->bit_width - 1;
media_cfg->bits_per_sample = mcfg->bit_width;
-
- audioreach_set_channel_mapping(media_cfg->channel_mapping,
- num_channels);
+ memcpy(media_cfg->channel_mapping, mcfg->channel_map, mcfg->num_channels);
rc = q6apm_send_cmd_sync(graph->apm, pkt, 0);
@@ -1163,9 +1161,7 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
cfg->q_factor = mcfg->bit_width - 1;
cfg->endianness = PCM_LITTLE_ENDIAN;
cfg->num_channels = mcfg->num_channels;
-
- audioreach_set_channel_mapping(cfg->channel_mapping,
- num_channels);
+ memcpy(cfg->channel_mapping, mcfg->channel_map, mcfg->num_channels);
} else {
rc = audioreach_set_compr_media_format(header, p, mcfg);
if (rc) {
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index cef9a9015dcc..6ae95eb85118 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -767,7 +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_set_default_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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 00bbd291be5c..5dfbd011bb97 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -8,6 +8,7 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <sound/soc.h>
+#include <sound/soc-dai.h>
#include <sound/soc-dapm.h>
#include <linux/spinlock.h>
#include <sound/pcm.h>
@@ -223,7 +224,10 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *soc_prtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(soc_prtd, 0);
struct q6apm_dai_rtd *prtd = runtime->private_data;
+ struct q6apm *apm = prtd->graph->apm;
struct audioreach_module_config cfg;
struct device *dev = component->dev;
struct q6apm_dai_data *pdata;
@@ -238,9 +242,17 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
return -EINVAL;
}
+ if (cpu_dai->id >= ARRAY_SIZE(apm->dai_config)) {
+ dev_err(dev, "Unsupported DAI ID number %d (%s)\n",
+ cpu_dai->id, cpu_dai->name);
+ return -EINVAL;
+ }
+
cfg.direction = substream->stream;
cfg.sample_rate = runtime->rate;
cfg.num_channels = runtime->channels;
+ memcpy(cfg.channel_map, apm->dai_config[cpu_dai->id].channel_map,
+ runtime->channels);
cfg.bit_width = prtd->bits_per_sample;
cfg.fmt = SND_AUDIOCODEC_PCM;
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index c29a2dd36992..f6fa15f42633 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -758,7 +758,7 @@ static int apm_probe(gpr_device_t *gdev)
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);
+ audioreach_set_default_channel_mapping(apm->dai_config[i].channel_map, 4);
init_waitqueue_head(&apm->wait);
INIT_LIST_HEAD(&apm->widget_list);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping
2024-05-07 10:27 [PATCH 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
` (2 preceding siblings ...)
2024-05-07 10:27 ` [PATCH 3/4] ASoC: qcom: q6dsp: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
@ 2024-05-07 10:27 ` Krzysztof Kozlowski
2024-05-07 13:20 ` Srinivas Kandagatla
3 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 10:27 UTC (permalink / raw)
To: James Schulman, David Rhodes, Richard Fitzgerald, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Lars-Peter Clausen,
Nuno Sá, Srinivas Kandagatla, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel,
Krzysztof Kozlowski
X1E80100 CRD board comes with four speakers arranged as left front+back
and then right front+back. Using default channel mapping causes front
right speaker to play left back stream.
Adjust the channel maps for frontend DAIs to fix stereo and four-channel
playback.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
sound/soc/qcom/x1e80100.c | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c
index c3c8bf7ffb5b..e90c68815b5c 100644
--- a/sound/soc/qcom/x1e80100.c
+++ b/sound/soc/qcom/x1e80100.c
@@ -12,6 +12,7 @@
#include "common.h"
#include "qdsp6/q6afe.h"
+#include "qdsp6/q6dsp-common.h"
#include "sdw.h"
struct x1e80100_snd_data {
@@ -74,7 +75,7 @@ static int x1e80100_snd_hw_params(struct snd_pcm_substream *substream,
return qcom_snd_sdw_hw_params(substream, params, &data->sruntime[cpu_dai->id]);
}
-static int x1e80100_snd_prepare(struct snd_pcm_substream *substream)
+static int x1e80100_snd_be_prepare(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
@@ -96,12 +97,34 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream)
&data->stream_prepared[cpu_dai->id]);
}
+static int x1e80100_snd_fe_prepare(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ const unsigned int rx_slot[4] = { PCM_CHANNEL_FL,
+ PCM_CHANNEL_LB,
+ PCM_CHANNEL_FR,
+ PCM_CHANNEL_RB };
+
+ snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, ARRAY_SIZE(rx_slot),
+ rx_slot);
+ }
+
+ return 0;
+}
+
static const struct snd_soc_ops x1e80100_be_ops = {
.startup = qcom_snd_sdw_startup,
.shutdown = x1e80100_snd_shutdown,
.hw_params = x1e80100_snd_hw_params,
.hw_free = x1e80100_snd_hw_free,
- .prepare = x1e80100_snd_prepare,
+ .prepare = x1e80100_snd_be_prepare,
+};
+
+static const struct snd_soc_ops x1e80100_fe_ops = {
+ .prepare = x1e80100_snd_fe_prepare,
};
static void x1e80100_add_be_ops(struct snd_soc_card *card)
@@ -118,6 +141,15 @@ static void x1e80100_add_be_ops(struct snd_soc_card *card)
}
}
+static int x1e80100_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *link)
+{
+ /* Add ops for Frontend DAIs coming from Topology */
+ if (link->dynamic && !link->no_pcm && !link->ops)
+ link->ops = &x1e80100_fe_ops;
+
+ return 0;
+}
+
static int x1e80100_platform_probe(struct platform_device *pdev)
{
struct snd_soc_card *card;
@@ -135,6 +167,7 @@ static int x1e80100_platform_probe(struct platform_device *pdev)
card->owner = THIS_MODULE;
card->dev = dev;
+ card->add_dai_link = x1e80100_add_dai_link;
dev_set_drvdata(dev, card);
snd_soc_card_set_drvdata(card, data);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping
2024-05-07 10:27 ` [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
@ 2024-05-07 13:20 ` Srinivas Kandagatla
2024-05-07 18:15 ` Krzysztof Kozlowski
0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Kandagatla @ 2024-05-07 13:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, James Schulman, David Rhodes,
Richard Fitzgerald, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, Lars-Peter Clausen, Nuno Sá, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel
Thanks Krzystof for the patch.
On 07/05/2024 11:27, Krzysztof Kozlowski wrote:
> X1E80100 CRD board comes with four speakers arranged as left front+back
> and then right front+back. Using default channel mapping causes front
> right speaker to play left back stream.
>
> Adjust the channel maps for frontend DAIs to fix stereo and four-channel
> playback.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> sound/soc/qcom/x1e80100.c | 37 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c
> index c3c8bf7ffb5b..e90c68815b5c 100644
> --- a/sound/soc/qcom/x1e80100.c
> +++ b/sound/soc/qcom/x1e80100.c
> @@ -12,6 +12,7 @@
>
> #include "common.h"
> #include "qdsp6/q6afe.h"
> +#include "qdsp6/q6dsp-common.h"
> #include "sdw.h"
>
> struct x1e80100_snd_data {
> @@ -74,7 +75,7 @@ static int x1e80100_snd_hw_params(struct snd_pcm_substream *substream,
> return qcom_snd_sdw_hw_params(substream, params, &data->sruntime[cpu_dai->id]);
> }
>
> -static int x1e80100_snd_prepare(struct snd_pcm_substream *substream)
> +static int x1e80100_snd_be_prepare(struct snd_pcm_substream *substream)
> {
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> @@ -96,12 +97,34 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream)
> &data->stream_prepared[cpu_dai->id]);
> }
>
> +static int x1e80100_snd_fe_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> +
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + const unsigned int rx_slot[4] = { PCM_CHANNEL_FL,
> + PCM_CHANNEL_LB,
> + PCM_CHANNEL_FR,
> + PCM_CHANNEL_RB };
> +
> + snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, ARRAY_SIZE(rx_slot),
> + rx_slot);
Channel mapping are specific to backend dais rather than front end pcm dais.
This will set all the playback pcms with this channel maps, which is a
problem.
example the 2 channel headset we will endup with data of front channel
and zeros on the right channel, however a speaker might work as you have
4 speakers in your system.
So No for this approach.
--srini
> + }
> +
> + return 0;
> +}
> +
> static const struct snd_soc_ops x1e80100_be_ops = {
> .startup = qcom_snd_sdw_startup,
> .shutdown = x1e80100_snd_shutdown,
> .hw_params = x1e80100_snd_hw_params,
> .hw_free = x1e80100_snd_hw_free,
> - .prepare = x1e80100_snd_prepare,
> + .prepare = x1e80100_snd_be_prepare,
> +};
> +
> +static const struct snd_soc_ops x1e80100_fe_ops = {
> + .prepare = x1e80100_snd_fe_prepare,
> };
>
> static void x1e80100_add_be_ops(struct snd_soc_card *card)
> @@ -118,6 +141,15 @@ static void x1e80100_add_be_ops(struct snd_soc_card *card)
> }
> }
>
> +static int x1e80100_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *link)
> +{
> + /* Add ops for Frontend DAIs coming from Topology */
> + if (link->dynamic && !link->no_pcm && !link->ops)
> + link->ops = &x1e80100_fe_ops;
> +
> + return 0;
> +}
> +
> static int x1e80100_platform_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> @@ -135,6 +167,7 @@ static int x1e80100_platform_probe(struct platform_device *pdev)
>
> card->owner = THIS_MODULE;
> card->dev = dev;
> + card->add_dai_link = x1e80100_add_dai_link;
> dev_set_drvdata(dev, card);
> snd_soc_card_set_drvdata(card, data);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] ASoC: qcom: x1e80100: Correct channel mapping
2024-05-07 13:20 ` Srinivas Kandagatla
@ 2024-05-07 18:15 ` Krzysztof Kozlowski
0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07 18:15 UTC (permalink / raw)
To: Srinivas Kandagatla, James Schulman, David Rhodes,
Richard Fitzgerald, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, Lars-Peter Clausen, Nuno Sá, Banajit Goswami
Cc: alsa-devel, patches, linux-sound, linux-kernel
On 07/05/2024 15:20, Srinivas Kandagatla wrote:
> Thanks Krzystof for the patch.
>
> On 07/05/2024 11:27, Krzysztof Kozlowski wrote:
>> X1E80100 CRD board comes with four speakers arranged as left front+back
>> and then right front+back. Using default channel mapping causes front
>> right speaker to play left back stream.
>>
>> Adjust the channel maps for frontend DAIs to fix stereo and four-channel
>> playback.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>> sound/soc/qcom/x1e80100.c | 37 +++++++++++++++++++++++++++++++++++--
>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c
>> index c3c8bf7ffb5b..e90c68815b5c 100644
>> --- a/sound/soc/qcom/x1e80100.c
>> +++ b/sound/soc/qcom/x1e80100.c
>> @@ -12,6 +12,7 @@
>>
>> #include "common.h"
>> #include "qdsp6/q6afe.h"
>> +#include "qdsp6/q6dsp-common.h"
>> #include "sdw.h"
>>
>> struct x1e80100_snd_data {
>> @@ -74,7 +75,7 @@ static int x1e80100_snd_hw_params(struct snd_pcm_substream *substream,
>> return qcom_snd_sdw_hw_params(substream, params, &data->sruntime[cpu_dai->id]);
>> }
>>
>> -static int x1e80100_snd_prepare(struct snd_pcm_substream *substream)
>> +static int x1e80100_snd_be_prepare(struct snd_pcm_substream *substream)
>> {
>> struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
>> @@ -96,12 +97,34 @@ static int x1e80100_snd_hw_free(struct snd_pcm_substream *substream)
>> &data->stream_prepared[cpu_dai->id]);
>> }
>>
>> +static int x1e80100_snd_fe_prepare(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
>> + struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
>> +
>> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> + const unsigned int rx_slot[4] = { PCM_CHANNEL_FL,
>> + PCM_CHANNEL_LB,
>> + PCM_CHANNEL_FR,
>> + PCM_CHANNEL_RB };
>> +
>> + snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, ARRAY_SIZE(rx_slot),
>> + rx_slot);
>
> Channel mapping are specific to backend dais rather than front end pcm dais.
>
> This will set all the playback pcms with this channel maps, which is a
> problem.
>
> example the 2 channel headset we will endup with data of front channel
> and zeros on the right channel, however a speaker might work as you have
> 4 speakers in your system.
>
>
> So No for this approach.
OK, I'll go with setting channels for MFC (and expecting MFC being part
of backend).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread