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

ucm2: Add initial support for AMD Vangogh (acp5x) on Steam Deck #233

Closed
wants to merge 1 commit into from

Conversation

cristicc
Copy link
Contributor

This has been tested on a Valve Steam Deck EV2 unit, using kernel v6.1-rc1.

ucm2/AMD/acp5x/acp5x.conf Outdated Show resolved Hide resolved
}
}

SectionDevice."Internal Mic".0 {
Copy link
Member

Choose a reason for hiding this comment

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

Follow https://github.com/alsa-project/alsa-lib/blob/master/include/use-case.h#L132 . Use "Mic" or "Mic1" here. Remove .0 suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


Value {
PlaybackPriority 100
PlaybackPCM "hw:acp5x,1"
Copy link
Member

Choose a reason for hiding this comment

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

Use "hw:${CardId},1" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

]

DisableSequence [
cset "name='Left AMP Enable Switch' off"
Copy link
Member

Choose a reason for hiding this comment

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

This setup is not in sync with the EnableSequence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

EnableSequence [
cset "name='Int Mic Switch' on"
cset "name='DMIC Enable Switch' on"
cset "name='Mic Volume' 252"
Copy link
Member

Choose a reason for hiding this comment

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

Fixed volume control. The application cannot use hw volume control (CaptureMixerElem).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

User configurable settings moved to BootSequence.

@@ -0,0 +1,122 @@
SectionVerb {
EnableSequence [
cset "name='Left DSP1 Preload Switch' 0"
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to study https://www.alsa-project.org/alsa-doc/alsa-lib/group__ucm__conf.html . The values which may be configurable by users should be put to the BootSequence.

Also the hw volume control is completely missing in this definition. The problem may be the Left/Right mono controls (apps expect the stereo controls for the stereo stream). This can be remaped using the alsa-lib's remap control plugin. We need to build probably a macro - see ucm2/common/ctl/remap.conf, https://www.alsa-project.org/alsa-doc/alsa-lib/control_plugins.html .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many thanks for the detailed review and the hints to further improve this! Hopefully I got it in a better shape this time..

@DocMAX
Copy link

DocMAX commented Dec 14, 2022

Works perfectly here. What are we still waiting for?...

@bell07
Copy link

bell07 commented Dec 23, 2022

The "off" Profile does not work as expected for me. The output devices are still available if I try to disable the port.

Steps to reproduce: Connect steam deck to TV. Play audio to builtin (acp5x) speaker by pulseaudio / pipewire.
Try to disable the the buitin speaker by
pactl set-card-profile alsa_card.pci-0000_04_00.5-platform-acp5x_mach.0 off

expected behaviour is the both acp5x output devices are not available and audio switch to HDMI output.

Currently I use the

This has been tested on a Valve Steam Deck EV2 unit, using kernel
v6.1-rc1.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
@cristicc
Copy link
Contributor Author

Thanks @DocMAX and @bell07 for testing this!

I've just pushed an update to properly handle the "off" profile. Please let me know if you still encounter issues.

@bell07
Copy link

bell07 commented Dec 29, 2022

Thank you! Does perfectly work for me now

@hexaheximal
Copy link

I tried copying the files in this pull request to my Steam Deck (which is currently running Debian bookworm/testing), and it did not show the HiFi option in pavucontrol.

I additionally tried running alsaucm reload, and it showed multiple errors:

ALSA lib parser.c:244:(error_node) UCM is not supported for this HDA model (HD-Audio Generic at 0x803c0000 irq 72)
ALSA lib main.c:1541:(snd_use_case_mgr_open) error: failed to import hw:0 use case configuration -6
alsaucm: error failed to open sound card hw:0: No such device or address

I have rebooted it multiple times, but it still doesn't show the HiFi option and only shows the HDMI options.

What configuration step am I missing?

@cristicc
Copy link
Contributor Author

@hexaheximal Although I haven't encountered that kind of issues so far, I would suggest to follow the installation procedure documented in README.md, just to exclude a possible configuration problem. Make sure to do a backup first, since this might break your setup if the installed ALSA binaries are too old and cannot properly handle the configuration update.

@hexaheximal
Copy link

hexaheximal commented Dec 30, 2022

@hexaheximal Although I haven't encountered that kind of issues so far, I would suggest to follow the installation procedure documented in README.md, just to exclude a possible configuration problem. Make sure to do a backup first, since this might break your setup if the installed ALSA binaries are too old and cannot properly handle the configuration update.

I'm using the version of alsa-ucm-conf (and the kernel) included in debian bookworm.
I don't see why it wouldn't work though.
I've heard it only works on kernel 6.1+ before, but I can't find anything that changed in 6.1 for the acp5x driver.

Also, backups are not a problem for me since I'm using a custom script that I wrote (https://github.com/deckian/deckian) to install debian on my Steam Deck which handles rootfs backups automatically so I can just reinstall it from a backup when I need to.

@cristicc
Copy link
Contributor Author

I'm using the version of alsa-ucm-conf (and the kernel) included in debian bookworm.

I only tested on an Arch Linux based distro after updating alsa-ucm-conf as indicated above.

I've heard it only works on kernel 6.1+ before, but I can't find anything that changed in 6.1 for the acp5x driver.

That's because not all the changes are on the driver itself, e.g. you need at least the following patch from 6.1 to get proper sound support:

9d08f700ab78 spi: amd: Setup all xfers before opcode execution

@waffshappen
Copy link

I am using this on a Steam Deck with Fedora 37 on the latest 6.1.2 kernel from koji.

While the patchset works generally and makes the speakers work they are very, very hushed and in any slightly louder environment almost completely inaudible at max volume (125% boost, gnome+pipewire). Booting back to SteamOS they're at the expected max loudness again. Is this expected for now?

@cristicc
Copy link
Contributor Author

cristicc commented Jan 6, 2023

@waffshappen It is expected to get a slightly lower loudness, but not quite to the levels you describe. Probably some of the mixer controls are not properly adjusted.

Also note SteamOS achieves max loudness by using a modified/tuned firmware for the cs35l41 audio codec. You may try to locate cs35l41-dsp1-spk-prot.wmfw and cs35l41-dsp1-spk-prot.bin on your Fedora distro (most probably they reside in /lib/firmware/cirrus) and replace them with /lib/firmware/{cs35l41-dsp1-spk-prot.wmfw,cs35l41-dsp1-spk-prot.bin} provided by SteamOS.

WARNING: Before doing any audio test with the replaced firmware, make sure to lower the volume first, otherwise you may risk damaging the speakers! On the SteamOS side, this is mitigated via the UCM configuration by restricting user access to some mixer controls.

@waffshappen
Copy link

@waffshappen It is expected to get a slightly lower loudness, but not quite to the levels you describe. Probably some of the mixer controls are not properly adjusted.
Also note SteamOS achieves max loudness by using a modified/tuned firmware for the cs35l41 audio codec. You may try to locate cs35l41-dsp1-spk-prot.wmfw and cs35l41-dsp1-spk-prot.bin on your Fedora distro (most probably they reside in /lib/firmware/cirrus) and replace them with /lib/firmware/{cs35l41-dsp1-spk-prot.wmfw,cs35l41-dsp1-spk-prot.bin} provided by SteamOS.
WARNING: Before doing any audio test with the replaced firmware, make sure to lower the volume first, otherwise you may risk damaging the speakers! On the SteamOS side, this is mitigated via the UCM configuration by restricting user access to some mixer controls.

I tested this with both the original steam deck and fedora firmware - both were initially equally silent until i found (alsamixer, disabled pw alsa redirection) the "Left Analog PCM" (and right variant) and set both to dB gain: 0,20

This made loudness equal to the original steam deck loudness on both firmware versions. Maybe something to adjust in the ucm config, if possible?

@cristicc
Copy link
Contributor Author

cristicc commented Jan 6, 2023

This made loudness equal to the original steam deck loudness on both firmware versions. Maybe something to adjust in the ucm config, if possible?

The adjustment is already handled in acp5x.conf:

BootSequence [
	...
	cset "name='Left Analog PCM Volume' 17"
	cset "name='Right Analog PCM Volume' 17"
	...
]

Both controls should have been automatically set to 0.17dB if the sound card state was not previously saved, i.e. file /var/lib/alsa/asound.state is missing. If you don't have this file, probably the mixer controls have been adjusted via pulse/pipewire?!

After rebooting the system, what does alsamixer show? I would expect either 0.20 (what you manually set) or 0.17 (what UCM provides as defaults).

@waffshappen
Copy link

This made loudness equal to the original steam deck loudness on both firmware versions. Maybe something to adjust in the ucm config, if possible?

The adjustment is already handled in acp5x.conf:

BootSequence [
	...
	cset "name='Left Analog PCM Volume' 17"
	cset "name='Right Analog PCM Volume' 17"
	...
]

Both controls should have been automatically set to 0.17dB if the sound card state was not previously saved, i.e. file /var/lib/alsa/asound.state is missing. If you don't have this file, probably the mixer controls have been adjusted via pulse/pipewire?!

After rebooting the system, what does alsamixer show? I would expect either 0.20 (what you manually set) or 0.17 (what UCM provides as defaults).

It is still on 0.20 as was set via alsamixer.

Could this be a sideeffect of having to install the distro first and then adding the configs and kernel 6.1 after first boot?

@cristicc
Copy link
Contributor Author

cristicc commented Jan 9, 2023

Could this be a sideeffect of having to install the distro first and then adding the configs and kernel 6.1 after first boot?

On Arch Linux (and most likely on Fedora as well), the alsa-utils package provides the alsa-restore.service responsible for saving the sound card state to /var/lib/alsa/asound.state on system shutdown and restoring it after booting:

$ systemctl status alsa-restore
● alsa-restore.service - Save/Restore Sound Card State
     Loaded: loaded (/usr/lib/systemd/system/alsa-restore.service; static)
[...]
Jan 08 22:37:11 systemd[1]: Starting Save/Restore Sound Card State...
Jan 08 22:37:11 systemd[1]: Finished Save/Restore Sound Card State.

The most plausible scenario is that the sound card state had been already persisted when you updated the UCM configuration, hence the BootSequence in acp5x.conf did not execute.

@perexg
Copy link
Member

perexg commented Jan 9, 2023

You may remove the old state using:

systemctl stop alsa-state
systemctl stop alsa-restore
rm /var/lib/alsa/asound.state
reboot

And yes, BootSequence is executed only when the previous state does not exist.

@cristicc
Copy link
Contributor Author

cristicc commented Jan 9, 2023

@perexg Is there anything else you'd like me to address/improve in order to get this merged?

@perexg perexg closed this in 6dd80ae Jan 29, 2023
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.

None yet

6 participants