pci/hda/patch_via.c - broken between 2.6.36 and 2.6.37 (bisected)

After the mentioned Kernel upgrade quite a view things went wrong on my hardware ( http://www.asrock.com/mb/overview.asp?Model=890GM%20Pro3 ):

* Microphone doesn't work anymore. neither the front mic nor the rear one. Tried all thinkable settings.

* Increasing Microphone levels leads to some strange internal feedback. That means, at some point when the feedback starts, it just starts, but i have no clue what audio source did actually trigger it.

* Touching the mixer channel "Smart 5.1" in alsamixer makes alsamicer crash.

After quite some days of frustration and the inability to get help i however managed to learn how to build a kernel and how to git bisect. So this is what i did:

root@Blackrock ~src/alsa-kernel (git)-[7b315bb...|bisect] # git bisect log git bisect start '--' 'sound/pci'
# bad: [81a2f603f94d6da108ddd8e3535294445097f730] ALSA: ac97: replace open- coded, error-prone stuff with AC97 bit defines
git bisect bad 81a2f603f94d6da108ddd8e3535294445097f730
# good: [f6f94e2ab1b33f0082ac22d71f66385a60d8157f] Linux 2.6.36 git bisect good f6f94e2ab1b33f0082ac22d71f66385a60d8157f
# bad: [28c4edb71d21623f1e47422194d865d2b6712fd4] ALSA: HDA: Rename "Int Mic" to "Internal Mic"
git bisect bad 28c4edb71d21623f1e47422194d865d2b6712fd4
# bad: [0e7adbe263f89ea2ef15b5af5e80a812b2a85025] ALSA: hda - Disable sticky PCM stream assignment for AD codecs
git bisect bad 0e7adbe263f89ea2ef15b5af5e80a812b2a85025
# bad: [5637edb2e1c2d13b276748508ae17f319fb7f066] ALSA: HDA: Sort CXT5066 quirk table
git bisect bad 5637edb2e1c2d13b276748508ae17f319fb7f066
# bad: [18675e4283f575594d55ef1239c14ab5b4de53b6] ALSA: hda - Add fixup for FSC Celsius H270
git bisect bad 18675e4283f575594d55ef1239c14ab5b4de53b6
# good: [eea7dc932bfa802ad0377755ea821f416f4f8623] ALSA: hda - Use new inputs[] field to parse input-pins for STAC/IDT codecs
git bisect good eea7dc932bfa802ad0377755ea821f416f4f8623
# bad: [263d0328c46995d8e4fb478005177839104483d2] ALSA: hda - Improve input control names for IDT/STAC codecs
git bisect bad 263d0328c46995d8e4fb478005177839104483d2
# bad: [9737731d64bfa64ab4fc04b46ae5b650f6432222] Merge branch 'fix/hda' into topic/hda
git bisect bad 9737731d64bfa64ab4fc04b46ae5b650f6432222
# bad: [7b315bb4980448250c80a7464c256b54d546cb26] ALSA: hda - Use new inputs[] field to parse input-pins for VIA codecs
git bisect bad 7b315bb4980448250c80a7464c256b54d546cb26

root@Blackrock ~src/alsa-kernel (git)-[7b315bb...|bisect] # git bisect bad 7b315bb4980448250c80a7464c256b54d546cb26 is the first bad commit commit 7b315bb4980448250c80a7464c256b54d546cb26
Author: Takashi Iwai <[email protected]>
Date:   Mon Aug 30 13:06:30 2010 +0200

   ALSA: hda - Use new inputs[] field to parse input-pins for VIA codecs

Signed-off-by Takashi Iwai <[email protected]>

:040000 040000 fb32bef12d78f90f00020cf375fddd36be1cdfbe 02f61291093ce6c7c8e7e1bd98fd87f9338702a1 M      sound

Oh, and here are debug infos from when i first tried to report the problem:
bad:
http://www.alsa-project.org/db/?f=0f29e6de98ea9fefae5c6e2adb43ff8f8eede697

good:
http://www.alsa-project.org/db/?f=11290fe50066e9bcd6a244f2c83b1c66c9dd201b

I hope someone can take it from here. I'm not much more than a user. Just because i managed to build a kernel doesn't mean i can fix this bug.

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec)

{
      struct via_spec *spec = codec->spec;
      const struct auto_pin_cfg *cfg = &spec->autocfg;
      unsigned int ctl;
      int i;

      for (i = 0; i < cfg->num_inputs; i++) {
              hda_nid_t nid = cfg->inputs[i].pin;
              if (spec->smart51_enabled && is_smart51_pins(spec, nid))
                      ctl = PIN_OUT;
              else if (i == AUTO_PIN_MIC)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
                      ctl = PIN_VREF50;
              else
                      ctl = PIN_IN;
              snd_hda_codec_write(codec, nid, 0,
                                  AC_VERB_SET_PIN_WIDGET_CONTROL, ctl);
      }
}

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i <= FRONT_MIC index).

2011/2/20 Mark Goldstein <[email protected]>

Quoted 25 lines

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec) {
      struct via_spec *spec = codec->spec;
      const struct auto_pin_cfg *cfg = &spec->autocfg; unsigned int ctl;
      int i;

      for (i = 0; i < cfg->num_inputs; i++) {
              hda_nid_t nid = cfg->inputs[i].pin;
              if (spec->smart51_enabled && is_smart51_pins(spec, nid)) ctl = PIN_OUT;
              else if (i == AUTO_PIN_MIC) ^^^^^^^^^^^^^^^^^^^^^^^^^
                      ctl = PIN_VREF50;
              else
                      ctl = PIN_IN; snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, ctl); }
}

Quoted 9 lines

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i <= FRONT_MIC index).

This may fix your vt1708s smart51 since only rear mic and line in are retasked

static int is_smart51_pins(struct via_spec *spec, hda_nid_t pin) {
   const struct auto_pin_cfg *cfg = &spec->autocfg;
   int i;
       if (cfg->line_outs != 1)
       return 0;
   for (i = 0; i < cfg->num_inputs; i++) {
       if (pin == cfg->inputs[i].pin) { switch(cfg->inputs[i].type){
           case AUTO_PIN_LINE_IN:
                   return 1;
                       case AUTO_PIN_MIC:
                               if ( strncmp(hda_get_autocfg_input_label(spec->codec,cfg,i),"Rear Mic",8) == 0 ) return 1;
               else
                   return 0;
           default:
               return 0;
                       }
               }
   }
   return 0;
}

http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=1564b2878f5cf160f60af99d4dbca1dd7809ee8a

Smart 5.1 is for 3-jacks model, to reuse input pins as outputs. While off, they act as "line out" / "line in" / "mic in". While on, they acts as "line out" / "back left/right" / "center/lfe".

Quoted 54 lines · Raymond Yau

2011/2/20 Mark Goldstein <[email protected]>

Quoted 25 lines

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec)

{
      struct via_spec *spec = codec->spec;
      const struct auto_pin_cfg *cfg = &spec->autocfg;
      unsigned int ctl;
      int i;

      for (i = 0; i < cfg->num_inputs; i++) {
              hda_nid_t nid = cfg->inputs[i].pin;
              if (spec->smart51_enabled && is_smart51_pins(spec, nid))
                      ctl = PIN_OUT;
              else if (i == AUTO_PIN_MIC)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
                      ctl = PIN_VREF50;
              else
                      ctl = PIN_IN;
              snd_hda_codec_write(codec, nid, 0,
                                  AC_VERB_SET_PIN_WIDGET_CONTROL, ctl);
      }
}

Quoted 9 lines

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i <= FRONT_MIC index).

This may fix your vt1708s smart51 since only rear mic and line in are retasked

static int is_smart51_pins(struct via_spec *spec, hda_nid_t pin)

{
   const struct auto_pin_cfg *cfg = &spec->autocfg;
   int i;
       if (cfg->line_outs != 1)
       return 0;
   for (i = 0; i < cfg->num_inputs; i++) {
       if (pin == cfg->inputs[i].pin) {
                       switch(cfg->inputs[i].type){
           case AUTO_PIN_LINE_IN:
                   return 1;
                       case AUTO_PIN_MIC:
                               if (

Quoted 1 line

strncmp(hda_get_autocfg_input_label(spec->codec,cfg,i),"Rear Mic",8) == 0

)

Quoted 7 lines

                          return 1;
               else
                   return 0;
           default:
               return 0;
                       }
               }

Quoted 5 lines

}
return 0;
}

http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=1564b2878f5cf160f60af99d4dbca1dd7809ee8a

Quoted 4 lines

Smart 5.1 is for 3-jacks model, to reuse input pins as outputs. While off, they act as "line out" / "line in" / "mic in". While on, they acts as "line out" / "back left/right" / "center/lfe".

Hi Raymond,
I've always had spart5.1 disabled and still Front Mic PIN has been configured as High Impedance and did not work until I changed the code as I described.

What I see in this function (is_smart51_pins) is that pin type is checked the same way I tried, that is cfg->inputs[i].type is compared to AUTO_PIN_MIC and not "i" itself as in the code of via_auto_init_analog_input.

So should the via_auto_init_analog_input be fixed? What do you think?

BTW for some reason I can't see the link included in your mail, I'm
getting: 403 Forbidden - No such project

Thank you & regards,
--
Mark Goldstein

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec)

{
        struct via_spec *spec = codec->spec;
        const struct auto_pin_cfg *cfg =&spec->autocfg;
        unsigned int ctl;
        int i;

        for (i = 0; i<  cfg->num_inputs; i++) {
                hda_nid_t nid = cfg->inputs[i].pin;
                if (spec->smart51_enabled&&  is_smart51_pins(spec, nid))
                        ctl = PIN_OUT;
                else if (i == AUTO_PIN_MIC)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^
                        ctl = PIN_VREF50;
                else
                        ctl = PIN_IN;
                snd_hda_codec_write(codec, nid, 0,
                                    AC_VERB_SET_PIN_WIDGET_CONTROL, ctl);
        }
}

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i<= FRONT_MIC index).

I'm also just the user of alsa driver, so can't decide whether this is the proper fix. Maybe someone of knowledgeable developers could check it.

Quoted 29 lines · Mark Goldstein

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec) {
       struct via_spec *spec = codec->spec;
       const struct auto_pin_cfg *cfg =&spec->autocfg; unsigned int ctl;
       int i;

       for (i = 0; i<  cfg->num_inputs; i++) {
               hda_nid_t nid = cfg->inputs[i].pin;
               if (spec->smart51_enabled&&  is_smart51_pins(spec, nid)) ctl = PIN_OUT;
               else if (i == AUTO_PIN_MIC) ^^^^^^^^^^^^^^^^^^^^^^^^^
                       ctl = PIN_VREF50;
               else
                       ctl = PIN_IN; snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, ctl); }
}

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

Quoted 8 lines

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i<= FRONT_MIC index).

I'm also just the user of alsa driver, so can't decide whether this is the proper fix. Maybe someone of knowledgeable developers could check it.

It looks like the proper fix to me. If you like the fame and fortune ;-) of having a commits in the kernel, I encourage you to submit a patch. Remember to have proper "Cc: [email protected]" and "Signed-off-by" lines in the the patch header, and to cc Takashi on the resulting email.

If not, I'll be happy to do it for you. It seems like an important fix to me.

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Quoted 44 lines · David Henningsson

Quoted 29 lines · Mark Goldstein

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec)

{
        struct via_spec *spec = codec->spec;
        const struct auto_pin_cfg *cfg =&spec->autocfg;
        unsigned int ctl;
        int i;

        for (i = 0; i<    cfg->num_inputs; i++) {
                hda_nid_t nid = cfg->inputs[i].pin;
                if (spec->smart51_enabled&&    is_smart51_pins(spec, nid))
                        ctl = PIN_OUT;
                else if (i == AUTO_PIN_MIC)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^
                        ctl = PIN_VREF50;
                else
                        ctl = PIN_IN;
                snd_hda_codec_write(codec, nid, 0,
                                    AC_VERB_SET_PIN_WIDGET_CONTROL, ctl);
        }
}

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

Quoted 8 lines

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i<= FRONT_MIC index).

I'm also just the user of alsa driver, so can't decide whether this is the proper fix. Maybe someone of knowledgeable developers could check it.

It looks like the proper fix to me. If you like the fame and fortune ;-) of having a commits in the kernel, I encourage you to submit a patch. Remember to have proper "Cc: [email protected]" and "Signed-off-by" lines in the the patch header, and to cc Takashi on the resulting email.

Quoted 7 lines

If not, I'll be happy to do it for you. It seems like an important fix to me.

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

David,

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Thank you,

From a28dd20815b47134bc342a6ec9b9da6e80f59837 Mon Sep 17 00:00:00 2001

Quoted 6 lines · Mark Goldstein

[...]

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Thank you,

Ok, here comes the patch!

Quoted 9 lines · David Henningsson

Quoted 6 lines · Mark Goldstein

[...]

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Thank you,

Ok, here comes the patch!

Tested, and Microphone now works! Thanks a lot to everyone involved. Good job so far!

On a side note:
One still must not touch "Smart 5.1" in alsamixer, because if one does, mixer will still crash and (sound)system will hang until pc is rebooted. I can't tell how related this two problems are, but as far as i can tell they both appeared with the same commit
( 7b315bb4980448250c80a7464c256b54d546cb26 ) i did track down on sunday.

Wolfgang Scheicher

Quoted 6 lines · David Henningsson

Quoted 5 lines · Mark Goldstein

[...]

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Quoted 3 lines

Quoted 1 line

Thank you,

Ok, here comes the patch!

Tested, and Microphone now works! Thanks a lot to everyone involved. Good job so far!

Quoted 12 lines · Wolfgang Scheicher

Quoted 9 lines · David Henningsson

Quoted 6 lines · Mark Goldstein

[...]

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Thank you,

Ok, here comes the patch!

Quoted 2 lines

Tested, and Microphone now works! Thanks a lot to everyone involved. Good job so far!

Hmm ... Just tried kernel 2.6.38 from debian sid, and Microphone is not working again. However, this fix is in.
What might have broken it this time?
I hope to find some time to track down the commit somewhen the next days ...

On Wed, Mar 23, 2011 at 12:41 PM, Wolfgang Scheicher

Quoted 5 lines

Hmm ... Just tried kernel 2.6.38 from debian sid, and Microphone is not working again. However, this fix is in.
What might have broken it this time?
I hope to find some time to track down the commit somewhen the next days
...

Wolfgang,

When I was fighting with Mic issue, I used hda-analyser script. Try it and check how the pin 0x1e (Front Mic) is configured.
My symptom was that this pin has been initiated with VREF Hi-Z. If you see that, try changing it to VREF 50. Mic should start working.

Quoted 54 lines · Mark Goldstein

Quoted 51 lines · David Henningsson

Quoted 29 lines · Mark Goldstein

I think the Mic part might be the same problem I had. I found out that the following code:

static void via_auto_init_analog_input(struct hda_codec *codec)

{
        struct via_spec *spec = codec->spec;
        const struct auto_pin_cfg *cfg =&spec->autocfg;
        unsigned int ctl;
        int i;

        for (i = 0; i<    cfg->num_inputs; i++) {
                hda_nid_t nid = cfg->inputs[i].pin;
                if (spec->smart51_enabled&&    is_smart51_pins(spec, nid))
                        ctl = PIN_OUT;
                else if (i == AUTO_PIN_MIC)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^
                        ctl = PIN_VREF50;
                else
                        ctl = PIN_IN;
                snd_hda_codec_write(codec, nid, 0,
                                    AC_VERB_SET_PIN_WIDGET_CONTROL, ctl);
        }
}

configures correctly only the first MIC (in my case it was Rear MIC). Front Mic pin was configured as HiZ.
Changing the underlined condition to

Quoted 8 lines

else if (cfg->input[i].type == AUTO_PIN_MIC)

resolved the issue for me. (In alsa driver 1.0.23 this code was different, it checked that i<= FRONT_MIC index).

I'm also just the user of alsa driver, so can't decide whether this is the proper fix. Maybe someone of knowledgeable developers could check it.

It looks like the proper fix to me. If you like the fame and fortune ;-) of having a commits in the kernel, I encourage you to submit a patch. Remember to have proper "Cc: [email protected]" and "Signed-off-by" lines in the the patch header, and to cc Takashi on the resulting email.

If not, I'll be happy to do it for you. It seems like an important fix to me.

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Quoted 6 lines

David,

I've never dealt with submitting patches, so probably it'll be better (and definitely faster) if you could do it.

Thank you,

Ok, here comes the patch!

Signature · 4 lines

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
[2 0001-ALSA-HDA-Fix-mic-initialization-in-VIA-auto-parser.patch <text/x-patch (7bit)>]

Quoted 1 line

From a28dd20815b47134bc342a6ec9b9da6e80f59837 Mon Sep 17 00:00:00 2001

From: David Henningsson <[email protected]>
Date: Mon, 21 Feb 2011 10:23:18 +0100
Subject: [PATCH] ALSA: HDA: Fix mic initialization in VIA auto parser

This typo caused some microphone inputs not to be correctly initialized on VIA codecs.

Reported-By Mark Goldstein <[email protected]>

Cc [email protected]

Signed-off-by David Henningsson <[email protected]>

---
 sound/pci/hda/patch_via.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

sound/pci/hda/patch_via.c +1 -1

index a76c326..63b0054 100644
@@ -567,7 +567,7 @@ static void via_auto_init_analog_input(struct hda_codec *codec)
 		hda_nid_t nid = cfg->inputs[i].pin;
 		if (spec->smart51_enabled && is_smart51_pins(spec, nid))
 			ctl = PIN_OUT;
-		else if (i == AUTO_PIN_MIC)
+		else if (cfg->inputs[i].type == AUTO_PIN_MIC)
 			ctl = PIN_VREF50;
 		else
 			ctl = PIN_IN;

Signature · 1 line

1.7.1

Quoted 11 lines

2011/2/20 Wolfgang Scheicher <[email protected]>

Quoted 9 lines

After the mentioned Kernel upgrade quite a view things went wrong on my hardware ( http://www.asrock.com/mb/overview.asp?Model=890GM%20Pro3 ):

* Microphone doesn't work anymore. neither the front mic nor the rear one. Tried all thinkable settings.

* Increasing Microphone levels leads to some strange internal feedback. That
means, at some point when the feedback starts, it just starts, but i

have

Quoted 7 lines

Quoted 5 lines

no
clue what audio source did actually trigger it.

* Touching the mixer channel "Smart 5.1" in alsamixer makes alsamicer crash.

Refer to the user manual , the motherboard has 5 audio jacks in rear

panel,

Quoted 9 lines

7.1 need retasking of Line in (blue jacks) as side

Audio Output Channels Front Speaker Rear Speaker Central / Bass Line In
(No. 9) (No. 6) (No. 5) (No. 8)
2 V -- -- --
4 V V -- --
6 V V V --
8 V V V V

Forgive me, but i don't understand.
I hope this doesn't mean i have to solder on my Mainboard if i intend to use a
newer kernel.

If there is something i can do, please explain in more detail.

--
Worf

Quoted 11 lines

2011/2/20 Wolfgang Scheicher <[email protected]>

Quoted 9 lines

After the mentioned Kernel upgrade quite a view things went wrong on my hardware ( http://www.asrock.com/mb/overview.asp?Model=890GM%20Pro3 ):

* Microphone doesn't work anymore. neither the front mic nor the rear one. Tried all thinkable settings.

* Increasing Microphone levels leads to some strange internal feedback. That
means, at some point when the feedback starts, it just starts, but i

have

Quoted 7 lines

Quoted 5 lines

no
clue what audio source did actually trigger it.

* Touching the mixer channel "Smart 5.1" in alsamixer makes alsamicer crash.

Refer to the user manual , the motherboard has 5 audio jacks in rear

panel,

Quoted 9 lines

7.1 need retasking of Line in (blue jacks) as side

Audio Output Channels Front Speaker Rear Speaker Central / Bass Line In
(No. 9) (No. 6) (No. 5) (No. 8)
2 V -- -- --
4 V V -- --
6 V V V --
8 V V V V

Forgive me, but i don't understand.
I hope this doesn't mean i have to solder on my Mainboard if i intend to use a
newer kernel.

If there is something i can do, please explain in more detail.

--
Worf