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: stable@kernel.org (3.3+) BugLink: https://bugs.launchpad.net/bugs/1040030 Signed-off-by: David Henningsson david.henningsson@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.