* [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping
@ 2024-05-09 6:51 Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 6:51 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, Charles Keepax,
Krzysztof Kozlowski
Hi,
First patch is a build dependency.
Changes in v3:
- Add missing Charles' Ack
- Link to v2: https://lore.kernel.org/r/20240508-asoc-x1e80100-4-channel-mapping-v2-0-ccef9a66a572@linaro.org
Changes in v2:
- Re-work most of the idea according to Srini comments: set channel
mapping for backend DAIs, not frontend.
- Patch #1: no changes
- Patch #2 is entirely replaced - now channel mapping is implemented in
q6apm-lpass-dais.
- Patch #3: rework to new approach, but most of the code stays.
- Patch #4: rework significantly, because only backend DAIs is now
affected.
- Link to v1: https://lore.kernel.org/r/20240507-asoc-x1e80100-4-channel-mapping-v1-0-b12c13e0a55d@linaro.org
Description
===========
X1E80100 CRD is the first board, which comes with four speakers, so we
still keep fixing and adding missing pieces.
The board has speaker 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.
Best regards,
Krzysztof
---
Krzysztof Kozlowski (4):
ASoC: Constify channel mapping array arguments in set_channel_map()
ASoC: qcom: q6apm-lpass-dais: Implement proper channel mapping
ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
ASoC: qcom: x1e80100: Correct channel mapping
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/audioreach.c | 30 +++++++-----------------------
sound/soc/qcom/qdsp6/audioreach.h | 2 +-
sound/soc/qcom/qdsp6/q6afe-dai.c | 16 ++++++++++------
sound/soc/qcom/qdsp6/q6apm-dai.c | 2 ++
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 21 +++++++++++++--------
sound/soc/qcom/x1e80100.c | 18 ++++++++++++++++++
sound/soc/soc-dai.c | 4 ++--
15 files changed, 79 insertions(+), 57 deletions(-)
---
base-commit: 2b84edefcad14934796fad37b16512b6a2ca467e
change-id: 20240507-asoc-x1e80100-4-channel-mapping-ea5f02b9e678
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/4] ASoC: Constify channel mapping array arguments in set_channel_map()
2024-05-09 6:51 [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
@ 2024-05-09 6:51 ` Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 2/4] ASoC: qcom: q6apm-lpass-dais: Implement proper channel mapping Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 6:51 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, Charles Keepax,
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).
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. None
---
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] 9+ messages in thread
* [PATCH v3 2/4] ASoC: qcom: q6apm-lpass-dais: Implement proper channel mapping
2024-05-09 6:51 [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
@ 2024-05-09 6:51 ` Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
3 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 6:51 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
The set_channel_map() implementation in q6apm-lpass-dais driver was
copying older pre-Audioreach code from q6afe-dai driver, but not really
using it. The code sets active channel mask based on passed channel
mapping, but Audioreach code does not use that mask ever. Audioreach
module configuration does have on the other hand proper channel mapping
field, which should supersed that active channel mask.
Drop the unused active channel mask and implement proper mapping of
channels in q6apm-lpass-dais driver.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Entirely replaced - now channel mapping is implemented in
q6apm-lpass-dais.
---
sound/soc/qcom/qdsp6/audioreach.h | 1 -
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 10 +++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 2c82917b7162..eb9306280988 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -755,7 +755,6 @@ struct audioreach_module_config {
u16 data_format;
u16 num_channels;
- u16 active_channels_mask;
u16 dp_idx;
u32 channel_allocation;
u32 sd_line_mask;
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 6bfbb52345e1..a4ad1d0e6abd 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -33,7 +33,7 @@ static int q6dma_set_channel_map(struct snd_soc_dai *dai,
struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
- int ch_mask;
+ int i;
switch (dai->id) {
case WSA_CODEC_DMA_TX_0:
@@ -58,7 +58,8 @@ static int q6dma_set_channel_map(struct snd_soc_dai *dai,
tx_num);
return -EINVAL;
}
- ch_mask = *tx_ch_mask;
+ for (i = 0; i < tx_num; i++)
+ cfg->channel_map[i] = tx_ch_mask[i];
break;
case WSA_CODEC_DMA_RX_0:
@@ -81,7 +82,8 @@ static int q6dma_set_channel_map(struct snd_soc_dai *dai,
rx_num);
return -EINVAL;
}
- ch_mask = *rx_ch_mask;
+ for (i = 0; i < rx_num; i++)
+ cfg->channel_map[i] = rx_ch_mask[i];
break;
default:
@@ -90,8 +92,6 @@ static int q6dma_set_channel_map(struct snd_soc_dai *dai,
return -EINVAL;
}
- cfg->active_channels_mask = ch_mask;
-
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
2024-05-09 6:51 [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 2/4] ASoC: qcom: q6apm-lpass-dais: Implement proper channel mapping Krzysztof Kozlowski
@ 2024-05-09 6:51 ` Krzysztof Kozlowski
2024-05-09 9:17 ` Srinivas Kandagatla
2024-05-09 6:51 ` [PATCH v3 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
3 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 6:51 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
When constructing packets to DSP, the Audioreach code uses 'struct
audioreach_module_config' to configure parameters like number of
channels, bitrate, sample rate etc, but uses defaults for the channel
mapping.
Rework this code to copy the channel mapping from 'struct
audioreach_module_config', instead of using the default. This requires
all callers to fill that structure: add missing initialization of
channel mapping.
Entire patch makes code more logical and easier to follow:
1. q6apm-dai and q6apm-lpass-dais code which allocates 'struct
audioreach_module_config' initializes it fully, so fills both
the number of channels and the channel mapping.
2. Audioreach code, which uses 'struct audioreach_module_config' when
constructing packets, copies entire contents of passed config, not
only pieces of it.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Extend commit msg, rationale.
2. Rework to new approach, but most of the code stays.
3. Export audioreach_set_channel_mapping() (needed by Q6APM DAIS and
LPASS DAIS)
4. Correct channel mapping also in audioreach_mfc_set_media_format(),
because MFC DAI is now part of backend.
5. Do not adjust dynamic DAIs (drop audioreach_dai_load()).
---
sound/soc/qcom/qdsp6/audioreach.c | 30 +++++++-----------------------
sound/soc/qcom/qdsp6/audioreach.h | 1 +
sound/soc/qcom/qdsp6/q6apm-dai.c | 2 ++
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 5 ++++-
4 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index 5291deac0a0b..750b8ba64211 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;
@@ -281,6 +281,7 @@ static void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
ch_map[3] = PCM_CHANNEL_RS;
}
}
+EXPORT_SYMBOL_GPL(audioreach_set_channel_mapping);
static void apm_populate_container_config(struct apm_container_obj *cfg,
struct audioreach_container *cont)
@@ -819,7 +820,7 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
uint32_t num_channels = cfg->num_channels;
int payload_size;
struct gpr_pkt *pkt;
- int rc;
+ int rc, i;
void *p;
payload_size = APM_MFC_CFG_PSIZE(media_format, num_channels) +
@@ -842,18 +843,8 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
media_format->sample_rate = cfg->sample_rate;
media_format->bit_width = cfg->bit_width;
media_format->num_channels = cfg->num_channels;
-
- if (num_channels == 1) {
- media_format->channel_mapping[0] = PCM_CHANNEL_FL;
- } else if (num_channels == 2) {
- media_format->channel_mapping[0] = PCM_CHANNEL_FL;
- media_format->channel_mapping[1] = PCM_CHANNEL_FR;
- } else if (num_channels == 4) {
- media_format->channel_mapping[0] = PCM_CHANNEL_FL;
- media_format->channel_mapping[1] = PCM_CHANNEL_FR;
- media_format->channel_mapping[2] = PCM_CHANNEL_LS;
- media_format->channel_mapping[3] = PCM_CHANNEL_RS;
- }
+ for (i = 0; i < num_channels; i++)
+ media_format->channel_mapping[i] = cfg->channel_map[i];
rc = q6apm_send_cmd_sync(graph->apm, pkt, 0);
@@ -883,9 +874,6 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
mp3_cfg->q_factor = mcfg->bit_width - 1;
mp3_cfg->endianness = PCM_LITTLE_ENDIAN;
mp3_cfg->num_channels = mcfg->num_channels;
-
- audioreach_set_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 +1092,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 +1149,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 eb9306280988..208b74e50445 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -766,6 +766,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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 00bbd291be5c..8ab55869e8a2 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
cfg.num_channels = runtime->channels;
cfg.bit_width = prtd->bits_per_sample;
cfg.fmt = SND_AUDIOCODEC_PCM;
+ audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);
if (prtd->state) {
/* clear the previous setup if any */
@@ -669,6 +670,7 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
cfg.num_channels = 2;
cfg.bit_width = prtd->bits_per_sample;
cfg.fmt = codec->id;
+ audioreach_set_channel_mapping(cfg.channel_map, cfg.num_channels);
memcpy(&cfg.codec, codec, sizeof(*codec));
ret = q6apm_graph_media_format_shmem(prtd->graph, &cfg);
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index a4ad1d0e6abd..8340e4fb78f4 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -106,6 +106,7 @@ static int q6hdmi_hw_params(struct snd_pcm_substream *substream,
cfg->bit_width = params_width(params);
cfg->sample_rate = params_rate(params);
cfg->num_channels = channels;
+ audioreach_set_channel_mapping(cfg->channel_map, channels);
switch (dai->id) {
case DISPLAY_PORT_RX_0:
@@ -130,10 +131,12 @@ static int q6dma_hw_params(struct snd_pcm_substream *substream,
{
struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
+ int channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
cfg->bit_width = params_width(params);
cfg->sample_rate = params_rate(params);
- cfg->num_channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
+ cfg->num_channels = channels;
+ audioreach_set_channel_mapping(cfg->channel_map, channels);
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/4] ASoC: qcom: x1e80100: Correct channel mapping
2024-05-09 6:51 [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
` (2 preceding siblings ...)
2024-05-09 6:51 ` [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
@ 2024-05-09 6:51 ` Krzysztof Kozlowski
3 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 6:51 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.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Rework significantly, because only backend DAIs is now affected.
---
sound/soc/qcom/x1e80100.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c
index c3c8bf7ffb5b..27f34c0873ab 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 {
@@ -80,6 +81,23 @@ static int x1e80100_snd_prepare(struct snd_pcm_substream *substream)
struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
+ const unsigned int rx_slot[4] = { PCM_CHANNEL_FL,
+ PCM_CHANNEL_LB,
+ PCM_CHANNEL_FR,
+ PCM_CHANNEL_RB };
+ int ret;
+
+ switch (cpu_dai->id) {
+ case WSA_CODEC_DMA_RX_0:
+ case WSA_CODEC_DMA_RX_1:
+ ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL,
+ ARRAY_SIZE(rx_slot), rx_slot);
+ if (ret)
+ return ret;
+ break;
+ default:
+ break;
+ }
return qcom_snd_sdw_prepare(substream, sruntime,
&data->stream_prepared[cpu_dai->id]);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
2024-05-09 6:51 ` [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
@ 2024-05-09 9:17 ` Srinivas Kandagatla
2024-05-09 11:43 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Srinivas Kandagatla @ 2024-05-09 9:17 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 Krzysztof for the patch.
On 09/05/2024 07:51, Krzysztof Kozlowski wrote:
> When constructing packets to DSP, the Audioreach code uses 'struct
> audioreach_module_config' to configure parameters like number of
> channels, bitrate, sample rate etc, but uses defaults for the channel
> mapping.
>
> Rework this code to copy the channel mapping from 'struct
> audioreach_module_config', instead of using the default. This requires
> all callers to fill that structure: add missing initialization of
> channel mapping.
Adding this new function call is logically fine but its going to
introducing some sequencing issues.
set_channel_map might be overwritten by this if not done correctly.
One such instance is in this patch..
>
> Entire patch makes code more logical and easier to follow:
> 1. q6apm-dai and q6apm-lpass-dais code which allocates 'struct
> audioreach_module_config' initializes it fully, so fills both
> the number of channels and the channel mapping.
> 2. Audioreach code, which uses 'struct audioreach_module_config' when
> constructing packets, copies entire contents of passed config, not
> only pieces of it.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes in v2:
> 1. Extend commit msg, rationale.
> 2. Rework to new approach, but most of the code stays.
> 3. Export audioreach_set_channel_mapping() (needed by Q6APM DAIS and
> LPASS DAIS)
> 4. Correct channel mapping also in audioreach_mfc_set_media_format(),
> because MFC DAI is now part of backend.
> 5. Do not adjust dynamic DAIs (drop audioreach_dai_load()).
> ---
> sound/soc/qcom/qdsp6/audioreach.c | 30 +++++++-----------------------
> sound/soc/qcom/qdsp6/audioreach.h | 1 +
> sound/soc/qcom/qdsp6/q6apm-dai.c | 2 ++
> sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 5 ++++-
> 4 files changed, 14 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
> index 5291deac0a0b..750b8ba64211 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;
> @@ -281,6 +281,7 @@ static void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
> ch_map[3] = PCM_CHANNEL_RS;
> }
> }
> +EXPORT_SYMBOL_GPL(audioreach_set_channel_mapping);
>
> static void apm_populate_container_config(struct apm_container_obj *cfg,
> struct audioreach_container *cont)
> @@ -819,7 +820,7 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
> uint32_t num_channels = cfg->num_channels;
> int payload_size;
> struct gpr_pkt *pkt;
> - int rc;
> + int rc, i;
> void *p;
>
> payload_size = APM_MFC_CFG_PSIZE(media_format, num_channels) +
> @@ -842,18 +843,8 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
> media_format->sample_rate = cfg->sample_rate;
> media_format->bit_width = cfg->bit_width;
> media_format->num_channels = cfg->num_channels;
> -
> - if (num_channels == 1) {
> - media_format->channel_mapping[0] = PCM_CHANNEL_FL;
> - } else if (num_channels == 2) {
> - media_format->channel_mapping[0] = PCM_CHANNEL_FL;
> - media_format->channel_mapping[1] = PCM_CHANNEL_FR;
> - } else if (num_channels == 4) {
> - media_format->channel_mapping[0] = PCM_CHANNEL_FL;
> - media_format->channel_mapping[1] = PCM_CHANNEL_FR;
> - media_format->channel_mapping[2] = PCM_CHANNEL_LS;
> - media_format->channel_mapping[3] = PCM_CHANNEL_RS;
> - }
> + for (i = 0; i < num_channels; i++)
> + media_format->channel_mapping[i] = cfg->channel_map[i];
>
> rc = q6apm_send_cmd_sync(graph->apm, pkt, 0);
>
> @@ -883,9 +874,6 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
> mp3_cfg->q_factor = mcfg->bit_width - 1;
> mp3_cfg->endianness = PCM_LITTLE_ENDIAN;
> mp3_cfg->num_channels = mcfg->num_channels;
> -
> - audioreach_set_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 +1092,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 +1149,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 eb9306280988..208b74e50445 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.h
> +++ b/sound/soc/qcom/qdsp6/audioreach.h
> @@ -766,6 +766,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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index 00bbd291be5c..8ab55869e8a2 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
> @@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
> cfg.num_channels = runtime->channels;
> cfg.bit_width = prtd->bits_per_sample;
> cfg.fmt = SND_AUDIOCODEC_PCM;
> + audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);
>
Prepare can be called multiple times.. so we have channels overwritten here.
--srini
> if (prtd->state) {
> /* clear the previous setup if any */
> @@ -669,6 +670,7 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
> cfg.num_channels = 2;
> cfg.bit_width = prtd->bits_per_sample;
> cfg.fmt = codec->id;
> + audioreach_set_channel_mapping(cfg.channel_map, cfg.num_channels);
> memcpy(&cfg.codec, codec, sizeof(*codec));
>
> ret = q6apm_graph_media_format_shmem(prtd->graph, &cfg);
> diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> index a4ad1d0e6abd..8340e4fb78f4 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> @@ -106,6 +106,7 @@ static int q6hdmi_hw_params(struct snd_pcm_substream *substream,
> cfg->bit_width = params_width(params);
> cfg->sample_rate = params_rate(params);
> cfg->num_channels = channels;
> + audioreach_set_channel_mapping(cfg->channel_map, channels);
>
> switch (dai->id) {
> case DISPLAY_PORT_RX_0:
> @@ -130,10 +131,12 @@ static int q6dma_hw_params(struct snd_pcm_substream *substream,
> {
> struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
> struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
> + int channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
>
> cfg->bit_width = params_width(params);
> cfg->sample_rate = params_rate(params);
> - cfg->num_channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
> + cfg->num_channels = channels;
> + audioreach_set_channel_mapping(cfg->channel_map, channels);
>
> return 0;
> }
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
2024-05-09 9:17 ` Srinivas Kandagatla
@ 2024-05-09 11:43 ` Krzysztof Kozlowski
2024-05-10 15:39 ` Srinivas Kandagatla
0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-09 11:43 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 09/05/2024 11:17, Srinivas Kandagatla wrote:
>> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
>> index eb9306280988..208b74e50445 100644
>> --- a/sound/soc/qcom/qdsp6/audioreach.h
>> +++ b/sound/soc/qcom/qdsp6/audioreach.h
>> @@ -766,6 +766,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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
>> index 00bbd291be5c..8ab55869e8a2 100644
>> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
>> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
>> @@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
>> cfg.num_channels = runtime->channels;
>> cfg.bit_width = prtd->bits_per_sample;
>> cfg.fmt = SND_AUDIOCODEC_PCM;
>> + audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);
>>
>
> Prepare can be called multiple times.. so we have channels overwritten here.
Which is expected - just like we overwrite number of channels.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
2024-05-09 11:43 ` Krzysztof Kozlowski
@ 2024-05-10 15:39 ` Srinivas Kandagatla
2024-05-20 13:57 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Srinivas Kandagatla @ 2024-05-10 15:39 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 09/05/2024 12:43, Krzysztof Kozlowski wrote:
> On 09/05/2024 11:17, Srinivas Kandagatla wrote:
>>> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
>>> index eb9306280988..208b74e50445 100644
>>> --- a/sound/soc/qcom/qdsp6/audioreach.h
>>> +++ b/sound/soc/qcom/qdsp6/audioreach.h
>>> @@ -766,6 +766,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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
>>> index 00bbd291be5c..8ab55869e8a2 100644
>>> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
>>> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
>>> @@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
>>> cfg.num_channels = runtime->channels;
>>> cfg.bit_width = prtd->bits_per_sample;
>>> cfg.fmt = SND_AUDIOCODEC_PCM;
>>> + audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);
>>>
>>
>> Prepare can be called multiple times.. so we have channels overwritten here.
>
> Which is expected - just like we overwrite number of channels.
This will work in q6apm-dai.c case as there is no set_channel_map callback.
lgtm.
Can you rename audioreach_set_channel_mapping to
audioreach_set_default_channel_mapping which makes it more obvious that
we are setting a default channel mappings.
--srini
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults
2024-05-10 15:39 ` Srinivas Kandagatla
@ 2024-05-20 13:57 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-20 13:57 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 10/05/2024 17:39, Srinivas Kandagatla wrote:
>
>
> On 09/05/2024 12:43, Krzysztof Kozlowski wrote:
>> On 09/05/2024 11:17, Srinivas Kandagatla wrote:
>>>> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
>>>> index eb9306280988..208b74e50445 100644
>>>> --- a/sound/soc/qcom/qdsp6/audioreach.h
>>>> +++ b/sound/soc/qcom/qdsp6/audioreach.h
>>>> @@ -766,6 +766,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-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
>>>> index 00bbd291be5c..8ab55869e8a2 100644
>>>> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
>>>> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
>>>> @@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
>>>> cfg.num_channels = runtime->channels;
>>>> cfg.bit_width = prtd->bits_per_sample;
>>>> cfg.fmt = SND_AUDIOCODEC_PCM;
>>>> + audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);
>>>>
>>>
>>> Prepare can be called multiple times.. so we have channels overwritten here.
>>
>> Which is expected - just like we overwrite number of channels.
> This will work in q6apm-dai.c case as there is no set_channel_map callback.
>
> lgtm.
>
> Can you rename audioreach_set_channel_mapping to
> audioreach_set_default_channel_mapping which makes it more obvious that
> we are setting a default channel mappings.
Ack.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-05-20 13:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-09 6:51 [PATCH v3 0/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 1/4] ASoC: Constify channel mapping array arguments in set_channel_map() Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 2/4] ASoC: qcom: q6apm-lpass-dais: Implement proper channel mapping Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults Krzysztof Kozlowski
2024-05-09 9:17 ` Srinivas Kandagatla
2024-05-09 11:43 ` Krzysztof Kozlowski
2024-05-10 15:39 ` Srinivas Kandagatla
2024-05-20 13:57 ` Krzysztof Kozlowski
2024-05-09 6:51 ` [PATCH v3 4/4] ASoC: qcom: x1e80100: Correct channel mapping Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).