thesofproject / sof-test

BSD 3-Clause "New" or "Revised" License
13 stars 45 forks source link

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

Closed marc-hb closed 2 months ago

marc-hb commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

aiChaoSONG commented 4 years ago

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

xiulipan commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago
  • 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 commented 4 years ago

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

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 3 years ago

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

marc-hb commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

@plbossart reported an interesting side effect of this in https://github.com/thesofproject/linux/pull/3645#issuecomment-1137902429

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 commented 2 years ago

@aiChaoSONG It seems to me that the idea of 'legacy HDA' was implemented in one of your patches, 3c6e7271c9e989667f537ec5ec2850fedc37a569 ('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 commented 2 years ago

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 commented 2 years ago

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

In https://github.com/thesofproject/linux/pull/3645#issuecomment-1137902429, 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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 6 months ago

New fw_profile worth noting:

marc-hb commented 2 months ago

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

So, closing.