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

topology2: pcm_caps: Remove defaults for rate_min/rate_max #9017

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Apr 9, 2024

Use 'rates' instead to specify that 48K is the default supported rate.

Use 'rates' instead to specify that 48K is the default supported rate.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi
Copy link
Contributor

Tests failing on all platforms: https://sof-ci.01.org/sofpr/PR9017/build3818/devicetest/index.html

and no dmesg

@ujfalusi
Copy link
Contributor

Something is wrong with CI, in the failing cases it tries to use 0Hz rate:
aplay -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 10 /dev/zero -v -q
arecord -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 3 /dev/null -v -q

resulting bad speed value 0

@ujfalusi
Copy link
Contributor

Something is wrong with CI, in the failing cases it tries to use 0Hz rate: aplay -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 10 /dev/zero -v -q arecord -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 3 /dev/null -v -q

resulting bad speed value 0

The reason is that the sof-test is parsing the topology and uses rate_min, rate_max which is now 0 if it is not set explicitly. The same issue with #9004 (converted back to draft).
I have not looked at the sof-test that closely, but tools/sof-tplgreader.py loadFile() only reads rate_min, rate_max

@ujfalusi
Copy link
Contributor

ujfalusi commented Apr 10, 2024

PIPELINE_LST[3]="pcm=HDA Analog;id=0;type=playback;cap_name=Analog Playback;eq=eqiir.2.1 eqfir.2.1;fmts=S16_LE S24_LE S32_LE;fmt=S16_LE;rate_min=48000;rate_max=48000;ch_min=2;ch_max=2;rate=48000;channel=2;dev=hw:0,0;snd=/dev/snd/pcmC0D0p;"

vs

PIPELINE_LST[3]="pcm=HDA Analog;id=0;type=playback;cap_name=Analog Playback;eq=eqiir.2.1 eqfir.2.1;fmts=S16_LE S24_LE S32_LE;fmt=S16_LE;rate_min=0;rate_max=0;ch_min=2;ch_max=2;rate=0;channel=2;dev=hw:0,0;snd=/dev/snd/pcmC0D0p;"

The PCM itself shows correct information:

./alsa_conformance_test/alsa_conformance_test -P hw:0,0 --dev_info_only
------DEVICE INFORMATION------
PCM handle name: hw:0,0
PCM type: HW
card: sofhdadsp [sof-hda-dsp]
device: HDA Analog (*) []
stream: PLAYBACK
available channels: 2
available formats: S16_LE S24_LE S32_LE
rate range: [48000, 48000]
available rates: 48000
period size range: [24, 4096]
buffer size range: [192, 16384]
------------------------------

@plbossart
Copy link
Member

that's a spectacular dependency on sof-test parsing of topology. Oh well. Glad it was caught, our CI results just improved over the last week, probably not wise to run CI into the ground again...

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 10, 2024

Something is wrong with CI, in the failing cases it tries to use 0Hz rate: aplay -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 10 /dev/zero -v -q arecord -Dhw:0,0 -r 0 -c 2 -f S16_LE -d 3 /dev/null -v -q
resulting bad speed value 0

The reason is that the sof-test is parsing the topology and uses rate_min, rate_max which is now 0 if it is not set explicitly. The same issue with #9004 (converted back to draft). I have not looked at the sof-test that closely, but tools/sof-tplgreader.py loadFile() only reads rate_min, rate_max

@ujfalusi yes softest relies on rate min/max. I have a fix coming soon

@lgirdwood
Copy link
Member

@ujfalusi any update ?

@fredoh9
Copy link
Contributor

fredoh9 commented Apr 17, 2024

SOFCI TEST

@fredoh9
Copy link
Contributor

fredoh9 commented Apr 17, 2024

After thesofproject/sof-test#1174 merged, I re-triggered the device test. It looks good so far.

@kv2019i kv2019i merged commit c77a4fe into thesofproject:main Apr 17, 2024
44 of 45 checks passed
@marc-hb
Copy link
Collaborator

marc-hb commented Apr 22, 2024

Another problem with this:

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.

7 participants