From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750837Ab3EaNE0 (ORCPT ); Fri, 31 May 2013 09:04:26 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49744 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751052Ab3EaNET (ORCPT ); Fri, 31 May 2013 09:04:19 -0400 Date: Fri, 31 May 2013 15:05:00 +0200 Message-ID: From: Takashi Iwai To: Alex Riesen Cc: alsa-devel@alsa-project.org, Linux Kernel Mailing List Subject: Re: regression: from 3.8 to 3.9: headphones output no sound on Intel HDA, codec VIA VT1802 In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: multipart/mixed; boundary="Multipart_Fri_May_31_15:05:00_2013-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: text/plain; charset=US-ASCII At Fri, 31 May 2013 12:31:19 +0200, Alex Riesen wrote: > > On Wed, May 29, 2013 at 5:42 PM, Takashi Iwai wrote: > > At Fri, 24 May 2013 23:32:14 +0200, Alex Riesen wrote: > >> I'm sorry to say that I will not be able to test it in the next > >> 8 or so days: I'll be traveling and without this particular laptop > >> with me. I hope someone with similar model (Sytem76 Lemur lemu4, i7) > >> can provide some testing in the meantime. Otherwise, I'll test > >> it as soon as I get back. > > > > Don't worry, the bug is more difficult than I thought :) > > I was afraid you will say that sooner or later :) Better than too late, no? ;) > > When you back, could you try the following and give the outputs? > > ... > > So, you'll get 4 alsa-info.sh outputs (step 3, 6, 7 and 8), and 4 > > trace logs (step 5, 6 and 7). > > Done. That's a lot of data, so please download it from here: > > http://familie-riesen.de/~raa/public/test/no-cached-write-test.tar.bz2 > > The system boot is in linuxrc-safemode, and the test itself in test.sh. > The logs and transcript in report/. Thanks a lot. Strangely, in these logs, I see no trace of setting the pin 0x25 with control 0x00. Also EAPD isn't changed. So, I have to conclude again that it's the hardware who resets these controls. Below is a series of patches. For simplicity, I just attach them, not inlining to the mail. They should be applicable cleanly to 3.9.4 as well. Let me know if this works. If this still doesn't work, I need to rewrite the patch to correct the pin-ctl / EAPD of the headphone pin after changing the speaker pin. Takashi --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0001-ALSA-hda-via-Disable-broken-dynamic-power-control.patch" Content-Transfer-Encoding: 7bit >>From 8ea5cb2a2a2a5cb6ba07cf6df5cf42f98489897f Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 31 May 2013 13:54:10 +0200 Subject: [PATCH 1/3] ALSA: hda/via - Disable broken dynamic power control Since the transition to the generic parser, the actual routes used there don't match always with the assumed static paths in some set_widgets_power_state callbacks. This results in the wrong power setup in the end. As a temporary workaround, we need to disable the calls together with the non-functional dynamic power control enum. Reported-by: Alex Riesen Cc: [v3.9] Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_via.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index e0dadcf..2f16605 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -231,9 +231,14 @@ static void vt1708_update_hp_work(struct hda_codec *codec) static void set_widgets_power_state(struct hda_codec *codec) { +#if 0 /* FIXME: the assumed connections don't match always with the + * actual routes by the generic parser, so better to disable + * the control for safety. + */ struct via_spec *spec = codec->spec; if (spec->set_widgets_power_state) spec->set_widgets_power_state(codec); +#endif } static void update_power_state(struct hda_codec *codec, hda_nid_t nid, @@ -435,8 +440,10 @@ static int via_build_controls(struct hda_codec *codec) if (err < 0) return err; +#if 0 /* FIXME: no dynamic power control, see set_widgets_power_state() */ if (spec->set_widgets_power_state) spec->mixers[spec->num_mixers++] = via_pin_power_ctl_enum; +#endif for (i = 0; i < spec->num_mixers; i++) { err = snd_hda_add_new_ctls(codec, spec->mixers[i]); -- 1.8.3 --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: text/plain; charset=US-ASCII --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0002-ALSA-hda-Allow-setting-automute-automic-hooks-after-.patch" >>From 5c7a129c62145fb21ee353b325506117a5696d27 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 31 May 2013 14:10:03 +0200 Subject: [PATCH 2/3] ALSA: hda - Allow setting automute/automic hooks after parsing Some codec drivers (VIA codecs and some Realtek fixups) set the automute and automic hooks after calling snd_hda_gen_parse_auto_config(). In the current code, the hook pointers are referred only in snd_hda_gen_parse_auto_config() and passed to snd_hda_jack_detect_enable_callback(), thus changing the hook values won't change the actually called callbacks properly. This patch fixes this bug by setting the static functions as the primary callback functions for the jack detection, and let them calling the appropriate hooks dynamically. Cc: [v3.9] Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_generic.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index ae85bbd2..fbc10b6 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3875,6 +3875,36 @@ static void update_automute_all(struct hda_codec *codec) snd_hda_gen_mic_autoswitch(codec, NULL); } +/* call appropriate hooks */ +static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack) +{ + struct hda_gen_spec *spec = codec->spec; + if (spec->hp_automute_hook) + spec->hp_automute_hook(codec, jack); + else + snd_hda_gen_hp_automute(codec, jack); +} + +static void call_line_automute(struct hda_codec *codec, + struct hda_jack_tbl *jack) +{ + struct hda_gen_spec *spec = codec->spec; + if (spec->line_automute_hook) + spec->line_automute_hook(codec, jack); + else + snd_hda_gen_line_automute(codec, jack); +} + +static void call_mic_autoswitch(struct hda_codec *codec, + struct hda_jack_tbl *jack) +{ + struct hda_gen_spec *spec = codec->spec; + if (spec->mic_autoswitch_hook) + spec->mic_autoswitch_hook(codec, jack); + else + snd_hda_gen_mic_autoswitch(codec, jack); +} + /* * Auto-Mute mode mixer enum support */ @@ -4009,9 +4039,7 @@ static int check_auto_mute_availability(struct hda_codec *codec) snd_printdd("hda-codec: Enable HP auto-muting on NID 0x%x\n", nid); snd_hda_jack_detect_enable_callback(codec, nid, HDA_GEN_HP_EVENT, - spec->hp_automute_hook ? - spec->hp_automute_hook : - snd_hda_gen_hp_automute); + call_hp_automute); spec->detect_hp = 1; } @@ -4024,9 +4052,7 @@ static int check_auto_mute_availability(struct hda_codec *codec) snd_printdd("hda-codec: Enable Line-Out auto-muting on NID 0x%x\n", nid); snd_hda_jack_detect_enable_callback(codec, nid, HDA_GEN_FRONT_EVENT, - spec->line_automute_hook ? - spec->line_automute_hook : - snd_hda_gen_line_automute); + call_line_automute); spec->detect_lo = 1; } spec->automute_lo_possible = spec->detect_hp; @@ -4068,9 +4094,7 @@ static bool auto_mic_check_imux(struct hda_codec *codec) snd_hda_jack_detect_enable_callback(codec, spec->am_entry[i].pin, HDA_GEN_MIC_EVENT, - spec->mic_autoswitch_hook ? - spec->mic_autoswitch_hook : - snd_hda_gen_mic_autoswitch); + call_mic_autoswitch); return true; } -- 1.8.3 --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: text/plain; charset=US-ASCII --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="0003-ALSA-hda-Add-volatile_pin_ctl-flag-to-generic-parser.patch" >>From 83aca44cf2ef21b4dd57e92d667331e323f08a31 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 31 May 2013 14:24:26 +0200 Subject: [PATCH 3/3] ALSA: hda - Add volatile_pin_ctl flag to generic parser It turned out that some VIA codecs handle the pin control value and EAPD dynamically in the hardware level even if you change the values explicitly in the driver. The driver assumes that the values are static until the driver changes the value itself, and this inconsistency resulted in unexpected surprises like the missing headphone outputs. This patch adds a flag to hda_gen_spec indicating that the codec may reset pins dynamically, so that the init and the automute functions will write the pin control and EAPD at each time even if it's same as the cached value. Reported-by: Alex Riesen Cc: [v3.9] Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_generic.c | 20 +++++++++++++++----- sound/pci/hda/hda_generic.h | 1 + sound/pci/hda/patch_via.c | 1 + 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index fbc10b6..29c9d00 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -175,13 +175,24 @@ static void parse_user_hints(struct hda_codec *codec) spec->mixer_nid = val; } +/* write the cached verb; if volatile_pin_ctl is set, don't trust caches */ +static void update_pin_verb(struct hda_codec *codec, hda_nid_t nid, + int verb, int val) +{ + struct hda_gen_spec *spec = codec->spec; + + if (spec->volatile_pin_ctl) + snd_hda_codec_write_cache(codec, nid, 0, verb, val); + else + snd_hda_codec_update_cache(codec, nid, 0, verb, val); +} + /* * pin control value accesses */ #define update_pin_ctl(codec, pin, val) \ - snd_hda_codec_update_cache(codec, pin, 0, \ - AC_VERB_SET_PIN_WIDGET_CONTROL, val) + update_pin_verb(codec, pin, AC_VERB_SET_PIN_WIDGET_CONTROL, val) /* restore the pinctl based on the cached value */ static inline void restore_pin_ctl(struct hda_codec *codec, hda_nid_t pin) @@ -788,9 +799,8 @@ static void set_pin_eapd(struct hda_codec *codec, hda_nid_t pin, bool enable) return; if (codec->inv_eapd) enable = !enable; - snd_hda_codec_update_cache(codec, pin, 0, - AC_VERB_SET_EAPD_BTLENABLE, - enable ? 0x02 : 0x00); + update_pin_verb(codec, pin, AC_VERB_SET_EAPD_BTLENABLE, + enable ? 0x02 : 0x00); } /* re-initialize the path specified by the given path index */ diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h index 54e6651..2d70a7d0 100644 --- a/sound/pci/hda/hda_generic.h +++ b/sound/pci/hda/hda_generic.h @@ -222,6 +222,7 @@ struct hda_gen_spec { unsigned int multi_cap_vol:1; /* allow multiple capture xxx volumes */ unsigned int inv_dmic_split:1; /* inverted dmic w/a for conexant */ unsigned int own_eapd_ctl:1; /* set EAPD by own function */ + unsigned int volatile_pin_ctl:1; /* pin contrl&EAPDs are volatile */ unsigned int vmaster_mute_enum:1; /* add vmaster mute mode enum */ unsigned int indep_hp:1; /* independent HP supported */ unsigned int prefer_hp_amp:1; /* enable HP amp for speaker if any */ diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 2f16605..4bbd4f3 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -136,6 +136,7 @@ static struct via_spec *via_new_spec(struct hda_codec *codec) spec->codec_type = VT1708S; spec->no_pin_power_ctl = 1; spec->gen.indep_hp = 1; + spec->gen.volatile_pin_ctl = 1; spec->gen.pcm_playback_hook = via_playback_pcm_hook; return spec; } -- 1.8.3 --Multipart_Fri_May_31_15:05:00_2013-1 Content-Type: text/plain; charset=US-ASCII --Multipart_Fri_May_31_15:05:00_2013-1--