Re: [alsa-devel] Surround speaker connection on Acer 8951G

[Takashi Iwai <tiwai@suse.de>]

On Wed, 27 Nov 2019 17:17:57 +0100,
Sergey 'Jin' Bostandzhyan wrote:
> 
> Hi Takashi,
> 
> On Wed, Nov 27, 2019 at 12:28:59PM +0100, Takashi Iwai wrote:
> > > sorry - it's me again about the Acer 8951G LFE speaker.
> > > 
> > > On Fri, Aug 30, 2019 at 01:45:10PM +0200, Sergey 'Jin' Bostandzhyan wrote:
> > > > > > The below HDA_FIXUP_VERBS does the trick, so I do have all 6 speakers working, 
> > > > > > finally!
> > > > > > 
> > > > > > {0x01, AC_VERB_SET_GPIO_DIRECTION, 0x02}
> > > > > 
> > > > > Actually this must be paired with the corresponding bit of GPIO_DATA,
> > > > > too.  Is the bit 0x02 of GPIO_DATA set or cleared?  Usually setting it
> > > > > turns on the amp, but sometimes inverted.
> > > > 
> > > > If I understood everything correctly, then the bit is set, meaning that the
> > > > GPIO signal is configured as output. I'll be honest, I exported the
> > > > hda-analyzer setting as a python script (nice feature btw) and deducted the
> > > > fixup verb setting from there (relevant part of the hda-analyzer export below):
> > > > 
> > > > def set(nid, verb, param):
> > > >   verb = (nid << 24) | (verb << 8) | param
> > > >   res = ioctl(FD, IOCTL_VERB_WRITE, struct.pack('II', verb, 0))  
> > > > 
> > > > set(0x01, 0x717,   0x02) # 0x01071702 (SET_GPIO_DIRECTION)
> > > 
> > > it seems I indeed missed something here regarding GPIO_DATA, I really am
> > > not sure what the influence is, but after updating to Fedora 31 my LFE
> > > stopped working, even with the self compiled 5.4-rc8 kernel which I am running
> > > now (all the time before I was on Fedora 29 and I just backported my patch to 
> > > 5.2.x and compiled the modules outside the tree after being done with the 
> > > patch submission).
> > > 
> > > So ultimately, it seems I now need to do the following in my fixup
> > > (original commit was 00066e9733f629e536f6b7957de2ce11a85fe15a):
> > > 
> > > --- a/sound/pci/hda/patch_realtek.c
> > > +++ b/sound/pci/hda/patch_realtek.c
> > > @@ -8875,7 +8875,7 @@ static const struct hda_fixup alc662_fixups[] = {
> > >                 .v.verbs = (const struct hda_verb[]) {
> > >                         {0x01, AC_VERB_SET_GPIO_MASK, 0x02},
> > >                         {0x01, AC_VERB_SET_GPIO_DIRECTION, 0x02},
> > > -                       {0x01, AC_VERB_SET_GPIO_DATA, 0x00},
> > > +                       {0x01, AC_VERB_SET_GPIO_DATA, 0x02},
> > >                         { }
> > >                 },
> > >                 .chained = true,
> > 
> > That makes more sense.  Usually GPIO pin is off as default, and the
> > driver needs to turn it up manually for a special usage.
> > 
> > > My question is: could something on the outside have influence on that? I am
> > > really very, very sure that I have tested LFE on kernel 5.4-rc before 
> > > submitting the original patch and it has been working as submitted.
> > > Why did the behavior change now? What else could I have missed?
> > 
> > Maybe the chip kept the GPIO pin on after warm boot from Windows or
> > such?
> 
> This is unlikely as I do not have Windows or any other OS installed on this
> system. I dug through the thread and found the following:
> 
> > > > set(0x01, 0x717,   0x02) # 0x01071702 (SET_GPIO_DIRECTION)
> > >
> > > This needs the paired SET_GPIO_DATA for a proper operation.  Your
> > > analysis implicit assumes some default value that is already set by
> > > the hardware.
> >
> >If I understand it correctly, then "some value" is zero on my hardware:
> >
> > # hda-verb /dev/snd/hwC0D0 0x01 GET_GPIO_DATA 0x02
> > nid = 0x1, verb = 0xf15, param = 0x2
> > value = 0x0
> > 
> > Meanwhile I also figured out that /proc/asound/card0/codec#0 is
> > providing this info as well:
> >
> >  IO[1]: enable=0, dir=1, wake=0, sticky=0, data=0, unsol=0
> >
> > So the value seems to be 0 and I can add an explicit SET_GPIO_DATA verb quirk
> > to set it in addition to SET_GPIO_DIRECTION, right?
> 
> You then helped me, explaining how I could properly initialize it, which I 
> incorporated in the original patch.
> 
> So we did check that and I am positive that the LFE did work back then, which 
> really confuses me now.

Hm, I don't know what happened then.  In most cases, the value wasn't
correctly reflected when you actually checked (e.g. the runtime PM was
triggered and it was cleared by some reason or such...)

> > Please make sure that which value actually is on and which is off.
> > You can change the GPIO bit dynamically via hda-verb, so you can check
> > whether the speaker works or not at each flip.
> 
> OK, so the starting point (now with my local update to the driver):
> # hda-verb /dev/snd/hwC0D0 0x01 GET_GPIO_DATA 0x02
> nid = 0x1, verb = 0xf15, param = 0x2
> value = 0x2
> 
> >From /proc/asound/card0/codec#0:
> 
> State of AFG node 0x01:
>   Power states:  D0 D1 D2 D3 CLKSTOP
>   Power: setting=D0, actual=D0
> GPIO: io=2, o=0, i=0, unsolicited=1, wake=0
>   IO[0]: enable=0, dir=0, wake=0, sticky=0, data=0, unsol=0
>   IO[1]: enable=1, dir=1, wake=0, sticky=0, data=1, unsol=0
> 
> Pulse profile "Analog Surround 5.1 Output + Analog Stereo Input" is active,
> speaker test (via the pulse/sound applet UI) delives audible noise on the LFE.
> 
> I'm flipping data in hda-analyzer now and rechecking afterwards:
> 
> # hda-verb /dev/snd/hwC0D0 0x01 GET_GPIO_DATA 0x02
> nid = 0x1, verb = 0xf15, param = 0x2
> value = 0x0
> 
> And:
> State of AFG node 0x01:
>   Power states:  D0 D1 D2 D3 CLKSTOP
>   Power: setting=D0, actual=D0
> GPIO: io=2, o=0, i=0, unsolicited=1, wake=0
>   IO[0]: enable=0, dir=0, wake=0, sticky=0, data=0, unsol=0
>   IO[1]: enable=1, dir=1, wake=0, sticky=0, data=0, unsol=0
> 
> LFE is no longer audible in speaker test.
> 
> Reenabling again, this time I just used hda-verb directly:
> # hda-verb /dev/snd/hwC0D0 0x01 SET_GPIO_DATA 0x02
> nid = 0x1, verb = 0x715, param = 0x2
> value = 0x0
> 
> And checking:
> # hda-verb /dev/snd/hwC0D0 0x01 GET_GPIO_DATA 0x02
> nid = 0x1, verb = 0xf15, param = 0x2
> value = 0x2
> 
> LFE becomes audible again.

OK, so it's clear now that the bit on turns on the LFE amp.

> Now, if that would help, I could try to install Fedora 29 on some external
> harddrive and reproduce my summer setup, to confirm that it has been working 
> with data pin disabled. Alltough I am certain that it was the case, because
> I retested this several times prior to submitting the patch.
> 
> Question is, if we would learn something from that?
> 
> How should I proceed? Just submit an update to have the data pin active on
> init or is this weirdness worth debugging?

The patch like below should work, I suppose?


Takashi

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -8424,6 +8424,8 @@ static void alc662_fixup_aspire_ethos_hp(struct hda_codec *codec,
 	case HDA_FIXUP_ACT_PRE_PROBE:
 		snd_hda_jack_detect_enable_callback(codec, 0x1b,
 				alc662_aspire_ethos_mute_speakers);
+		/* subwoofer needs an extra GPIO setting to become audible */
+		alc_setup_gpio(codec, 0x02);
 		break;
 	case HDA_FIXUP_ACT_INIT:
 		/* Make sure to start in a correct state, i.e. if
@@ -8506,7 +8508,6 @@ enum {
 	ALC662_FIXUP_USI_HEADSET_MODE,
 	ALC662_FIXUP_LENOVO_MULTI_CODECS,
 	ALC669_FIXUP_ACER_ASPIRE_ETHOS,
-	ALC669_FIXUP_ACER_ASPIRE_ETHOS_SUBWOOFER,
 	ALC669_FIXUP_ACER_ASPIRE_ETHOS_HEADSET,
 };
 
@@ -8838,18 +8839,6 @@ static const struct hda_fixup alc662_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc662_fixup_aspire_ethos_hp,
 	},
-	[ALC669_FIXUP_ACER_ASPIRE_ETHOS_SUBWOOFER] = {
-		.type = HDA_FIXUP_VERBS,
-		/* subwoofer needs an extra GPIO setting to become audible */
-		.v.verbs = (const struct hda_verb[]) {
-			{0x01, AC_VERB_SET_GPIO_MASK, 0x02},
-			{0x01, AC_VERB_SET_GPIO_DIRECTION, 0x02},
-			{0x01, AC_VERB_SET_GPIO_DATA, 0x00},
-			{ }
-		},
-		.chained = true,
-		.chain_id = ALC669_FIXUP_ACER_ASPIRE_ETHOS_HEADSET
-	},
 	[ALC669_FIXUP_ACER_ASPIRE_ETHOS] = {
 		.type = HDA_FIXUP_PINS,
 		.v.pins = (const struct hda_pintbl[]) {
@@ -8859,7 +8848,7 @@ static const struct hda_fixup alc662_fixups[] = {
 			{ }
 		},
 		.chained = true,
-		.chain_id = ALC669_FIXUP_ACER_ASPIRE_ETHOS_SUBWOOFER
+		.chain_id = ALC669_FIXUP_ACER_ASPIRE_ETHOS_HEADSET
 	},
 };
 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel