At Wed, 22 Aug 2012 16:27:35 +0200, David Henningsson wrote: > > On 08/22/2012 03:52 PM, Takashi Iwai wrote: > > At Wed, 22 Aug 2012 15:46:59 +0200, > > David Henningsson wrote: > >> > >> On 08/22/2012 02:58 PM, Takashi Iwai wrote: > >>> At Wed, 22 Aug 2012 14:39:17 +0200, > >>> David Henningsson wrote: > >>>> > >>>> On 08/22/2012 02:22 PM, Takashi Iwai wrote: > >>>>> At Wed, 22 Aug 2012 14:01:41 +0200, > >>>>> David Henningsson wrote: > >>>>>> > >>>>>> The HDMI codec (an NVIDIA one in this case) forgot that its pins > >>>>>> were unsol enabled, while it was suspended. Therefore jack detection > >>>>>> was broken after S3. > >>>>>> With this patch, we reenable the unsol events on resume, > >>>>>> and also do an extra check afterwards, to see if the HDMI monitor was > >>>>>> plugged/unplugged while in S3. > >>>>>> > >>>>>> Cc: sta...@kernel.org (3.3+) > >>>>>> BugLink: https://bugs.launchpad.net/bugs/1040030 > >>>>>> Signed-off-by: David Henningsson <david.hennings...@canonical.com> > >>>>>> --- > >>>>>> sound/pci/hda/patch_hdmi.c | 13 +++++++++++++ > >>>>>> 1 file changed, 13 insertions(+) > >>>>>> > >>>>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > >>>>>> index 8f23374..6a3ac05 100644 > >>>>>> --- a/sound/pci/hda/patch_hdmi.c > >>>>>> +++ b/sound/pci/hda/patch_hdmi.c > >>>>>> @@ -1315,6 +1315,16 @@ static int generic_hdmi_init(struct hda_codec > >>>>>> *codec) > >>>>>> return 0; > >>>>>> } > >>>>>> > >>>>>> +#ifdef CONFIG_PM > >>>>>> +static int generic_hdmi_resume(struct hda_codec *codec) > >>>>>> +{ > >>>>>> + snd_hda_codec_resume_cache(codec); > >>>>>> + snd_hda_jack_set_dirty_all(codec); > >>>>>> + snd_hda_jack_report_sync(codec); > >>>>>> + return 0; > >>>>> > >>>>> Hm, is this really needed? > >>>>> > >>>>> snd_hda_jack_set_dirty_all() is already called in > >>>>> hda_call_codec_resume(), and snd_hda_jack_report_sync() is called in > >>>>> the init callback. > >>>> > >>>> The tester (who has the hardware) has gone for the day, so I can't > >>>> really verify different scenarios right now, but after having looked at > >>>> hda_call_codec_resume I see what you mean... > >>>> > >>>> I do notice one difference though - the order. > >>>> snd_hda_codec_resume_cache, which is what writes the unsol_enable verbs, > >>>> should probably be before the set_dirty_all / report_sync. If not for > >>>> anything else, so for the race condition of somebody plugging/unplugging > >>>> the monitor after checking the jack but before the unsol is enabled. > >>> > >>> Calling snd_hda_jack_report_sync() in init means that all jacks are > >>> checked at first, then the caches are resumed. So this won't change > >>> ENABLE_UNSOL verb setups. > >> > >> I'm not sure I'm following. Here's the order in hda_call_codec_resume: > >> > >> snd_hda_jack_set_dirty_all() > >> generic_hdmi_init -> snd_hda_jack_report_sync() - get_pin_sense > >> snd_hda_codec_resume_cache() - set_unsol_enable > >> > >> With the (theoretical?) race condition being a change of pin sense > >> between snd_hda_jack_report_sync() and snd_hda_codec_resume_cache(), > >> which will not be picked up. > > > > In that race, yes. But this should be irrelevant with this bug :) > > Yes, the bug needs further verification. > > >> The patch changed the order to: > >> > >> snd_hda_codec_resume_cache() - set_unsol_enable > >> snd_hda_jack_set_dirty_all() > >> snd_hda_jack_report_sync() - get_pin_sense > >> > >> Which eliminates that race condition. > > > > Well, what I'm thinking now is rather to call > > snd_hda_jack_report_sync() generically in hda_call_codec_resume(). > > The call doesn't have to be in all places. > > Ditto for the initialization case. It'll clean up many lines. > > Fine with me - the more code being generic, the better.
OK, here we go. Takashi === From: Takashi Iwai <ti...@suse.de> Subject: [PATCH] ALSA: hda - Call snd_hda_jack_report_sync() generically in hda_codec.c Instead of calling the jack sync in the init callback of each codec, call it generically at initialization and resume. By calling it at the last of resume sequence, a possible race between the jack sync and the unsol event enablement in the current code will be closed, too. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/pci/hda/hda_codec.c | 2 ++ sound/pci/hda/patch_cirrus.c | 2 -- sound/pci/hda/patch_conexant.c | 1 - sound/pci/hda/patch_hdmi.c | 2 -- sound/pci/hda/patch_realtek.c | 2 -- sound/pci/hda/patch_sigmatel.c | 2 -- sound/pci/hda/patch_via.c | 1 - 7 files changed, 2 insertions(+), 10 deletions(-) diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index a6c34dc..4efd271 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3618,6 +3618,7 @@ static void hda_call_codec_resume(struct hda_codec *codec) snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); } + snd_hda_jack_report_sync(codec); snd_hda_power_down(codec); /* flag down before returning */ } #endif /* CONFIG_PM */ @@ -3663,6 +3664,7 @@ int snd_hda_codec_build_controls(struct hda_codec *codec) err = codec->patch_ops.build_controls(codec); if (err < 0) return err; + snd_hda_jack_report_sync(codec); /* call at the last init point */ return 0; } diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c index 0c4c1a6..0bddb3e 100644 --- a/sound/pci/hda/patch_cirrus.c +++ b/sound/pci/hda/patch_cirrus.c @@ -1193,7 +1193,6 @@ static int cs_init(struct hda_codec *codec) init_output(codec); init_input(codec); init_digital(codec); - snd_hda_jack_report_sync(codec); return 0; } @@ -1643,7 +1642,6 @@ static int cs421x_init(struct hda_codec *codec) init_output(codec); init_input(codec); init_cs421x_digital(codec); - snd_hda_jack_report_sync(codec); return 0; } diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index 5e22a8f..172895a 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -4061,7 +4061,6 @@ static int cx_auto_init(struct hda_codec *codec) cx_auto_init_output(codec); cx_auto_init_input(codec); cx_auto_init_digital(codec); - snd_hda_jack_report_sync(codec); snd_hda_sync_vmaster_hook(&spec->vmaster_mute); return 0; } diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8f23374..d9439c5 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1311,7 +1311,6 @@ static int generic_hdmi_init(struct hda_codec *codec) hdmi_init_pin(codec, pin_nid); snd_hda_jack_detect_enable(codec, pin_nid, pin_nid); } - snd_hda_jack_report_sync(codec); return 0; } @@ -1428,7 +1427,6 @@ static int simple_playback_init(struct hda_codec *codec) snd_hda_codec_write(codec, pin, 0, AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE); snd_hda_jack_detect_enable(codec, pin, pin); - snd_hda_jack_report_sync(codec); return 0; } diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 4f81dd4..ce99cc9 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -2053,8 +2053,6 @@ static int alc_init(struct hda_codec *codec) alc_apply_fixup(codec, ALC_FIXUP_ACT_INIT); - snd_hda_jack_report_sync(codec); - hda_call_check_power_status(codec, 0x01); return 0; } diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index ea5775a..4352954 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -4418,8 +4418,6 @@ static int stac92xx_init(struct hda_codec *codec) stac_toggle_power_map(codec, nid, 0); } - snd_hda_jack_report_sync(codec); - /* sync mute LED */ if (spec->gpio_led) { if (spec->vmaster_mute.hook) diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 4307717..4b0796b 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -2815,7 +2815,6 @@ static int via_init(struct hda_codec *codec) via_hp_automute(codec); vt1708_update_hp_work(spec); - snd_hda_jack_report_sync(codec); return 0; } -- 1.7.11.4 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1040030 Title: HDMI jack detection broken after suspend/resume To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1040030/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs