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

Remove sof-test dependency on topology file, use /proc instead #471

Closed
marc-hb opened this issue Oct 24, 2020 · 24 comments
Closed

Remove sof-test dependency on topology file, use /proc instead #471

marc-hb opened this issue Oct 24, 2020 · 24 comments
Assignees
Labels
P3 Low-impact bugs or features type:discussion Open ended discussion topic

Comments

@marc-hb
Copy link
Collaborator

marc-hb commented Oct 24, 2020

From @ranj063

@plbossart @aiChaoSONG @marc-hb this PR (#430) is in the right direction. I think we should decouple sof-test from the topology file even when testing with the SOF driver and use the procfs info for the card/pcm info and even for the topology graph using the asoc dapm info. This will also make sure that we dont have assumptions about the default card number. What do you think?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 24, 2020

From @plbossart

Agree that if we can use the /proc info then we have all the information needed for tests with PCM devices.
For probes it's actually the only solution since we expose a compressed interface that will never be shown in the topology but will be shown in the card properties (if it's not show it's a bug). The probes are like 'blue wires', they are not set in stone and not shown in the topology.

Having the topology would only help to show the graph as an information but would not have an functional input into the tests.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Oct 24, 2020

Agree that if we can use the /proc info then we have all the information needed for tests with PCM devices.

PR #430 has the following code:

for pcm in card_info['pcm']:
                # There are limited pipeline parameters in the proc file system,
                # add some default parameters to make use of sof-test for legacy HDA test
                pcm['fmt'] = 'S16_LE'
                pcm['fmts'] = 'S16_LE S24_LE S32_LE'
                pcm['rate'] = '48000'
                pcm['channel'] = '2'
                pcm['dev'] = 'hw:{},{}'.format(card_id, pcm['id'])

@ranj063 , @plbossart Is this not actually needed for testing?

BTW Auxiliary devices in SOF break SOFCARD #466 seems related.

EDIT, somewhat related check-alsabat.sh: remove use of plughw #241

@marc-hb marc-hb changed the title Remove sof-test dependency on topology file Remove sof-test dependency on topology file, use /proc instead Oct 24, 2020
@aiChaoSONG
Copy link

@marc-hb For SOF test, we have to depend on topology. Because we cannot get the information from proc, eg:

  • We can't identify WoV pipeline/Echo Reference pipeline/DSM pipeline from proc, which need special test case for the feature.
  • We can't get component parameters from proc, eg: SOF_TKN_VOLUME_RAMP_STEP_TYPE SOF_TKN_VOLUME_RAMP_STEP_MS

@xiulipan
Copy link
Contributor

Agree that if we can use the /proc info then we have all the information needed for tests with PCM devices.

PR #430 has the following code:

for pcm in card_info['pcm']:
                # There are limited pipeline parameters in the proc file system,
                # add some default parameters to make use of sof-test for legacy HDA test
                pcm['fmt'] = 'S16_LE'
                pcm['fmts'] = 'S16_LE S24_LE S32_LE'
                pcm['rate'] = '48000'
                pcm['channel'] = '2'
                pcm['dev'] = 'hw:{},{}'.format(card_id, pcm['id'])

@marc-hb right, you have found the limitation for the proc info. This is the workaround code to provide a fake PCM capability (hw_params) to the PCMs from proc. In TPLG, we can get those info by parse the binary.
When you try to cat the hw_params info from a PCM in proc, you will only get

/proc/asound/card0/pcm0c/sub0$ cat hw_params
closed

Another gaps we have with proc info is that we could not get the info about the comps in pipeline for the PCM. Some test case is designed for special COMP (eg: MUX, EQ, WOV). So I think we can use proc as a fallback, but we could not relay on the proc info for our test.

@ranj063
Copy link
Contributor

ranj063 commented Oct 26, 2020

Another gaps we have with proc info is that we could not get the info about the comps in pipeline for the PCM. Some test case is designed for special COMP (eg: MUX, EQ, WOV). So I think we can use proc as a fallback, but we could not relay on the proc info for our test.

@xiulipan what are we missing for these special comps that we read from topology?

@ranj063
Copy link
Contributor

ranj063 commented Oct 27, 2020

@marc-hb For SOF test, we have to depend on topology. Because we cannot get the information from proc, eg:

  • We can't identify WoV pipeline/Echo Reference pipeline/DSM pipeline from proc, which need special test case for the feature.

@aiChaoSONG wov is always the DMIC16k device. For DSM, the speaker PCM device will have the same name for the feedback pipeline and for echoref, the pipeline is simply called echo. What more info do we need from tplg?

@ranj063
Copy link
Contributor

ranj063 commented Oct 27, 2020

  • We can't get component parameters from proc, eg: SOF_TKN_VOLUME_RAMP_STEP_TYPE SOF_TKN_VOLUME_RAMP_STEP_MS

What do we use these for?

@aiChaoSONG
Copy link

@ranj063 I think below useful information cannot be extracted from proc, either.

  • rate, channel count and supported formats, need to query with alsa API.
  • SOF_TKN_VOLUME_RAMP_STEP_MS: this defines the duration of the volume ramp in a PGA, which is useful in volume test
  • PCM/pipeline <-> PGA mapping,

wov is always the DMIC16k device
Yes, But the reverse may not be true, right? should we run WoV test on all devices with DMIC16k, even it may not act as WoV pipeline? We do have such device with DMIC16k, but without WoV support.

@xiulipan
Copy link
Contributor

@ranj063 @aiChaoSONG I think /proc info only is not enough to get similar info as TPLG parser can do.
We may need to use ALSA API to query the info (eg: aplay --dump-hw-params), we can do some experiment later to see if this would work in most case and get correct info for the test.

If above steps can work, TPLG reader can still be kept for PCM info validation test and some audio quality test that may need detailed configuration value.

@plbossart
Copy link
Member

@xiulipan even for legacy HDaudio it's much better to query the capabilities instead of hard-coding 48kHz as suggested.

That said you are not going to do a volume test or PCM/pipeline->PGA mapping for legacy HDaudio, so we probably need to move to a mode where the topology is only used when absolutely needed, instead of being the source of all information.

@xiulipan
Copy link
Contributor

@plbossart I think I have made some alignment with @ranj063 @aiChaoSONG to have a feature to read /proc or query from ALSA API to get the correct PCM info. But this will take time to make sure the read out info is trustful and won't break SOF FW before we start the test.

reference to get those info by API:

@aiChaoSONG
Copy link

some parameter can only be extracted from topology, so a full replacement is impossible, but for platforms use legacy HDA drivers like RKL-S, we can run sof-test on there platforms by reading /proc. close? @marc-hb

@aiChaoSONG aiChaoSONG added the P3 Low-impact bugs or features label Jan 26, 2021
@xiulipan xiulipan added the type:discussion Open ended discussion topic label Jan 26, 2021
@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 28, 2021

so we probably need to move to a mode where the topology is only used when absolutely needed, instead of being the source of all information.

How far from that are we?

@xiulipan
Copy link
Contributor

so we probably need to move to a mode where the topology is only used when absolutely needed, instead of being the source of all information.

How far from that are we?

For legacy HDA DUT, we are using /proc but with FAKED rate, channel and format info. I do not think using /proc like this can satisfy our test case requirement.

To get fully info, we will need to query the PCM with --dump-hw-params which makes test case more complex.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Feb 18, 2022

I just made sof-get-default-tplg.sh compatible with the new cavs / IPC4 topologies:

It works by scanning the kernel logs so it does not work until the driver has been loaded.

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 25, 2022

@plbossart reported an interesting side effect of this in thesofproject/linux#3645 (comment)

When everything goes well, the list of pipelines comes from the $TPLG file. This file obviously does not include any USB device.

When the firmware crashes or does not load for some reason, sof-test/case-lib/pipeline.sh#func_pipeline_export() assumes the lack of SOF devices requires running in "legacy HDA" mode. This falls back on sof-dump-status.py -e "$filter_str" which does read (fewer data fields?) from /proc. Unlike the $TPLG file, /proc does include USB devices. Confusion ensues.

There's probably a bug somewhere.

@plbossart
Copy link
Member

@aiChaoSONG It seems to me that the idea of 'legacy HDA' was implemented in one of your patches, 3c6e727 ('sof-dump-status: dump pipeline info from proc for legacy HDA')

Can you chime in on what the idea of this 'legacy HDA' feature was? I don't recall ever seeing a test report on snd-hda-intel tested with HDaudio, and it seems like this will select USB audio if the SOF card is not present - major problem IMHO.

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 26, 2022

Quoting one of the commit messages in (long)

This patch adds is_sof_used function to check (whether) we are running test on SOF platform or legacy HDA platform.

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 26, 2022

From experience, automated fallbacks are a generous source of "green failures".

In thesofproject/linux#3645 (comment), this automated fallback certainly tries to hide an SOF crash.

Maybe sof-test should by default test SOF device and nothing else.

If someone wants to test non-SOF devices, they can but they should have to ask explicitly.

Testing is about finding and reporting as many problems as possible. Fallbacks do the exact opposite.

@aiChaoSONG
Copy link

aiChaoSONG commented May 26, 2022

@plbossart @marc-hb We do test legacy-hda on RKL device, check report 12889. But looks no one has ever checked it.

sh-rkls-rvp-hda-03:~$ cat /proc/asound/cards
 0 [PCH            ]: HDA-Intel - HDA Intel PCH
                      HDA Intel PCH at 0x6001110000 irq 155

@plbossart
Copy link
Member

Agree, if people want to test on HDaudio or USB or whatever, they should use the card name explicitly. That's a great feature to allow sof-test to be used in a generic way.

Fallback is evil in this case, we end-up testing the wrong card if by accident the one we want was not registered. Big conceptual fail IMHO. Typical issues will be using the NVidia HDMI card or USB audio, not of which are intended in our CI tests.

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 27, 2022

Agree, if people want to test on HDaudio or USB or whatever, they should use the card name explicitly. That's a great feature to allow sof-test to be used in a generic way.

I think it would be best but it would be some coding effort. In the meantime here's a quickfix from @aiChaoSONG that turns off the fallback by default:

@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 9, 2024

@marc-hb marc-hb self-assigned this Aug 13, 2024
@marc-hb
Copy link
Collaborator Author

marc-hb commented Aug 15, 2024

Maybe reading /proc/ is better in some cases, but sof-test will always need information from topologies. Recent examples:

So, closing.

@marc-hb marc-hb closed this as completed Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low-impact bugs or features type:discussion Open ended discussion topic
Projects
None yet
Development

No branches or pull requests

5 participants