[alsa-devel] [PATCH] Acer aspire 3830TG and Conexant 506c/20588

[Takashi Iwai]

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 at 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 at 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