Test for [PATCH v3 00/23] ASoC: Replace dpcm_playback/capture to playback/capture_assertion #4937
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
https://sof-ci.01.org/linuxpr/PR4937/build2357/devicetest/index.html |
|
SOFCI TEST |
|
Looks like https://sof-ci.01.org/linuxpr/PR4937/build2361/devicetest/index.html still has a problem on Helios, @marc-hb @fredoh9 do you know if we can get the logs? |
|
I'm working on fixing thesofproject/sof-test#1112 and I made some progress but it's not done yet, so for now you need ssh when the firmware does not load :-( So I did use ssh and here it the regression below. I found that error 8 times and every time it was with this PR and kernel merge. Commit: Merge 9be6af3a99f4 into 4dc32018572e |
|
ok this is clearly a problem with this PR, thanks @marc-hb, will report upstream. |
Current soc_get_playback_capture() (A) is checking playback/capture
availability for DPCM (X) / Normal (Y) / Codec2Codec (Z) connections.
(A) static int soc_get_playback_capture(...)
{
...
^ if (dai_link->dynamic || dai_link->no_pcm) {
| ...
|(a) if (dai_link->dpcm_playback) {
| ...
| ^ for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
|(*) ...
| v }
| ...
(X) }
|(b) if (dai_link->dpcm_capture) {
| ...
| ^ for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
|(*) ...
| v }
| ...
v }
} else {
^ ^ /* Adapt stream for codec2codec links */
|(Z) int cpu_capture = ...
| v int cpu_playback = ...
(Y)
| ^ for_each_rtd_ch_maps(rtd, i, ch_maps) {
|(*) ...
v v }
}
...
}
(*) part is checking each DAI's availability.
(X) part is for DPCM, and it checks playback/capture availability
if dai_link has dpcm_playback/capture flag (a)(b).
This availability check should be available not only for DPCM, but for
all connections. But it is not mandatory option. Let's name it as
assertion.
In case of having assertion flag, non specific side will be disabled.
For example if it has playback_assertion but not have capture_assertion,
capture will be force disabled.
dpcm_playback -> playabck_assertion = 1
dpcm_capture -> capture_assertion = 1
playback_only -> playback_assertion = 1
capture_assertion = 0
capture_only -> playback_assertion = 0
capture_assertion = 1
By expanding this assertion to all connections, we can use same code
for all connections, this means code can be more cleanup.
Here, current CPU / Codec validation check relationship is like this
DPCM
[CPU/xxxx]-[yyyy/Codec]
^^^^ ^^^^
non DPCM
[CPU/Codec]
^^^^^^^^^^^
DPCM part (X) is checking only CPU DAI, and
non DPCM part (Y) is checking both CPU/Codec DAI
Current validation check on DPCM ignores Codec DAI, but there is no
reason to do it. We should check both CPU/Codec in all connection.
This patch expands validation check to all cases.
[CPU/xxxx]-[yyyy/Codec]
*****
In many case (not all case), above [xxxx][yyyy] part are "dummy" DAI
which is always valid for both playback/capture.
But unfortunately DPCM BE Codec (**** part) had been no validation
check before, and some Codec driver doesn't have necessary settings for
it. This means all cases validation check breaks compatibility on some
vender's drivers. Thus this patch temporary uses dummy DAI at BPCM BE
Codec part, and avoid compatibility error. But it should be removed in
the future.
Signed-off-by: Kuninori Morimoto <[email protected]>
Historically, ASoC doesn't have validation check for DPCM BE Codec,
but it should have. Current ASoC is ignoring it same as before,
but let's indicate the warning about that.
This warning and code should be removed and cleaned if all DPCM BE
Codec driver has necessary settings in the future.
One of the big user which doesn't have it is Intel.
Below is at least already known settings missing driver.
--- sound/soc/codecs/hda.c ---
static struct snd_soc_dai_driver card_binder_dai = {
.id = -1,
.name = "codec-probing-DAI",
+ .capture.channels_min = 1,
+ .playback.channels_min = 1,
};
--- sound/pci/hda/patch_hdmi.c ---
static int generic_hdmi_build_pcms(...)
{
...
for (...) {
...
+ pstr->channels_min = 1;
}
return 0;
}
Link: https://lore.kernel.org/r/[email protected]
Cc: Amadeusz Sławiński <[email protected]>
Signed-off-by: Kuninori Morimoto <[email protected]>
snd_soc_dai_link_set_capabilities() checks all CPU/Codec DAI (Y)(Z)
for Playback/Capture (X) and checks its validation (A), and setup
dpcm_playback/capture flags (a).
void snd_soc_dai_link_set_capabilities(...)
{
...
(X) for_each_pcm_streams(direction) {
...
(Y) for_each_link_cpus(dai_link, i, cpu) {
...
(A) if (... snd_soc_dai_stream_valid(...)) {
...
}
}
(Z) for_each_link_codecs(dai_link, i, codec) {
...
(A) if (... snd_soc_dai_stream_valid(...)) {
...
}
}
...
}
(a) dai_link->dpcm_playback = supported[...];
(a) dai_link->dpcm_capture = supported[...];
}
This validation check will be automatically done on new
soc_get_playback_capture(). snd_soc_dai_link_set_capabilities() is no
longer needed. Let's remove it.
Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…ertion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…ertion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…_assertion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…_assertion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…apture_assertion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts dpcm_xxx flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…rtion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
…rtion soc_get_playback_capture() is now handling DPCM and normal comprehensively for playback/capture stream in same code. This patch converts xxx_only flag to xxx_assertion. Signed-off-by: Kuninori Morimoto <[email protected]>
No driver is using dpcm_playback/capture and playback/capture_only flag, let's remove these. Signed-off-by: Kuninori Morimoto <[email protected]>
.dpcm_playback/capture flags are converted to playback/capture_assertion flag. Let's follow it on Documentation. Signed-off-by: Kuninori Morimoto <[email protected]>
The patches added a requirement that the cpu and codec dais support the direction set by the dailink. One some Intel platform, we "cheat" when the echo reference is generated. The dailink is marked as supporting capture even though the amplifier cannot generate the echo reference; the echo reference is actually generated in the firmware and the DAI not used for capture proper. Signed-off-by: Pierre-Louis Bossart <[email protected]>
plbossart
force-pushed
the
morimoto/dpcm-cleanup
branch
from
b237412 to
c102f11
Compare
|
I think I know the issue: these patches added a stricter check that both cpu and codec dais support a direction. That is not true when we have an echo reference generated in the firmware: the codec dai does not support capture at all. |
|
3 hours and the build is not complete? |
|
SOFCI TEST |
|
These look relevant: |
|
Yeah, the errors above validate my theory that the new criteria are too strict. The Helios machine driver have a capture dailink that's connected to SSP1, even though the amplifiers don't support capture. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.