HDMI jack detection broken after suspend/resume
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
linux (Ubuntu) |
Fix Released
|
Undecided
|
David Henningsson |
Bug Description
After suspend/resume, the HDMI port does not show up, but it can still be used (if you use e g pavucontrol to select it). Sometimes, doing seemingly unrelated things, such as running the alsa-info script, restarting PulseAudio, etc can make it show up again.
Found on both NVidia and Intel hardware.
Fixed upstream by http://
Which kernels should we consider backporting to?
* 3.2-Precise
* 3.5-Quantal
* upstream stable
tags: | added: precise |
David Henningsson (diwic) wrote : [PATCH] ALSA: hda - fix broken HDMI jack detection after S3 | #1 |
Changed in alsa-driver (Ubuntu): | |
assignee: | nobody → David Henningsson (diwic) |
status: | New → In Progress |
affects: | alsa-driver (Ubuntu) → linux (Ubuntu) |
description: | updated |
Takashi Iwai (tiwai) wrote : | #2 |
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 address hidden> (3.3+)
> BugLink: https:/
> Signed-off-by: David Henningsson <email address hidden>
> ---
> sound/pci/
> 1 file changed, 13 insertions(+)
>
> diff --git a/sound/
> index 8f23374..6a3ac05 100644
> --- a/sound/
> +++ b/sound/
> @@ -1315,6 +1315,16 @@ static int generic_
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int generic_
> +{
> + snd_hda_
> + snd_hda_
> + snd_hda_
> + return 0;
Hm, is this really needed?
snd_hda_
hda_call_
the init callback.
Takashi
> +}
> +#endif
> +
> static void generic_
> {
> struct hdmi_spec *spec = codec->spec;
> @@ -1338,6 +1348,9 @@ static const struct hda_codec_ops generic_
> .build_pcms = generic_
> .build_controls = generic_
> .unsol_event = hdmi_unsol_event,
> +#ifdef CONFIG_PM
> + .resume = generic_
> +#endif
> };
>
> static int patch_generic_
> --
> 1.7.9.5
>
David Henningsson (diwic) wrote : | #3 |
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 address hidden> (3.3+)
>> BugLink: https:/
>> Signed-off-by: David Henningsson <email address hidden>
>> ---
>> sound/pci/
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/sound/
>> index 8f23374..6a3ac05 100644
>> --- a/sound/
>> +++ b/sound/
>> @@ -1315,6 +1315,16 @@ static int generic_
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM
>> +static int generic_
>> +{
>> + snd_hda_
>> + snd_hda_
>> + snd_hda_
>> + return 0;
>
> Hm, is this really needed?
>
> snd_hda_
> hda_call_
> 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_
I do notice one difference though - the order.
snd_hda_
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.
Could it be that the card gives wrong info if unsol is not enabled, or
perhaps needs SET_PIN_SENSE in that case? I don't know.
But yeah, I'll have to wait with this patch then until I've verified
more things.
Thanks.
--
David Henningsson, Canonical Ltd.
https:/
Takashi Iwai (tiwai) wrote : | #4 |
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 address hidden> (3.3+)
> >> BugLink: https:/
> >> Signed-off-by: David Henningsson <email address hidden>
> >> ---
> >> sound/pci/
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/sound/
> >> index 8f23374..6a3ac05 100644
> >> --- a/sound/
> >> +++ b/sound/
> >> @@ -1315,6 +1315,16 @@ static int generic_
> >> return 0;
> >> }
> >>
> >> +#ifdef CONFIG_PM
> >> +static int generic_
> >> +{
> >> + snd_hda_
> >> + snd_hda_
> >> + snd_hda_
> >> + return 0;
> >
> > Hm, is this really needed?
> >
> > snd_hda_
> > hda_call_
> > 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_
>
> I do notice one difference though - the order.
> snd_hda_
> 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_
checked at first, then the caches are resumed. So this won't change
ENABLE_UNSOL verb setups.
The bug report doesn't give any details, so it's hard to guess what
actually doesn't work. alsa-info.sh output before and after suspend
would be helpful to understand.
Takashi
David Henningsson (diwic) wrote : | #5 |
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 address hidden> (3.3+)
>>>> BugLink: https:/
>>>> Signed-off-by: David Henningsson <email address hidden>
>>>> ---
>>>> sound/pci/
>>>> 1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/sound/
>>>> index 8f23374..6a3ac05 100644
>>>> --- a/sound/
>>>> +++ b/sound/
>>>> @@ -1315,6 +1315,16 @@ static int generic_
>>>> return 0;
>>>> }
>>>>
>>>> +#ifdef CONFIG_PM
>>>> +static int generic_
>>>> +{
>>>> + snd_hda_
>>>> + snd_hda_
>>>> + snd_hda_
>>>> + return 0;
>>>
>>> Hm, is this really needed?
>>>
>>> snd_hda_
>>> hda_call_
>>> 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_
>>
>> I do notice one difference though - the order.
>> snd_hda_
>> 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_
> 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_
snd_hda_
generic_hdmi_init -> snd_hda_
snd_hda_
With the (theoretical?) race condition being a change of pin sense
between snd_hda_
which will not be picked up.
The patch changed the order to:
snd_hda_
snd_hda_
snd_hda_
Which eliminates that race condition.
> The bug report doesn't give any details, so it's hard to guess what
> actually doesn't work. alsa-info.sh output before and after suspend
> would be helpful to understand.
Sorry about that. The machine is not yet released, that's why.
--
David Henningsson, Canonical Ltd.
https:/
Takashi Iwai (tiwai) wrote : | #6 |
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 address hidden> (3.3+)
> >>>> BugLink: https:/
> >>>> Signed-off-by: David Henningsson <email address hidden>
> >>>> ---
> >>>> sound/pci/
> >>>> 1 file changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/sound/
> >>>> index 8f23374..6a3ac05 100644
> >>>> --- a/sound/
> >>>> +++ b/sound/
> >>>> @@ -1315,6 +1315,16 @@ static int generic_
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +#ifdef CONFIG_PM
> >>>> +static int generic_
> >>>> +{
> >>>> + snd_hda_
> >>>> + snd_hda_
> >>>> + snd_hda_
> >>>> + return 0;
> >>>
> >>> Hm, is this really needed?
> >>>
> >>> snd_hda_
> >>> hda_call_
> >>> 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_
> >>
> >> I do notice one difference though - the order.
> >> snd_hda_
> >> 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_
> > 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_
>
> snd_hda_
> generic_hdmi_init -> snd_hda_
> snd_hda_
>
> With the (theoretical?) race condition being a change of pin sense
> between snd_hda_
> 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_
> snd_hda_
> snd_hda_
>
> Which eliminates that race condition.
Well, what I'm thinking now is rather to call
snd_h...
David Henningsson (diwic) wrote : | #7 |
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 address hidden> (3.3+)
>>>>>> BugLink: https:/
>>>>>> Signed-off-by: David Henningsson <email address hidden>
>>>>>> ---
>>>>>> sound/pci/
>>>>>> 1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/sound/
>>>>>> index 8f23374..6a3ac05 100644
>>>>>> --- a/sound/
>>>>>> +++ b/sound/
>>>>>> @@ -1315,6 +1315,16 @@ static int generic_
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +#ifdef CONFIG_PM
>>>>>> +static int generic_
>>>>>> +{
>>>>>> + snd_hda_
>>>>>> + snd_hda_
>>>>>> + snd_hda_
>>>>>> + return 0;
>>>>>
>>>>> Hm, is this really needed?
>>>>>
>>>>> snd_hda_
>>>>> hda_call_
>>>>> 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_
>>>>
>>>> I do notice one difference though - the order.
>>>> snd_hda_
>>>> 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_
>>> 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_
>>
>> snd_hda_
>> generic_hdmi_init -> snd_hda_
>> snd_hda_
>>
>> With the (theoretical?) race condition being a change of pin sense
>> between snd_hda_
>> 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_
>> snd_hda_
>> snd_hda_
Takashi Iwai (tiwai) wrote : | #8 |
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: <email address hidden> (3.3+)
> >>>>>> BugLink: https:/
> >>>>>> Signed-off-by: David Henningsson <email address hidden>
> >>>>>> ---
> >>>>>> sound/pci/
> >>>>>> 1 file changed, 13 insertions(+)
> >>>>>>
> >>>>>> diff --git a/sound/
> >>>>>> index 8f23374..6a3ac05 100644
> >>>>>> --- a/sound/
> >>>>>> +++ b/sound/
> >>>>>> @@ -1315,6 +1315,16 @@ static int generic_
> >>>>>> return 0;
> >>>>>> }
> >>>>>>
> >>>>>> +#ifdef CONFIG_PM
> >>>>>> +static int generic_
> >>>>>> +{
> >>>>>> + snd_hda_
> >>>>>> + snd_hda_
> >>>>>> + snd_hda_
> >>>>>> + return 0;
> >>>>>
> >>>>> Hm, is this really needed?
> >>>>>
> >>>>> snd_hda_
> >>>>> hda_call_
> >>>>> 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_
> >>>>
> >>>> I do notice one difference though - the order.
> >>>> snd_hda_
> >>>> 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_
> >>> 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_
> >>
> >> snd_hda_
> >> generic_hdmi_init -> snd_hda_
> >> snd_hda_
> >>
> >> With the (theoretical?) race condition being a change of pin sense
> >> between snd_hda_
> >> which will not be picked up.
> >
> > In that race, yes. But this should be irrelevant with ...
David Henningsson (diwic) wrote : | #9 |
On 08/22/2012 04:49 PM, Takashi Iwai wrote:
> From: Takashi Iwai <email address hidden>
> Subject: [PATCH] ALSA: hda - Call snd_hda_
>
> 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 <email address hidden>
Acked-by: David Henningsson <email address hidden>
> ---
> sound/pci/
> sound/pci/
> sound/pci/
> sound/pci/
> sound/pci/
> sound/pci/
> sound/pci/
> 7 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/sound/
> index a6c34dc..4efd271 100644
> --- a/sound/
> +++ b/sound/
> @@ -3618,6 +3618,7 @@ static void hda_call_
> snd_hda_
> snd_hda_
> }
> + snd_hda_
> snd_hda_
> }
> #endif /* CONFIG_PM */
> @@ -3663,6 +3664,7 @@ int snd_hda_
> err = codec->
> if (err < 0)
> return err;
> + snd_hda_
> return 0;
> }
>
> diff --git a/sound/
> index 0c4c1a6..0bddb3e 100644
> --- a/sound/
> +++ b/sound/
> @@ -1193,7 +1193,6 @@ static int cs_init(struct hda_codec *codec)
> init_output(codec);
> init_input(codec);
> init_digital(
> - snd_hda_
>
> return 0;
> }
> @@ -1643,7 +1642,6 @@ static int cs421x_init(struct hda_codec *codec)
> init_output(codec);
> init_input(codec);
> init_cs421x_
> - snd_hda_
>
> return 0;
> }
> diff --git a/sound/
> index 5e22a8f..172895a 100644
> --- a/sound/
> +++ b/sound/
> @@ -4061,7 +4061,6 @@ static int cx_auto_init(struct hda_codec *codec)
> cx_auto_
> cx_auto_
> cx_auto_
> - snd_hda_
> snd_hda_
> return 0;
> }
> diff --git a/sound/
> index 8f23374..d9439c5 100644
> --- a/sound/
> +++ b/sound/
> @@ -1311,7 +1311,6 @@ static int generic_
> hdmi_init_
> snd_hda_
> }
> - snd_hda_
> return 0;
> }
>
> @@ -1428,7 +1427,6 @@ static int simple_
tags: | added: blocks-hwcert-enablement |
description: | updated |
David Henningsson (diwic) wrote : | #10 |
- alsa-hda-dkms_0.1_all.deb Edit (299.5 KiB, application/x-debian-package)
This dkms package contains the quantal hda tree + the patch in comment #1.
Now, as Takashi points out, in theory there shouldn't be a difference. Please verify that in practice there actually is a difference, before I attempt to get it into the quantal kernel. Thanks!
David Henningsson (diwic) wrote : | #11 |
Should be fixed in 3.8. Please reopen this bug if it isn't.
Changed in linux (Ubuntu): | |
status: | In Progress → Fix Released |
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 address hidden> (3.3+) /bugs.launchpad .net/bugs/ 1040030 pci/hda/ patch_hdmi. c | 13 +++++++++++++
BugLink: https:/
Signed-off-by: David Henningsson <email address hidden>
---
sound/
1 file changed, 13 insertions(+)
diff --git a/sound/ pci/hda/ patch_hdmi. c b/sound/ pci/hda/ patch_hdmi. c pci/hda/ patch_hdmi. c pci/hda/ patch_hdmi. c hdmi_init( struct hda_codec *codec)
index 8f23374..6a3ac05 100644
--- a/sound/
+++ b/sound/
@@ -1315,6 +1315,16 @@ static int generic_
return 0;
}
+#ifdef CONFIG_PM hdmi_resume( struct hda_codec *codec) codec_resume_ cache(codec) ; jack_set_ dirty_all( codec); jack_report_ sync(codec) ; hdmi_free( struct hda_codec *codec) hdmi_patch_ ops = { hdmi_build_ pcms, hdmi_build_ controls, hdmi_resume,
+static int generic_
+{
+ snd_hda_
+ snd_hda_
+ snd_hda_
+ return 0;
+}
+#endif
+
static void generic_
{
struct hdmi_spec *spec = codec->spec;
@@ -1338,6 +1348,9 @@ static const struct hda_codec_ops generic_
.build_pcms = generic_
.build_controls = generic_
.unsol_event = hdmi_unsol_event,
+#ifdef CONFIG_PM
+ .resume = generic_
+#endif
};
static int patch_generic_ hdmi(struct hda_codec *codec)
--
1.7.9.5