[alsa-devel] [PATCH] ALSA: patch_realtek: handle unset external amp bits
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;
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; }
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
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
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!
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
participants (2)
-
Kyle McMartin
-
Takashi Iwai