* [PATCH] ALSA: patch_realtek: handle unset external amp bits
@ 2010-11-25 18:57 Kyle McMartin
2010-11-26 5:51 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Kyle McMartin @ 2010-11-25 18:57 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, linux-kernel
From: Kyle McMartin <kyle@redhat.com>
Commit ef5dbbcc removes the default initialization of init_amp to
ALC_INIT_DEFAULT, with the justification that it should be set by
alc_subsystem_id, which contains a switch case:
tmp = (ass & 0x38) >> 3; /* external Amp control */
switch (tmp) {
case 1:
...
case 5:
}
Which handles values 1,3,7 for GPIOs, or 5 to set _DEFAULT.
But... what if it's 0? :)
I got a bug report this morning from a user whose sound had broken
between 2.6.34 and 2.6.35, and I bisected it down to this commit.
[ 32.366183] ALSA sound/pci/hda/patch_realtek.c:1374: realtek: Enabling
init ASM_ID=0x8a05 CODEC_ID=10ec0888
(0x8a05 & 0x38) >> 3 = 0;
But otherwise the SKU looks fine (since if I revert that commit, it
seems to work correctly.)
I think the best thing to do would be to either restore the 'redundant'
default initialization, or quirk it.
https://bugzilla.redhat.com/show_bug.cgi?id=657388
Signed-off-by: Kyle McMartin <kyle@redhat.com>
---
Revert "ALSA: hda - Remove superfluous external amp setup for ALC888"
This reverts commit ef5dbbccbbfa7d2211fa8efcc095a9f4a7912dda.
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 76c4968..b6ba1d5 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -10604,6 +10604,9 @@ static int patch_alc882(struct hda_codec *codec)
spec->stream_digital_playback = &alc882_pcm_digital_playback;
spec->stream_digital_capture = &alc882_pcm_digital_capture;
+ if (codec->vendor_id == 0x10ec0888)
+ spec->init_amp = ALC_INIT_DEFAULT; /* always initialize */
+
if (!spec->adc_nids && spec->input_mux) {
int i, j;
spec->num_adc_nids = 0;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: patch_realtek: handle unset external amp bits
2010-11-25 18:57 [PATCH] ALSA: patch_realtek: handle unset external amp bits Kyle McMartin
@ 2010-11-26 5:51 ` Takashi Iwai
2010-11-26 13:29 ` Kyle McMartin
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2010-11-26 5:51 UTC (permalink / raw)
To: Kyle McMartin; +Cc: alsa-devel, linux-kernel
At Thu, 25 Nov 2010 13:57:30 -0500,
Kyle McMartin wrote:
>
> From: Kyle McMartin <kyle@redhat.com>
>
> Commit ef5dbbcc removes the default initialization of init_amp to
> ALC_INIT_DEFAULT, with the justification that it should be set by
> alc_subsystem_id, which contains a switch case:
>
> tmp = (ass & 0x38) >> 3; /* external Amp control */
> switch (tmp) {
> case 1:
> ...
> case 5:
> }
>
> Which handles values 1,3,7 for GPIOs, or 5 to set _DEFAULT.
>
> But... what if it's 0? :)
>
> I got a bug report this morning from a user whose sound had broken
> between 2.6.34 and 2.6.35, and I bisected it down to this commit.
>
> [ 32.366183] ALSA sound/pci/hda/patch_realtek.c:1374: realtek: Enabling
> init ASM_ID=0x8a05 CODEC_ID=10ec0888
>
> (0x8a05 & 0x38) >> 3 = 0;
>
> But otherwise the SKU looks fine (since if I revert that commit, it
> seems to work correctly.)
>
> I think the best thing to do would be to either restore the 'redundant'
> default initialization, or quirk it.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=657388
>
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
How about setting default to ALC_INIT_DEFAULT instead like the patch
below? This is safer than forcing ALC_INIT_DEFAULT to all, I
suppose.
thanks,
Takashi
---
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 81a2a49..886d7c7 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1614,6 +1614,7 @@ do_sku:
spec->init_amp = ALC_INIT_GPIO3;
break;
case 5:
+ default:
spec->init_amp = ALC_INIT_DEFAULT;
break;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: patch_realtek: handle unset external amp bits
2010-11-26 5:51 ` Takashi Iwai
@ 2010-11-26 13:29 ` Kyle McMartin
2010-11-26 14:04 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Kyle McMartin @ 2010-11-26 13:29 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Kyle McMartin, alsa-devel, linux-kernel
On Fri, Nov 26, 2010 at 06:51:11AM +0100, Takashi Iwai wrote:
> How about setting default to ALC_INIT_DEFAULT instead like the patch
> below? This is safer than forcing ALC_INIT_DEFAULT to all, I
> suppose.
>
As I understand the code, that should work fine. I'll build a new kernel
for my tester to confirm, I suspect it will though. :)
Thanks!
--Kyle
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: patch_realtek: handle unset external amp bits
2010-11-26 13:29 ` Kyle McMartin
@ 2010-11-26 14:04 ` Takashi Iwai
2010-11-26 16:07 ` Kyle McMartin
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2010-11-26 14:04 UTC (permalink / raw)
To: Kyle McMartin; +Cc: alsa-devel, linux-kernel
At Fri, 26 Nov 2010 08:29:22 -0500,
Kyle McMartin wrote:
>
> On Fri, Nov 26, 2010 at 06:51:11AM +0100, Takashi Iwai wrote:
> > How about setting default to ALC_INIT_DEFAULT instead like the patch
> > below? This is safer than forcing ALC_INIT_DEFAULT to all, I
> > suppose.
> >
>
> As I understand the code, that should work fine. I'll build a new kernel
> for my tester to confirm, I suspect it will though. :)
Thanks!
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: patch_realtek: handle unset external amp bits
2010-11-26 14:04 ` Takashi Iwai
@ 2010-11-26 16:07 ` Kyle McMartin
2010-11-26 16:18 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Kyle McMartin @ 2010-11-26 16:07 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Kyle McMartin, alsa-devel, linux-kernel
On Fri, Nov 26, 2010 at 03:04:13PM +0100, Takashi Iwai wrote:
> > As I understand the code, that should work fine. I'll build a new kernel
> > for my tester to confirm, I suspect it will though. :)
>
> Thanks!
>
Tester confirms audio works after the patch. Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: patch_realtek: handle unset external amp bits
2010-11-26 16:07 ` Kyle McMartin
@ 2010-11-26 16:18 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2010-11-26 16:18 UTC (permalink / raw)
To: Kyle McMartin; +Cc: alsa-devel, linux-kernel
At Fri, 26 Nov 2010 11:07:14 -0500,
Kyle McMartin wrote:
>
> On Fri, Nov 26, 2010 at 03:04:13PM +0100, Takashi Iwai wrote:
> > > As I understand the code, that should work fine. I'll build a new kernel
> > > for my tester to confirm, I suspect it will though. :)
> >
> > Thanks!
> >
>
> Tester confirms audio works after the patch. Thanks!
Thanks, now applied it.
I'm going to send a pull request to Linus tomorrow.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-26 16:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-25 18:57 [PATCH] ALSA: patch_realtek: handle unset external amp bits Kyle McMartin
2010-11-26 5:51 ` Takashi Iwai
2010-11-26 13:29 ` Kyle McMartin
2010-11-26 14:04 ` Takashi Iwai
2010-11-26 16:07 ` Kyle McMartin
2010-11-26 16:18 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.