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: [email protected] (3.3+) >>>>>> BugLink: https://bugs.launchpad.net/bugs/1040030 >>>>>> Signed-off-by: David Henningsson <[email protected]> >>>>>> --- >>>>>> 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. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic -- 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 [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
