Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] ASoC Cpu/codec M/N dai update #4632

Closed

Conversation

plbossart
Copy link
Member

@plbossart
Copy link
Member Author

of course this set of patches conflict with my own changes... Will fix tomorrow.

@plbossart plbossart force-pushed the morimoto/m-n-update branch from 516105c to e1bfb4d Compare October 12, 2023 18:20
@plbossart
Copy link
Member Author

SOFCI TEST

@plbossart
Copy link
Member Author

@plbossart
Copy link
Member Author

SOFCI TEST

@plbossart
Copy link
Member Author

Results are reproducible and really bad. @bardliao any idea what could cause this?

@bardliao
Copy link
Collaborator

@plbossart The issue is that __soc_pcm_hw_params is called by both FE and BE dai links, and only BE dai links have ch_mask.

                if (rtd->dai_link->num_cpus >= rtd->dai_link->num_codecs) {
                        /* .ch_map is from CPU */
                        ch_mask = rtd->dai_link->ch_maps[i].ch_mask;

kernel NULL pointer dereference happens at that point.

dmesg.txt

Can you try below change?

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 0bfff2ea111d..ce84d9c1d8be 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1054,6 +1054,9 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
                /* copy params for each cpu */
                tmp_params = *params;

+               /* ch_map is only set in BE dai link */
+               if (rtd->dai_link->dynamic)
+                       goto run;
                /*
                 * construct cpu channel mask by combining ch_mask of each
                 * codec which maps to the cpu.
@@ -1075,7 +1078,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
                /* fixup cpu channel number */
                if (ch_mask)
                        soc_pcm_codec_params_fixup(&tmp_params, ch_mask);
-
+run:
                ret = snd_soc_dai_hw_params(cpu_dai, substream, &tmp_params);
                if (ret < 0)
                        goto out;

@plbossart
Copy link
Member Author

still a really bad result for CML_SKU0955_HDA https://sof-ci.01.org/linuxpr/PR4632/build422/devicetest/index.html

whaaat?

@plbossart plbossart force-pushed the morimoto/m-n-update branch from 7761c52 to 366b708 Compare October 19, 2023 13:17
@plbossart plbossart closed this Oct 19, 2023
morimoto and others added 10 commits October 23, 2023 16:48
Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
but it is used for N < M case only. We want to use it for any case.

By this patch, not only N:M connection, but all existing connection
(1:1, 1:N, N:N) will use same connection mapping. Then, because it will
use default mapping, no conversion patch is needed to exising drivers.

More over, CPU:Codec = N:M (N > M) also supported in the same time.

ch_maps array will has CPU/Codec index by this patch.

Image
	CPU0 <---> Codec0
	CPU1 <-+-> Codec1
	CPU2 <-/

ch_map
	ch_map[0].cpu = 0	ch_map[0].codec = 0
	ch_map[1].cpu = 1	ch_map[1].codec = 1
	ch_map[2].cpu = 2	ch_map[2].codec = 1

Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kuninori Morimoto <[email protected]>
Now ASoC is supporting CPU:Codec = N:M support.
This patch enables it on Audio Graph Card2.

Signed-off-by: Kuninori Morimoto <[email protected]>
Now ASoC is supporting CPU/Codec = N:M connection.
This patch adds its sample settings.

But One note here is that it has many type of samples, it reached to
maximum of sound minor number. Therefore, new sample is disabled so far.
If you want to try it, you need to disable some other one instead.

Signed-off-by: Kuninori Morimoto <[email protected]>
This patch adds ch-maps property to enable handling CPU:Codec = N:M
connection.

Signed-off-by: Kuninori Morimoto <[email protected]>
…ggregated dailink

Use new 'ch_maps' and 'connected_node' introduced by Morimoto-san

Signed-off-by: Pierre-Louis Bossart <[email protected]>
use 'ch_maps' and 'connected_node' introduced by Morimoto-san

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Otherwise there's a kernel oops

Suggested-by: Bard Liao <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
…ggregated dailink

Signed-off-by: Pierre-Louis Bossart <[email protected]>
This reverts commit f4ab7fa.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
@plbossart plbossart reopened this Oct 23, 2023
@plbossart plbossart force-pushed the morimoto/m-n-update branch from 366b708 to 7019e7e Compare October 23, 2023 22:24
@plbossart
Copy link
Member Author

testing v5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants