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

X13s volume fixes #335

Conversation

Srinivas-Kandagatla
Copy link
Contributor

This patchset fixes some of the Volume issues that are seen on X13s, most of the existing ucm was controlling Digital Gains instead of Analog Gains. With this patchset the volume on both speakers are much better and not saturated.

@@ -5,6 +5,6 @@ EnableSequence [
cset "name='WSA_RX1 INP0' RX1"
cset "name='WSA_COMP1 Switch' 1"
cset "name='WSA_COMP2 Switch' 1"
cset "name='WSA_RX0 Digital Volume' 68"
cset "name='WSA_RX1 Digital Volume' 68"
cset "name='WSA_RX0 Digital Volume' 84"
Copy link
Member

Choose a reason for hiding this comment

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

Won't be better to remove this settings from the EnableSequence and keep the initailization only in the BootSequence ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, will update the PR with suggested changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, branch is now pushed with suggested change.

@jhovold
Copy link
Contributor

jhovold commented Sep 12, 2023

@Srinivas-Kandagatla, your updated pull-request is not functionally equivalent to your first one and causes the speaker volume to again be set too low.

I tried adding back the volume csets to ucm2/codecs/qcom-lpass/wsa-macro/SpeakerEnableSeq.conf and that fixed the issue.

Please investigate and respin ASAP so we can get this fixed as audio is currently broken on the X13s without these changes merged.

@Srinivas-Kandagatla
Copy link
Contributor Author

@jhovold thanks for testing and reporting this, I think I found one issue here. We seems to be setting the same mixer WSA_RX0/WSA_RX1 Digital Volume in SpeakerDisableSeq.conf as well. So we will end up with low volume because of this. I have fixed this now, Will push to this PR with those changes.

Current setup used Digital Volume to control Headset Volume which is
pretty saturated after centain gain. Fix the Digital gain at 0dB and use
Analog gain to do the volume control.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Add combined analog volume controls for Speakers in the init conf

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Make use of Speakers volume control to control analog gain on WSA
Speakers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
cset "name='WSA_RX0 Digital Volume' 84"
cset "name='WSA_RX1 Digital Volume' 84"
]

Copy link
Contributor

Choose a reason for hiding this comment

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

You now also set these in BootSequence in wsa_macro/init.conf, do you really need both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it should be removed.

cset "name='RX_RX0 Digital Volume' 80"
cset "name='RX_RX1 Digital Volume' 80"
cset "name='RX_RX0 Digital Volume' 84"
cset "name='RX_RX1 Digital Volume' 84"
cset "name='ADC2 Volume' 12"
Copy link
Contributor

Choose a reason for hiding this comment

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

This currently has no effect since you also set these to 80 in the EnableSequence. You need to drop the latter.

For some reason we ended up with a Digital gain below 0dB, resulting in
a very low speaker volume.
Fix this to 0dB and let Analog gain control speakers volume.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
@jhovold
Copy link
Contributor

jhovold commented Sep 29, 2023

The updated fixes seem to work as intended.

Note that with the addition of rx-macro/init.conf you now have a redundant initialisation of "RX_RX0 Digital Volume" in wcd938x/init.conf instead. Not sure if you want to respin this PR again or if you can just drop that as a follow-up clean up.

For some reason we ended up with a Digital gain below 0dB, resulting in
a very low HP volume.
Fix this to 0dB and let Analog gain control HP volume.
Also remove all redundant setting of this control.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
@jhovold
Copy link
Contributor

jhovold commented Sep 29, 2023

Still works after dropping the redundant initialisation so these should be good to go:

Reviewed-by: Johan Hovold johan+linaro@kernel.org
Tested-by: Johan Hovold johan+linaro@kernel.org

Note that users will need to clear their asound.state after updating the ucm configuration, and that after doing so headphone output is broken until a second reboot. So there still appears to be some bugs in these configuration files, but as I assume this issue was not introduced by this series that should be fixed separately.

@perexg, is it possible to get this merged and included in the next release?

@perexg perexg closed this in 8c3200a Oct 30, 2023
perexg pushed a commit that referenced this pull request Oct 30, 2023
Add combined analog volume controls for Speakers in the init conf

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Make use of Speakers volume control to control analog gain on WSA
Speakers.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low speaker volume.
Fix this to 0dB and let Analog gain control speakers volume.

CloseS: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low HP volume.
Fix this to 0dB and let Analog gain control HP volume.
Also remove all redundant setting of this control.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Current setup used Digital Volume to control Headset Volume which is
pretty saturated after centain gain. Fix the Digital gain at 0dB and use
Analog gain to do the volume control.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Add combined analog volume controls for Speakers in the init conf

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Make use of Speakers volume control to control analog gain on WSA
Speakers.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low speaker volume.
Fix this to 0dB and let Analog gain control speakers volume.

CloseS: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low HP volume.
Fix this to 0dB and let Analog gain control HP volume.
Also remove all redundant setting of this control.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Current setup used Digital Volume to control Headset Volume which is
pretty saturated after centain gain. Fix the Digital gain at 0dB and use
Analog gain to do the volume control.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Add combined analog volume controls for Speakers in the init conf

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
Make use of Speakers volume control to control analog gain on WSA
Speakers.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low speaker volume.
Fix this to 0dB and let Analog gain control speakers volume.

CloseS: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
perexg pushed a commit that referenced this pull request Oct 30, 2023
For some reason we ended up with a Digital gain below 0dB, resulting in
a very low HP volume.
Fix this to 0dB and let Analog gain control HP volume.
Also remove all redundant setting of this control.

Closes: #335
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
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.

3 participants