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

sof-essx8336: update HiFi.conf #246

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

junocomp
Copy link

@junocomp junocomp commented Dec 1, 2022

This enables the speakers from startup, detects the HDMI audio and enables the internal microphone. I have tested this on 3 devices with Debian now and it seems to work. Can other people test it too please. If it works, it will be worth pushing it upstream.

@junocomp junocomp mentioned this pull request Dec 4, 2022
@perexg
Copy link
Member

perexg commented Dec 7, 2022

I don't think that it's the right way to update this configuration. I assume that @plbossart and @mchehab tested the previous configs. It would be probably good to gather output from alsa-info.sh --no-upload script as first and do the changes step-by-step.

@perexg perexg changed the title Update HiFi.conf sof-essx8336: update HiFi.conf Dec 7, 2022
@perexg
Copy link
Member

perexg commented Dec 7, 2022

Also, do not forget that there are probably multiple hardware variants which are sharing the similar configuration file, so removing devices and/or modifying the configuration unconditionally without asking the original contributors will result with a regression.

@junocomp
Copy link
Author

junocomp commented Dec 7, 2022

@perexg

alsa-info.txt

@junocomp
Copy link
Author

@perexg did you check my alsa-info?

@junocomp
Copy link
Author

I submited this two weeks ago, I was wondering if someone could take a look at it.

Copy link
Member

@perexg perexg left a comment

Choose a reason for hiding this comment

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

I've tried to do an review from the UCM configuration maintainer perspective. I admit that my hardware knowledge is limited in this case.

@@ -1,71 +1,31 @@
SectionVerb {
EnableSequence [
disdevall ""
#disdevall ""
Copy link
Member

Choose a reason for hiding this comment

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

The Left / Right DAC switches are already set in the BootSequence (sof-essx8336.conf) file. Also, keep disdevall line uncommented. It will execute all DisableSequence commands for all defined UCM devices.

]
}

If.amic {
Copy link
Member

Choose a reason for hiding this comment

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

Do not move amic outside If blocks. According alsa-info dump, there are no Components defined for this card thus this analog mic block should be activated.

}
}

If.dmic {
Copy link
Member

Choose a reason for hiding this comment

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

Do not remove blindly devices which may be used for another hardware variants.

cset "name='Internal Mic Switch' on"
cset "name='Headset Mic Switch' on"
Copy link
Member

Choose a reason for hiding this comment

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

Headset mic device was defined bellow (and removed with your change). If you insert a microphone, which Jack control changes? You may use amixer -c 0 events command to check the control state changes.

@@ -77,10 +37,12 @@ SectionDevice."Speaker" {

EnableSequence [
cset "name='Speaker Switch' on"
cset "name='Differential Mux' lin1-rin1"
Copy link
Member

Choose a reason for hiding this comment

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

Changing controls which are evidently assigned to the input for the output device seems very suspicious. I would remove this addition.

EnableSequence [
cset "name='Headset Mic Switch' on"
cset "name='Speaker Switch' off"
cset "name='Differential Mux' lin2-rin2"
Copy link
Member

Choose a reason for hiding this comment

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

Changing controls which are evidently assigned to the input for the output device seems very suspicious. I would remove this addition. Also do not use controls which are already assigned to other devices (Speaker).

@@ -109,35 +81,4 @@ SectionDevice."Headphones" {
}
}

SectionDevice."Headset" {
Copy link
Member

Choose a reason for hiding this comment

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

Keep this section. If it does not make sense for your hardware, use If. blocks.

@perexg
Copy link
Member

perexg commented Dec 19, 2022

As I already wrote, I would like probably to see the step-by-step configuration changes e.g. what are minimal configuration changes to make Speaker device work on your hardware?

@mchehab
Copy link
Contributor

mchehab commented Dec 24, 2022

A large part of the issues here is due to wrong/missing quirks. Basically, 2 gpios are used to control speaker and headphone; each of them can be either triggered on low or on high. So, there are 4 different combinations, when both are used - and some systems may use just one gpio for both.

We're working towards improving its detection via ACPI device ESSX _DSM method (see thesofproject/linux#4112). After having those patches applied, there will be just one quirk that won't be auto-detected: if GPIO 0 is speaker or headphone.

Yet, UCM changeset 15a1250 ("ucm2: put disdevall to HDA/Soundwire configs") caused a regression, breaking support for the system's microphone. It should probably be partially reverted for es8336, until we figure out what caused the regressions.

I submitted a PR with such partial revert: #258

@mchehab
Copy link
Contributor

mchehab commented Dec 24, 2022

@perexg

alsa-info.txt

Btw, if you want to compare, this is the alsa-info.txt from Huawei Matebook D15:
huawei-matebook-d15-alsa-info.txt

@dgox16
Copy link

dgox16 commented Jan 2, 2023

This fixed the audio problem along with some adjustments in alsamixer, I have a fully functional speaker. A microphone is recognized now but when using it it only detects noise, it does not detect voice or external noises.

@junocomp
Copy link
Author

junocomp commented Jan 2, 2023

@dgox16 Did you try my commit?

@dgox16
Copy link

dgox16 commented Jan 2, 2023

@dgox16 Did you try my commit?

Yes. I did it in my Matebook d15
Only microphone not worked but is detected

@mchehab
Copy link
Contributor

mchehab commented Jan 2, 2023

@dgox16 Did you try my commit?

Yes. I did it in my Matebook d15 Only microphone not worked but is detected

It won't work, as the patch removed support for DMIC (which is used on Matebook). Try, instead, #258

@junocomp
Copy link
Author

Has anyone managed to get this to work?

@ghost
Copy link

ghost commented Jun 10, 2023

I have tested this configuration and the audio indeed works at max volume when the system starts up in a Positivo Master N1250 ROHS, the microphone on the other hand does not work.

@xry111
Copy link
Contributor

xry111 commented Jun 22, 2023

As I already wrote, I would like probably to see the step-by-step configuration changes e.g. what are minimal
configuration changes to make Speaker device work on your hardware?

On my system (a Hasee X5-2021S5H, shipping Tigerlake i5-11300H with ES8336) #328 is the minimal change I've found to make speaker, headphone, and internal digital mic to work. I've not tested headset (with a mic) because I don't have one. See the commit message for the rationale of the changes.

If we remove disdevall, I guess the first change in my PR won't be needed anymore.

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.

6 participants