Message ID | 7dcc5148-dd01-ddbb-516b-51ffd4f1c31a@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> Some questions for you: > > 1) I think enabling micbias2 may be a problem on devices with a DMIC, > could this be a problem? Currently the codec driver itself contains > quirks for dmic pin-mapping, maybe we should leave micbias2 disabled > if dmic pin-mapping is specified through a quirk? No, micbias2 and DMIC are independent and micbias2 will not be enabled if we don't add it in the audio route. So I don't think we need to disable it if dmic pin-mapping is specified. > > 2) Do headset mics normally need a bias current? Could this be headset > dependent? Yes, but we don't need to set specific register for that. It means you don't need to add "micbias1" in the audio route for headset mic. I think all headset mic will need a bias current. > > Regards, > > Hans >
Hi, On 29-12-17 03:10, Bard Liao wrote: >> Some questions for you: >> >> 1) I think enabling micbias2 may be a problem on devices with a DMIC, >> could this be a problem? Currently the codec driver itself contains >> quirks for dmic pin-mapping, maybe we should leave micbias2 disabled >> if dmic pin-mapping is specified through a quirk? > > No, micbias2 and DMIC are independent and micbias2 will not be enabled > if we don't add it in the audio route. So I don't think we need to disable it > if dmic pin-mapping is specified. Right, but currently we use the "Int Mic" switch in both the DMIC and analog mic paths, and the changes to the machine driver enable micbias2 when Int Mic gets turned on. After my changes the machine driver has: {"IN2P", NULL, "Int Mic"}, {"IN2N", NULL, "Int Mic"}, {"DMIC L1", NULL, "Int Mic"}, {"DMIC R1", NULL, "Int Mic"}, ... {"Int Mic", NULL, "micbias2"}, Since the analog mic is currently not working anyways, maybe we should use Int Mic2 for the analog mic, so make the above: {"IN2P", NULL, "Int Mic2"}, {"IN2N", NULL, "Int Mic2"}, {"DMIC L1", NULL, "Int Mic"}, {"DMIC R1", NULL, "Int Mic"}, ... {"Int Mic2", NULL, "micbias2"}, And then use cset "name='Int Mic Switch2' on" In the ucm file in the analog mic enable sequence? This way the already working dmic support stays unchanged and the analog mic support I'm adding does not enable micbias2 when using dmic inputs. ? Regards, Hans
> -----Original Message----- > From: Hans de Goede [mailto:hdegoede@redhat.com] > Sent: Monday, January 01, 2018 8:32 PM > To: Bard Liao > Cc: Pierre-Louis Bossart; alsa-devel@alsa-project.org > Subject: Re: Cherry Trail + RT5645 devices analog microphone not working. > > Hi, > > On 29-12-17 03:10, Bard Liao wrote: > >> Some questions for you: > >> > >> 1) I think enabling micbias2 may be a problem on devices with a DMIC, > >> could this be a problem? Currently the codec driver itself contains > >> quirks for dmic pin-mapping, maybe we should leave micbias2 disabled > >> if dmic pin-mapping is specified through a quirk? > > > > No, micbias2 and DMIC are independent and micbias2 will not be enabled > > if we don't add it in the audio route. So I don't think we need to disable it > > if dmic pin-mapping is specified. > > Right, but currently we use the "Int Mic" switch in both the DMIC and analog > mic paths, and the changes to the machine driver enable micbias2 when Int > Mic > gets turned on. After my changes the machine driver has: > > {"IN2P", NULL, "Int Mic"}, > {"IN2N", NULL, "Int Mic"}, > {"DMIC L1", NULL, "Int Mic"}, > {"DMIC R1", NULL, "Int Mic"}, > ... > {"Int Mic", NULL, "micbias2"}, > > Since the analog mic is currently not working anyways, maybe we should > use Int Mic2 for the analog mic, so make the above: Do you mean the analog mic doesn't work with above change? Could you dump registers for me? > > {"IN2P", NULL, "Int Mic2"}, > {"IN2N", NULL, "Int Mic2"}, > {"DMIC L1", NULL, "Int Mic"}, > {"DMIC R1", NULL, "Int Mic"}, > ... > {"Int Mic2", NULL, "micbias2"}, > > And then use > > cset "name='Int Mic Switch2' on" It will be cset "name='Int Mic2 Switch' on" And you will also need to add "Int Mic2" in cht_mc_controls[]. > > In the ucm file in the analog mic enable sequence? > > This way the already working dmic support stays unchanged and the > analog mic support I'm adding does not enable micbias2 when using > dmic inputs. If you are using dmic inputs, you will need to set below switches on "Sto1 ADC MIXL ADC2 Switch" "Sto1 ADC MIXR ADC2 Switch" And below switches are for analog mics "Sto1 ADC MIXL ADC1 Switch" "Sto1 ADC MIXR ADC1 Switch" So you can select dmic or amic by rt5645's mixer. But, anyway it's a good idea to add a new widget for analog mic. However, it is better to have a more specific name for it. Something like "Int A Mic" and "Int D Mic". > > ? > > Regards, > > Hans > > ------Please consider the environment before printing this e-mail.
Hi Bard, On 02-01-18 03:41, Bard Liao wrote: >> -----Original Message----- >> From: Hans de Goede [mailto:hdegoede@redhat.com] >> Sent: Monday, January 01, 2018 8:32 PM >> To: Bard Liao >> Cc: Pierre-Louis Bossart; alsa-devel@alsa-project.org >> Subject: Re: Cherry Trail + RT5645 devices analog microphone not working. >> >> Hi, >> >> On 29-12-17 03:10, Bard Liao wrote: >>>> Some questions for you: >>>> >>>> 1) I think enabling micbias2 may be a problem on devices with a DMIC, >>>> could this be a problem? Currently the codec driver itself contains >>>> quirks for dmic pin-mapping, maybe we should leave micbias2 disabled >>>> if dmic pin-mapping is specified through a quirk? >>> >>> No, micbias2 and DMIC are independent and micbias2 will not be enabled >>> if we don't add it in the audio route. So I don't think we need to disable it >>> if dmic pin-mapping is specified. >> >> Right, but currently we use the "Int Mic" switch in both the DMIC and analog >> mic paths, and the changes to the machine driver enable micbias2 when Int >> Mic >> gets turned on. After my changes the machine driver has: >> >> {"IN2P", NULL, "Int Mic"}, >> {"IN2N", NULL, "Int Mic"}, >> {"DMIC L1", NULL, "Int Mic"}, >> {"DMIC R1", NULL, "Int Mic"}, >> ... >> {"Int Mic", NULL, "micbias2"}, >> >> Since the analog mic is currently not working anyways, maybe we should >> use Int Mic2 for the analog mic, so make the above: > > Do you mean the analog mic doesn't work with above change? It does work with the above changes, what I mean is that it does not work without the patch series which I've just send out, sorry for the confusion. What I was trying to say is that using a new dapm widget with a new name for the analog mic will NOT break existing ucm files since on older kernels, without the latest patches it will not work because of the lacking patches. > Could you dump registers for me? No need, everything works now, also see the series I just send out thank you for all your help! >> {"IN2P", NULL, "Int Mic2"}, >> {"IN2N", NULL, "Int Mic2"}, >> {"DMIC L1", NULL, "Int Mic"}, >> {"DMIC R1", NULL, "Int Mic"}, >> ... >> {"Int Mic2", NULL, "micbias2"}, >> >> And then use >> >> cset "name='Int Mic Switch2' on" > > It will be cset "name='Int Mic2 Switch' on" > And you will also need to add "Int Mic2" in cht_mc_controls[]. > >> >> In the ucm file in the analog mic enable sequence? >> >> This way the already working dmic support stays unchanged and the >> analog mic support I'm adding does not enable micbias2 when using >> dmic inputs. > > If you are using dmic inputs, you will need to set below switches on > "Sto1 ADC MIXL ADC2 Switch" > "Sto1 ADC MIXR ADC2 Switch" > And below switches are for analog mics > "Sto1 ADC MIXL ADC1 Switch" > "Sto1 ADC MIXR ADC1 Switch" > So you can select dmic or amic by rt5645's mixer. > But, anyway it's a good idea to add a new widget for analog mic. Ok, I've just send out a patch series with your fixes to the codec driver + a patch to sound/soc/intel/boards/cht_bsw_rt5645.c adding a new "Int Analog Mic" DAPM widget for this. Regards, Hans
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -126,6 +126,8 @@ static const struct snd_soc_dapm_widget cht_dapm_widgets[] = static const struct snd_soc_dapm_route cht_rt5645_audio_map[] = { {"IN1P", NULL, "Headset Mic"}, {"IN1N", NULL, "Headset Mic"}, + {"IN2P", NULL, "Int Mic"}, + {"IN2N", NULL, "Int Mic"}, {"DMIC L1", NULL, "Int Mic"}, {"DMIC R1", NULL, "Int Mic"}, {"Headphone", NULL, "HPOL"}, @@ -135,6 +137,7 @@ static const struct snd_soc_dapm_route cht_rt5645_audio_map[ {"Headphone", NULL, "Platform Clock"}, {"Headset Mic", NULL, "Platform Clock"}, {"Int Mic", NULL, "Platform Clock"}, + {"Int Mic", NULL, "micbias2"}, {"Ext Spk", NULL, "Platform Clock"}, };