| Message ID | 20240507-asoc-x1e80100-4-channel-mapping-v1-4-b12c13e0a55d@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: qcom: x1e80100: Correct channel mapping | expand |
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); > >
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
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);
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(-)