linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] HDA: Fix digital microphone on CS420x
@ 2012-11-04  5:19 Daniel J Blueman
  2012-11-04  5:19 ` [PATCH 2/2] HDA: Mark CS260x immutable structures const Daniel J Blueman
  2012-11-04  8:16 ` [PATCH 1/2] HDA: Fix digital microphone on CS420x Takashi Iwai
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel J Blueman @ 2012-11-04  5:19 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Alexander Stein, Jaroslav Kysela, linux-kernel,
	Daniel J Blueman

Correctly enable the digital microphones with the right bits in the right coeffecient
registers on Cirrus CS4206/7 codecs. It also prevents misconfiguring ADC1/2.

This fixes the digital mic on the Macbook Pro 10,1/Retina.

Based-on-patch-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
 sound/pci/hda/patch_cirrus.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 61a7113..859a119 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -101,8 +101,8 @@ enum {
 #define CS420X_VENDOR_NID	0x11
 #define CS_DIG_OUT1_PIN_NID	0x10
 #define CS_DIG_OUT2_PIN_NID	0x15
-#define CS_DMIC1_PIN_NID	0x12
-#define CS_DMIC2_PIN_NID	0x0e
+#define CS_DMIC1_PIN_NID	0x0e
+#define CS_DMIC2_PIN_NID	0x12
 
 /* coef indices */
 #define IDX_SPDIF_STAT		0x0000
@@ -1079,14 +1079,18 @@ static void init_input(struct hda_codec *codec)
 			cs_automic(codec, NULL);
 
 		coef = 0x000a; /* ADC1/2 - Digital and Analog Soft Ramp */
+		cs_vendor_coef_set(codec, IDX_ADC_CFG, coef);
+
+		coef = cs_vendor_coef_get(codec, IDX_BEEP_CFG);
 		if (is_active_pin(codec, CS_DMIC2_PIN_NID))
-			coef |= 0x0500; /* DMIC2 2 chan on, GPIO1 off */
+			coef |= 1 << 4; /* DMIC2 2 chan on, GPIO1 off */
 		if (is_active_pin(codec, CS_DMIC1_PIN_NID))
-			coef |= 0x1800; /* DMIC1 2 chan on, GPIO0 off
+			coef |= 1 << 3; /* DMIC1 2 chan on, GPIO0 off
 					 * No effect if SPDIF_OUT2 is
 					 * selected in IDX_SPDIF_CTL.
 					*/
-		cs_vendor_coef_set(codec, IDX_ADC_CFG, coef);
+
+		cs_vendor_coef_set(codec, IDX_BEEP_CFG, coef);
 	} else {
 		if (spec->mic_detect)
 			cs_automic(codec, NULL);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] HDA: Mark CS260x immutable structures const
  2012-11-04  5:19 [PATCH 1/2] HDA: Fix digital microphone on CS420x Daniel J Blueman
@ 2012-11-04  5:19 ` Daniel J Blueman
  2012-11-04  8:16 ` [PATCH 1/2] HDA: Fix digital microphone on CS420x Takashi Iwai
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel J Blueman @ 2012-11-04  5:19 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Alexander Stein, Jaroslav Kysela, linux-kernel,
	Daniel J Blueman

Mark structures that won't change const.

Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
 sound/pci/hda/patch_cirrus.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 859a119..d5f3a26 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1732,8 +1732,7 @@ static int cs421x_mux_enum_put(struct snd_kcontrol *kcontrol,
 
 }
 
-static struct snd_kcontrol_new cs421x_capture_source = {
-
+static const struct snd_kcontrol_new cs421x_capture_source = {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "Capture Source",
 	.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
@@ -1950,7 +1949,7 @@ static int cs421x_suspend(struct hda_codec *codec)
 }
 #endif
 
-static struct hda_codec_ops cs421x_patch_ops = {
+static const struct hda_codec_ops cs421x_patch_ops = {
 	.build_controls = cs421x_build_controls,
 	.build_pcms = cs_build_pcms,
 	.init = cs421x_init,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] HDA: Fix digital microphone on CS420x
  2012-11-04  5:19 [PATCH 1/2] HDA: Fix digital microphone on CS420x Daniel J Blueman
  2012-11-04  5:19 ` [PATCH 2/2] HDA: Mark CS260x immutable structures const Daniel J Blueman
@ 2012-11-04  8:16 ` Takashi Iwai
       [not found]   ` <CAMVG2sv2dD=WXAiJnpEO7e-pnJ5gOnCn0JK5P+B_SWc4Wqg36w@mail.gmail.com>
  1 sibling, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2012-11-04  8:16 UTC (permalink / raw)
  To: Daniel J Blueman
  Cc: alsa-devel, Alexander Stein, Jaroslav Kysela, linux-kernel

At Sun,  4 Nov 2012 13:19:03 +0800,
Daniel J Blueman wrote:
> 
> Correctly enable the digital microphones with the right bits in the right coeffecient
> registers on Cirrus CS4206/7 codecs. It also prevents misconfiguring ADC1/2.
> 
> This fixes the digital mic on the Macbook Pro 10,1/Retina.
> 
> Based-on-patch-by: Alexander Stein <alexander.stein@systec-electronic.com>
> Signed-off-by: Daniel J Blueman <daniel@quora.org>

Thanks, this looks more comprehensive.  DIG1 and DIG2 seem to have
been set wrongly in the original code, based on the Cirrus's example
code.  Is the right-only recording problem fixed by this patch?

Alexander, could you check this doesn't break your machine?


Takashi

> ---
>  sound/pci/hda/patch_cirrus.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 61a7113..859a119 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -101,8 +101,8 @@ enum {
>  #define CS420X_VENDOR_NID	0x11
>  #define CS_DIG_OUT1_PIN_NID	0x10
>  #define CS_DIG_OUT2_PIN_NID	0x15
> -#define CS_DMIC1_PIN_NID	0x12
> -#define CS_DMIC2_PIN_NID	0x0e
> +#define CS_DMIC1_PIN_NID	0x0e
> +#define CS_DMIC2_PIN_NID	0x12
>  
>  /* coef indices */
>  #define IDX_SPDIF_STAT		0x0000
> @@ -1079,14 +1079,18 @@ static void init_input(struct hda_codec *codec)
>  			cs_automic(codec, NULL);
>  
>  		coef = 0x000a; /* ADC1/2 - Digital and Analog Soft Ramp */
> +		cs_vendor_coef_set(codec, IDX_ADC_CFG, coef);
> +
> +		coef = cs_vendor_coef_get(codec, IDX_BEEP_CFG);
>  		if (is_active_pin(codec, CS_DMIC2_PIN_NID))
> -			coef |= 0x0500; /* DMIC2 2 chan on, GPIO1 off */
> +			coef |= 1 << 4; /* DMIC2 2 chan on, GPIO1 off */
>  		if (is_active_pin(codec, CS_DMIC1_PIN_NID))
> -			coef |= 0x1800; /* DMIC1 2 chan on, GPIO0 off
> +			coef |= 1 << 3; /* DMIC1 2 chan on, GPIO0 off
>  					 * No effect if SPDIF_OUT2 is
>  					 * selected in IDX_SPDIF_CTL.
>  					*/
> -		cs_vendor_coef_set(codec, IDX_ADC_CFG, coef);
> +
> +		cs_vendor_coef_set(codec, IDX_BEEP_CFG, coef);
>  	} else {
>  		if (spec->mic_detect)
>  			cs_automic(codec, NULL);
> -- 
> 1.7.10.4
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] HDA: Fix digital microphone on CS420x
       [not found]   ` <CAMVG2sv2dD=WXAiJnpEO7e-pnJ5gOnCn0JK5P+B_SWc4Wqg36w@mail.gmail.com>
@ 2012-11-05  8:59     ` Alexander Stein
  2012-11-05 11:38     ` Takashi Iwai
  1 sibling, 0 replies; 5+ messages in thread
From: Alexander Stein @ 2012-11-05  8:59 UTC (permalink / raw)
  To: Daniel J Blueman; +Cc: Takashi Iwai, alsa-devel, Jaroslav Kysela, Linux Kernel

Hello,

On Monday 05 November 2012 07:22:24, Daniel J Blueman wrote:
> On 4 November 2012 16:16, Takashi Iwai <tiwai@suse.de> wrote:
> > Thanks, this looks more comprehensive.  DIG1 and DIG2 seem to have
> > been set wrongly in the original code, based on the Cirrus's example
> > code.  Is the right-only recording problem fixed by this patch?
> >
> 
> It is fixed. In the mbp101 init verbs, setting the vendor widget ADC
> configuration bit 12 to copy the ADC2 right channel to the left (0x1004 to
> nid 0x11 coef 2) is superfluous here, so presumably for the analogue mic
> in. I'll get time later to check all this.
> 
> The second coefficient index initialisation (0x000f to nid 0x11 coef 4) is
> now redundant, as we enable the mic bits later correctly now. I'll send an
> updated patch which drops this.
> 
> Alexander, could you check this doesn't break your machine?

I replaced my 2nd patch "hda: Cirrus: Fix wrong ADC config" with Daniel's 
patch "HDA: Fix digital microphone on CS420x" and I still get stereo in both 
ADC channels I'm using. So, no breakage, but I can't comment on the digital 
mic patch, because I'm not using it.

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
August-Bebel-Str. 29
D-07973 Greiz

Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
eMail:    Alexander.Stein@systec-electronic.com
Internet: http://www.systec-electronic.com

Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Jena, HRB 205563

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] HDA: Fix digital microphone on CS420x
       [not found]   ` <CAMVG2sv2dD=WXAiJnpEO7e-pnJ5gOnCn0JK5P+B_SWc4Wqg36w@mail.gmail.com>
  2012-11-05  8:59     ` Alexander Stein
@ 2012-11-05 11:38     ` Takashi Iwai
  1 sibling, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2012-11-05 11:38 UTC (permalink / raw)
  To: Daniel J Blueman
  Cc: alsa-devel, Alexander Stein, Jaroslav Kysela, Linux Kernel

At Mon, 5 Nov 2012 07:22:24 +0800,
Daniel J Blueman wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> On 4 November 2012 16:16, Takashi Iwai <tiwai@suse.de> wrote:
> 
> > At Sun,  4 Nov 2012 13:19:03 +0800,
> > Daniel J Blueman wrote:
> > >
> > > Correctly enable the digital microphones with the right bits in the
> > right coeffecient
> > > registers on Cirrus CS4206/7 codecs. It also prevents misconfiguring
> > ADC1/2.
> > >
> > > This fixes the digital mic on the Macbook Pro 10,1/Retina.
> > >
> > > Based-on-patch-by: Alexander Stein <
> > alexander.stein@systec-electronic.com>
> > > Signed-off-by: Daniel J Blueman <daniel@quora.org>
> >
> > Thanks, this looks more comprehensive.  DIG1 and DIG2 seem to have
> > been set wrongly in the original code, based on the Cirrus's example
> > code.  Is the right-only recording problem fixed by this patch?
> >
> 
> It is fixed. In the mbp101 init verbs, setting the vendor widget ADC
> configuration bit 12 to copy the ADC2 right channel to the left (0x1004 to
> nid 0x11 coef 2) is superfluous here, so presumably for the analogue mic
> in. I'll get time later to check all this.
> 
> The second coefficient index initialisation (0x000f to nid 0x11 coef 4) is
> now redundant, as we enable the mic bits later correctly now. I'll send an
> updated patch which drops this.

OK, I applied fix patches now to sound git tree for-linus branch,
queued for 3.7-rc5.

Feel free to work on further clean ups.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-11-05 11:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-04  5:19 [PATCH 1/2] HDA: Fix digital microphone on CS420x Daniel J Blueman
2012-11-04  5:19 ` [PATCH 2/2] HDA: Mark CS260x immutable structures const Daniel J Blueman
2012-11-04  8:16 ` [PATCH 1/2] HDA: Fix digital microphone on CS420x Takashi Iwai
     [not found]   ` <CAMVG2sv2dD=WXAiJnpEO7e-pnJ5gOnCn0JK5P+B_SWc4Wqg36w@mail.gmail.com>
2012-11-05  8:59     ` Alexander Stein
2012-11-05 11:38     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).