Skip to content

alsactl DUT restoration #1270

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.

Sign up for GitHub

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

Open
LukaszMrugala wants to merge 5 commits into
base: main
Choose a base branch
from
Open

alsactl DUT restoration #1270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4cde89
alsactl: Implementation for LNL, MTL, PTL
LukaszMrugala May 8, 2025
f9f2401
log: Add dlogi at set_alsa_settings start
LukaszMrugala Jul 4, 2025
107aeb3
alsa: Add common setup_alsa in ALSA tests
LukaszMrugala Jul 4, 2025
0c76c00
alsa: remove crash on init
LukaszMrugala Jul 11, 2025
e208838
Fix typo
LukaszMrugala Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions alsa_settings/LNLM_RVP_HDA.sh
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions alsa_settings/LNLM_SDW_AIOC.sh
View file
Open in desktop

This file was deleted.

11 changes: 0 additions & 11 deletions alsa_settings/MTLP_RVP_HDA.sh
View file
Open in desktop

This file was deleted.

17 changes: 0 additions & 17 deletions alsa_settings/MTLP_RVP_SDW.sh
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion alsa_settings/MTLP_SDW_AIOC.sh
View file
Open in desktop

This file was deleted.

12 changes: 0 additions & 12 deletions alsa_settings/PTLH_HDA_AIOC.sh
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions alsa_settings/PTLH_SDW_RT712.sh
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions alsa_settings/PTLP_RVP_SDW.sh
View file
Open in desktop

This file was deleted.

19 changes: 19 additions & 0 deletions alsa_settings/alsactl/LNLM_RVP_HDA.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
state.sofhdadsp {
control.1 {
iface MIXER
name 'Headphone Playback Volume'
value.0 60
value.1 60
}
control.6 {
iface MIXER
name 'Capture Volume'
value.0 30
value.1 30
}
control.9 {
iface MIXER
name 'Master Playback Volume'
value 87
}
}
14 changes: 14 additions & 0 deletions alsa_settings/alsactl/LNLM_SDW_AIOC.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
state.sofsoundwire {
control.1 {
iface MIXER
name 'rt711 FU05 Playback Volume'
value.0 80
value.1 80
}
control.5 {
iface MIXER
name 'rt711 FU0F Capture Volume'
value.0 30
value.1 30
}
}
19 changes: 19 additions & 0 deletions alsa_settings/alsactl/MTLP_RVP_HDA.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
state.sofhdadsp {
control.1 {
iface MIXER
name 'Headphone Playback Volume'
value.0 60
value.1 60
}
control.6 {
iface MIXER
name 'Capture Volume'
value.0 30
value.1 30
}
control.9 {
iface MIXER
name 'Master Playback Volume'
value 87
}
}
14 changes: 14 additions & 0 deletions alsa_settings/alsactl/MTLP_RVP_SDW.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
state.sofsoundwire {
control.1 {
iface MIXER
name 'rt711 FU05 Playback Volume'
value.0 80
value.1 80
}
control.5 {
iface MIXER
name 'rt711 FU0F Capture Volume'
value.0 30
value.1 30
}
}
1 change: 1 addition & 0 deletions alsa_settings/alsactl/MTLP_SDW_AIOC.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
./MTLP_RVP_SDW.state
8 changes: 8 additions & 0 deletions alsa_settings/alsactl/PTLH_HDA_AIOC.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
state.sofhdadsp {
control.1 {
iface MIXER
name 'Headphone Playback Volume'
value.0 60
value.1 60
}
}
36 changes: 36 additions & 0 deletions alsa_settings/alsactl/PTLH_SDW_RT712.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
state.sofsoundwire {
control.1 {
iface MIXER
name 'rt712 FU06 Playback Volume'
value.0 60
value.1 60
}
control.2 {
iface MIXER
name 'rt712 FU05 Playback Volume'
value.0 60
value.1 60
}
control.3 {
iface MIXER
name 'rt712 FU0F Capture Switch'
value.0 on
value.1 on
}
control.4 {
iface MIXER
name 'rt712 FU0F Capture Volume'
value.0 46
value.1 46
}
control.14 {
iface MIXER
name 'Headphone Switch'
value.0 on
}
control.15 {
iface MIXER
name 'Headset Mic Switch'
value.0 on
}
}
14 changes: 14 additions & 0 deletions alsa_settings/alsactl/PTLP_RVP_SDW.state
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
state.sofsoundwire {
control.3 {
iface MIXER
name 'rt722 FU0F Capture Volume'
value.0 15
value.1 15
}
control.5 {
iface MIXER
name 'rt722 FU06 Playback Volume'
value.0 50
value.1 50
}
}
7 changes: 7 additions & 0 deletions alsa_settings/alsactl/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Creating a .state file

1. Gain access to a machine you'd like to create a state file for.
Copy link
Member

@golowanow golowanow May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How all the *.state files in this PR were obtained ? (context: platform in default state, kernel, version, topology, whatever else what's matter)

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 4, 2025

Choose a reason for hiding this comment

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

sof and linux builds were taken from that day's CI relevant run. Then I've run the PR with alsa-info print added in. This gave me the full .state file I could then adjust.

Copy link
Member

@golowanow golowanow Jun 9, 2025

Choose a reason for hiding this comment

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

as I understand the idea behind this PoC proposal, these *.state files are transitional having their content specific to the SOF/Linux components currently deployed on a DUT, thus in most cases they should be explicitly created at some early stage of a test plan execution to keep the appropriate defaults to reset before each test case, where it is important.

Copy link
Collaborator

@marc-hb marc-hb Jun 9, 2025

Choose a reason for hiding this comment

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

these *.state files are transitional having their content specific to the SOF/Linux components currently deployed on a DUT, ...

That looks like a great catch, I feel bad I missed this.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 11, 2025

Choose a reason for hiding this comment

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

I do not seem to understand. They necessarily cannot be transitional (temporary in their existence), as they are the definition of the DUT state we want to achieve, just as the previously-used .sh files. I would not call the .sh files transitional as I wouldn't call the .state files transitional.

I also do not understand had would we create the .state files during testplan execution, given that those files are our source of truth when it comes to out desired state. It would be akin to creating a MAVEN file in Java for every compilation - the benefits would be slim and the process unnecessarily onerous.

Copy link
Member

@golowanow golowanow Jun 11, 2025

Choose a reason for hiding this comment

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

.. given that those files are our source of truth when it comes to out desired state

right, my question is about these *.state files which are included into this PR, and what the 'desired state' they set.

If the basic goal is to reset the state before each test to defaults, then there is no need to keep these files in the sof-test repo as a 'golden source', but call get_alsactl_state_or_create() once at the test plan beginning right after the DUT deployment stage, and then call restore_settings_via_alsactl() before each test it needs. Isn't it enough ? Did you try/consider to call alsactl init for the same purpose ?

When there is a need to do some custom/experimental state change, it can be done e.g. by a CI script deploying a .state file here from some other repo/source with the platform-specific BKC, or a state file placed here by some test case script during its test plan execution.

Copy link
Collaborator

@marc-hb marc-hb Jun 11, 2025

Choose a reason for hiding this comment

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

You wrote in #1270 (comment)

This approach should only modify the values mentioned in the .state file. So while a typical machine would have a couple dozen values, only the few specified in the .state file will be changed. That should mean this approach is equivalent.

But then in #1270 (comment)

sof and linux builds were taken from that day's CI relevant run. Then I've run the PR with alsa-info print added in. This gave me the full .state file I could then adjust.

When you write "adjust", do you mean "trimmed down to an incremental change, strictly equivalent to the .sh files removed"?

If that's what you meant then that looks OK to me.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 11, 2025

Choose a reason for hiding this comment

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

@marc-hb Yes, that's exactly what I do - full .state files can have over 60 controls specified, while we oftentimes need less than 10, we I trim them to just what the .sh had specified.

@golowanow The full .state file has all the controls and their current values. However, the current .sh files specify only some controls to some values that can be different from what currently exist on the machine. That's why we do not create them during the runs - we'd be restoring the current machine state rather than restoring the desired machine state.

alsactl init is just alsactl restore on a specific file in a predetermined place. Why obfuscate the files to use rather than keep them in the same repo that uses them? Those aren't multi-gigabyte behemoths to be wary of downloading them unnecessarily.

Copy link
Member

@golowanow golowanow Jun 12, 2025

Choose a reason for hiding this comment

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

Why we expect that after alsactl init the DUT's state is wrong and we need some handmade *.state file to set correct defaults ?

I think this PR should evolve from its current 'proof-of-concept' to some runnable test case, then it will be easier to see how it solves the problem and ready for merge.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 23, 2025

Choose a reason for hiding this comment

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

Why we expect that after alsactl init the DUT's state is wrong and we need some handmade *.state file to set correct defaults ?

As we have experienced tests failing in our current CI operation and we have never used alsactl init's files contained by default in /usr/share/alsa/init/ to restore the DUT state to our liking, why should we assume that alsactl init will bring the DUT to a state we desire? We have no information that would indicate that ALSA's default state is congruent with our desired state. Thusly, we are to use custom .state files, as that's the status quo.

I think this PR should evolve from its current 'proof-of-concept' to some runnable test case, then it will be easier to see how it solves the problem and ready for merge.

In my understanding, this repo houses tests for ALSA configurations and tools to that end. Separate tests for the tools inside the sof-test repo seem missing and it would be awkward to task this PR specifically with establishing a base for tests of sof-test itself.

2. Run `alsa-info` or use the print_alsa_info from the lib.sh file.
3. Copy the `!!Alsactl output` part of the `alsa-info` file to your `.state` file.
4. Modify values you'd like to set at a specific amount and delete the rest.
5. Name the file <MODEL_NAME>.state
85 changes: 83 additions & 2 deletions case-lib/lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1184,13 +1184,92 @@ re_enable_ntp_sync()
sudo timedatectl set-ntp true
}

# Get alsa-info of DUTs in CI
print_alsa_info()
{
dlogi "----------- ↓ alsa-info ↓ ------------"
alsa-info --stdout --with-aplay --with-amixer --with-alsactl --with-configs --no-upload
dlogi "----------- ↑ alsa-info ↑ ------------"
}

# Check whether a relevant .state file exists
# If not, save current state for future use
# param1: script home path
# param2: platform name
get_alsactl_state_or_create()
{
ALSACTL_STATE_FILE_PATH="$1"/alsa_settings/alsactl/"$2".state
# .state file does not exist. Creating one from current setup.
if [ ! -f "$ALSACTL_STATE_FILE_PATH" ]; then
alsactl store --file="$ALSACTL_STATE_FILE_PATH"
Copy link
Member

@golowanow golowanow May 30, 2025

Choose a reason for hiding this comment

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

alsactl will store all devices present

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 4, 2025

Choose a reason for hiding this comment

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

That shouldn't be a problem, right?

Copy link
Member

@golowanow golowanow Jun 12, 2025

Choose a reason for hiding this comment

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

It will look different to what is in the handcrafted *.state files which are for only one card at the moment.
Can you try your PR on a NOCODEC dut ?

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 23, 2025

Choose a reason for hiding this comment

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

It will look different to what is in the handcrafted *.state files which are for only one card at the moment.

It will look different, but that specific function is unused in the automatic code and is left as a tool for new .state file creation.

Can you try your PR on a NOCODEC dut ?

The current code does not touch NOCODEC DUTs. The run No. 53589 contains a NOCODEC platform.

fi
echo "$ALSACTL_STATE_FILE_PATH"
}

# Check whether a relevant .state file exists
# and return its file path
# param1: script home path
# param2: platform name
get_alsactl_state()
{
ALSACTL_STATE_FILE_PATH="$1"/alsa_settings/alsactl/"$2".state
# .state file does not exist.
if [ ! -f "$ALSACTL_STATE_FILE_PATH" ]; then
return 1;
fi
echo "$ALSACTL_STATE_FILE_PATH"
}

# Use `alsactl restore` to restore the alsa settings.
# param1: platform name
restore_settings_via_alsactl()
{
dlogi "Using experimental ALSACTL method"

ALSACTL_STATE_FILE_PATH=$(get_alsactl_state "$SCRIPT_HOME" "$1")

alsactl restore --file="$ALSACTL_STATE_FILE_PATH" --pedantic --no-init-fallback --no-ucm
}

# One-stop shop to initialise alsa-related stuff for tests using it.
setup_alsa()
{
check_locale_for_alsabat

# reset sof volume to 0dB
reset_sof_volume

# If MODEL is defined, set proper gain for the platform
if [ -z "$MODEL" ]; then
# treat as warning only
dlogw "NO MODEL is defined. Please define MODEL to run MODEL-based settings configuration"
else
#dlogi "apply alsa settings for alsa_settings/MODEL.sh"
set_alsa_settings "$MODEL"
fi
}

# check-alsabat.sh need to run optimum alsa control settings
# param1: platform name
set_alsa_settings()
{
# This will bring the machine ALSA state to a common known point - a good baseline
dlogi "Initialising ALSA configuration for $PNAME with alsactl"

# If alsactl init does not recognise the device, it will return error code 99 and bring down the whole test.
# We do not care for unrecognised devices and will ignore it instead.
E_FLAG=0
[[ "$-" == *e* ]] || E_FLAG=$?
set +e
alsactl init
Copy link
Member

@golowanow golowanow Jun 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Currently set_alsa_settings() is called from just two places:

https://github.com/thesofproject/sof-test/blob/main/test-case/check-alsabat.sh#L87-L96
and
https://github.com/thesofproject/sof-test/blob/main/test-case/latency-metrics.sh#L147-L155

both have it after reset_sof_volume() which seems strange now.

I think we should move set_alsa() to lib.sh and reuse it adding there

Suggested change
alsactl init
dlogi "Initialize ALSA cards"
alsactl init

-d gives too much output

Copy link
Member

@golowanow golowanow Jun 26, 2025

Choose a reason for hiding this comment

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

I think that we even need it this way to reset before applying MODEL.state or MODEL.sh:

    alsactl init
    # restores ALSA defaults from /var/lib/alsa/asound.state
    sudo alsactl restore

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jul 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion, we should first have a good set_alsa_settings merged and then we'll add it to all tests using ALSA, while also removing reset_... from them.

With that, all ALSA tests should be pretty much guaranteed to have a common, reproducible state.

I've previously verified which tests use ALSA - I've used that list in the PR #1279 and I'll be able to reuse it here.

Copy link
Member

@golowanow golowanow Jul 3, 2025

Choose a reason for hiding this comment

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

This PR will already affect two tests, so the changes need to be aligned.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jul 4, 2025

Choose a reason for hiding this comment

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

I've created a consisted setup_alsa and added it to all tests using ALSA. Run tests before and after that change.

Copy link
Member

@golowanow golowanow Jul 11, 2025

Choose a reason for hiding this comment

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

I think that we even need it this way to reset before applying MODEL.state or MODEL.sh:

    alsactl init
    # restores ALSA defaults from /var/lib/alsa/asound.state
    sudo alsactl restore

IMO it is essential to execute alsactl restore to reset properly.

if [ "$E_FLAG" -eq 0 ]; then
set -e
fi

# ZEPHYR platform shares same tplg, remove '_ZEPHYR' from platform name
local PNAME="${1%_ZEPHYR}"
dlogi "Run alsa setting for $PNAME"
# Apply configuration on top of the default init
case $PNAME in
APL_UP2_NOCODEC | CML_RVP_NOCODEC | JSL_RVP_NOCODEC | TGLU_RVP_NOCODEC | ADLP_RVP_NOCODEC | TGLH_RVP_NOCODEC | ADLP_RVP_NOCODEC-ipc3 | CML_RVP_NOCODEC-ipc3 | JSL_RVP_NOCODEC-ipc3)
# common nocodec alsa settings
Expand All @@ -1202,10 +1281,12 @@ set_alsa_settings()
;;
TGLU_RVP_NOCODEC_IPC4ZPH | ADLP_RVP_NOCODEC_IPC4ZPH | ADLP_RVP_NOCODEC-ipc4 | TGLU_RVP_NOCODEC-ipc4 | MTLP_RVP_NOCODEC | MTLP_RVP_NOCODEC-multicore-2cores | MTLP_RVP_NOCODEC-multicore-3cores | LNLM_RVP_NOCODEC)
dlogi "Use reset_sof_volume function to set amixer setting."
;;
;;
*)
# if script name is same as platform name, default case will handle all
if [ -f "$SCRIPT_HOME"/alsa_settings/"$PNAME".sh ]; then
if get_alsactl_state "$SCRIPT_HOME" "$PNAME"; then
restore_settings_via_alsactl "$PNAME"
elif [ -f "$SCRIPT_HOME"/alsa_settings/"$PNAME".sh ]; then
Comment on lines +1287 to +1289
Copy link
Member

@golowanow golowanow Jun 16, 2025

Choose a reason for hiding this comment

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

Previously-used switch-case statement checks whether a relevant .state file exists,
based on the $MODEL variable and uses it if able.
If not, it uses the previous method as a fallback.

This statement - .. uses it if able. If not .. in the commit description is confusing: it sounds like when the new method is unable to restore from the .state file, then the .sh script is expected to run.
Also, with all current *.sh scripts removed, no fallback will be possible, so it is better to keep them.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This statement - .. uses it if able. If not .. in the commit description is confusing: it sounds like when the new method is unable to restore from the .state file, then the .sh script is expected to run.

There's no need to overcomplicate - anyone confused by the commit message can verify that the mentioned switch-case does not have any try-catch mechanism. Additionally, "if able" connects with the previous line's "checks whether a relevant .state file exists".

Also, with all current *.sh scripts removed, no fallback will be possible, so it is better to keep them.

New .state files are supposed to replace the old .sh files. We should not assume that alsactl shall break by itself, lest we try to have ksh fallbacks in case bash breaks. Keeping the equivalent .sh files is superfluous with new equivalent .state files present.

Copy link
Member

@golowanow golowanow Jun 25, 2025

Choose a reason for hiding this comment

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

There's no need to overcomplicate

right, let's remove alss_settings/*.sh files describing the change as .. whether a relevant alsa_settings/alsactl/$MODEL.state file exists, or alsa_settings/$MODEL.sh is executed instead, if present.

We should not assume that alsactl break by itself,

We should consider it is possible for alsactl restore to fail when the .state file is incorrect, and then exit the test case with FAIL to avoid running it in some unexpected state.

Copy link
Contributor Author

@LukaszMrugala LukaszMrugala Jul 3, 2025

Choose a reason for hiding this comment

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

We should consider it is possible for alsactl restore to fail when the .state file is incorrect, and then exit the test case with FAIL to avoid running it in some unexpected state.

There hasn't been such a mechanism in the set_alsa_settings() before and introducing such error handling should be comprehensive, encompassing the whole function, and a subject of a separate Issue.

"$SCRIPT_HOME"/alsa_settings/"$PNAME".sh
else
dlogw "alsa setting for $PNAME is not available"
Expand Down
14 changes: 1 addition & 13 deletions test-case/check-alsabat.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,7 @@ then
exit 2
fi

check_locale_for_alsabat

# reset sof volume to 0dB
reset_sof_volume

# If MODEL is defined, set proper gain for the platform
if [ -z "$MODEL" ]; then
# treat as warning only
dlogw "NO MODEL is defined. Please define MODEL to run alsa_settings/MODEL.sh"
else
#dlogi "apply alsa settings for alsa_settings/MODEL.sh"
set_alsa_settings "$MODEL"
fi
setup_alsa

logger_disabled || func_lib_start_log_collect

Expand Down
1 change: 1 addition & 0 deletions test-case/check-audio-equalizer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ sofcard=${SOFCARD:-0}

start_test
setup_kernel_check_point
setup_alsa

# Test equalizer
func_test_eq()
Expand Down
1 change: 1 addition & 0 deletions test-case/check-capture.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ start_test
logger_disabled || func_lib_start_log_collect

setup_kernel_check_point
setup_alsa
func_lib_check_sudo
func_pipeline_export "$tplg" "type:capture & ${OPT_VAL['S']}"

Expand Down
Loading
Loading