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 :) > 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. Takashi -- 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