From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Henningsson Subject: Re: [PATCH] Acer aspire 3830TG and Conexant 506c/20588 Date: Thu, 30 Jun 2011 07:55:58 +0100 Message-ID: <4E0C1DFE.8000605@canonical.com> References: <4E09BE93.8060503@canonical.com> <4E0AED83.6020005@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by alsa0.perex.cz (Postfix) with ESMTP id 60916244AC for ; Thu, 30 Jun 2011 08:56:02 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 >>> 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 >>> --- >>> 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