[PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[David Henningsson]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

I spend last Friday together with Bartłomiej Żogała fixing up a long 
standing issue with Lenovo Y530, which has 4+1 internal speakers on a 
Realtek 888. And we all want that supported by the auto parser, don't 
we? So here comes the patches. Would be nice to have in 2.6.38.


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ALSA-HDA-Enable-surround-and-subwoofer-on-Lenovo-Ide.patch --]
[-- Type: text/x-patch; name="0001-ALSA-HDA-Enable-surround-and-subwoofer-on-Lenovo-Ide.patch", Size: 1918 bytes --]

>From 869a246337ece559ea0792ae853ee75daf328f40 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Fri, 4 Mar 2011 13:37:50 +0100
Subject: [PATCH 1/4] ALSA: HDA: Enable surround and subwoofer on Lenovo Ideapad Y530
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The pin config values would change the association instead of the
sequence, this commit fixes that up.

Tested-by: Bartłomiej Żogała <nusch88@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_realtek.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index f721a18..ed4873c 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -10747,6 +10747,7 @@ static struct alc_config_preset alc882_presets[] = {
  */
 enum {
 	PINFIX_ABIT_AW9D_MAX,
+	PINFIX_LENOVO_Y530,
 	PINFIX_PB_M5210,
 	PINFIX_ACER_ASPIRE_7736,
 };
@@ -10761,6 +10762,14 @@ static const struct alc_fixup alc882_fixups[] = {
 			{ }
 		}
 	},
+	[PINFIX_LENOVO_Y530] = {
+		.type = ALC_FIXUP_PINS,
+		.v.pins = (const struct alc_pincfg[]) {
+			{ 0x15, 0x99130112 }, /* rear int speakers */
+			{ 0x16, 0x99130111 }, /* subwoofer */
+			{ }
+		}
+	},
 	[PINFIX_PB_M5210] = {
 		.type = ALC_FIXUP_VERBS,
 		.v.verbs = (const struct hda_verb[]) {
@@ -10776,6 +10785,7 @@ static const struct alc_fixup alc882_fixups[] = {
 
 static struct snd_pci_quirk alc882_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x1025, 0x0155, "Packard-Bell M5120", PINFIX_PB_M5210),
+	SND_PCI_QUIRK(0x17aa, 0x3a0d, "Lenovo Y530", PINFIX_LENOVO_Y530),
 	SND_PCI_QUIRK(0x147b, 0x107a, "Abit AW9D-MAX", PINFIX_ABIT_AW9D_MAX),
 	SND_PCI_QUIRK(0x1025, 0x0296, "Acer Aspire 7736z", PINFIX_ACER_ASPIRE_7736),
 	{}
-- 
1.7.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ALSA-HDA-Fix-volume-control-naming-for-surround-spea.patch --]
[-- Type: text/x-patch; name="0002-ALSA-HDA-Fix-volume-control-naming-for-surround-spea.patch", Size: 1103 bytes --]

>From 36a9f29b1de6a101dfd2d083b619cc55fbcf38ae Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Fri, 4 Mar 2011 14:08:30 +0100
Subject: [PATCH 2/4] ALSA: HDA: Fix volume control naming for surround speakers on Realtek auto-parser

When more than one pair of internal speakers is present, allow names
according to their channels.

Tested-by: Bartłomiej Żogała <nusch88@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_realtek.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index ed4873c..fe45c61 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5150,7 +5150,9 @@ static const char *alc_get_line_out_pfx(const struct auto_pin_cfg *cfg,
 
 	switch (cfg->line_out_type) {
 	case AUTO_PIN_SPEAKER_OUT:
-		return "Speaker";
+		if (cfg->line_outs == 1)
+			return "Speaker";
+		break;
 	case AUTO_PIN_HP_OUT:
 		return "Headphone";
 	default:
-- 
1.7.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-ALSA-HDA-Fixup-unnecessary-volume-control-index-on-R.patch --]
[-- Type: text/x-patch; name="0003-ALSA-HDA-Fixup-unnecessary-volume-control-index-on-R.patch", Size: 1485 bytes --]

>From 59ffe41f2ea198787be49996e36c43f32cc794a7 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Fri, 4 Mar 2011 14:22:25 +0100
Subject: [PATCH 3/4] ALSA: HDA: Fixup unnecessary volume control index on Realtek ALC88x

Without this change, a volume control named "Surround" or "Side" would
get an unnecessary index, causing it to be ignored by the vmaster and
PulseAudio.

Tested-by: Bartłomiej Żogała <nusch88@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_realtek.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index fe45c61..f94b12f 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5206,16 +5206,19 @@ static int alc880_auto_create_multi_out_ctls(struct alc_spec *spec,
 				return err;
 		} else {
 			const char *name = pfx;
-			if (!name)
+			int index = i;
+			if (!name) {
 				name = chname[i];
+				index = 0;
+			}
 			err = __add_pb_vol_ctrl(spec, ALC_CTL_WIDGET_VOL,
-						name, i,
+						name, index,
 					  HDA_COMPOSE_AMP_VAL(nid, 3, 0,
 							      HDA_OUTPUT));
 			if (err < 0)
 				return err;
 			err = __add_pb_sw_ctrl(spec, ALC_CTL_BIND_MUTE,
-					       name, i,
+					       name, index,
 					  HDA_COMPOSE_AMP_VAL(nid, 3, 2,
 							      HDA_INPUT));
 			if (err < 0)
-- 
1.7.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-ALSA-HDA-Realtek-ALC88x-Do-not-over-initialize-speak.patch --]
[-- Type: text/x-patch; name="0004-ALSA-HDA-Realtek-ALC88x-Do-not-over-initialize-speak.patch", Size: 2338 bytes --]

>From 247b0dbb5c5df7cdb8d9d5b08f48577c33c0afee Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Fri, 4 Mar 2011 16:54:52 +0100
Subject: [PATCH 4/4] ALSA: HDA: Realtek ALC88x: Do not over-initialize speakers and hp that are primary outputs

Do not initialize again the what has already been initialized as
multi outs, as this breaks surround speakers.

Tested-by: Bartłomiej Żogała <nusch88@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_realtek.c |   37 +++++++++++++++++++++----------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index f94b12f..3dc88ba 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -10843,23 +10843,28 @@ static void alc882_auto_init_hp_out(struct hda_codec *codec)
 	hda_nid_t pin, dac;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(spec->autocfg.hp_pins); i++) {
-		pin = spec->autocfg.hp_pins[i];
-		if (!pin)
-			break;
-		dac = spec->multiout.hp_nid;
-		if (!dac)
-			dac = spec->multiout.dac_nids[0]; /* to front */
-		alc882_auto_set_output_and_unmute(codec, pin, PIN_HP, dac);
+	if (spec->autocfg.line_out_type != AUTO_PIN_HP_OUT) {
+		for (i = 0; i < ARRAY_SIZE(spec->autocfg.hp_pins); i++) {
+			pin = spec->autocfg.hp_pins[i];
+			if (!pin)
+				break;
+			dac = spec->multiout.hp_nid;
+			if (!dac)
+				dac = spec->multiout.dac_nids[0]; /* to front */
+			alc882_auto_set_output_and_unmute(codec, pin, PIN_HP, dac);
+		}
 	}
-	for (i = 0; i < ARRAY_SIZE(spec->autocfg.speaker_pins); i++) {
-		pin = spec->autocfg.speaker_pins[i];
-		if (!pin)
-			break;
-		dac = spec->multiout.extra_out_nid[0];
-		if (!dac)
-			dac = spec->multiout.dac_nids[0]; /* to front */
-		alc882_auto_set_output_and_unmute(codec, pin, PIN_OUT, dac);
+
+	if (spec->autocfg.line_out_type != AUTO_PIN_SPEAKER_OUT) {
+		for (i = 0; i < ARRAY_SIZE(spec->autocfg.speaker_pins); i++) {
+			pin = spec->autocfg.speaker_pins[i];
+			if (!pin)
+				break;
+			dac = spec->multiout.extra_out_nid[0];
+			if (!dac)
+				dac = spec->multiout.dac_nids[0]; /* to front */
+			alc882_auto_set_output_and_unmute(codec, pin, PIN_OUT, dac);
+		}
 	}
 }
 
-- 
1.7.1


[-- Attachment #6: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[Takashi Iwai]

At Mon, 07 Mar 2011 09:22:42 +0100,
David Henningsson wrote:
> 
> I spend last Friday together with Bartłomiej Żogała fixing up a long 
> standing issue with Lenovo Y530, which has 4+1 internal speakers on a 
> Realtek 888. And we all want that supported by the auto parser, don't 
> we?

Well, it's a bit flaky.  The current behavior assigning speakers only
as "Speaker" is intentional.  This is a simplification to avoid the
conflict with the case where both multiple line-outs and multiple
speakers are present.

And, in general, I don't like to get rid of "Speaker" notation.  If
any, we should keep "Speaker" with a channel prefix.

> So here comes the patches. Would be nice to have in 2.6.38.

Sorry, the speaker-change is too intrusive for 2.6.38.
Since I already sent a pull request yesterday, which is supposed to be
the final one unless major fixes come up, I'll queue this later with
stable-kernel tag.

Looking at each patch...

> Subject: [PATCH 1/4] ALSA: HDA: Enable surround and subwoofer on Lenovo Ideapad Y530
> 
> The pin config values would change the association instead of the
> sequence, this commit fixes that up.

This looks good.

> Subject: [PATCH 2/4] ALSA: HDA: Fix volume control naming for surround speakers on Realtek auto-parser
> 
> When more than one pair of internal speakers is present, allow names
> according to their channels.

This one is for re-consideration.

> From: David Henningsson <david.henningsson@canonical.com>
> Date: Fri, 4 Mar 2011 14:22:25 +0100
> Subject: [PATCH 3/4] ALSA: HDA: Fixup unnecessary volume control index on Realtek ALC88x
> 
> Without this change, a volume control named "Surround" or "Side" would
> get an unnecessary index, causing it to be ignored by the vmaster and
> PulseAudio.

vmaster should handle multiple indices properly, but maybe PA not.
Nevertheless, the fix is correct.

> Subject: [PATCH 4/4] ALSA: HDA: Realtek ALC88x: Do not over-initialize speakers and hp that are primary outputs
> 
> Do not initialize again the what has already been initialized as
> multi outs, as this breaks surround speakers.

This looks correct, too.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[David Henningsson]

On 2011-03-07 10:37, Takashi Iwai wrote:
> At Mon, 07 Mar 2011 09:22:42 +0100,
> David Henningsson wrote:
>>
>> I spend last Friday together with Bartłomiej Żogała fixing up a long
>> standing issue with Lenovo Y530, which has 4+1 internal speakers on a
>> Realtek 888. And we all want that supported by the auto parser, don't
>> we?
>
> Well, it's a bit flaky.  The current behavior assigning speakers only
> as "Speaker" is intentional.  This is a simplification to avoid the
> conflict with the case where both multiple line-outs and multiple
> speakers are present.

In that case, nothing changes - this code path is not taken, since 
line_out_type wouldn't be AUTO_PIN_SPEAKER_OUT.

> And, in general, I don't like to get rid of "Speaker" notation.  If
> any, we should keep "Speaker" with a channel prefix.

This is indeed tricky, and I think we've stranded on a similar issue 
here once before.

Btw, in the case of this particular machine, the "Front" controls the 
headphones as well, so the name is accidentally correct.

I've even seen a machine where a DAC controlled two out of three fronts 
(e g headphones and line-out but not speaker, or something like that) - 
how would you name that?

Perhaps it is time to start to come up with a more reliable naming 
scheme for 2.6.39 or 2.6.40, that would take into account the more 
trickier combinations as well.

>
>> So here comes the patches. Would be nice to have in 2.6.38.
>
> Sorry, the speaker-change is too intrusive for 2.6.38.
> Since I already sent a pull request yesterday, which is supposed to be
> the final one unless major fixes come up, I'll queue this later with
> stable-kernel tag.

Ok, that works well for me.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[Takashi Iwai]

At Mon, 07 Mar 2011 11:46:29 +0100,
David Henningsson wrote:
> 
> On 2011-03-07 10:37, Takashi Iwai wrote:
> > At Mon, 07 Mar 2011 09:22:42 +0100,
> > David Henningsson wrote:
> >>
> >> I spend last Friday together with Bartłomiej Żogała fixing up a long
> >> standing issue with Lenovo Y530, which has 4+1 internal speakers on a
> >> Realtek 888. And we all want that supported by the auto parser, don't
> >> we?
> >
> > Well, it's a bit flaky.  The current behavior assigning speakers only
> > as "Speaker" is intentional.  This is a simplification to avoid the
> > conflict with the case where both multiple line-outs and multiple
> > speakers are present.
> 
> In that case, nothing changes - this code path is not taken, since 
> line_out_type wouldn't be AUTO_PIN_SPEAKER_OUT.

Hm, I see.

> > And, in general, I don't like to get rid of "Speaker" notation.  If
> > any, we should keep "Speaker" with a channel prefix.
> 
> This is indeed tricky, and I think we've stranded on a similar issue 
> here once before.
> 
> Btw, in the case of this particular machine, the "Front" controls the 
> headphones as well, so the name is accidentally correct.

OK, then it's good to take.
Let me check these patches a bit.  Then I'll merge them.

> I've even seen a machine where a DAC controlled two out of three fronts 
> (e g headphones and line-out but not speaker, or something like that) - 
> how would you name that?

It's a difficult choice, yeah.  If I would have to choose, I'd take
"Headphone" and "Speaker", and make line-out implicit.  But this is
neither ideal solution, of course.

> Perhaps it is time to start to come up with a more reliable naming 
> scheme for 2.6.39 or 2.6.40, that would take into account the more 
> trickier combinations as well.

The time for 2.6.39 is almost closed :)
But I fully agree with a major re-design for more consistent control
names.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Four patches to fixup surround internal speakers on Realtek 88x

[Takashi Iwai]

At Mon, 07 Mar 2011 11:54:58 +0100,
Takashi Iwai wrote:
> 
> At Mon, 07 Mar 2011 11:46:29 +0100,
> David Henningsson wrote:
> > 
> > > And, in general, I don't like to get rid of "Speaker" notation.  If
> > > any, we should keep "Speaker" with a channel prefix.
> > 
> > This is indeed tricky, and I think we've stranded on a similar issue 
> > here once before.
> > 
> > Btw, in the case of this particular machine, the "Front" controls the 
> > headphones as well, so the name is accidentally correct.
> 
> OK, then it's good to take.
> Let me check these patches a bit.  Then I'll merge them.

Applied these patches now.


thanks,

Takashi