All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Acer aspire 3830TG and Conexant 506c/20588
@ 2011-06-28 11:44 David Henningsson
  2011-06-28 12:15 ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: David Henningsson @ 2011-06-28 11:44 UTC (permalink / raw)
  To: tiwai, alsa-devel

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

The new Conexant 5066 auto-parser is barely finished and already we 
might need a quirk system for it...

Anyway, in this bug, the internal speaker is not working (and was not 
working before the auto-parser either). I'm attaching two patches that 
are quite simple, and at least the first one (add ID 506c) I think 
should be applied right away.

The problem: The internal speaker is at node 0x1f and you need to turn 
on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to 
fix this in the best way. An "init verb" quirk, or a "use EAPD from this 
pin" quirk? Try to copy-paste realtek's quirk system, and if so, 
refactor it into hda_codec.c, or write something new?

What are your ideas?

Alsa-info reference: https://launchpadlibrarian.net/73966502/alsa-info
Bug reference: 
https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/783582

-- 
David Henningsson
http://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-HDA-Add-a-new-Conexant-codec-ID-506c.patch --]
[-- Type: text/x-patch, Size: 1546 bytes --]

>From 69bb710e13ae842d77d584a75088e008834a1722 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 21 Jun 2011 20:51:34 +0200
Subject: [PATCH 1/2] ALSA: HDA: Add a new Conexant codec ID (506c)

Conexant ID 506c was found on Acer Aspire 3830TG. As users report
no playback, sending to stable should be safe.

Cc: stable@kernel.org
BugLink: https://bugs.launchpad.net/bugs/783582
Reported-by: andROOM
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_conexant.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 6e86427..6c9b742 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -4390,6 +4390,8 @@ static const struct hda_codec_preset snd_hda_preset_conexant[] = {
 	  .patch = patch_cxt5066 },
 	{ .id = 0x14f15069, .name = "CX20585",
 	  .patch = patch_cxt5066 },
+	{ .id = 0x14f1506c, .name = "CX20588",
+	  .patch = patch_cxt5066 },
 	{ .id = 0x14f1506e, .name = "CX20590",
 	  .patch = patch_cxt5066 },
 	{ .id = 0x14f15097, .name = "CX20631",
@@ -4418,6 +4420,7 @@ MODULE_ALIAS("snd-hda-codec-id:14f15066");
 MODULE_ALIAS("snd-hda-codec-id:14f15067");
 MODULE_ALIAS("snd-hda-codec-id:14f15068");
 MODULE_ALIAS("snd-hda-codec-id:14f15069");
+MODULE_ALIAS("snd-hda-codec-id:14f1506c");
 MODULE_ALIAS("snd-hda-codec-id:14f1506e");
 MODULE_ALIAS("snd-hda-codec-id:14f15097");
 MODULE_ALIAS("snd-hda-codec-id:14f15098");
-- 
1.7.4.1


[-- Attachment #3: 0002-ALSA-HDA-Add-model-auto-quirk-for-Acer-Aspire-3830TG.patch --]
[-- Type: text/x-patch, Size: 1116 bytes --]

>From 9505d48fb556f977c5247c9564481301de39b258 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Tue, 21 Jun 2011 21:01:52 +0200
Subject: [PATCH 2/2] ALSA: HDA: Add model=auto quirk for Acer Aspire 3830TG

Since we're not using the new auto parser as a fallback yet,
add it manually as a quirk.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_conexant.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 6c9b742..91c8c0c 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -3075,6 +3075,7 @@ static const char * const cxt5066_models[CXT5066_MODELS] = {
 };
 
 static const struct snd_pci_quirk cxt5066_cfg_tbl[] = {
+	SND_PCI_QUIRK(0x1025, 0x054c, "Acer Aspire 3830TG", CXT5066_AUTO),
 	SND_PCI_QUIRK_MASK(0x1025, 0xff00, 0x0400, "Acer", CXT5066_IDEAPAD),
 	SND_PCI_QUIRK(0x1028, 0x02d8, "Dell Vostro", CXT5066_DELL_VOSTRO),
 	SND_PCI_QUIRK(0x1028, 0x02f5, "Dell Vostro 320", CXT5066_IDEAPAD),
-- 
1.7.4.1


[-- Attachment #4: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-28 11:44 [PATCH] Acer aspire 3830TG and Conexant 506c/20588 David Henningsson
@ 2011-06-28 12:15 ` Takashi Iwai
  2011-06-29  6:46   ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-06-28 12:15 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Tue, 28 Jun 2011 12:44:19 +0100,
David Henningsson wrote:
> 
> The new Conexant 5066 auto-parser is barely finished and already we 
> might need a quirk system for it...
> 
> Anyway, in this bug, the internal speaker is not working (and was not 
> working before the auto-parser either). I'm attaching two patches that 
> are quite simple, and at least the first one (add ID 506c) I think 
> should be applied right away.

Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
branch for 3.1 to enable model=auto as default for Conexant (Thanks
for remind :)

> The problem: The internal speaker is at node 0x1f and you need to turn 
> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to 
> fix this in the best way. An "init verb" quirk, or a "use EAPD from this 
> pin" quirk? Try to copy-paste realtek's quirk system, and if so, 
> refactor it into hda_codec.c, or write something new?
> 
> What are your ideas?
 
We can simply turn on all EAPDs for 5066 and older chips.
These codecs have just one or two EAPD pins, and we have no tightly
coupled EAPD control (yet), so it'd be safe to turn all on.


Takashi

> Alsa-info reference: https://launchpadlibrarian.net/73966502/alsa-info
> Bug reference: 
> https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/783582
> 
> -- 
> David Henningsson
> http://launchpad.net/~diwic
> [2 0001-ALSA-HDA-Add-a-new-Conexant-codec-ID-506c.patch <text/x-patch (7bit)>]
> >From 69bb710e13ae842d77d584a75088e008834a1722 Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson@canonical.com>
> Date: Tue, 21 Jun 2011 20:51:34 +0200
> Subject: [PATCH 1/2] ALSA: HDA: Add a new Conexant codec ID (506c)
> 
> Conexant ID 506c was found on Acer Aspire 3830TG. As users report
> no playback, sending to stable should be safe.
> 
> Cc: stable@kernel.org
> BugLink: https://bugs.launchpad.net/bugs/783582
> Reported-by: andROOM
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/pci/hda/patch_conexant.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 6e86427..6c9b742 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -4390,6 +4390,8 @@ static const struct hda_codec_preset snd_hda_preset_conexant[] = {
>  	  .patch = patch_cxt5066 },
>  	{ .id = 0x14f15069, .name = "CX20585",
>  	  .patch = patch_cxt5066 },
> +	{ .id = 0x14f1506c, .name = "CX20588",
> +	  .patch = patch_cxt5066 },
>  	{ .id = 0x14f1506e, .name = "CX20590",
>  	  .patch = patch_cxt5066 },
>  	{ .id = 0x14f15097, .name = "CX20631",
> @@ -4418,6 +4420,7 @@ MODULE_ALIAS("snd-hda-codec-id:14f15066");
>  MODULE_ALIAS("snd-hda-codec-id:14f15067");
>  MODULE_ALIAS("snd-hda-codec-id:14f15068");
>  MODULE_ALIAS("snd-hda-codec-id:14f15069");
> +MODULE_ALIAS("snd-hda-codec-id:14f1506c");
>  MODULE_ALIAS("snd-hda-codec-id:14f1506e");
>  MODULE_ALIAS("snd-hda-codec-id:14f15097");
>  MODULE_ALIAS("snd-hda-codec-id:14f15098");
> -- 
> 1.7.4.1
> 
> [3 0002-ALSA-HDA-Add-model-auto-quirk-for-Acer-Aspire-3830TG.patch <text/x-patch (7bit)>]
> >From 9505d48fb556f977c5247c9564481301de39b258 Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson@canonical.com>
> Date: Tue, 21 Jun 2011 21:01:52 +0200
> Subject: [PATCH 2/2] ALSA: HDA: Add model=auto quirk for Acer Aspire 3830TG
> 
> Since we're not using the new auto parser as a fallback yet,
> add it manually as a quirk.
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/pci/hda/patch_conexant.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 6c9b742..91c8c0c 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -3075,6 +3075,7 @@ static const char * const cxt5066_models[CXT5066_MODELS] = {
>  };
>  
>  static const struct snd_pci_quirk cxt5066_cfg_tbl[] = {
> +	SND_PCI_QUIRK(0x1025, 0x054c, "Acer Aspire 3830TG", CXT5066_AUTO),
>  	SND_PCI_QUIRK_MASK(0x1025, 0xff00, 0x0400, "Acer", CXT5066_IDEAPAD),
>  	SND_PCI_QUIRK(0x1028, 0x02d8, "Dell Vostro", CXT5066_DELL_VOSTRO),
>  	SND_PCI_QUIRK(0x1028, 0x02f5, "Dell Vostro 320", CXT5066_IDEAPAD),
> -- 
> 1.7.4.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-28 12:15 ` Takashi Iwai
@ 2011-06-29  6:46   ` Takashi Iwai
  2011-06-29  9:16     ` David Henningsson
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-06-29  6:46 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Tue, 28 Jun 2011 14:15:40 +0200,
Takashi Iwai wrote:
> 
> At Tue, 28 Jun 2011 12:44:19 +0100,
> David Henningsson wrote:
> > 
> > The new Conexant 5066 auto-parser is barely finished and already we 
> > might need a quirk system for it...
> > 
> > Anyway, in this bug, the internal speaker is not working (and was not 
> > working before the auto-parser either). I'm attaching two patches that 
> > are quite simple, and at least the first one (add ID 506c) I think 
> > should be applied right away.
> 
> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> for remind :)
> 
> > The problem: The internal speaker is at node 0x1f and you need to turn 
> > on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to 
> > fix this in the best way. An "init verb" quirk, or a "use EAPD from this 
> > pin" quirk? Try to copy-paste realtek's quirk system, and if so, 
> > refactor it into hda_codec.c, or write something new?
> > 
> > What are your ideas?
>  
> We can simply turn on all EAPDs for 5066 and older chips.
> These codecs have just one or two EAPD pins, and we have no tightly
> coupled EAPD control (yet), so it'd be safe to turn all on.

Or, maybe something like below would be cleverer...
(Totally untested, of course.)


Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser

There seem some machines referring to EAPD bit of the unassigned pin
for the speaker EAPD of a different pin, thus we need to control the
EAPD of unused pins as well.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 4ca880b..dbea3d0 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -155,6 +155,10 @@ struct conexant_spec {
 	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
 
 	unsigned int beep_amp;
+
+	/* extra EAPD pins */
+	unsigned int num_eapds;
+	hda_nid_t eapds[4];
 };
 
 static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
@@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
 	/* if LO is a copy of either HP or Speaker, don't need to handle it */
 	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
 	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
-		return;
+		goto turn_extra_eapd;
 	if (spec->auto_mute) {
 		/* mute LO in auto-mode when HP jack is present */
 		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
@@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
 			on = 1;
 	}
 	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
+ turn_extra_eapd:
+	/* turn on/off extra EAPDs, too */
+	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
 }
 
 static void cx_auto_hp_automute(struct hda_codec *codec)
@@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
 #define cx_auto_parse_beep(codec)
 #endif
 
+static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
+{
+	int i;
+	for (i = 0; i < nums; i++)
+		if (list[i] == nid)
+			return true;
+	return false;
+}
+
+/* parse extra-EAPD that aren't assigned to any pins */
+static void cx_auto_parse_eapd(struct hda_codec *codec)
+{
+	struct conexant_spec *spec = codec->spec;
+	struct auto_pin_cfg *cfg = &spec->autocfg;
+	hda_nid_t nid, end_nid;
+
+	end_nid = codec->start_nid + codec->num_nodes;
+	for (nid = codec->start_nid; nid < end_nid; nid++) {
+		if (get_wcaps_type(get_wcaps(codec, nid)) != AC_WID_PIN)
+			continue;
+		if (!(snd_hda_query_pin_caps(codec, nid) & AC_PINCAP_EAPD))
+			continue;
+		if (found_in_nid_list(nid, cfg->line_out_pins, cfg->line_outs) ||
+		    found_in_nid_list(nid, cfg->hp_pins, cfg->hp_outs) ||
+		    found_in_nid_list(nid, cfg->speaker_pins, cfg->speaker_outs))
+			continue;
+		spec->eapds[spec->num_eapds++] = nid;
+		if (spec->num_eapds >= ARRAY_SIZE(spec->eapds))
+			break;
+	}
+}
+
 static int cx_auto_parse_auto_config(struct hda_codec *codec)
 {
 	struct conexant_spec *spec = codec->spec;
@@ -3914,6 +3953,7 @@ static int cx_auto_parse_auto_config(struct hda_codec *codec)
 	cx_auto_parse_input(codec);
 	cx_auto_parse_digital(codec);
 	cx_auto_parse_beep(codec);
+	cx_auto_parse_eapd(codec);
 	return 0;
 }
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-29  6:46   ` Takashi Iwai
@ 2011-06-29  9:16     ` David Henningsson
  2011-06-29  9:34       ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: David Henningsson @ 2011-06-29  9:16 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 2011-06-29 07:46, Takashi Iwai wrote:
> At Tue, 28 Jun 2011 14:15:40 +0200,
> Takashi Iwai wrote:
>>
>> At Tue, 28 Jun 2011 12:44:19 +0100,
>> David Henningsson wrote:
>>>
>>> The new Conexant 5066 auto-parser is barely finished and already we
>>> might need a quirk system for it...
>>>
>>> Anyway, in this bug, the internal speaker is not working (and was not
>>> working before the auto-parser either). I'm attaching two patches that
>>> are quite simple, and at least the first one (add ID 506c) I think
>>> should be applied right away.
>>
>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
>> for remind :)
>>
>>> The problem: The internal speaker is at node 0x1f and you need to turn
>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
>>> refactor it into hda_codec.c, or write something new?
>>>
>>> What are your ideas?
>>
>> We can simply turn on all EAPDs for 5066 and older chips.
>> These codecs have just one or two EAPD pins, and we have no tightly
>> coupled EAPD control (yet), so it'd be safe to turn all on.
>
> Or, maybe something like below would be cleverer...
> (Totally untested, of course.)

Yes, very nice, I was thinking of something like that. That will be best 
from a power consumption perspective. My only worry is that if turning 
EAPD on and off causes pops (I don't know if they do), it will be better 
to leave it on all the time.

> From: Takashi Iwai<tiwai@suse.de>
> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
>
> There seem some machines referring to EAPD bit of the unassigned pin
> for the speaker EAPD of a different pin, thus we need to control the
> EAPD of unused pins as well.
>
> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> ---
>   sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 41 insertions(+), 1 deletions(-)
>
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 4ca880b..dbea3d0 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -155,6 +155,10 @@ struct conexant_spec {
>   	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
>
>   	unsigned int beep_amp;
> +
> +	/* extra EAPD pins */
> +	unsigned int num_eapds;
> +	hda_nid_t eapds[4];
>   };
>
>   static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>   	/* if LO is a copy of either HP or Speaker, don't need to handle it */
>   	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
>   	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> -		return;
> +		goto turn_extra_eapd;
>   	if (spec->auto_mute) {
>   		/* mute LO in auto-mode when HP jack is present */
>   		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>   			on = 1;
>   	}
>   	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> + turn_extra_eapd:
> +	/* turn on/off extra EAPDs, too */
> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
>   }
>
>   static void cx_auto_hp_automute(struct hda_codec *codec)
> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
>   #define cx_auto_parse_beep(codec)
>   #endif
>
> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> +{

Hmm, I've seen variations of this function at more than one place 
already. Maybe a function like this would be good to have in hda_codec.c?

int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
{
	int i;
	for (i = 0; i < list_length; i++)
		if (list[i] == nid)
			return i;
	return -1;
}

#define bool snd_hda_nid_exists(nid, list, list_length) 
(snd_hda_find_nid(nid, list, list_length) != -1)

> +	int i;
> +	for (i = 0; i<  nums; i++)
> +		if (list[i] == nid)
> +			return true;
> +	return false;
> +}
> +
> +/* parse extra-EAPD that aren't assigned to any pins */
> +static void cx_auto_parse_eapd(struct hda_codec *codec)
> +{
> +	struct conexant_spec *spec = codec->spec;
> +	struct auto_pin_cfg *cfg =&spec->autocfg;
> +	hda_nid_t nid, end_nid;
> +
> +	end_nid = codec->start_nid + codec->num_nodes;
> +	for (nid = codec->start_nid; nid<  end_nid; nid++) {
> +		if (get_wcaps_type(get_wcaps(codec, nid)) != AC_WID_PIN)
> +			continue;
> +		if (!(snd_hda_query_pin_caps(codec, nid)&  AC_PINCAP_EAPD))
> +			continue;
> +		if (found_in_nid_list(nid, cfg->line_out_pins, cfg->line_outs) ||
> +		    found_in_nid_list(nid, cfg->hp_pins, cfg->hp_outs) ||
> +		    found_in_nid_list(nid, cfg->speaker_pins, cfg->speaker_outs))
> +			continue;
> +		spec->eapds[spec->num_eapds++] = nid;
> +		if (spec->num_eapds>= ARRAY_SIZE(spec->eapds))
> +			break;
> +	}
> +}
> +
>   static int cx_auto_parse_auto_config(struct hda_codec *codec)
>   {
>   	struct conexant_spec *spec = codec->spec;
> @@ -3914,6 +3953,7 @@ static int cx_auto_parse_auto_config(struct hda_codec *codec)
>   	cx_auto_parse_input(codec);
>   	cx_auto_parse_digital(codec);
>   	cx_auto_parse_beep(codec);
> +	cx_auto_parse_eapd(codec);
>   	return 0;
>   }
>


-- 
David Henningsson
http://launchpad.net/~diwic

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-29  9:16     ` David Henningsson
@ 2011-06-29  9:34       ` Takashi Iwai
  2011-06-30  6:55         ` David Henningsson
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-06-29  9:34 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Wed, 29 Jun 2011 10:16:51 +0100,
David Henningsson wrote:
> 
> On 2011-06-29 07:46, Takashi Iwai wrote:
> > At Tue, 28 Jun 2011 14:15:40 +0200,
> > Takashi Iwai wrote:
> >>
> >> At Tue, 28 Jun 2011 12:44:19 +0100,
> >> David Henningsson wrote:
> >>>
> >>> The new Conexant 5066 auto-parser is barely finished and already we
> >>> might need a quirk system for it...
> >>>
> >>> Anyway, in this bug, the internal speaker is not working (and was not
> >>> working before the auto-parser either). I'm attaching two patches that
> >>> are quite simple, and at least the first one (add ID 506c) I think
> >>> should be applied right away.
> >>
> >> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> >> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> >> for remind :)
> >>
> >>> The problem: The internal speaker is at node 0x1f and you need to turn
> >>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
> >>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
> >>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
> >>> refactor it into hda_codec.c, or write something new?
> >>>
> >>> What are your ideas?
> >>
> >> We can simply turn on all EAPDs for 5066 and older chips.
> >> These codecs have just one or two EAPD pins, and we have no tightly
> >> coupled EAPD control (yet), so it'd be safe to turn all on.
> >
> > Or, maybe something like below would be cleverer...
> > (Totally untested, of course.)
> 
> Yes, very nice, I was thinking of something like that. That will be best 
> from a power consumption perspective. My only worry is that if turning 
> EAPD on and off causes pops (I don't know if they do), it will be better 
> to leave it on all the time.

Turning all EAPD on would be even much easier.
OTOH, we already control EAPD dynamically for the active pins, so
this behavior could be determined better by some switch.

> > From: Takashi Iwai<tiwai@suse.de>
> > Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
> >
> > There seem some machines referring to EAPD bit of the unassigned pin
> > for the speaker EAPD of a different pin, thus we need to control the
> > EAPD of unused pins as well.
> >
> > Signed-off-by: Takashi Iwai<tiwai@suse.de>
> > ---
> >   sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
> >   1 files changed, 41 insertions(+), 1 deletions(-)
> >
> > diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> > index 4ca880b..dbea3d0 100644
> > --- a/sound/pci/hda/patch_conexant.c
> > +++ b/sound/pci/hda/patch_conexant.c
> > @@ -155,6 +155,10 @@ struct conexant_spec {
> >   	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
> >
> >   	unsigned int beep_amp;
> > +
> > +	/* extra EAPD pins */
> > +	unsigned int num_eapds;
> > +	hda_nid_t eapds[4];
> >   };
> >
> >   static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> > @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >   	/* if LO is a copy of either HP or Speaker, don't need to handle it */
> >   	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
> >   	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> > -		return;
> > +		goto turn_extra_eapd;
> >   	if (spec->auto_mute) {
> >   		/* mute LO in auto-mode when HP jack is present */
> >   		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> > @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >   			on = 1;
> >   	}
> >   	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> > + turn_extra_eapd:
> > +	/* turn on/off extra EAPDs, too */
> > +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
> >   }
> >
> >   static void cx_auto_hp_automute(struct hda_codec *codec)
> > @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
> >   #define cx_auto_parse_beep(codec)
> >   #endif
> >
> > +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> > +{
> 
> Hmm, I've seen variations of this function at more than one place 
> already. Maybe a function like this would be good to have in hda_codec.c?
> 
> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
> {
> 	int i;
> 	for (i = 0; i < list_length; i++)
> 		if (list[i] == nid)
> 			return i;
> 	return -1;
> }
> 
> #define bool snd_hda_nid_exists(nid, list, list_length) 
> (snd_hda_find_nid(nid, list, list_length) != -1)

Yes, I was thinking of it (and actually wrote some code yesterday),
but for now I implemented it as runnable on 3.0 as is ;)
We can clean up later.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-29  9:34       ` Takashi Iwai
@ 2011-06-30  6:55         ` David Henningsson
  2011-06-30  7:04           ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: David Henningsson @ 2011-06-30  6:55 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 2011-06-29 10:34, Takashi Iwai wrote:
> At Wed, 29 Jun 2011 10:16:51 +0100,
> David Henningsson wrote:
>>
>> On 2011-06-29 07:46, Takashi Iwai wrote:
>>> At Tue, 28 Jun 2011 14:15:40 +0200,
>>> Takashi Iwai wrote:
>>>>
>>>> At Tue, 28 Jun 2011 12:44:19 +0100,
>>>> David Henningsson wrote:
>>>>>
>>>>> The new Conexant 5066 auto-parser is barely finished and already we
>>>>> might need a quirk system for it...
>>>>>
>>>>> Anyway, in this bug, the internal speaker is not working (and was not
>>>>> working before the auto-parser either). I'm attaching two patches that
>>>>> are quite simple, and at least the first one (add ID 506c) I think
>>>>> should be applied right away.
>>>>
>>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
>>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
>>>> for remind :)
>>>>
>>>>> The problem: The internal speaker is at node 0x1f and you need to turn
>>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
>>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
>>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
>>>>> refactor it into hda_codec.c, or write something new?
>>>>>
>>>>> What are your ideas?
>>>>
>>>> We can simply turn on all EAPDs for 5066 and older chips.
>>>> These codecs have just one or two EAPD pins, and we have no tightly
>>>> coupled EAPD control (yet), so it'd be safe to turn all on.
>>>
>>> Or, maybe something like below would be cleverer...
>>> (Totally untested, of course.)
>>
>> Yes, very nice, I was thinking of something like that. That will be best
>> from a power consumption perspective. My only worry is that if turning
>> EAPD on and off causes pops (I don't know if they do), it will be better
>> to leave it on all the time.
>
> Turning all EAPD on would be even much easier.
> OTOH, we already control EAPD dynamically for the active pins, so
> this behavior could be determined better by some switch.
>
>>> From: Takashi Iwai<tiwai@suse.de>
>>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
>>>
>>> There seem some machines referring to EAPD bit of the unassigned pin
>>> for the speaker EAPD of a different pin, thus we need to control the
>>> EAPD of unused pins as well.
>>>
>>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
>>> ---
>>>    sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
>>>    1 files changed, 41 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
>>> index 4ca880b..dbea3d0 100644
>>> --- a/sound/pci/hda/patch_conexant.c
>>> +++ b/sound/pci/hda/patch_conexant.c
>>> @@ -155,6 +155,10 @@ struct conexant_spec {
>>>    	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
>>>
>>>    	unsigned int beep_amp;
>>> +
>>> +	/* extra EAPD pins */
>>> +	unsigned int num_eapds;
>>> +	hda_nid_t eapds[4];
>>>    };
>>>
>>>    static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
>>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>>>    	/* if LO is a copy of either HP or Speaker, don't need to handle it */
>>>    	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
>>>    	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
>>> -		return;
>>> +		goto turn_extra_eapd;
>>>    	if (spec->auto_mute) {
>>>    		/* mute LO in auto-mode when HP jack is present */
>>>    		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
>>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>>>    			on = 1;
>>>    	}
>>>    	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
>>> + turn_extra_eapd:
>>> +	/* turn on/off extra EAPDs, too */
>>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
>>>    }
>>>
>>>    static void cx_auto_hp_automute(struct hda_codec *codec)
>>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
>>>    #define cx_auto_parse_beep(codec)
>>>    #endif
>>>
>>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
>>> +{
>>
>> Hmm, I've seen variations of this function at more than one place
>> already. Maybe a function like this would be good to have in hda_codec.c?
>>
>> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
>> {
>> 	int i;
>> 	for (i = 0; i<  list_length; i++)
>> 		if (list[i] == nid)
>> 			return i;
>> 	return -1;
>> }
>>
>> #define bool snd_hda_nid_exists(nid, list, list_length)
>> (snd_hda_find_nid(nid, list, list_length) != -1)
>
> Yes, I was thinking of it (and actually wrote some code yesterday),
> but for now I implemented it as runnable on 3.0 as is ;)

Sorry, did you actually commit this code (I didn't see it in your tree), 
or were you expecting me to ask downstream to test it first?

-- 
David Henningsson
http://launchpad.net/~diwic

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-30  6:55         ` David Henningsson
@ 2011-06-30  7:04           ` Takashi Iwai
  2011-07-04 11:02             ` David Henningsson
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-06-30  7:04 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Thu, 30 Jun 2011 07:55:58 +0100,
David Henningsson wrote:
> 
> On 2011-06-29 10:34, Takashi Iwai wrote:
> > At Wed, 29 Jun 2011 10:16:51 +0100,
> > David Henningsson wrote:
> >>
> >> On 2011-06-29 07:46, Takashi Iwai wrote:
> >>> At Tue, 28 Jun 2011 14:15:40 +0200,
> >>> Takashi Iwai wrote:
> >>>>
> >>>> At Tue, 28 Jun 2011 12:44:19 +0100,
> >>>> David Henningsson wrote:
> >>>>>
> >>>>> The new Conexant 5066 auto-parser is barely finished and already we
> >>>>> might need a quirk system for it...
> >>>>>
> >>>>> Anyway, in this bug, the internal speaker is not working (and was not
> >>>>> working before the auto-parser either). I'm attaching two patches that
> >>>>> are quite simple, and at least the first one (add ID 506c) I think
> >>>>> should be applied right away.
> >>>>
> >>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> >>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> >>>> for remind :)
> >>>>
> >>>>> The problem: The internal speaker is at node 0x1f and you need to turn
> >>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
> >>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
> >>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
> >>>>> refactor it into hda_codec.c, or write something new?
> >>>>>
> >>>>> What are your ideas?
> >>>>
> >>>> We can simply turn on all EAPDs for 5066 and older chips.
> >>>> These codecs have just one or two EAPD pins, and we have no tightly
> >>>> coupled EAPD control (yet), so it'd be safe to turn all on.
> >>>
> >>> Or, maybe something like below would be cleverer...
> >>> (Totally untested, of course.)
> >>
> >> Yes, very nice, I was thinking of something like that. That will be best
> >> from a power consumption perspective. My only worry is that if turning
> >> EAPD on and off causes pops (I don't know if they do), it will be better
> >> to leave it on all the time.
> >
> > Turning all EAPD on would be even much easier.
> > OTOH, we already control EAPD dynamically for the active pins, so
> > this behavior could be determined better by some switch.
> >
> >>> From: Takashi Iwai<tiwai@suse.de>
> >>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
> >>>
> >>> There seem some machines referring to EAPD bit of the unassigned pin
> >>> for the speaker EAPD of a different pin, thus we need to control the
> >>> EAPD of unused pins as well.
> >>>
> >>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> >>> ---
> >>>    sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
> >>>    1 files changed, 41 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> >>> index 4ca880b..dbea3d0 100644
> >>> --- a/sound/pci/hda/patch_conexant.c
> >>> +++ b/sound/pci/hda/patch_conexant.c
> >>> @@ -155,6 +155,10 @@ struct conexant_spec {
> >>>    	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
> >>>
> >>>    	unsigned int beep_amp;
> >>> +
> >>> +	/* extra EAPD pins */
> >>> +	unsigned int num_eapds;
> >>> +	hda_nid_t eapds[4];
> >>>    };
> >>>
> >>>    static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> >>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >>>    	/* if LO is a copy of either HP or Speaker, don't need to handle it */
> >>>    	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
> >>>    	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> >>> -		return;
> >>> +		goto turn_extra_eapd;
> >>>    	if (spec->auto_mute) {
> >>>    		/* mute LO in auto-mode when HP jack is present */
> >>>    		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> >>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >>>    			on = 1;
> >>>    	}
> >>>    	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> >>> + turn_extra_eapd:
> >>> +	/* turn on/off extra EAPDs, too */
> >>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
> >>>    }
> >>>
> >>>    static void cx_auto_hp_automute(struct hda_codec *codec)
> >>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
> >>>    #define cx_auto_parse_beep(codec)
> >>>    #endif
> >>>
> >>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> >>> +{
> >>
> >> Hmm, I've seen variations of this function at more than one place
> >> already. Maybe a function like this would be good to have in hda_codec.c?
> >>
> >> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
> >> {
> >> 	int i;
> >> 	for (i = 0; i<  list_length; i++)
> >> 		if (list[i] == nid)
> >> 			return i;
> >> 	return -1;
> >> }
> >>
> >> #define bool snd_hda_nid_exists(nid, list, list_length)
> >> (snd_hda_find_nid(nid, list, list_length) != -1)
> >
> > Yes, I was thinking of it (and actually wrote some code yesterday),
> > but for now I implemented it as runnable on 3.0 as is ;)
> 
> Sorry, did you actually commit this code (I didn't see it in your tree), 
> or were you expecting me to ask downstream to test it first?

Not committed yet.  At least for 3.0, I'd commit only the fix that was
tested.  If this can be tested quickly and the result is positive,
I'll queue it up for 3.0.  Otherwise put it to the branch for 3.1.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-06-30  7:04           ` Takashi Iwai
@ 2011-07-04 11:02             ` David Henningsson
  2011-07-04 11:05               ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: David Henningsson @ 2011-07-04 11:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 2011-06-30 09:04, Takashi Iwai wrote:
> At Thu, 30 Jun 2011 07:55:58 +0100,
> David Henningsson wrote:
>>
>> On 2011-06-29 10:34, Takashi Iwai wrote:
>>> At Wed, 29 Jun 2011 10:16:51 +0100,
>>> David Henningsson wrote:
>>>>
>>>> On 2011-06-29 07:46, Takashi Iwai wrote:
>>>>> At Tue, 28 Jun 2011 14:15:40 +0200,
>>>>> Takashi Iwai wrote:
>>>>>>
>>>>>> At Tue, 28 Jun 2011 12:44:19 +0100,
>>>>>> David Henningsson wrote:
>>>>>>>
>>>>>>> The new Conexant 5066 auto-parser is barely finished and already we
>>>>>>> might need a quirk system for it...
>>>>>>>
>>>>>>> Anyway, in this bug, the internal speaker is not working (and was not
>>>>>>> working before the auto-parser either). I'm attaching two patches that
>>>>>>> are quite simple, and at least the first one (add ID 506c) I think
>>>>>>> should be applied right away.
>>>>>>
>>>>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
>>>>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
>>>>>> for remind :)
>>>>>>
>>>>>>> The problem: The internal speaker is at node 0x1f and you need to turn
>>>>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
>>>>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
>>>>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
>>>>>>> refactor it into hda_codec.c, or write something new?
>>>>>>>
>>>>>>> What are your ideas?
>>>>>>
>>>>>> We can simply turn on all EAPDs for 5066 and older chips.
>>>>>> These codecs have just one or two EAPD pins, and we have no tightly
>>>>>> coupled EAPD control (yet), so it'd be safe to turn all on.
>>>>>
>>>>> Or, maybe something like below would be cleverer...
>>>>> (Totally untested, of course.)
>>>>
>>>> Yes, very nice, I was thinking of something like that. That will be best
>>>> from a power consumption perspective. My only worry is that if turning
>>>> EAPD on and off causes pops (I don't know if they do), it will be better
>>>> to leave it on all the time.
>>>
>>> Turning all EAPD on would be even much easier.
>>> OTOH, we already control EAPD dynamically for the active pins, so
>>> this behavior could be determined better by some switch.
>>>
>>>>> From: Takashi Iwai<tiwai@suse.de>
>>>>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
>>>>>
>>>>> There seem some machines referring to EAPD bit of the unassigned pin
>>>>> for the speaker EAPD of a different pin, thus we need to control the
>>>>> EAPD of unused pins as well.
>>>>>
>>>>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
>>>>> ---
>>>>>     sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
>>>>>     1 files changed, 41 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
>>>>> index 4ca880b..dbea3d0 100644
>>>>> --- a/sound/pci/hda/patch_conexant.c
>>>>> +++ b/sound/pci/hda/patch_conexant.c
>>>>> @@ -155,6 +155,10 @@ struct conexant_spec {
>>>>>     	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
>>>>>
>>>>>     	unsigned int beep_amp;
>>>>> +
>>>>> +	/* extra EAPD pins */
>>>>> +	unsigned int num_eapds;
>>>>> +	hda_nid_t eapds[4];
>>>>>     };
>>>>>
>>>>>     static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
>>>>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>>>>>     	/* if LO is a copy of either HP or Speaker, don't need to handle it */
>>>>>     	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
>>>>>     	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
>>>>> -		return;
>>>>> +		goto turn_extra_eapd;
>>>>>     	if (spec->auto_mute) {
>>>>>     		/* mute LO in auto-mode when HP jack is present */
>>>>>     		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
>>>>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
>>>>>     			on = 1;
>>>>>     	}
>>>>>     	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
>>>>> + turn_extra_eapd:
>>>>> +	/* turn on/off extra EAPDs, too */
>>>>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
>>>>>     }
>>>>>
>>>>>     static void cx_auto_hp_automute(struct hda_codec *codec)
>>>>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
>>>>>     #define cx_auto_parse_beep(codec)
>>>>>     #endif
>>>>>
>>>>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
>>>>> +{
>>>>
>>>> Hmm, I've seen variations of this function at more than one place
>>>> already. Maybe a function like this would be good to have in hda_codec.c?
>>>>
>>>> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
>>>> {
>>>> 	int i;
>>>> 	for (i = 0; i<   list_length; i++)
>>>> 		if (list[i] == nid)
>>>> 			return i;
>>>> 	return -1;
>>>> }
>>>>
>>>> #define bool snd_hda_nid_exists(nid, list, list_length)
>>>> (snd_hda_find_nid(nid, list, list_length) != -1)
>>>
>>> Yes, I was thinking of it (and actually wrote some code yesterday),
>>> but for now I implemented it as runnable on 3.0 as is ;)
>>
>> Sorry, did you actually commit this code (I didn't see it in your tree),
>> or were you expecting me to ask downstream to test it first?
>
> Not committed yet.  At least for 3.0, I'd commit only the fix that was
> tested.  If this can be tested quickly and the result is positive,
> I'll queue it up for 3.0.  Otherwise put it to the branch for 3.1.

Hmm, that came back with mixed reviews. First tester said it didn't help 
(still had to manually turn on EAPD on 0x1b), second tester said it did 
work, but that headphones stopped working instead...

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-07-04 11:02             ` David Henningsson
@ 2011-07-04 11:05               ` Takashi Iwai
  2011-07-04 16:06                 ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-07-04 11:05 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Mon, 04 Jul 2011 13:02:17 +0200,
David Henningsson wrote:
> 
> On 2011-06-30 09:04, Takashi Iwai wrote:
> > At Thu, 30 Jun 2011 07:55:58 +0100,
> > David Henningsson wrote:
> >>
> >> On 2011-06-29 10:34, Takashi Iwai wrote:
> >>> At Wed, 29 Jun 2011 10:16:51 +0100,
> >>> David Henningsson wrote:
> >>>>
> >>>> On 2011-06-29 07:46, Takashi Iwai wrote:
> >>>>> At Tue, 28 Jun 2011 14:15:40 +0200,
> >>>>> Takashi Iwai wrote:
> >>>>>>
> >>>>>> At Tue, 28 Jun 2011 12:44:19 +0100,
> >>>>>> David Henningsson wrote:
> >>>>>>>
> >>>>>>> The new Conexant 5066 auto-parser is barely finished and already we
> >>>>>>> might need a quirk system for it...
> >>>>>>>
> >>>>>>> Anyway, in this bug, the internal speaker is not working (and was not
> >>>>>>> working before the auto-parser either). I'm attaching two patches that
> >>>>>>> are quite simple, and at least the first one (add ID 506c) I think
> >>>>>>> should be applied right away.
> >>>>>>
> >>>>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> >>>>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> >>>>>> for remind :)
> >>>>>>
> >>>>>>> The problem: The internal speaker is at node 0x1f and you need to turn
> >>>>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
> >>>>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
> >>>>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
> >>>>>>> refactor it into hda_codec.c, or write something new?
> >>>>>>>
> >>>>>>> What are your ideas?
> >>>>>>
> >>>>>> We can simply turn on all EAPDs for 5066 and older chips.
> >>>>>> These codecs have just one or two EAPD pins, and we have no tightly
> >>>>>> coupled EAPD control (yet), so it'd be safe to turn all on.
> >>>>>
> >>>>> Or, maybe something like below would be cleverer...
> >>>>> (Totally untested, of course.)
> >>>>
> >>>> Yes, very nice, I was thinking of something like that. That will be best
> >>>> from a power consumption perspective. My only worry is that if turning
> >>>> EAPD on and off causes pops (I don't know if they do), it will be better
> >>>> to leave it on all the time.
> >>>
> >>> Turning all EAPD on would be even much easier.
> >>> OTOH, we already control EAPD dynamically for the active pins, so
> >>> this behavior could be determined better by some switch.
> >>>
> >>>>> From: Takashi Iwai<tiwai@suse.de>
> >>>>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
> >>>>>
> >>>>> There seem some machines referring to EAPD bit of the unassigned pin
> >>>>> for the speaker EAPD of a different pin, thus we need to control the
> >>>>> EAPD of unused pins as well.
> >>>>>
> >>>>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> >>>>> ---
> >>>>>     sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
> >>>>>     1 files changed, 41 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> >>>>> index 4ca880b..dbea3d0 100644
> >>>>> --- a/sound/pci/hda/patch_conexant.c
> >>>>> +++ b/sound/pci/hda/patch_conexant.c
> >>>>> @@ -155,6 +155,10 @@ struct conexant_spec {
> >>>>>     	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
> >>>>>
> >>>>>     	unsigned int beep_amp;
> >>>>> +
> >>>>> +	/* extra EAPD pins */
> >>>>> +	unsigned int num_eapds;
> >>>>> +	hda_nid_t eapds[4];
> >>>>>     };
> >>>>>
> >>>>>     static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> >>>>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >>>>>     	/* if LO is a copy of either HP or Speaker, don't need to handle it */
> >>>>>     	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
> >>>>>     	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> >>>>> -		return;
> >>>>> +		goto turn_extra_eapd;
> >>>>>     	if (spec->auto_mute) {
> >>>>>     		/* mute LO in auto-mode when HP jack is present */
> >>>>>     		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> >>>>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> >>>>>     			on = 1;
> >>>>>     	}
> >>>>>     	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> >>>>> + turn_extra_eapd:
> >>>>> +	/* turn on/off extra EAPDs, too */
> >>>>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
> >>>>>     }
> >>>>>
> >>>>>     static void cx_auto_hp_automute(struct hda_codec *codec)
> >>>>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
> >>>>>     #define cx_auto_parse_beep(codec)
> >>>>>     #endif
> >>>>>
> >>>>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> >>>>> +{
> >>>>
> >>>> Hmm, I've seen variations of this function at more than one place
> >>>> already. Maybe a function like this would be good to have in hda_codec.c?
> >>>>
> >>>> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
> >>>> {
> >>>> 	int i;
> >>>> 	for (i = 0; i<   list_length; i++)
> >>>> 		if (list[i] == nid)
> >>>> 			return i;
> >>>> 	return -1;
> >>>> }
> >>>>
> >>>> #define bool snd_hda_nid_exists(nid, list, list_length)
> >>>> (snd_hda_find_nid(nid, list, list_length) != -1)
> >>>
> >>> Yes, I was thinking of it (and actually wrote some code yesterday),
> >>> but for now I implemented it as runnable on 3.0 as is ;)
> >>
> >> Sorry, did you actually commit this code (I didn't see it in your tree),
> >> or were you expecting me to ask downstream to test it first?
> >
> > Not committed yet.  At least for 3.0, I'd commit only the fix that was
> > tested.  If this can be tested quickly and the result is positive,
> > I'll queue it up for 3.0.  Otherwise put it to the branch for 3.1.
> 
> Hmm, that came back with mixed reviews. First tester said it didn't help 
> (still had to manually turn on EAPD on 0x1b), second tester said it did 
> work, but that headphones stopped working instead...

The latter comment suggests that EAPD must be always on, then?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-07-04 11:05               ` Takashi Iwai
@ 2011-07-04 16:06                 ` Takashi Iwai
  2011-07-11 12:52                   ` Takashi Iwai
  0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2011-07-04 16:06 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Mon, 04 Jul 2011 13:05:44 +0200,
Takashi Iwai wrote:
> 
> At Mon, 04 Jul 2011 13:02:17 +0200,
> David Henningsson wrote:
> > 
> > On 2011-06-30 09:04, Takashi Iwai wrote:
> > > At Thu, 30 Jun 2011 07:55:58 +0100,
> > > David Henningsson wrote:
> > >>
> > >> On 2011-06-29 10:34, Takashi Iwai wrote:
> > >>> At Wed, 29 Jun 2011 10:16:51 +0100,
> > >>> David Henningsson wrote:
> > >>>>
> > >>>> On 2011-06-29 07:46, Takashi Iwai wrote:
> > >>>>> At Tue, 28 Jun 2011 14:15:40 +0200,
> > >>>>> Takashi Iwai wrote:
> > >>>>>>
> > >>>>>> At Tue, 28 Jun 2011 12:44:19 +0100,
> > >>>>>> David Henningsson wrote:
> > >>>>>>>
> > >>>>>>> The new Conexant 5066 auto-parser is barely finished and already we
> > >>>>>>> might need a quirk system for it...
> > >>>>>>>
> > >>>>>>> Anyway, in this bug, the internal speaker is not working (and was not
> > >>>>>>> working before the auto-parser either). I'm attaching two patches that
> > >>>>>>> are quite simple, and at least the first one (add ID 506c) I think
> > >>>>>>> should be applied right away.
> > >>>>>>
> > >>>>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> > >>>>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> > >>>>>> for remind :)
> > >>>>>>
> > >>>>>>> The problem: The internal speaker is at node 0x1f and you need to turn
> > >>>>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
> > >>>>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
> > >>>>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
> > >>>>>>> refactor it into hda_codec.c, or write something new?
> > >>>>>>>
> > >>>>>>> What are your ideas?
> > >>>>>>
> > >>>>>> We can simply turn on all EAPDs for 5066 and older chips.
> > >>>>>> These codecs have just one or two EAPD pins, and we have no tightly
> > >>>>>> coupled EAPD control (yet), so it'd be safe to turn all on.
> > >>>>>
> > >>>>> Or, maybe something like below would be cleverer...
> > >>>>> (Totally untested, of course.)
> > >>>>
> > >>>> Yes, very nice, I was thinking of something like that. That will be best
> > >>>> from a power consumption perspective. My only worry is that if turning
> > >>>> EAPD on and off causes pops (I don't know if they do), it will be better
> > >>>> to leave it on all the time.
> > >>>
> > >>> Turning all EAPD on would be even much easier.
> > >>> OTOH, we already control EAPD dynamically for the active pins, so
> > >>> this behavior could be determined better by some switch.
> > >>>
> > >>>>> From: Takashi Iwai<tiwai@suse.de>
> > >>>>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
> > >>>>>
> > >>>>> There seem some machines referring to EAPD bit of the unassigned pin
> > >>>>> for the speaker EAPD of a different pin, thus we need to control the
> > >>>>> EAPD of unused pins as well.
> > >>>>>
> > >>>>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> > >>>>> ---
> > >>>>>     sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
> > >>>>>     1 files changed, 41 insertions(+), 1 deletions(-)
> > >>>>>
> > >>>>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> > >>>>> index 4ca880b..dbea3d0 100644
> > >>>>> --- a/sound/pci/hda/patch_conexant.c
> > >>>>> +++ b/sound/pci/hda/patch_conexant.c
> > >>>>> @@ -155,6 +155,10 @@ struct conexant_spec {
> > >>>>>     	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
> > >>>>>
> > >>>>>     	unsigned int beep_amp;
> > >>>>> +
> > >>>>> +	/* extra EAPD pins */
> > >>>>> +	unsigned int num_eapds;
> > >>>>> +	hda_nid_t eapds[4];
> > >>>>>     };
> > >>>>>
> > >>>>>     static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> > >>>>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> > >>>>>     	/* if LO is a copy of either HP or Speaker, don't need to handle it */
> > >>>>>     	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
> > >>>>>     	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> > >>>>> -		return;
> > >>>>> +		goto turn_extra_eapd;
> > >>>>>     	if (spec->auto_mute) {
> > >>>>>     		/* mute LO in auto-mode when HP jack is present */
> > >>>>>     		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> > >>>>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> > >>>>>     			on = 1;
> > >>>>>     	}
> > >>>>>     	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> > >>>>> + turn_extra_eapd:
> > >>>>> +	/* turn on/off extra EAPDs, too */
> > >>>>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
> > >>>>>     }
> > >>>>>
> > >>>>>     static void cx_auto_hp_automute(struct hda_codec *codec)
> > >>>>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
> > >>>>>     #define cx_auto_parse_beep(codec)
> > >>>>>     #endif
> > >>>>>
> > >>>>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> > >>>>> +{
> > >>>>
> > >>>> Hmm, I've seen variations of this function at more than one place
> > >>>> already. Maybe a function like this would be good to have in hda_codec.c?
> > >>>>
> > >>>> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
> > >>>> {
> > >>>> 	int i;
> > >>>> 	for (i = 0; i<   list_length; i++)
> > >>>> 		if (list[i] == nid)
> > >>>> 			return i;
> > >>>> 	return -1;
> > >>>> }
> > >>>>
> > >>>> #define bool snd_hda_nid_exists(nid, list, list_length)
> > >>>> (snd_hda_find_nid(nid, list, list_length) != -1)
> > >>>
> > >>> Yes, I was thinking of it (and actually wrote some code yesterday),
> > >>> but for now I implemented it as runnable on 3.0 as is ;)
> > >>
> > >> Sorry, did you actually commit this code (I didn't see it in your tree),
> > >> or were you expecting me to ask downstream to test it first?
> > >
> > > Not committed yet.  At least for 3.0, I'd commit only the fix that was
> > > tested.  If this can be tested quickly and the result is positive,
> > > I'll queue it up for 3.0.  Otherwise put it to the branch for 3.1.
> > 
> > Hmm, that came back with mixed reviews. First tester said it didn't help 
> > (still had to manually turn on EAPD on 0x1b), second tester said it did 
> > work, but that headphones stopped working instead...
> 
> The latter comment suggests that EAPD must be always on, then?

So, what about the patch below instead?


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 4ca880b..884f67b 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -155,6 +155,10 @@ struct conexant_spec {
 	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
 
 	unsigned int beep_amp;
+
+	/* extra EAPD pins */
+	unsigned int num_eapds;
+	hda_nid_t eapds[4];
 };
 
 static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
@@ -3901,6 +3905,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
 #define cx_auto_parse_beep(codec)
 #endif
 
+static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
+{
+	int i;
+	for (i = 0; i < nums; i++)
+		if (list[i] == nid)
+			return true;
+	return false;
+}
+
+/* parse extra-EAPD that aren't assigned to any pins */
+static void cx_auto_parse_eapd(struct hda_codec *codec)
+{
+	struct conexant_spec *spec = codec->spec;
+	struct auto_pin_cfg *cfg = &spec->autocfg;
+	hda_nid_t nid, end_nid;
+
+	end_nid = codec->start_nid + codec->num_nodes;
+	for (nid = codec->start_nid; nid < end_nid; nid++) {
+		if (get_wcaps_type(get_wcaps(codec, nid)) != AC_WID_PIN)
+			continue;
+		if (!(snd_hda_query_pin_caps(codec, nid) & AC_PINCAP_EAPD))
+			continue;
+		if (found_in_nid_list(nid, cfg->line_out_pins, cfg->line_outs) ||
+		    found_in_nid_list(nid, cfg->hp_pins, cfg->hp_outs) ||
+		    found_in_nid_list(nid, cfg->speaker_pins, cfg->speaker_outs))
+			continue;
+		spec->eapds[spec->num_eapds++] = nid;
+		if (spec->num_eapds >= ARRAY_SIZE(spec->eapds))
+			break;
+	}
+}
+
 static int cx_auto_parse_auto_config(struct hda_codec *codec)
 {
 	struct conexant_spec *spec = codec->spec;
@@ -3914,6 +3950,7 @@ static int cx_auto_parse_auto_config(struct hda_codec *codec)
 	cx_auto_parse_input(codec);
 	cx_auto_parse_digital(codec);
 	cx_auto_parse_beep(codec);
+	cx_auto_parse_eapd(codec);
 	return 0;
 }
 
@@ -4001,6 +4038,8 @@ static void cx_auto_init_output(struct hda_codec *codec)
 		}
 	}
 	cx_auto_update_speakers(codec);
+	/* turn on/off extra EAPDs, too */
+	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, true);
 }
 
 static void cx_auto_init_input(struct hda_codec *codec)

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588
  2011-07-04 16:06                 ` Takashi Iwai
@ 2011-07-11 12:52                   ` Takashi Iwai
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2011-07-11 12:52 UTC (permalink / raw)
  To: alsa-devel

At Mon, 04 Jul 2011 18:06:06 +0200,
Takashi Iwai wrote:
> 
> At Mon, 04 Jul 2011 13:05:44 +0200,
> Takashi Iwai wrote:
> > 
> > At Mon, 04 Jul 2011 13:02:17 +0200,
> > David Henningsson wrote:
> > > 
> > > On 2011-06-30 09:04, Takashi Iwai wrote:
> > > > At Thu, 30 Jun 2011 07:55:58 +0100,
> > > > David Henningsson wrote:
> > > >>
> > > >> On 2011-06-29 10:34, Takashi Iwai wrote:
> > > >>> At Wed, 29 Jun 2011 10:16:51 +0100,
> > > >>> David Henningsson wrote:
> > > >>>>
> > > >>>> On 2011-06-29 07:46, Takashi Iwai wrote:
> > > >>>>> At Tue, 28 Jun 2011 14:15:40 +0200,
> > > >>>>> Takashi Iwai wrote:
> > > >>>>>>
> > > >>>>>> At Tue, 28 Jun 2011 12:44:19 +0100,
> > > >>>>>> David Henningsson wrote:
> > > >>>>>>>
> > > >>>>>>> The new Conexant 5066 auto-parser is barely finished and already we
> > > >>>>>>> might need a quirk system for it...
> > > >>>>>>>
> > > >>>>>>> Anyway, in this bug, the internal speaker is not working (and was not
> > > >>>>>>> working before the auto-parser either). I'm attaching two patches that
> > > >>>>>>> are quite simple, and at least the first one (add ID 506c) I think
> > > >>>>>>> should be applied right away.
> > > >>>>>>
> > > >>>>>> Thanks, applied to fix/hda for 3.0-rc.  Meanwhile, I'll change the
> > > >>>>>> branch for 3.1 to enable model=auto as default for Conexant (Thanks
> > > >>>>>> for remind :)
> > > >>>>>>
> > > >>>>>>> The problem: The internal speaker is at node 0x1f and you need to turn
> > > >>>>>>> on EAPD on node 0x1b for the speaker to sound. Now I'm not sure how to
> > > >>>>>>> fix this in the best way. An "init verb" quirk, or a "use EAPD from this
> > > >>>>>>> pin" quirk? Try to copy-paste realtek's quirk system, and if so,
> > > >>>>>>> refactor it into hda_codec.c, or write something new?
> > > >>>>>>>
> > > >>>>>>> What are your ideas?
> > > >>>>>>
> > > >>>>>> We can simply turn on all EAPDs for 5066 and older chips.
> > > >>>>>> These codecs have just one or two EAPD pins, and we have no tightly
> > > >>>>>> coupled EAPD control (yet), so it'd be safe to turn all on.
> > > >>>>>
> > > >>>>> Or, maybe something like below would be cleverer...
> > > >>>>> (Totally untested, of course.)
> > > >>>>
> > > >>>> Yes, very nice, I was thinking of something like that. That will be best
> > > >>>> from a power consumption perspective. My only worry is that if turning
> > > >>>> EAPD on and off causes pops (I don't know if they do), it will be better
> > > >>>> to leave it on all the time.
> > > >>>
> > > >>> Turning all EAPD on would be even much easier.
> > > >>> OTOH, we already control EAPD dynamically for the active pins, so
> > > >>> this behavior could be determined better by some switch.
> > > >>>
> > > >>>>> From: Takashi Iwai<tiwai@suse.de>
> > > >>>>> Subject: [PATCH] ALSA: hda - Handle extra EAPD in Conexant auto-parser
> > > >>>>>
> > > >>>>> There seem some machines referring to EAPD bit of the unassigned pin
> > > >>>>> for the speaker EAPD of a different pin, thus we need to control the
> > > >>>>> EAPD of unused pins as well.
> > > >>>>>
> > > >>>>> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> > > >>>>> ---
> > > >>>>>     sound/pci/hda/patch_conexant.c |   42 +++++++++++++++++++++++++++++++++++++++-
> > > >>>>>     1 files changed, 41 insertions(+), 1 deletions(-)
> > > >>>>>
> > > >>>>> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> > > >>>>> index 4ca880b..dbea3d0 100644
> > > >>>>> --- a/sound/pci/hda/patch_conexant.c
> > > >>>>> +++ b/sound/pci/hda/patch_conexant.c
> > > >>>>> @@ -155,6 +155,10 @@ struct conexant_spec {
> > > >>>>>     	unsigned int mic_boost; /* offset into cxt5066_analog_mic_boost */
> > > >>>>>
> > > >>>>>     	unsigned int beep_amp;
> > > >>>>> +
> > > >>>>> +	/* extra EAPD pins */
> > > >>>>> +	unsigned int num_eapds;
> > > >>>>> +	hda_nid_t eapds[4];
> > > >>>>>     };
> > > >>>>>
> > > >>>>>     static int conexant_playback_pcm_open(struct hda_pcm_stream *hinfo,
> > > >>>>> @@ -3485,7 +3489,7 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> > > >>>>>     	/* if LO is a copy of either HP or Speaker, don't need to handle it */
> > > >>>>>     	if (cfg->line_out_pins[0] == cfg->hp_pins[0] ||
> > > >>>>>     	    cfg->line_out_pins[0] == cfg->speaker_pins[0])
> > > >>>>> -		return;
> > > >>>>> +		goto turn_extra_eapd;
> > > >>>>>     	if (spec->auto_mute) {
> > > >>>>>     		/* mute LO in auto-mode when HP jack is present */
> > > >>>>>     		if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT ||
> > > >>>>> @@ -3495,6 +3499,9 @@ static void cx_auto_update_speakers(struct hda_codec *codec)
> > > >>>>>     			on = 1;
> > > >>>>>     	}
> > > >>>>>     	do_automute(codec, cfg->line_outs, cfg->line_out_pins, on);
> > > >>>>> + turn_extra_eapd:
> > > >>>>> +	/* turn on/off extra EAPDs, too */
> > > >>>>> +	cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, on);
> > > >>>>>     }
> > > >>>>>
> > > >>>>>     static void cx_auto_hp_automute(struct hda_codec *codec)
> > > >>>>> @@ -3901,6 +3908,38 @@ static void cx_auto_parse_beep(struct hda_codec *codec)
> > > >>>>>     #define cx_auto_parse_beep(codec)
> > > >>>>>     #endif
> > > >>>>>
> > > >>>>> +static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums)
> > > >>>>> +{
> > > >>>>
> > > >>>> Hmm, I've seen variations of this function at more than one place
> > > >>>> already. Maybe a function like this would be good to have in hda_codec.c?
> > > >>>>
> > > >>>> int snd_hda_find_nid(hda_nid_t nid, const hda_nid_t *list, int list_length)
> > > >>>> {
> > > >>>> 	int i;
> > > >>>> 	for (i = 0; i<   list_length; i++)
> > > >>>> 		if (list[i] == nid)
> > > >>>> 			return i;
> > > >>>> 	return -1;
> > > >>>> }
> > > >>>>
> > > >>>> #define bool snd_hda_nid_exists(nid, list, list_length)
> > > >>>> (snd_hda_find_nid(nid, list, list_length) != -1)
> > > >>>
> > > >>> Yes, I was thinking of it (and actually wrote some code yesterday),
> > > >>> but for now I implemented it as runnable on 3.0 as is ;)
> > > >>
> > > >> Sorry, did you actually commit this code (I didn't see it in your tree),
> > > >> or were you expecting me to ask downstream to test it first?
> > > >
> > > > Not committed yet.  At least for 3.0, I'd commit only the fix that was
> > > > tested.  If this can be tested quickly and the result is positive,
> > > > I'll queue it up for 3.0.  Otherwise put it to the branch for 3.1.
> > > 
> > > Hmm, that came back with mixed reviews. First tester said it didn't help 
> > > (still had to manually turn on EAPD on 0x1b), second tester said it did 
> > > work, but that headphones stopped working instead...
> > 
> > The latter comment suggests that EAPD must be always on, then?
> 
> So, what about the patch below instead?

FYI, this patch was already included in topic/hda branch
commit 19110595c89b2d606883b7cb99260c7e47fd2143.


Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-07-11 12:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-28 11:44 [PATCH] Acer aspire 3830TG and Conexant 506c/20588 David Henningsson
2011-06-28 12:15 ` Takashi Iwai
2011-06-29  6:46   ` Takashi Iwai
2011-06-29  9:16     ` David Henningsson
2011-06-29  9:34       ` Takashi Iwai
2011-06-30  6:55         ` David Henningsson
2011-06-30  7:04           ` Takashi Iwai
2011-07-04 11:02             ` David Henningsson
2011-07-04 11:05               ` Takashi Iwai
2011-07-04 16:06                 ` Takashi Iwai
2011-07-11 12:52                   ` 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.