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

Volume reset for alsabat testing #1010

Merged
merged 3 commits into from
Mar 28, 2023

Conversation

keqiaozhang
Copy link
Contributor

No description provided.

@keqiaozhang keqiaozhang requested a review from a team as a code owner March 13, 2023 08:11
@marc-hb
Copy link
Collaborator

marc-hb commented Mar 13, 2023

BTW there's a list of volume issues in

@marc-hb marc-hb requested a review from singalsu March 13, 2023 17:26
@@ -833,7 +833,7 @@ reset_sof_volume()
amixer -Dhw:0 scontrols | sed -e "s/^.*'\(.*\)'.*/\1/" |egrep 'PGA|gain' |
while read -r mixer_name
do
amixer -Dhw:0 -- sset "$mixer_name" 0dB
Copy link
Contributor

Choose a reason for hiding this comment

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

@ujfalusi Do you have idea why 0 dB is not working?

Copy link
Contributor

Choose a reason for hiding this comment

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

Note: In tplg1 100% could be +20 or +40 dB in some cases.

case-lib/lib.sh Outdated
{
# set all PGA* volume to 0dB
amixer -Dhw:0 scontrols | sed -e "s/^.*'\(.*\)'.*/\1/" |grep PGA |
amixer -Dhw:0 scontrols | sed -e "s/^.*'\(.*\)'.*/\1/" |egrep 'PGA|gain' |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
amixer -Dhw:0 scontrols | sed -e "s/^.*'\(.*\)'.*/\1/" |egrep 'PGA|gain' |
amixer -Dhw:0 scontrols | sed -e "s/^.*'\(.*\)'.*/\1/" | grep -E 'PGA|gain' |

see shellcheck

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 14, 2023

@keqiaozhang
Copy link
Contributor Author

keqiaozhang commented Mar 14, 2023

@keqiaozhang
Copy link
Contributor Author

After more tests, I found not all gain controls can set to 0dB,
For example:

amixer -Dhw:0 -- sset 'gain.0.1 1 Playback Volume 0' 0dB
amixer: Invalid command!

amixer -Dhw:0 -- sset "gain.100.1 DMIC0 Capture Volume 1" 0dB
amixer: Invalid command!

This is still a problem that needs to be clarified.

@singalsu
Copy link
Contributor

singalsu commented Mar 14, 2023

I'm wondering if there has ever been a working dB setting in amixer. 0dB has worked but it doesn't seem accept negative values for attenuation.

$ amixer sset 'PGA1.0 1 Master' 0dB
Simple mixer control 'PGA1.0 1 Master',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 32
  Mono:
  Front Left: Playback 32 [100%] [0.00dB]
  Front Right: Playback 32 [100%] [0.00dB]
$ amixer sset 'PGA1.0 1 Master' -6dB
amixer: invalid option -- '6'
Invalid switch or option -? needs an argument.
amixer: invalid option -- 'B'
Invalid switch or option -? needs an argument.

The manual pages suggests that it should be possible but maybe there's some bug

       set or sset <SCONTROL> <PARAMETER> ...
              Sets  the  simple  mixer control contents. The parameter can be the volume either as a percentage from 0% to
              100% with % suffix, a dB gain with dB suffix (like -12.5dB), or an exact hardware value.  The dB gain can be
              used only for the mixer elements with available dB information.  When plus(+) or minus(-) letter is appended
              after volume value, the volume is incremented or decremented from the current value, respectively.

It could be command line quotation issue too. Setting first to 0 dB and then decrement 6 dB works.

$ amixer sset 'PGA1.0 1 Master' 0dB
Simple mixer control 'PGA1.0 1 Master',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 32
  Mono:
  Front Left: Playback 32 [100%] [0.00dB]
  Front Right: Playback 32 [100%] [0.00dB]
$ amixer sset 'PGA1.0 1 Master' 6dB-
Simple mixer control 'PGA1.0 1 Master',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 32
  Mono:
  Front Left: Playback 29 [91%] [-6.00dB]
  Front Right: Playback 29 [91%] [-6.00dB]

@singalsu
Copy link
Contributor

I checked with UPX set up to IPC4, works there too:

$ amixer sset 'gain.1.1 1 2nd' 0dB
Simple mixer control 'gain.1.1 1 2nd',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 45
  Mono:
  Front Left: Playback 45 [100%] [0.00dB]
  Front Right: Playback 45 [100%] [0.00dB]
$ amixer sset 'gain.1.1 1 2nd' 6dB-
Simple mixer control 'gain.1.1 1 2nd',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 45
  Mono:
  Front Left: Playback 42 [93%] [-6.00dB]
  Front Right: Playback 42 [93%] [-6.00dB]

@singalsu
Copy link
Contributor

This is still a problem that needs to be clarified.

Maybe there's a difference in topology in working and non-working gains?

@marc-hb marc-hb requested a review from plbossart March 15, 2023 00:01
the volume ctl name PGA is for IPC3, IPC4 uses keyword gain.
so rename the function to a more generic name to cover IPC4.

Signed-off-by: Keqiao Zhang <[email protected]>
No all IPC4 gain controls can set 0dB directly, maybe there're
some problems there. As the alternative, use 100% for IPC4 gain.
Will be unified after the problem is fixed.

Signed-off-by: Keqiao Zhang <[email protected]>
We have aligned that to use 0dB for all sof volume controls in CI test.

Signed-off-by: Keqiao Zhang <[email protected]>
@keqiaozhang
Copy link
Contributor Author

keqiaozhang commented Mar 15, 2023

Maybe there's a difference in topology in working and non-working gains?

Not sure about it. Since alsabat test occasionally failed on some IPC4 platforms. So for IPC4 platforms, let's set the gain volume to 100%. We can switch it back to 0dB after we fixed this issue.

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 15, 2023

Fixing this shellcheck warning is not trivial:
https://github.com/thesofproject/sof-test/actions/runs/4423370532/jobs/7756041716
https://www.shellcheck.net/wiki/SC2207

 ----- shellcheck -x test-case/volume-basic-test.sh ----


In test-case/volume-basic-test.sh line 61:
pgalist=($(amixer -c"$sofcard" controls | grep -i PGA | sed 's/ /_/g;' | awk -Fname= '{print $2}'))
         ^-- SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).

Here's one way how. Note PGA is now gain in IPC4 (thx @fredoh9 !) so I added it.

ubuntu@sh-mtlp-rvp-hda-03:~$ mapfile pgalist < <(amixer -c"$sofcard" controls |
         awk -Fname= '/PGA|gain/ { gsub(" ","_", $2 ); print $2}')

ubuntu@sh-mtlp-rvp-hda-03:~$ declare -p pgalist
declare -a pgalist=([0]=$'\'gain.1.1_1_2nd_Playback_Volume\'\n' [1]=$'\'gain.11.1_DMIC0_Capture_Volume_1\'\n' [2]=$'\'gain.15.1_Deepbuffer_Volume\'\n' [3]=$'\'gain.2.1_2_Main_Playback_Volume\'\n')

@keqiaozhang
Copy link
Contributor Author

Here's one way how. Note PGA is now gain in IPC4 (thx @fredoh9 !) so I added it.

volume-basic-test.sh only support IPC3 ATM, IPC4 is not supported.
In this PR, I only renamed the reset_PGA_volume to a generic name reset_sof_volume. Will add the volume test for IPC4 platforms later.

@keqiaozhang
Copy link
Contributor Author

CI Test passed on all platforms. Can we merge this PR? @marc-hb @singalsu @fredoh9
For more tests, Please refer to:#1012 (comment)

@marc-hb
Copy link
Collaborator

marc-hb commented Mar 21, 2023

@keqiaozhang
Copy link
Contributor Author

@keqiaozhang keqiaozhang merged commit ba2f6c5 into thesofproject:main Mar 28, 2023
@keqiaozhang keqiaozhang deleted the volume_reset branch March 28, 2023 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants