Problems with multiple simultaneous ac97 quirks

Problems with multiple simultaneous ac97 quirks

[Matthew Garrett]

Hi,

Lots of current HP hardware would benefit from both AC97_TUNE_MUTE_LED 
and AC97_TUNE_HP_ONLY. However, this is made difficult by the fact that 
both of these options set the master volume switch put function to their 
own one. This is easy enough to work around by adding another quirk, 
but that would soon lead to combinatorial explosion. What's the best 
approach to being able to add multiple interacting quirks?

-- 
Matthew Garrett | mjg59@srcf.ucam.org


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Takashi Iwai]

At Thu, 15 Sep 2005 19:04:15 +0100,
Matthew Garrett wrote:
> 
> Hi,
> 
> Lots of current HP hardware would benefit from both AC97_TUNE_MUTE_LED 
> and AC97_TUNE_HP_ONLY. However, this is made difficult by the fact that 
> both of these options set the master volume switch put function to their 
> own one. This is easy enough to work around by adding another quirk, 
> but that would soon lead to combinatorial explosion. What's the best 
> approach to being able to add multiple interacting quirks?

I think the new quirk is the only solution in this case.
And there won't be so many combinations in the real world, I'd
expect.


Takashi


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Matthew Garrett]

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

On Thu, Sep 15, 2005 at 09:49:41PM +0200, Takashi Iwai wrote:

> I think the new quirk is the only solution in this case.
> And there won't be so many combinations in the real world, I'd
> expect.

Ok. Something like the following patch? It introduces 
AC97_TUNE_HP_MUTE_LED (yes, it's a poor name...) which is identical to 
AC97_TUNE_MUTE_LED except it calls bind_hp_volsw_put instead of 
snd_ac97_put_volsw and removes the headphone mixer controls.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[-- Attachment #2: ac97.diff --]
[-- Type: text/plain, Size: 599 bytes --]

--- sound/pci/ac97/ac97_codec.c.orig	2005-09-13 15:45:35.000000000 +0100
+++ linux/sound/pci/ac97/ac97_codec.c	2005-09-13 15:46:05.000000000 +0100
@@ -2551,9 +2551,10 @@ int snd_ac97_tune_hardware(ac97_t *ac97,
 				continue;
 			snd_printdd("ac97 quirk for %s (%04x:%04x)\n", quirk->name, ac97->subsystem_vendor, ac97->subsystem_device);
 			result = apply_quirk(ac97, quirk->type);
-			if (result < 0)
+			if (result < 0) {
 				snd_printk(KERN_ERR "applying quirk type %d for %s failed (%d)\n", quirk->type, quirk->name, result);
-			return result;
+				return result;
+			}
 		}
 	}
 	return 0;

Re: Problems with multiple simultaneous ac97 quirks

[Takashi Iwai]

At Thu, 15 Sep 2005 21:28:58 +0100,
Matthew Garrett wrote:
> 
> On Thu, Sep 15, 2005 at 09:49:41PM +0200, Takashi Iwai wrote:
> 
> > I think the new quirk is the only solution in this case.
> > And there won't be so many combinations in the real world, I'd
> > expect.
> 
> Ok. Something like the following patch? It introduces 
> AC97_TUNE_HP_MUTE_LED (yes, it's a poor name...) which is identical to 
> AC97_TUNE_MUTE_LED except it calls bind_hp_volsw_put instead of 
> snd_ac97_put_volsw and removes the headphone mixer controls.

Looks like the most of patch is missing...


Takashi


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Matthew Garrett]

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

On Thu, Sep 15, 2005 at 09:28:58PM +0100, Matthew Garrett wrote:

> Ok. Something like the following patch? It introduces 
> AC97_TUNE_HP_MUTE_LED (yes, it's a poor name...) which is identical to 
> AC97_TUNE_MUTE_LED except it calls bind_hp_volsw_put instead of 
> snd_ac97_put_volsw and removes the headphone mixer controls.

Oops. Entirely the wrong patch. Here we go:

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[-- Attachment #2: hpledquirk.diff --]
[-- Type: text/plain, Size: 2059 bytes --]

--- include/sound/ac97_codec.h.orig	2005-09-15 19:05:14.000000000 +0100
+++ b/include/sound/ac97_codec.h	2005-09-15 19:05:47.000000000 +0100
@@ -570,6 +570,7 @@
 	AC97_TUNE_ALC_JACK,	/* for Realtek, enable JACK detection */
 	AC97_TUNE_INV_EAPD,	/* inverted EAPD implementation */
 	AC97_TUNE_MUTE_LED,	/* EAPD bit works as mute LED */
+	AC97_TUNE_HP_MUTE_LED,  /* EAPD bit works as mute LED, use headphone control as master */
 };
 
 struct ac97_quirk {
--- sound/pci/ac97/ac97_codec.c.orig	2005-09-15 21:25:24.000000000 +0100
+++ b/sound/pci/ac97/ac97_codec.c	2005-09-15 19:09:55.000000000 +0100
@@ -2470,6 +2470,40 @@
 	return 0;
 }
 
+static int hp_master_mute_sw_put(snd_kcontrol_t *kcontrol, snd_ctl_elem_value_t *ucontrol)
+{
+	int err = bind_hp_volsw_put(kcontrol, ucontrol);
+	if (err > 0) {
+		ac97_t *ac97 = snd_kcontrol_chip(kcontrol);
+		int shift = (kcontrol->private_value >> 8) & 0x0f;
+		int rshift = (kcontrol->private_value >> 12) & 0x0f;
+		unsigned short mask;
+		if (shift != rshift)
+			mask = 0x8080;
+		else
+			mask = 0x8000;
+		snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000,
+				     (ac97->regs[AC97_MASTER] & mask) == mask ?
+				     0x8000 : 0);
+	}
+	return err;
+}
+
+static int tune_hp_mute_led(ac97_t *ac97)
+{
+	snd_kcontrol_t *msw = ctl_find(ac97, "Master Playback Switch", NULL);
+	snd_kcontrol_t *mvol = ctl_find(ac97, "Master Playback Volume", NULL);
+	if (! msw || ! mvol)
+		return -ENOENT;
+	msw->put = hp_master_mute_sw_put;
+	mvol->put = bind_hp_volsw_put;
+	snd_ac97_remove_ctl(ac97, "External Amplifier", NULL);
+	snd_ac97_remove_ctl(ac97, "Headphone Playback", "Switch");
+	snd_ac97_remove_ctl(ac97, "Headphone Playback", "Volume");
+	snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, 0x8000); /* mute LED on */
+	return 0;
+}
+
 struct quirk_table {
 	const char *name;
 	int (*func)(ac97_t *);
@@ -2484,6 +2518,7 @@
 	{ "alc_jack", tune_alc_jack },
 	{ "inv_eapd", tune_inv_eapd },
 	{ "mute_led", tune_mute_led },
+	{ "hp_mute_led", tune_hp_mute_led },
 };
 
 /* apply the quirk with the given type */

Re: Problems with multiple simultaneous ac97 quirks

[John W. Linville]

On Fri, Sep 16, 2005 at 11:41:37AM +0100, Matthew Garrett wrote:
> On Thu, Sep 15, 2005 at 09:28:58PM +0100, Matthew Garrett wrote:
> 
> > Ok. Something like the following patch? It introduces 
> > AC97_TUNE_HP_MUTE_LED (yes, it's a poor name...) which is identical to 
> > AC97_TUNE_MUTE_LED except it calls bind_hp_volsw_put instead of 
> > snd_ac97_put_volsw and removes the headphone mixer controls.

I don't suppose you have a list of PCI sub-system IDs that this helps?

Thanks,

John
-- 
John W. Linville
linville@tuxdriver.com


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Matthew Garrett]

On Fri, Sep 16, 2005 at 08:37:13AM -0400, John W. Linville wrote:

> I don't suppose you have a list of PCI sub-system IDs that this helps?

Yup:

AC97_TUNE_HP_MUTE_LED

4200    103c:0938
6120	103c:099c
6220    103c:0944
8220    103c:0934

All of Those plus the following also want to be added to the list in 
check_ad1981_hp_jack_sense():

4000/10 0e11:005a

The 6125 wants AC97_TUNE_MUTE_LED - it's an ATI chipset, using the 
snd-atiixp driver. It's 103c:308b. However, it doesn't need TUNE_HP_ONLY 
and doesn't need to be added to check_ad1981_hp_jack_sense().

I'd guess that the nx9600 probably needs the same, but I don't have 
access to one of those to check. The 8230 and 8240 have the same IDs as 
the 8220, and the 6110 shares with the 6120 (it doesn't actually have a 
mute LED, but the motherboard is identical). The 6230 ought to be 
identical to the 6220.

I'd guess that some of the consumer range also has these requirements, 
but I'm afraid I don't have any available to test.
-- 
Matthew Garrett | mjg59@srcf.ucam.org


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Takashi Iwai]

At Fri, 16 Sep 2005 15:25:44 +0100,
Matthew Garrett wrote:
> 
> On Fri, Sep 16, 2005 at 08:37:13AM -0400, John W. Linville wrote:
> 
> > I don't suppose you have a list of PCI sub-system IDs that this helps?
> 
> Yup:
> 
> AC97_TUNE_HP_MUTE_LED
> 
> 4200    103c:0938
> 6120	103c:099c
> 6220    103c:0944
> 8220    103c:0934
> 
> All of Those plus the following also want to be added to the list in 
> check_ad1981_hp_jack_sense():
> 
> 4000/10 0e11:005a
> 
> The 6125 wants AC97_TUNE_MUTE_LED - it's an ATI chipset, using the 
> snd-atiixp driver. It's 103c:308b. However, it doesn't need TUNE_HP_ONLY 
> and doesn't need to be added to check_ad1981_hp_jack_sense().

Care to create a patch for the above?


> I'd guess that the nx9600 probably needs the same, but I don't have 
> access to one of those to check. The 8230 and 8240 have the same IDs as 
> the 8220, and the 6110 shares with the 6120 (it doesn't actually have a 
> mute LED, but the motherboard is identical). The 6230 ought to be 
> identical to the 6220.

They can be fixed later.


Thanks,

Takashi


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Problems with multiple simultaneous ac97 quirks

[Takashi Iwai]

At Fri, 16 Sep 2005 11:41:37 +0100,
Matthew Garrett wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Thu, Sep 15, 2005 at 09:28:58PM +0100, Matthew Garrett wrote:
> 
> > Ok. Something like the following patch? It introduces 
> > AC97_TUNE_HP_MUTE_LED (yes, it's a poor name...) which is identical to 
> > AC97_TUNE_MUTE_LED except it calls bind_hp_volsw_put instead of 
> > snd_ac97_put_volsw and removes the headphone mixer controls.
> 
> Oops. Entirely the wrong patch. Here we go:

Then the last change to allow double entries is no longer needed.
Shall we take it back?

Also, it'd be nice if you update ALSA-Configuration.txt, too.
And, don't forget Signed-off-by, too.


Thanks,

Takashi


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php