[alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
From: Mengdong Lin mengdong.lin@intel.com
In system resume, Haswell codec cannot be programmed to D0 before Gfx driver initializes the display pipeline and audio, which will trigger an unsol event on the pin with HDMI/DP cable connected. Otherwise, the connected pin will stay in D3 with right channel muted and thus no sound can be heard.
This patch - adds a codec flag to delay resuming a codec. System resume will skip the codecs if this flag is set, and these codecs will be resumed on later codec access. - adds a set_power_state ops for Haswell HDMI codec. In a delayed resume, this ops will enable and wait for the unsol event, and then resume the codec. A 300ms timeout is set in case unsol event is lost.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 04b5738..bcb7205 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -5508,11 +5508,15 @@ int snd_hda_resume(struct hda_bus *bus) struct hda_codec *codec;
list_for_each_entry(codec, &bus->codec_list, list) { - hda_call_codec_resume(codec); + if (codec->support_delay_resume) + codec->resume_delayed = 1; + else + hda_call_codec_resume(codec); } return 0; } EXPORT_SYMBOL_HDA(snd_hda_resume); + #endif /* CONFIG_PM */
/* diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 23ca172..5b5e5f4 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -886,6 +886,8 @@ struct hda_codec { unsigned int d3_stop_clk:1; /* support D3 operation without BCLK */ unsigned int pm_down_notified:1; /* PM notified to controller */ unsigned int in_pm:1; /* suspend/resume being performed */ + unsigned int support_delay_resume:1; /* codec support delay resume */ + unsigned int resume_delayed:1; /* resume delayed by PM */ int power_transition; /* power-state in transition */ int power_count; /* current (global) power refcount */ struct delayed_work power_work; /* delayed task for powerdown */ diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 78e1827..d116908 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -98,6 +98,14 @@ struct hdmi_spec { */ struct hda_multi_out multiout; struct hda_pcm_stream pcm_playback; + +#ifdef CONFIG_PM + /* + * Non-generic Intel Haswell specific + */ + unsigned int ready_to_resume:1; + wait_queue_head_t resume_wq; +#endif };
@@ -977,6 +985,13 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) if (pin_idx < 0) return;
+#ifdef CONFIG_PM + if (codec->resume_delayed) { + spec->ready_to_resume = 1; + wake_up(&spec->resume_wq); + } +#endif + hdmi_present_sense(&spec->pins[pin_idx], 1); snd_hda_jack_report_sync(codec); } @@ -1846,6 +1861,63 @@ static const struct hda_fixup hdmi_fixups[] = { };
+#ifdef CONFIG_PM +static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + spec->ready_to_resume = 0; + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin; + hda_nid_t pin_nid; + struct hda_jack_tbl *jack; + + per_pin = &spec->pins[pin_idx]; + pin_nid = per_pin->pin_nid; + jack = snd_hda_jack_tbl_get(codec, pin_nid); + if (jack) + snd_hda_codec_write(codec, pin_nid, 0, + AC_VERB_SET_UNSOLICITED_ENABLE, + AC_USRSP_EN | jack->tag); + } + + wait_event_timeout(spec->resume_wq, + spec->ready_to_resume, msecs_to_jiffies(300)); + if (!spec->ready_to_resume) + snd_printd(KERN_WARNING "HDMI: Haswell not ready to resume\n"); +} + +static void intel_haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg, + unsigned int power_state) +{ + if (codec->resume_delayed && power_state == AC_PWRST_D0) { + intel_haswell_wait_ready_to_resume(codec); + codec->resume_delayed = 0; + } + + snd_hda_codec_read(codec, fg, 0, + AC_VERB_SET_POWER_STATE, + power_state); + + snd_hda_codec_set_power_to_all(codec, fg, power_state); +} + +static inline void intel_haswell_allow_delay_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + + init_waitqueue_head(&spec->resume_wq); + codec->support_delay_resume = 1; + + codec->patch_ops.set_power_state = + intel_haswell_set_power_state; +} +#else +define intel_haswell_allow_delay_resume NULL +#endif + static int patch_generic_hdmi(struct hda_codec *codec) { struct hdmi_spec *spec; @@ -1868,6 +1940,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) return -EINVAL; } codec->patch_ops = generic_hdmi_patch_ops; + + if (codec->vendor_id == 0x80862807) + intel_haswell_allow_delay_resume(codec); + generic_hdmi_init_per_pins(codec);
init_channel_allocations();
Is it okay to add a new parameter to snd_hda_resume(), to indicate whether codec resume should be delayed or not? Is because snd_hda_resume() is only only called by azx_resume() but also called by azx_bus_reset(). The latter can be called on a fatal verb execution failure and codec resume delay is not suitable for this case.
Thanks Mengdong
At Tue, 26 Mar 2013 07:02:54 +0000, Lin, Mengdong wrote:
Is it okay to add a new parameter to snd_hda_resume(), to indicate whether codec resume should be delayed or not? Is because snd_hda_resume() is only only called by azx_resume() but also called by azx_bus_reset(). The latter can be called on a fatal verb execution failure and codec resume delay is not suitable for this case.
Well, I prefer creating a new function (e.g. snd_hda_bus_reset()) called from azx_bus_reset().
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 02, 2013 5:53 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Tue, 26 Mar 2013 07:02:54 +0000, Lin, Mengdong wrote:
Is it okay to add a new parameter to snd_hda_resume(), to indicate whether
codec resume should be delayed or not?
Is because snd_hda_resume() is only only called by azx_resume() but also
called by azx_bus_reset().
The latter can be called on a fatal verb execution failure and codec resume
delay is not suitable for this case.
Well, I prefer creating a new function (e.g. snd_hda_bus_reset()) called from azx_bus_reset().
Takashi
Hi Takashi,
Do you mean that in azx_bus_reset(), we can merge the snd_hda_suspend() and snd_hda_resume() to a new function like snd_hda_bus_reset()? snd_hda_suspend() and snd_hda_resume() look symmetrical here and so I don't want to destroy the symmetry.
static void azx_bus_reset(struct hda_bus *bus) { ... #ifdef CONFIG_PM if (chip->initialized) { struct azx_pcm *p; list_for_each_entry(p, &chip->pcm_list, list) snd_pcm_suspend_all(p->pcm); snd_hda_suspend(chip->bus); snd_hda_resume(chip->bus); } #endif bus->in_reset = 0; }
Thanks Mengdong
At Fri, 5 Apr 2013 16:58:54 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 02, 2013 5:53 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Tue, 26 Mar 2013 07:02:54 +0000, Lin, Mengdong wrote:
Is it okay to add a new parameter to snd_hda_resume(), to indicate whether
codec resume should be delayed or not?
Is because snd_hda_resume() is only only called by azx_resume() but also
called by azx_bus_reset().
The latter can be called on a fatal verb execution failure and codec resume
delay is not suitable for this case.
Well, I prefer creating a new function (e.g. snd_hda_bus_reset()) called from azx_bus_reset().
Takashi
Hi Takashi,
Do you mean that in azx_bus_reset(), we can merge the snd_hda_suspend() and snd_hda_resume() to a new function like snd_hda_bus_reset()?
Yes. Calling snd_hda_suspend() following snd_hda_resume() immediately is just an ad hoc way of reset. It should have been a dedicated reset function from the beginning.
Takashi
snd_hda_suspend() and snd_hda_resume() look symmetrical here and so I don't want to destroy the symmetry.
static void azx_bus_reset(struct hda_bus *bus) { ... #ifdef CONFIG_PM if (chip->initialized) { struct azx_pcm *p; list_for_each_entry(p, &chip->pcm_list, list) snd_pcm_suspend_all(p->pcm); snd_hda_suspend(chip->bus); snd_hda_resume(chip->bus); } #endif bus->in_reset = 0; }
Thanks Mengdong
On 03/26/2013 07:12 PM, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
In system resume, Haswell codec cannot be programmed to D0 before Gfx driver initializes the display pipeline and audio, which will trigger an unsol event on the pin with HDMI/DP cable connected. Otherwise, the connected pin will stay in D3 with right channel muted and thus no sound can be heard.
Thanks for this patch!
Just a question: Is there a difference between real system S3 here, and just the normal idle power down? I e, I'm a little unsure myself, but it seems like this 300 ms delay would trigger in both cases, causing an (unnecessary?) 300 ms delay every time the codec is idle and you want to start playback. Is this correct, or did I get something wrong?
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, March 26, 2013 6:58 PM
Just a question: Is there a difference between real system S3 here, and just the normal idle power down? I e, I'm a little unsure myself, but it seems like this 300 ms delay would trigger in both cases, causing an (unnecessary?) 300 ms delay every time the codec is idle and you want to start playback. Is this correct, or did I get something wrong?
Hi David,
Thanks for your feedback! There will not be 300ms delay every time in normal idle power down case.
Only the system resume will delay resuming a codec and cause a waiting on first codec access (eg. 1st playback ) after a system resume. Snd_hda_resume(), called by azx_resume(), will check whether a codec can support delay resume, skip these codecs and set the flag "resume_delayed" on them. Normal idle power down will not touch this flag.
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
BTW, would you please tell us how Ubuntu decides whether to enable the normal idle power down (parameter "power_save")? We found this bug on a Haswell mobile machine with "power_save" disabled, which means only system resume will program the codec back to D0. But on a another machine with "power_save" enabled, this bug is not visible because later runtime codec power up can program the codec to D0 and unmute the pin again.
Thanks Mengdong
On 03/27/2013 03:03 AM, Lin, Mengdong wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, March 26, 2013 6:58 PM
Just a question: Is there a difference between real system S3 here, and just the normal idle power down? I e, I'm a little unsure myself, but it seems like this 300 ms delay would trigger in both cases, causing an (unnecessary?) 300 ms delay every time the codec is idle and you want to start playback. Is this correct, or did I get something wrong?
Hi David,
Thanks for your feedback! There will not be 300ms delay every time in normal idle power down case.
Only the system resume will delay resuming a codec and cause a waiting on first codec access (eg. 1st playback ) after a system resume. Snd_hda_resume(), called by azx_resume(), will check whether a codec can support delay resume, skip these codecs and set the flag "resume_delayed" on them. Normal idle power down will not touch this flag.
Right.
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
BTW, would you please tell us how Ubuntu decides whether to enable the normal idle power down (parameter "power_save")? We found this bug on a Haswell mobile machine with "power_save" disabled, which means only system resume will program the codec back to D0. But on a another machine with "power_save" enabled, this bug is not visible because later runtime codec power up can program the codec to D0 and unmute the pin again.
Hmm. I don't think there is any difference between the upstream default and Ubuntu in this case. I remember we had to turn off power_save for Pantherpoint once, before keep_power_link_on for PantherPoint was discovered and upstreamed. But that was quite a while ago.
Then there is of course people playing with powertop and other tools, to override the defaults.
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
And I think there will not be race between unsol event and idle time out. Because until the first user operation trigger the delayed resume, the codec keeps in D3. And only until intel_haswell_set_power_state(), which is called by hda_call_codec_resume(), finishes waiting and programing the codec to D0, __snd_hda_power_up() will keep 'codec->power_transition' to '1'. Thus __snd_hda_power_down() will not schedule idle time out.
BTW, would you please tell us how Ubuntu decides whether to enable the
normal idle power down (parameter "power_save")?
We found this bug on a Haswell mobile machine with "power_save" disabled,
which means only system resume will program the codec back to D0. But on a another machine with "power_save" enabled, this bug is not visible because later runtime codec power up can program the codec to D0 and unmute the pin again.
Hmm. I don't think there is any difference between the upstream default and Ubuntu in this case. I remember we had to turn off power_save for Pantherpoint once, before keep_power_link_on for PantherPoint was discovered and upstreamed. But that was quite a while ago.
Then there is of course people playing with powertop and other tools, to override the defaults.
The default power_value is 0 in Kconfig, which means off. But on some HSW machines with ALC889 codec and Haswell display codec, both with EPSS support, I found that power_save is 1 after boot. On another HSW machine with ALC262, no EPSS, power_save is 0.
So I suspect the rule is that if all codecs support EPSS, power_save will be enabled. But who enables this?
Thanks Mengdong
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
And I think there will not be race between unsol event and idle time out. Because until the first user operation trigger the delayed resume, the codec keeps in D3. And only until intel_haswell_set_power_state(), which is called by hda_call_codec_resume(), finishes waiting and programing the codec to D0, __snd_hda_power_up() will keep 'codec->power_transition' to '1'. Thus __snd_hda_power_down() will not schedule idle time out.
BTW, would you please tell us how Ubuntu decides whether to enable the
normal idle power down (parameter "power_save")?
We found this bug on a Haswell mobile machine with "power_save" disabled,
which means only system resume will program the codec back to D0. But on a another machine with "power_save" enabled, this bug is not visible because later runtime codec power up can program the codec to D0 and unmute the pin again.
Hmm. I don't think there is any difference between the upstream default and Ubuntu in this case. I remember we had to turn off power_save for Pantherpoint once, before keep_power_link_on for PantherPoint was discovered and upstreamed. But that was quite a while ago.
Then there is of course people playing with powertop and other tools, to override the defaults.
The default power_value is 0 in Kconfig, which means off. But on some HSW machines with ALC889 codec and Haswell display codec, both with EPSS support, I found that power_save is 1 after boot. On another HSW machine with ALC262, no EPSS, power_save is 0.
So I suspect the rule is that if all codecs support EPSS, power_save will be enabled. But who enables this?
That does not seem to make sense. It might be something different?
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Takashi
On 04/02/2013 02:18 PM, Takashi Iwai wrote:
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Lin, would it be possible to add some chain of events description to the bug commit? The different contexts are just boggling my mind :-)
Assume we have two cases, system idle after S3 and immediate playback after S3.
Is this correct:
System idle: 1. System skips hda_call_codec_resume and sets codec->resume_delayed. 2. Since the codec is not reinitialized, nothing powers up the codec, so this is all that happens. 3. Since unsol events are not enabled (?), we have a bug that jack detection does not work and cannot be detected from userspace...?
Immediate playback: 1. System skips hda_call_codec_resume and sets codec->resume_delayed. 2. Process context wants to start playback, which powers up the codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
At Tue, 02 Apr 2013 14:49:40 +0200, David Henningsson wrote:
On 04/02/2013 02:18 PM, Takashi Iwai wrote:
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Lin, would it be possible to add some chain of events description to the bug commit? The different contexts are just boggling my mind :-)
Yeah, a nice code flow picture would be helpful :)
Assume we have two cases, system idle after S3 and immediate playback after S3.
Is this correct:
System idle:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Since the codec is not reinitialized, nothing powers up the codec,
so this is all that happens. 3. Since unsol events are not enabled (?), we have a bug that jack detection does not work and cannot be detected from userspace...?
Immediate playback:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Process context wants to start playback, which powers up the codec
and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
Takashi
--- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 67e11ba..d0eafee 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1740,6 +1740,9 @@ 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); } + + spec->ready_to_resume = 0; + return 0; }
@@ -1876,8 +1879,6 @@ static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) struct hdmi_spec *spec = codec->spec; int pin_idx;
- spec->ready_to_resume = 0; - for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid;
At Fri, 05 Apr 2013 15:01:47 +0200, Takashi Iwai wrote:
At Tue, 02 Apr 2013 14:49:40 +0200, David Henningsson wrote:
On 04/02/2013 02:18 PM, Takashi Iwai wrote:
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
> The Haswell codec set_power_state ops (intel_haswell_set_power_state) will only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq. It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Lin, would it be possible to add some chain of events description to the bug commit? The different contexts are just boggling my mind :-)
Yeah, a nice code flow picture would be helpful :)
Assume we have two cases, system idle after S3 and immediate playback after S3.
Is this correct:
System idle:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Since the codec is not reinitialized, nothing powers up the codec,
so this is all that happens. 3. Since unsol events are not enabled (?), we have a bug that jack detection does not work and cannot be detected from userspace...?
Immediate playback:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Process context wants to start playback, which powers up the codec
and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
Takashi
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 67e11ba..d0eafee 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1740,6 +1740,9 @@ 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); }
- spec->ready_to_resume = 0;
Here needs #ifdef CONFIG_PM, obviously...
Takashi
return 0; }
@@ -1876,8 +1879,6 @@ static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) struct hdmi_spec *spec = codec->spec; int pin_idx;
- spec->ready_to_resume = 0;
- for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid;
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, April 05, 2013 9:02 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Tue, 02 Apr 2013 14:49:40 +0200, David Henningsson wrote:
On 04/02/2013 02:18 PM, Takashi Iwai wrote:
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, March 27, 2013 4:19 PM
> The Haswell codec set_power_state ops > (intel_haswell_set_power_state) will only wait if this is a delayed resume and clear this flag after waiting. And actually, there is no waiting even in this case. Because when 1st user operation after system resume happens, Gfx already finishes resuming and audio initialization, so as long as intel_haswell_wait_ready_to_resume() enable the unsol event, the unsol event comes and so so waiting finishes. The 300ms time out is set for safety consideration in case unsol event is lost. I've not
observed any unsol event lost till now.
As for the timeout, I suggest you use the codec->bus->workq instead of creating a new workq. I think that will also give us some serialisation, i e, protection against race conditions if the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work
queue like codec->bus->workq.
It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Lin, would it be possible to add some chain of events description to the bug commit? The different contexts are just boggling my mind :-)
Yeah, a nice code flow picture would be helpful :)
Assume we have two cases, system idle after S3 and immediate playback after S3.
Is this correct:
System idle:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Since the codec is not reinitialized, nothing powers up the
codec, so this is all that happens. 3. Since unsol events are not enabled (?), we have a bug that jack detection does not work and cannot be detected from userspace...?
Yes. This is a side effect of this patch. The unsol event is delayed until the user operation triggers the delayed resume. Then intel_haswell_wait_ready_to_resume() will enable unsol events.
Immediate playback:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume() enables the unsol event on the pins. So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Thanks Mengdong
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 67e11ba..d0eafee 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1740,6 +1740,9 @@ 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); }
- spec->ready_to_resume = 0;
- return 0;
}
@@ -1876,8 +1879,6 @@ static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) struct hdmi_spec *spec = codec->spec; int pin_idx;
- spec->ready_to_resume = 0;
- for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid;
At Fri, 5 Apr 2013 16:42:43 +0000, Lin, Mengdong wrote:
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, April 05, 2013 9:02 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Tue, 02 Apr 2013 14:49:40 +0200, David Henningsson wrote:
On 04/02/2013 02:18 PM, Takashi Iwai wrote:
At Tue, 02 Apr 2013 13:53:16 +0200, David Henningsson wrote:
On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
> -----Original Message----- > From: David Henningsson [mailto:david.henningsson@canonical.com] > Sent: Wednesday, March 27, 2013 4:19 PM
>> The Haswell codec set_power_state ops >> (intel_haswell_set_power_state) will > only wait if this is a delayed resume and clear this flag after > waiting. And actually, there is no waiting even in this case. > Because when 1st user operation after system resume happens, Gfx > already finishes resuming and audio initialization, so as long as > intel_haswell_wait_ready_to_resume() enable the unsol event, the > unsol event comes and so so waiting finishes. The 300ms time out > is set for safety consideration in case unsol event is lost. I've not
observed any unsol event lost till now.
> > As for the timeout, I suggest you use the codec->bus->workq > instead of creating a new workq. I think that will also give us > some serialisation, i e, protection against race conditions if > the timeout happen at the same time as the unsol event.
Hi David,
The new added "resume_wq" for hdmi codec is a wait queue, not a work
queue like codec->bus->workq.
It's expected to wake up as soon as the unsol event is got.
Sure; but I don't see why you need a wait queue for that? Why don't you just call the resume path from the unsol event handler (hdmi_present_sense, or its caller), and then also cancel the timeout handler (which can then be in the normal workq)?
Because the delayed resume actually fakes as if the resume is done. This is necessary not to block other device's resume operation.
Since it looks as if ready, user-space might restart things soon again before the delayed resume is really finished. So, some serialization is required there.
Lin, would it be possible to add some chain of events description to the bug commit? The different contexts are just boggling my mind :-)
Yeah, a nice code flow picture would be helpful :)
Assume we have two cases, system idle after S3 and immediate playback after S3.
Is this correct:
System idle:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Since the codec is not reinitialized, nothing powers up the
codec, so this is all that happens. 3. Since unsol events are not enabled (?), we have a bug that jack detection does not work and cannot be detected from userspace...?
Yes. This is a side effect of this patch. The unsol event is delayed until the user operation triggers the delayed resume. Then intel_haswell_wait_ready_to_resume() will enable unsol events.
Immediate playback:
- System skips hda_call_codec_resume and sets codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume() enables the unsol event on the pins. So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Practically your patch would work well, but theoretically the unsol event might be issued and handled by hdmi_intrinsic_event() before you go into intel_haswell_wait_ready_to_resume(). That is, you _clear_ spec->ready_to_resume again and wait for the unsol event that had been already processed. That's the race David pointed.
The fix is simply not to clear spec->ready_to_resume in intel_haswell_wait_ready_to_resume() but clear it much earlier already before the resume path as my patch does. Then wait_event() passes immediately (and the unsol enablement of is anyway harmless).
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, April 06, 2013 1:05 AM
Immediate playback:
- System skips hda_call_codec_resume and sets
codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume()
enables the unsol event on the pins.
So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Practically your patch would work well, but theoretically the unsol event might be issued and handled by hdmi_intrinsic_event() before you go into intel_haswell_wait_ready_to_resume(). That is, you _clear_ spec->ready_to_resume again and wait for the unsol event that had been already processed. That's the race David pointed.
The fix is simply not to clear spec->ready_to_resume in intel_haswell_wait_ready_to_resume() but clear it much earlier already before the resume path as my patch does. Then wait_event() passes immediately (and the unsol enablement of is anyway harmless).
Hi Takashi,
I see. Thanks for clarifying!
I'll revise the patch and test it again when I'm in office on Monday. Also I'll rebase the patch to for-next branch of sound git tree and fix the indentations and spaces errors.
Thanks Mengdong
On 04/07/2013 02:10 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, April 06, 2013 1:05 AM
Immediate playback:
- System skips hda_call_codec_resume and sets
codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume()
enables the unsol event on the pins.
So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Practically your patch would work well, but theoretically the unsol event might be issued and handled by hdmi_intrinsic_event() before you go into intel_haswell_wait_ready_to_resume(). That is, you _clear_ spec->ready_to_resume again and wait for the unsol event that had been already processed. That's the race David pointed.
The fix is simply not to clear spec->ready_to_resume in intel_haswell_wait_ready_to_resume() but clear it much earlier already before the resume path as my patch does. Then wait_event() passes immediately (and the unsol enablement of is anyway harmless).
Hi Takashi,
I see. Thanks for clarifying!
I'll revise the patch and test it again when I'm in office on Monday. Also I'll rebase the patch to for-next branch of sound git tree and fix the indentations and spaces errors.
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow? And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
On 04/07/2013 02:10 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, April 06, 2013 1:05 AM
Immediate playback:
- System skips hda_call_codec_resume and sets
codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume()
enables the unsol event on the pins.
So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Practically your patch would work well, but theoretically the unsol event might be issued and handled by hdmi_intrinsic_event() before you go into intel_haswell_wait_ready_to_resume(). That is, you _clear_ spec->ready_to_resume again and wait for the unsol event that had been already processed. That's the race David pointed.
The fix is simply not to clear spec->ready_to_resume in intel_haswell_wait_ready_to_resume() but clear it much earlier already before the resume path as my patch does. Then wait_event() passes immediately (and the unsol enablement of is anyway harmless).
Hi Takashi,
I see. Thanks for clarifying!
I'll revise the patch and test it again when I'm in office on Monday. Also I'll rebase the patch to for-next branch of sound git tree and fix the indentations and spaces errors.
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
Thanks Mengdong
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3, can we extend the "try to set the pin 10 times" algorithm in hda_set_power_state to check all the pins for haswell HDMI?
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED toggling).
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Takashi
can we extend the "try to set the pin 10 times" algorithm in hda_set_power_state to check all the pins for haswell HDMI?
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED toggling).
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote:
I'm still not understanding this patch.
We of course cannot have a situation where HDMI jack detection is not correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED toggling).
Second, I just saw on a machine we're working on, the symptoms: one of the pins in D3 and the right channel muted. And this was on a clean boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
To look at the problem again: If the problem is that something must be done on the graphics driver side first and then on the audio side, wouldn't the solution be for the video driver and audio driver to communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
And resume (and possibly init?) would not complete until first the graphics driver has done its resume, and after that, the audio driver. I e, userspace will remain frozen until both drivers have completed a correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Takashi
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 6:24 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: > > I'm still not understanding this patch. > > We of course cannot have a situation where HDMI jack detection is not > correctly working after S3, which looks like would be the case here?
No. The patch itself has nothing to do with the HDMI jack detection at all. This intrinsic unsol event is (according to Intel) guaranteed to be issued once at the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED toggling).
> Second, I just saw on a machine we're working on, the symptoms: one of > the pins in D3 and the right channel muted. And this was on a clean > boot, not after S3 suspend/resume cycle.
Maybe another problem. Maybe not. Does the problem persist if you reload the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
> To look at the problem again: If the problem is that something must be > done on the graphics driver side first and then on the audio side, > wouldn't the solution be for the video driver and audio driver to > communicate somehow?
Yes, the pm domain support is necessary sooner or later, as I already discussed shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
> And resume (and possibly init?) would not complete until first the > graphics driver has done its resume, and after that, the audio driver. > I e, userspace will remain frozen until both drivers have completed a > correct resume.
... the resume problem can't be fixed only by the usual pm domain serialization. The graphics initialization for the audio bit is done asynchronously, thus we can't guarantee the audio codec is really ready after the graphics driver returns from the resume callback. It shows as if it's ready (you can turn it to D0) but actually it doesn't change on the hardware. Thus, the serialization with an unsol event wait seems mandatory for the time being.
It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote:
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Monday, April 08, 2013 6:24 PM > To: David Henningsson > Cc: Lin, Mengdong; alsa-devel@alsa-project.org > Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec > in system resume > > At Mon, 08 Apr 2013 11:49:09 +0200, > David Henningsson wrote: >> >> I'm still not understanding this patch. >> >> We of course cannot have a situation where HDMI jack detection is not >> correctly working after S3, which looks like would be the case here? > > No. The patch itself has nothing to do with the HDMI jack detection at all. > This intrinsic unsol event is (according to Intel) guaranteed to be issued once at > the audio codec initialization, at least for Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED toggling).
>> Second, I just saw on a machine we're working on, the symptoms: one of >> the pins in D3 and the right channel muted. And this was on a clean >> boot, not after S3 suspend/resume cycle. > > Maybe another problem. Maybe not. Does the problem persist if you reload > the driver (without invocation of alsactl via udev)?
Hi Takashi and David,
This is the same dependency issue as after system suspend/resume cycles, with same phenomenon. I duplicated this error if I boot without an HDMI/DP cable connected and connect the cable later. Maybe we need to delay codec operations on initialization like in resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
>> To look at the problem again: If the problem is that something must be >> done on the graphics driver side first and then on the audio side, >> wouldn't the solution be for the video driver and audio driver to >> communicate somehow? > > Yes, the pm domain support is necessary sooner or later, as I already discussed > shortly with Dave Airlie. However...
Liam will propose Gfx driver team to implement a pm domain.
>> And resume (and possibly init?) would not complete until first the >> graphics driver has done its resume, and after that, the audio driver. >> I e, userspace will remain frozen until both drivers have completed a >> correct resume. > > ... the resume problem can't be fixed only by the usual pm domain serialization. > The graphics initialization for the audio bit is done asynchronously, thus we > can't guarantee the audio codec is really ready after the graphics driver returns > from the resume callback. > It shows as if it's ready (you can turn it to D0) but actually it doesn't change on > the hardware. Thus, the serialization with an unsol event wait seems > mandatory for the time being. > It seems so. We have not found better solution now :-( I've revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 9:50 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote:
On 04/08/2013 01:06 PM, Lin, Mengdong wrote: >> -----Original Message----- >> From: Takashi Iwai [mailto:tiwai@suse.de] >> Sent: Monday, April 08, 2013 6:24 PM >> To: David Henningsson >> Cc: Lin, Mengdong; alsa-devel@alsa-project.org >> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >> haswell hdmi codec in system resume >> >> At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: >>> >>> I'm still not understanding this patch. >>> >>> We of course cannot have a situation where HDMI jack detection >>> is not correctly working after S3, which looks like would be the case
here?
>> >> No. The patch itself has nothing to do with the HDMI jack detection
at all.
>> This intrinsic unsol event is (according to Intel) guaranteed >> to be issued once at the audio codec initialization, at least for
Haswell.
Well, if no process is using the HDMI codec actively, nothing will initialize the resume, and the HDMI codec will not initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
(The counter argument would be; if nobody uses HDMI codec, does it matter that it is not initialized, but I'm unsure if e g "amixer contents" or listeners on legacy /dev/input actually powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED
toggling).
>>> Second, I just saw on a machine we're working on, the >>> symptoms: one of the pins in D3 and the right channel muted. >>> And this was on a clean boot, not after S3 suspend/resume cycle. >> >> Maybe another problem. Maybe not. Does the problem persist if >> you reload the driver (without invocation of alsactl via udev)? > > Hi Takashi and David, > > This is the same dependency issue as after system suspend/resume
cycles, with same phenomenon.
> I duplicated this error if I boot without an HDMI/DP cable connected
and connect the cable later.
> Maybe we need to delay codec operations on initialization like in
resume case.
So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
>>> To look at the problem again: If the problem is that something >>> must be done on the graphics driver side first and then on the >>> audio side, wouldn't the solution be for the video driver and >>> audio driver to communicate somehow? >> >> Yes, the pm domain support is necessary sooner or later, as I >> already discussed shortly with Dave Airlie. However... > > Liam will propose Gfx driver team to implement a pm domain. > >>> And resume (and possibly init?) would not complete until first >>> the graphics driver has done its resume, and after that, the audio
driver.
>>> I e, userspace will remain frozen until both drivers have >>> completed a correct resume. >> >> ... the resume problem can't be fixed only by the usual pm domain
serialization.
>> The graphics initialization for the audio bit is done >> asynchronously, thus we can't guarantee the audio codec is >> really ready after the graphics driver returns from the resume
callback.
>> It shows as if it's ready (you can turn it to D0) but actually >> it doesn't change on the hardware. Thus, the serialization >> with an unsol event wait seems mandatory for the time being. >> > It seems so. We have not found better solution now :-( I've > revised the patch for system suspend/resume case. Please review.
If this problem can be detected by looking at the pin and finding that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
This is same as what I observed after S3 or connect HDMI/DP cable after boot, on machines with power_save is '0'. Amp-Out vals: [0x00 0x80] Power: setting=D3, actual=D3
If we set the pin state to D0 and unmute the pin again, HDMI can work. So I still think this is also caused by dependency on Gfx.
But the controller cannot find the error when it programs the codec to D0 - when audio driver programs the codec and pins to D0, HW does not return error - I also added code to check their power state immediately after setting state to D0, HW reports their state did change to D0. However, after Gfx driver finishes setting up the display pipeline and enabling audio, the pin's state changed to D3 and get muted, as shown by /proc. It seems to me that Gfx HW initialization can override some audio driver settings. So we choose to delay resuming the codec until the unsol event which indicates Gfx is ready.
Thanks Mengdong
On 04/08/2013 05:47 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 9:50 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote: > > On 04/08/2013 01:06 PM, Lin, Mengdong wrote: >>> -----Original Message----- >>> From: Takashi Iwai [mailto:tiwai@suse.de] >>> Sent: Monday, April 08, 2013 6:24 PM >>> To: David Henningsson >>> Cc: Lin, Mengdong; alsa-devel@alsa-project.org >>> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >>> haswell hdmi codec in system resume >>> >>> At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: >>>> >>>> I'm still not understanding this patch. >>>> >>>> We of course cannot have a situation where HDMI jack detection >>>> is not correctly working after S3, which looks like would be the case
here?
>>> >>> No. The patch itself has nothing to do with the HDMI jack detection
at all.
>>> This intrinsic unsol event is (according to Intel) guaranteed >>> to be issued once at the audio codec initialization, at least for
Haswell.
> > Well, if no process is using the HDMI codec actively, nothing > will initialize the resume, and the HDMI codec will not > initialized at all, i e, no unsol events enabled in the first place?
No, the intrinsic unsol event that is issued for this seems irrelevant from the jack state (i.e. active usage).
> (The counter argument would be; if nobody uses HDMI codec, does > it matter that it is not initialized, but I'm unsure if e g > "amixer contents" or listeners on legacy /dev/input actually > powers up the chip.)
The problem is that the codec resume is always performed no matter whether the device is used or not. This was different in the past but it's been changed in that way because there are cases where you need to initialize the hardware properly once (e.g. mute-LED
toggling).
>>>> Second, I just saw on a machine we're working on, the >>>> symptoms: one of the pins in D3 and the right channel muted. >>>> And this was on a clean boot, not after S3 suspend/resume cycle. >>> >>> Maybe another problem. Maybe not. Does the problem persist if >>> you reload the driver (without invocation of alsactl via udev)? >> >> Hi Takashi and David, >> >> This is the same dependency issue as after system suspend/resume
cycles, with same phenomenon.
>> I duplicated this error if I boot without an HDMI/DP cable connected
and connect the cable later.
>> Maybe we need to delay codec operations on initialization like in
resume case.
> > So it's a problem both on hotplug and S3...
In the case of hotplug, you don't hit the inconsistent power sate we're trying to solve. The graphics has been already powered up.
>>>> To look at the problem again: If the problem is that something >>>> must be done on the graphics driver side first and then on the >>>> audio side, wouldn't the solution be for the video driver and >>>> audio driver to communicate somehow? >>> >>> Yes, the pm domain support is necessary sooner or later, as I >>> already discussed shortly with Dave Airlie. However... >> >> Liam will propose Gfx driver team to implement a pm domain. >> >>>> And resume (and possibly init?) would not complete until first >>>> the graphics driver has done its resume, and after that, the audio
driver.
>>>> I e, userspace will remain frozen until both drivers have >>>> completed a correct resume. >>> >>> ... the resume problem can't be fixed only by the usual pm domain
serialization.
>>> The graphics initialization for the audio bit is done >>> asynchronously, thus we can't guarantee the audio codec is >>> really ready after the graphics driver returns from the resume
callback.
>>> It shows as if it's ready (you can turn it to D0) but actually >>> it doesn't change on the hardware. Thus, the serialization >>> with an unsol event wait seems mandatory for the time being. >>> >> It seems so. We have not found better solution now :-( I've >> revised the patch for system suspend/resume case. Please review. > > If this problem can be detected by looking at the pin and finding > that it is in D3,
No, you can't see that it's D3. The controller chip _shows_ it's in D0 while it's actually in D3. That's the problem.
So, this is actually a hardware design bug. But we need to live with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
This is same as what I observed after S3 or connect HDMI/DP cable after boot, on machines with power_save is '0'. Amp-Out vals: [0x00 0x80] Power: setting=D3, actual=D3
If we set the pin state to D0 and unmute the pin again, HDMI can work. So I still think this is also caused by dependency on Gfx.
But the controller cannot find the error when it programs the codec to D0
- when audio driver programs the codec and pins to D0, HW does not return error
- I also added code to check their power state immediately after setting state to D0, HW reports their state did change to D0.
However, after Gfx driver finishes setting up the display pipeline and enabling audio, the pin's state changed to D3 and get muted, as shown by /proc. It seems to me that Gfx HW initialization can override some audio driver settings. So we choose to delay resuming the codec until the unsol event which indicates Gfx is ready.
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events) - Any time an unsol event comes in, have a haswell specific function that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it. - This check could possibly also be done in the prepare hook for extra safety.
At Tue, 09 Apr 2013 08:45:32 +0200, David Henningsson wrote:
On 04/08/2013 05:47 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 9:50 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote:
On 04/08/2013 01:59 PM, Takashi Iwai wrote: > At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote: >> >> On 04/08/2013 01:06 PM, Lin, Mengdong wrote: >>>> -----Original Message----- >>>> From: Takashi Iwai [mailto:tiwai@suse.de] >>>> Sent: Monday, April 08, 2013 6:24 PM >>>> To: David Henningsson >>>> Cc: Lin, Mengdong; alsa-devel@alsa-project.org >>>> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >>>> haswell hdmi codec in system resume >>>> >>>> At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: >>>>> >>>>> I'm still not understanding this patch. >>>>> >>>>> We of course cannot have a situation where HDMI jack detection >>>>> is not correctly working after S3, which looks like would be the case
here?
>>>> >>>> No. The patch itself has nothing to do with the HDMI jack detection
at all.
>>>> This intrinsic unsol event is (according to Intel) guaranteed >>>> to be issued once at the audio codec initialization, at least for
Haswell.
>> >> Well, if no process is using the HDMI codec actively, nothing >> will initialize the resume, and the HDMI codec will not >> initialized at all, i e, no unsol events enabled in the first place? > > No, the intrinsic unsol event that is issued for this seems > irrelevant from the jack state (i.e. active usage). > >> (The counter argument would be; if nobody uses HDMI codec, does >> it matter that it is not initialized, but I'm unsure if e g >> "amixer contents" or listeners on legacy /dev/input actually >> powers up the chip.) > > The problem is that the codec resume is always performed no matter > whether the device is used or not. This was different in the past > but it's been changed in that way because there are cases where > you need to initialize the hardware properly once (e.g. mute-LED
toggling).
> > >>>>> Second, I just saw on a machine we're working on, the >>>>> symptoms: one of the pins in D3 and the right channel muted. >>>>> And this was on a clean boot, not after S3 suspend/resume cycle. >>>> >>>> Maybe another problem. Maybe not. Does the problem persist if >>>> you reload the driver (without invocation of alsactl via udev)? >>> >>> Hi Takashi and David, >>> >>> This is the same dependency issue as after system suspend/resume
cycles, with same phenomenon.
>>> I duplicated this error if I boot without an HDMI/DP cable connected
and connect the cable later.
>>> Maybe we need to delay codec operations on initialization like in
resume case.
>> >> So it's a problem both on hotplug and S3... > > In the case of hotplug, you don't hit the inconsistent power sate > we're trying to solve. The graphics has been already powered up. > > >>>>> To look at the problem again: If the problem is that something >>>>> must be done on the graphics driver side first and then on the >>>>> audio side, wouldn't the solution be for the video driver and >>>>> audio driver to communicate somehow? >>>> >>>> Yes, the pm domain support is necessary sooner or later, as I >>>> already discussed shortly with Dave Airlie. However... >>> >>> Liam will propose Gfx driver team to implement a pm domain. >>> >>>>> And resume (and possibly init?) would not complete until first >>>>> the graphics driver has done its resume, and after that, the audio
driver.
>>>>> I e, userspace will remain frozen until both drivers have >>>>> completed a correct resume. >>>> >>>> ... the resume problem can't be fixed only by the usual pm domain
serialization.
>>>> The graphics initialization for the audio bit is done >>>> asynchronously, thus we can't guarantee the audio codec is >>>> really ready after the graphics driver returns from the resume
callback.
>>>> It shows as if it's ready (you can turn it to D0) but actually >>>> it doesn't change on the hardware. Thus, the serialization >>>> with an unsol event wait seems mandatory for the time being. >>>> >>> It seems so. We have not found better solution now :-( I've >>> revised the patch for system suspend/resume case. Please review. >> >> If this problem can be detected by looking at the pin and finding >> that it is in D3, > > No, you can't see that it's D3. The controller chip _shows_ it's > in > D0 while it's actually in D3. That's the problem. > > So, this is actually a hardware design bug. But we need to live > with that.
Then I'm not sure it's the same problem; because for me I can see the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
This is same as what I observed after S3 or connect HDMI/DP cable after boot, on machines with power_save is '0'. Amp-Out vals: [0x00 0x80] Power: setting=D3, actual=D3
If we set the pin state to D0 and unmute the pin again, HDMI can work. So I still think this is also caused by dependency on Gfx.
But the controller cannot find the error when it programs the codec to D0
- when audio driver programs the codec and pins to D0, HW does not return error
- I also added code to check their power state immediately after setting state to D0, HW reports their state did change to D0.
However, after Gfx driver finishes setting up the display pipeline and enabling audio, the pin's state changed to D3 and get muted, as shown by /proc. It seems to me that Gfx HW initialization can override some audio driver settings. So we choose to delay resuming the codec until the unsol event which indicates Gfx is ready.
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events)
- Any time an unsol event comes in, have a haswell specific function
that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it.
- This check could possibly also be done in the prepare hook for extra
safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than that.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
Takashi
On 04/09/2013 08:53 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 08:45:32 +0200, David Henningsson wrote:
On 04/08/2013 05:47 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 9:50 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote:
At Mon, 08 Apr 2013 14:20:28 +0200, David Henningsson wrote: > > On 04/08/2013 01:59 PM, Takashi Iwai wrote: >> At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote: >>> >>> On 04/08/2013 01:06 PM, Lin, Mengdong wrote: >>>>> -----Original Message----- >>>>> From: Takashi Iwai [mailto:tiwai@suse.de] >>>>> Sent: Monday, April 08, 2013 6:24 PM >>>>> To: David Henningsson >>>>> Cc: Lin, Mengdong; alsa-devel@alsa-project.org >>>>> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >>>>> haswell hdmi codec in system resume >>>>> >>>>> At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: >>>>>> >>>>>> I'm still not understanding this patch. >>>>>> >>>>>> We of course cannot have a situation where HDMI jack detection >>>>>> is not correctly working after S3, which looks like would be the case
here?
>>>>> >>>>> No. The patch itself has nothing to do with the HDMI jack detection
at all.
>>>>> This intrinsic unsol event is (according to Intel) guaranteed >>>>> to be issued once at the audio codec initialization, at least for
Haswell.
>>> >>> Well, if no process is using the HDMI codec actively, nothing >>> will initialize the resume, and the HDMI codec will not >>> initialized at all, i e, no unsol events enabled in the first place? >> >> No, the intrinsic unsol event that is issued for this seems >> irrelevant from the jack state (i.e. active usage). >> >>> (The counter argument would be; if nobody uses HDMI codec, does >>> it matter that it is not initialized, but I'm unsure if e g >>> "amixer contents" or listeners on legacy /dev/input actually >>> powers up the chip.) >> >> The problem is that the codec resume is always performed no matter >> whether the device is used or not. This was different in the past >> but it's been changed in that way because there are cases where >> you need to initialize the hardware properly once (e.g. mute-LED
toggling).
>> >> >>>>>> Second, I just saw on a machine we're working on, the >>>>>> symptoms: one of the pins in D3 and the right channel muted. >>>>>> And this was on a clean boot, not after S3 suspend/resume cycle. >>>>> >>>>> Maybe another problem. Maybe not. Does the problem persist if >>>>> you reload the driver (without invocation of alsactl via udev)? >>>> >>>> Hi Takashi and David, >>>> >>>> This is the same dependency issue as after system suspend/resume
cycles, with same phenomenon.
>>>> I duplicated this error if I boot without an HDMI/DP cable connected
and connect the cable later.
>>>> Maybe we need to delay codec operations on initialization like in
resume case.
>>> >>> So it's a problem both on hotplug and S3... >> >> In the case of hotplug, you don't hit the inconsistent power sate >> we're trying to solve. The graphics has been already powered up. >> >> >>>>>> To look at the problem again: If the problem is that something >>>>>> must be done on the graphics driver side first and then on the >>>>>> audio side, wouldn't the solution be for the video driver and >>>>>> audio driver to communicate somehow? >>>>> >>>>> Yes, the pm domain support is necessary sooner or later, as I >>>>> already discussed shortly with Dave Airlie. However... >>>> >>>> Liam will propose Gfx driver team to implement a pm domain. >>>> >>>>>> And resume (and possibly init?) would not complete until first >>>>>> the graphics driver has done its resume, and after that, the audio
driver.
>>>>>> I e, userspace will remain frozen until both drivers have >>>>>> completed a correct resume. >>>>> >>>>> ... the resume problem can't be fixed only by the usual pm domain
serialization.
>>>>> The graphics initialization for the audio bit is done >>>>> asynchronously, thus we can't guarantee the audio codec is >>>>> really ready after the graphics driver returns from the resume
callback.
>>>>> It shows as if it's ready (you can turn it to D0) but actually >>>>> it doesn't change on the hardware. Thus, the serialization >>>>> with an unsol event wait seems mandatory for the time being. >>>>> >>>> It seems so. We have not found better solution now :-( I've >>>> revised the patch for system suspend/resume case. Please review. >>> >>> If this problem can be detected by looking at the pin and finding >>> that it is in D3, >> >> No, you can't see that it's D3. The controller chip _shows_ it's >> in >> D0 while it's actually in D3. That's the problem. >> >> So, this is actually a hardware design bug. But we need to live >> with that. > > Then I'm not sure it's the same problem; because for me I can see > the D3 in the codec proc output.
I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
BTW, the main problem was about the FG node, which you can't judge from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
This is same as what I observed after S3 or connect HDMI/DP cable after boot, on machines with power_save is '0'. Amp-Out vals: [0x00 0x80] Power: setting=D3, actual=D3
If we set the pin state to D0 and unmute the pin again, HDMI can work. So I still think this is also caused by dependency on Gfx.
But the controller cannot find the error when it programs the codec to D0
- when audio driver programs the codec and pins to D0, HW does not return error
- I also added code to check their power state immediately after setting state to D0, HW reports their state did change to D0.
However, after Gfx driver finishes setting up the display pipeline and enabling audio, the pin's state changed to D3 and get muted, as shown by /proc. It seems to me that Gfx HW initialization can override some audio driver settings. So we choose to delay resuming the codec until the unsol event which indicates Gfx is ready.
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events)
- Any time an unsol event comes in, have a haswell specific function
that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it.
- This check could possibly also be done in the prepare hook for extra
safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
At Tue, 09 Apr 2013 09:10:27 +0200, David Henningsson wrote:
On 04/09/2013 08:53 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 08:45:32 +0200, David Henningsson wrote:
On 04/08/2013 05:47 PM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 9:50 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Mon, 08 Apr 2013 15:30:25 +0200, David Henningsson wrote:
On 04/08/2013 02:23 PM, Takashi Iwai wrote: > At Mon, 08 Apr 2013 14:20:28 +0200, > David Henningsson wrote: >> >> On 04/08/2013 01:59 PM, Takashi Iwai wrote: >>> At Mon, 08 Apr 2013 13:35:25 +0200, David Henningsson wrote: >>>> >>>> On 04/08/2013 01:06 PM, Lin, Mengdong wrote: >>>>>> -----Original Message----- >>>>>> From: Takashi Iwai [mailto:tiwai@suse.de] >>>>>> Sent: Monday, April 08, 2013 6:24 PM >>>>>> To: David Henningsson >>>>>> Cc: Lin, Mengdong; alsa-devel@alsa-project.org >>>>>> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >>>>>> haswell hdmi codec in system resume >>>>>> >>>>>> At Mon, 08 Apr 2013 11:49:09 +0200, David Henningsson wrote: >>>>>>> >>>>>>> I'm still not understanding this patch. >>>>>>> >>>>>>> We of course cannot have a situation where HDMI jack detection >>>>>>> is not correctly working after S3, which looks like would be the case
here?
>>>>>> >>>>>> No. The patch itself has nothing to do with the HDMI jack detection
at all.
>>>>>> This intrinsic unsol event is (according to Intel) guaranteed >>>>>> to be issued once at the audio codec initialization, at least for
Haswell.
>>>> >>>> Well, if no process is using the HDMI codec actively, nothing >>>> will initialize the resume, and the HDMI codec will not >>>> initialized at all, i e, no unsol events enabled in the first place? >>> >>> No, the intrinsic unsol event that is issued for this seems >>> irrelevant from the jack state (i.e. active usage). >>> >>>> (The counter argument would be; if nobody uses HDMI codec, does >>>> it matter that it is not initialized, but I'm unsure if e g >>>> "amixer contents" or listeners on legacy /dev/input actually >>>> powers up the chip.) >>> >>> The problem is that the codec resume is always performed no matter >>> whether the device is used or not. This was different in the past >>> but it's been changed in that way because there are cases where >>> you need to initialize the hardware properly once (e.g. mute-LED
toggling).
>>> >>> >>>>>>> Second, I just saw on a machine we're working on, the >>>>>>> symptoms: one of the pins in D3 and the right channel muted. >>>>>>> And this was on a clean boot, not after S3 suspend/resume cycle. >>>>>> >>>>>> Maybe another problem. Maybe not. Does the problem persist if >>>>>> you reload the driver (without invocation of alsactl via udev)? >>>>> >>>>> Hi Takashi and David, >>>>> >>>>> This is the same dependency issue as after system suspend/resume
cycles, with same phenomenon.
>>>>> I duplicated this error if I boot without an HDMI/DP cable connected
and connect the cable later.
>>>>> Maybe we need to delay codec operations on initialization like in
resume case.
>>>> >>>> So it's a problem both on hotplug and S3... >>> >>> In the case of hotplug, you don't hit the inconsistent power sate >>> we're trying to solve. The graphics has been already powered up. >>> >>> >>>>>>> To look at the problem again: If the problem is that something >>>>>>> must be done on the graphics driver side first and then on the >>>>>>> audio side, wouldn't the solution be for the video driver and >>>>>>> audio driver to communicate somehow? >>>>>> >>>>>> Yes, the pm domain support is necessary sooner or later, as I >>>>>> already discussed shortly with Dave Airlie. However... >>>>> >>>>> Liam will propose Gfx driver team to implement a pm domain. >>>>> >>>>>>> And resume (and possibly init?) would not complete until first >>>>>>> the graphics driver has done its resume, and after that, the audio
driver.
>>>>>>> I e, userspace will remain frozen until both drivers have >>>>>>> completed a correct resume. >>>>>> >>>>>> ... the resume problem can't be fixed only by the usual pm domain
serialization.
>>>>>> The graphics initialization for the audio bit is done >>>>>> asynchronously, thus we can't guarantee the audio codec is >>>>>> really ready after the graphics driver returns from the resume
callback.
>>>>>> It shows as if it's ready (you can turn it to D0) but actually >>>>>> it doesn't change on the hardware. Thus, the serialization >>>>>> with an unsol event wait seems mandatory for the time being. >>>>>> >>>>> It seems so. We have not found better solution now :-( I've >>>>> revised the patch for system suspend/resume case. Please review. >>>> >>>> If this problem can be detected by looking at the pin and finding >>>> that it is in D3, >>> >>> No, you can't see that it's D3. The controller chip _shows_ it's >>> in >>> D0 while it's actually in D3. That's the problem. >>> >>> So, this is actually a hardware design bug. But we need to live >>> with that. >> >> Then I'm not sure it's the same problem; because for me I can see >> the D3 in the codec proc output. > > I suppose you didn't set the powersave for it, right?
AFAIK, powersave remains at upstream default.
The option is overridden by user-space, so checking only the default value doesn't make any sense :)
> BTW, the main problem was about the FG node, which you can't judge > from the proc output, unfortunately.
Here's what it looks like for me. Right channel muted and pin in D3.
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP Control: name="HDMI/DP,pcm=3 Jack", index=0, device=0 Control: name="IEC958 Playback Con Mask", index=0, device=0 Control: name="IEC958 Playback Pro Mask", index=0, device=0 Control: name="IEC958 Playback Default", index=0, device=0 Control: name="IEC958 Playback Switch", index=0, device=0 Control: name="ELD", index=0, device=3 Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1 Amp-Out vals: [0x00 0x80] Pincap 0x0b000094: OUT Detect HBR HDMI DP Pin Default 0x18560010: [Jack] Digital Out at Int HDMI Conn = Digital, Color = Unknown DefAssociation = 0x1, Sequence = 0x0 Pin-ctls: 0x40: OUT Unsolicited: tag=01, enabled=1 Power states: D0 D3 EPSS Power: setting=D3, actual=D3 Connection: 3 0x02* 0x03 0x04
Then this might be the different issue.
This is same as what I observed after S3 or connect HDMI/DP cable after boot, on machines with power_save is '0'. Amp-Out vals: [0x00 0x80] Power: setting=D3, actual=D3
If we set the pin state to D0 and unmute the pin again, HDMI can work. So I still think this is also caused by dependency on Gfx.
But the controller cannot find the error when it programs the codec to D0
- when audio driver programs the codec and pins to D0, HW does not return error
- I also added code to check their power state immediately after setting state to D0, HW reports their state did change to D0.
However, after Gfx driver finishes setting up the display pipeline and enabling audio, the pin's state changed to D3 and get muted, as shown by /proc. It seems to me that Gfx HW initialization can override some audio driver settings. So we choose to delay resuming the codec until the unsol event which indicates Gfx is ready.
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events)
- Any time an unsol event comes in, have a haswell specific function
that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it.
- This check could possibly also be done in the prepare hook for extra
safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events)
- Any time an unsol event comes in, have a haswell specific
function that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it.
- This check could possibly also be done in the prepare hook for
extra safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than
that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
Thanks Mengdong
On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events) - Any time an unsol event comes in, have a haswell specificfunction that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it. - This check could possibly also be done in the prepare hook for extra safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than
that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
I've been answering the power_save questions a little bit uncertain so far, and that's because I don't really know. I've not investigated power management much.
But the person with the hardware reports that this problem happens when on AC power only. And that makes sense with your observations about power_save - there could very well be something that turns power_save off when on AC power and on when on battery power.
So, if it's just a matter of re-initializing the pin, what do you think about this solution:
- Resume as normal (this will enable unsol events) - Any time an unsol event comes in, have a haswell specificfunction that checks relevant pins to see that is still in D0 and mute state correct. If not, correct it. - This check could possibly also be done in the prepare hook for extra safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than
that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
/*
- Callbacks
*/ @@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);- if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
> I've been answering the power_save questions a little bit uncertain > so far, and that's because I don't really know. I've not > investigated power management much. > > But the person with the hardware reports that this problem happens > when on AC power only. And that makes sense with your observations > about power_save - there could very well be something that turns > power_save off when on AC power and on when on battery power. > > So, if it's just a matter of re-initializing the pin, what do you > think about this solution: > > - Resume as normal (this will enable unsol events) > - Any time an unsol event comes in, have a haswell specific > function that checks relevant pins to see that is still in D0 and > mute state correct. If not, correct it. > - This check could possibly also be done in the prepare hook for > extra safety.
It won't work reliably. The problem is that the codec might be set up wrongly if it's used before the unsol event comes up. For example, a PCM resume may be performed immediately after the normal resume is finished. Meanwhile the first unsol may arrive much later than
that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
Thus we need to delay the resume operation in anyway. And if so, there is no merit to perform the normal resume operation at the first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
- /*
*/
- Callbacks
@@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);- if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
At Wed, 10 Apr 2013 07:29:51 +0200, David Henningsson wrote:
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
>> I've been answering the power_save questions a little bit uncertain >> so far, and that's because I don't really know. I've not >> investigated power management much. >> >> But the person with the hardware reports that this problem happens >> when on AC power only. And that makes sense with your observations >> about power_save - there could very well be something that turns >> power_save off when on AC power and on when on battery power. >> >> So, if it's just a matter of re-initializing the pin, what do you >> think about this solution: >> >> - Resume as normal (this will enable unsol events) >> - Any time an unsol event comes in, have a haswell specific >> function that checks relevant pins to see that is still in D0 and >> mute state correct. If not, correct it. >> - This check could possibly also be done in the prepare hook for >> extra safety. > > It won't work reliably. The problem is that the codec might be set > up wrongly if it's used before the unsol event comes up. For > example, a PCM resume may be performed immediately after the normal > resume is finished. Meanwhile the first unsol may arrive much later than
that.
Right, but if this will only mean a few samples (max 300 ms?) will never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
> Thus we need to delay the resume operation in anyway. And if so, > there is no merit to perform the normal resume operation at the > first step.
The normal resume operation enables unsol events, which is important, otherwise we don't get the unsol notification from when the gfx pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
I guess the question here is how much of the codec setup is really destroyed by the gfx driver? Is it just a pin in D3 and a right channel mute, or is it much more that needs to be re-done after the gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
.... if the graphics / audio resume serialization is implemented. Currently not, so we take a brute-force way.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
As mentioned, you can't see it from user-space. If you access, the codec is always woken up.
Actually, I prefer the delayed resume to band-aiding the broken setup afterwards. The delayed resume mechanism was proved to work well for long time (it was so as default). We switched to the full resume just because of a few other devices.
So, the biggest concern in the scheme Mengdong suggested is only about the reliability of the unsol event.
Takashi
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
- /*
*/
- Callbacks
@@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);- if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 04/10/2013 07:47 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 07:29:51 +0200, David Henningsson wrote:
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, April 09, 2013 3:43 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
>>> I've been answering the power_save questions a little bit uncertain >>> so far, and that's because I don't really know. I've not >>> investigated power management much. >>> >>> But the person with the hardware reports that this problem happens >>> when on AC power only. And that makes sense with your observations >>> about power_save - there could very well be something that turns >>> power_save off when on AC power and on when on battery power. >>> >>> So, if it's just a matter of re-initializing the pin, what do you >>> think about this solution: >>> >>> - Resume as normal (this will enable unsol events) >>> - Any time an unsol event comes in, have a haswell specific >>> function that checks relevant pins to see that is still in D0 and >>> mute state correct. If not, correct it. >>> - This check could possibly also be done in the prepare hook for >>> extra safety. >> >> It won't work reliably. The problem is that the codec might be set >> up wrongly if it's used before the unsol event comes up. For >> example, a PCM resume may be performed immediately after the normal >> resume is finished. Meanwhile the first unsol may arrive much later than that. > > Right, but if this will only mean a few samples (max 300 ms?) will > never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
It's not only about the missing few samples. The whole setup isn't reliable at that point, and you don't know what to re-setup for the *running* stream.
>> Thus we need to delay the resume operation in anyway. And if so, >> there is no merit to perform the normal resume operation at the >> first step. > > The normal resume operation enables unsol events, which is important, > otherwise we don't get the unsol notification from when the gfx > pipeline has finished.
It seems that the intrinsic unsol event is always issued once when the unsol event is enabled, according to Intel. So, you won't miss it.
But heck, we need more test coverage, obviously.
> I guess the question here is how much of the codec setup is really > destroyed by the gfx driver? Is it just a pin in D3 and a right > channel mute, or is it much more that needs to be re-done after the > gfx driver has finished?
I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
.... if the graphics / audio resume serialization is implemented. Currently not, so we take a brute-force way.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
As mentioned, you can't see it from user-space. If you access, the codec is always woken up.
Actually, I prefer the delayed resume to band-aiding the broken setup afterwards. The delayed resume mechanism was proved to work well for long time (it was so as default). We switched to the full resume just because of a few other devices.
So, the biggest concern in the scheme Mengdong suggested is only about the reliability of the unsol event.
I'm going to make one more try to explain what I think won't work with this scheme.
1. System wakes up from S3 suspend/resume. 2. No initialization is performed because resume is delayed. 3. HDMI/DP cable is plugged into the system. 4. Because unsol events are not enabled (due to the lack of initialization), userspace is not notified that HDMI/DP cable has been plugged in. 5. Userspace now has the wrong idea about whether HDMI/DP cable is plugged in or not.
Takashi
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
- /*
*/
- Callbacks
@@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);- if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
At Wed, 10 Apr 2013 08:02:23 +0200, David Henningsson wrote:
On 04/10/2013 07:47 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 07:29:51 +0200, David Henningsson wrote:
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote:
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, April 09, 2013 3:43 PM > To: David Henningsson > Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R > Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec > in system resume
>>>> I've been answering the power_save questions a little bit uncertain >>>> so far, and that's because I don't really know. I've not >>>> investigated power management much. >>>> >>>> But the person with the hardware reports that this problem happens >>>> when on AC power only. And that makes sense with your observations >>>> about power_save - there could very well be something that turns >>>> power_save off when on AC power and on when on battery power. >>>> >>>> So, if it's just a matter of re-initializing the pin, what do you >>>> think about this solution: >>>> >>>> - Resume as normal (this will enable unsol events) >>>> - Any time an unsol event comes in, have a haswell specific >>>> function that checks relevant pins to see that is still in D0 and >>>> mute state correct. If not, correct it. >>>> - This check could possibly also be done in the prepare hook for >>>> extra safety. >>> >>> It won't work reliably. The problem is that the codec might be set >>> up wrongly if it's used before the unsol event comes up. For >>> example, a PCM resume may be performed immediately after the normal >>> resume is finished. Meanwhile the first unsol may arrive much later than > that. >> >> Right, but if this will only mean a few samples (max 300 ms?) will >> never reach the HDMI cable, it might be the least bad workaround.
I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP.
> It's not only about the missing few samples. The whole setup isn't reliable at > that point, and you don't know what to re-setup for the > *running* stream. > >>> Thus we need to delay the resume operation in anyway. And if so, >>> there is no merit to perform the normal resume operation at the >>> first step. >> >> The normal resume operation enables unsol events, which is important, >> otherwise we don't get the unsol notification from when the gfx >> pipeline has finished. > > It seems that the intrinsic unsol event is always issued once when the unsol > event is enabled, according to Intel. So, you won't miss it. > > But heck, we need more test coverage, obviously. > >> I guess the question here is how much of the codec setup is really >> destroyed by the gfx driver? Is it just a pin in D3 and a right >> channel mute, or is it much more that needs to be re-done after the >> gfx driver has finished? > > I'm afraid it's too native to assume that...
Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
.... if the graphics / audio resume serialization is implemented. Currently not, so we take a brute-force way.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
As mentioned, you can't see it from user-space. If you access, the codec is always woken up.
Actually, I prefer the delayed resume to band-aiding the broken setup afterwards. The delayed resume mechanism was proved to work well for long time (it was so as default). We switched to the full resume just because of a few other devices.
So, the biggest concern in the scheme Mengdong suggested is only about the reliability of the unsol event.
I'm going to make one more try to explain what I think won't work with this scheme.
- System wakes up from S3 suspend/resume.
- No initialization is performed because resume is delayed.
- HDMI/DP cable is plugged into the system.
- Because unsol events are not enabled (due to the lack of
initialization), userspace is not notified that HDMI/DP cable has been plugged in. 5. Userspace now has the wrong idea about whether HDMI/DP cable is plugged in or not.
Well, we can enable only unsol event without setting to D0. For example, instead of adding codec->support_delay_resume, add a new codec ops, codec->patch_ops.delay_resume(). For HDMI codecs, this callback just calls
spec->ready_to_resume = 0;
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid; struct hda_jack_tbl *jack;
per_pin = &spec->pins[pin_idx]; pin_nid = per_pin->pin_nid; jack = snd_hda_jack_tbl_get(codec, pin_nid); if (jack) snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN | jack->tag); }
and returns. The rest would work almost as is. The pin-detection itself should work even in D3 for this chip.
Takashi
Takashi
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
- /*
*/
- Callbacks
@@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);- if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 04/10/2013 08:52 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 08:02:23 +0200, David Henningsson wrote:
On 04/10/2013 07:47 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 07:29:51 +0200, David Henningsson wrote:
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, Mengdong wrote: >> -----Original Message----- >> From: Takashi Iwai [mailto:tiwai@suse.de] >> Sent: Tuesday, April 09, 2013 3:43 PM >> To: David Henningsson >> Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R >> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec >> in system resume > >>>>> I've been answering the power_save questions a little bit uncertain >>>>> so far, and that's because I don't really know. I've not >>>>> investigated power management much. >>>>> >>>>> But the person with the hardware reports that this problem happens >>>>> when on AC power only. And that makes sense with your observations >>>>> about power_save - there could very well be something that turns >>>>> power_save off when on AC power and on when on battery power. >>>>> >>>>> So, if it's just a matter of re-initializing the pin, what do you >>>>> think about this solution: >>>>> >>>>> - Resume as normal (this will enable unsol events) >>>>> - Any time an unsol event comes in, have a haswell specific >>>>> function that checks relevant pins to see that is still in D0 and >>>>> mute state correct. If not, correct it. >>>>> - This check could possibly also be done in the prepare hook for >>>>> extra safety. >>>> >>>> It won't work reliably. The problem is that the codec might be set >>>> up wrongly if it's used before the unsol event comes up. For >>>> example, a PCM resume may be performed immediately after the normal >>>> resume is finished. Meanwhile the first unsol may arrive much later than >> that. >>> >>> Right, but if this will only mean a few samples (max 300 ms?) will >>> never reach the HDMI cable, it might be the least bad workaround. > > I've tried enabling the unsol event on system resume, the unsol event will arrive in 70~80ms for HDMI and about 10ms for DP. > >> It's not only about the missing few samples. The whole setup isn't reliable at >> that point, and you don't know what to re-setup for the >> *running* stream. >> >>>> Thus we need to delay the resume operation in anyway. And if so, >>>> there is no merit to perform the normal resume operation at the >>>> first step. >>> >>> The normal resume operation enables unsol events, which is important, >>> otherwise we don't get the unsol notification from when the gfx >>> pipeline has finished. >> >> It seems that the intrinsic unsol event is always issued once when the unsol >> event is enabled, according to Intel. So, you won't miss it. >> >> But heck, we need more test coverage, obviously. >> >>> I guess the question here is how much of the codec setup is really >>> destroyed by the gfx driver? Is it just a pin in D3 and a right >>> channel mute, or is it much more that needs to be re-done after the >>> gfx driver has finished? >> >> I'm afraid it's too native to assume that... > > Actually, we're not sure how much setting are destroyed, although it seems only the pin power state and mute status are affected.
I made a test patch (attached), sprinkled with printk, and asked the person with hw to test it. He said it fixed the issue. Now, I'm sure he did a simple playback test only, rather than having streams running during S3 or anything like that.
The patch simply sets the pin to D0 and the copies the mute value from the left channel to the right channel. The printk's confirmed this happened.
But at least it should show that not too much of the settings are destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
.... if the graphics / audio resume serialization is implemented. Currently not, so we take a brute-force way.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
As mentioned, you can't see it from user-space. If you access, the codec is always woken up.
Actually, I prefer the delayed resume to band-aiding the broken setup afterwards. The delayed resume mechanism was proved to work well for long time (it was so as default). We switched to the full resume just because of a few other devices.
So, the biggest concern in the scheme Mengdong suggested is only about the reliability of the unsol event.
I'm going to make one more try to explain what I think won't work with this scheme.
- System wakes up from S3 suspend/resume.
- No initialization is performed because resume is delayed.
- HDMI/DP cable is plugged into the system.
- Because unsol events are not enabled (due to the lack of
initialization), userspace is not notified that HDMI/DP cable has been plugged in. 5. Userspace now has the wrong idea about whether HDMI/DP cable is plugged in or not.
Well, we can enable only unsol event without setting to D0.
Yes, this can work. My point is that we can't just do nothing on S3 resume.
It seems Mengdong is not around today. While pm domain synchronisation and more advanced patches are discussed and written, I might try to deploy the attached patch in Ubuntu as an intermediate solution. Feel free to do the same if you wish.
For example, instead of adding codec->support_delay_resume, add a new codec ops, codec->patch_ops.delay_resume(). For HDMI codecs, this callback just calls
spec->ready_to_resume = 0;
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid; struct hda_jack_tbl *jack;
per_pin = &spec->pins[pin_idx]; pin_nid = per_pin->pin_nid; jack = snd_hda_jack_tbl_get(codec, pin_nid); if (jack) snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN | jack->tag);}
and returns. The rest would work almost as is. The pin-detection itself should work even in D3 for this chip.
Takashi
Takashi
Also, a different thought: what are the possibilities that Mengdong can fix the gfx driver side not to destroy the settings in the first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..edee301 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) hdmi_non_intrinsic_event(codec, res); }
+static void haswell_verify_pin_D0(struct hda_codec *codec, hda_nid_t nid) +{
- int pwr, lamp, ramp;
- printk("Haswell: Verify pin D0 on pin 0x%x\n", nid);
- pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);
- pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;
- printk("Haswell: Found pin in state D%d\n", pwr);
- if (pwr != AC_PWRST_D0) {
printk("Haswell: Trying to set power to D0\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_POWER_STATE,AC_PWRST_D0);msleep(40);pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0);pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT;printk("Haswell: Found pin in state D%d\n", pwr);- }
- lamp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);- ramp = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);- printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);
- if (lamp != ramp) {
printk("Haswell: Setting r amp mute to l amp mute\n");snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_AMP_GAIN_MUTE,AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT | lamp);lamp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);ramp = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_AMP_GAIN_MUTE,AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid, lamp, ramp);- }
+}
- /*
*/
- Callbacks
@@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, int pinctl; int new_pinctl = 0;
- if (codec->vendor_id == 0x80862807)
haswell_verify_pin_D0(codec, pin_nid);if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
Hi David and Takashi,
I'm sorry for the late response. I was assigned other tasks this week
We don't have a better solution for this issue now, still trying. The delay_resume() ops for a codec can help not delaying the unsol event. So our patch can solve the problem after system suspend/resume if the cable is connected. But I'm afraid that if the HDMI/DP cable removed during system suspend and connected again sometime after system is resumed, and if the codec access happens before the cable is connected, waiting for unsol event can be time out and codec cannot be properly resumed. And if runtime power saving is also disabled, the audio driver has no chance to resume the codec again. I cannot verify such hot-plug case during system/suspend now because my Haswell machines cannot reach a stable S3 and then resume. But we do observed a similar bug: if cable is connected after boot, the pin is in D3 with right channel muted, as the codec is initialized before unsol event comes. Audio driver cannot find a suitable time out to wait for the usnsol event as we don't know when the cable will be connected.
We'll try if we can fix this dependency issue in Gfx driver side, and by sync Gfx and audio driver processing.
And we cannot implement pm domain atm. Last Thursday, we have a meeting with Gfx team, for Gfx power well support and audio dependency on Gfx. Linux PM maintainer Rafael also attended the meeting. He suggested us not use pm domain now because it cannot fully support PCI devices (PM domains ops will override PCI bus ops). We will use and extend the existing private gfx driver API to control the powerwell and sequence the initialization/suspend/resume events between gfx and audio for internal releases. Once this is working well with the private API, we will then look at either implementing this functionality as PM domain or PM runtime depending on the best fit and upstream.
I'm working on HD-A RTD3 for legacy audio in one or two weeks. After that, I'll continue to work on this issue.
Thanks Mengdong
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Wednesday, April 10, 2013 6:39 PM To: Takashi Iwai Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 04/10/2013 08:52 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 08:02:23 +0200, David Henningsson wrote:
On 04/10/2013 07:47 AM, Takashi Iwai wrote:
At Wed, 10 Apr 2013 07:29:51 +0200, David Henningsson wrote:
On 04/09/2013 11:26 AM, Takashi Iwai wrote:
At Tue, 09 Apr 2013 11:15:13 +0200, David Henningsson wrote: > > [1 <text/plain; ISO-8859-1 (7bit)>] On 04/09/2013 10:18 AM, Lin, > Mengdong wrote: >>> -----Original Message----- >>> From: Takashi Iwai [mailto:tiwai@suse.de] >>> Sent: Tuesday, April 09, 2013 3:43 PM >>> To: David Henningsson >>> Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam >>> R >>> Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume >>> haswell hdmi codec in system resume >> >>>>>> I've been answering the power_save questions a little bit >>>>>> uncertain so far, and that's because I don't really know. >>>>>> I've not investigated power management much. >>>>>> >>>>>> But the person with the hardware reports that this problem >>>>>> happens when on AC power only. And that makes sense with >>>>>> your observations about power_save - there could very well >>>>>> be something that turns power_save off when on AC power and
on when on battery power.
>>>>>> >>>>>> So, if it's just a matter of re-initializing the pin, what >>>>>> do you think about this solution: >>>>>> >>>>>> - Resume as normal (this will enable unsol events) >>>>>> - Any time an unsol event comes in, have a haswell >>>>>> specific function that checks relevant pins to see that is >>>>>> still in D0 and mute state correct. If not, correct it. >>>>>> - This check could possibly also be done in the >>>>>> prepare hook for extra safety. >>>>> >>>>> It won't work reliably. The problem is that the codec might >>>>> be set up wrongly if it's used before the unsol event comes >>>>> up. For example, a PCM resume may be performed immediately >>>>> after the normal resume is finished. Meanwhile the first >>>>> unsol may arrive much later than >>> that. >>>> >>>> Right, but if this will only mean a few samples (max 300 ms?) >>>> will never reach the HDMI cable, it might be the least bad
workaround.
>> >> I've tried enabling the unsol event on system resume, the unsol event
will arrive in 70~80ms for HDMI and about 10ms for DP.
>> >>> It's not only about the missing few samples. The whole setup >>> isn't reliable at that point, and you don't know what to >>> re-setup for the >>> *running* stream. >>> >>>>> Thus we need to delay the resume operation in anyway. And if >>>>> so, there is no merit to perform the normal resume operation >>>>> at the first step. >>>> >>>> The normal resume operation enables unsol events, which is >>>> important, otherwise we don't get the unsol notification from >>>> when the gfx pipeline has finished. >>> >>> It seems that the intrinsic unsol event is always issued once >>> when the unsol event is enabled, according to Intel. So, you won't
miss it.
>>> >>> But heck, we need more test coverage, obviously. >>> >>>> I guess the question here is how much of the codec setup is >>>> really destroyed by the gfx driver? Is it just a pin in D3 and >>>> a right channel mute, or is it much more that needs to be >>>> re-done after the gfx driver has finished? >>> >>> I'm afraid it's too native to assume that... >> >> Actually, we're not sure how much setting are destroyed, although it
seems only the pin power state and mute status are affected.
> > I made a test patch (attached), sprinkled with printk, and asked > the person with hw to test it. He said it fixed the issue. Now, > I'm sure he did a simple playback test only, rather than having > streams running during S3 or anything like that. > > The patch simply sets the pin to D0 and the copies the mute value > from the left channel to the right channel. The printk's confirmed this
happened.
> > But at least it should show that not too much of the settings are > destroyed...or would that be jumping to conclusions too early?
In your test case, the whole stream setup is called after the gfx power up. It's much different from the case where the stream is resumed before the gfx power up. This is the biggest concern.
That being said, the PCM can't be prepared before the gfx chip gets ready.
Shall we have another stab at this today...
I think we need to figure out what actually happens if we do start a stream before gfx is initialized. Mengdong, could you give more information on this?
If it's something we can easily recover from (e g by just executing a few verbs in the unsol event handler), that might be the best option.
If it's not, then we might need the wait event to synchronise the PCM start with the unsol event handler. But there is still no need to delay rest of the chip initialization/resume for that.
.... if the graphics / audio resume serialization is implemented. Currently not, so we take a brute-force way.
But part of this is also that we're discussing two different symptoms here. I've only seen the D3 + right channel mute problem so far. We can easily recover from that, as my workaround patch shows. Takashi was seeing the fg node being D3 (?). I have not seen that problem, does Mengdong know if it also exists on the latest hardware revision?
As mentioned, you can't see it from user-space. If you access, the codec is always woken up.
Actually, I prefer the delayed resume to band-aiding the broken setup afterwards. The delayed resume mechanism was proved to work well for long time (it was so as default). We switched to the full resume just because of a few other devices.
So, the biggest concern in the scheme Mengdong suggested is only about the reliability of the unsol event.
I'm going to make one more try to explain what I think won't work with this scheme.
- System wakes up from S3 suspend/resume.
- No initialization is performed because resume is delayed.
- HDMI/DP cable is plugged into the system.
- Because unsol events are not enabled (due to the lack of
initialization), userspace is not notified that HDMI/DP cable has been plugged in. 5. Userspace now has the wrong idea about whether HDMI/DP cable is plugged in or not.
Well, we can enable only unsol event without setting to D0.
Yes, this can work. My point is that we can't just do nothing on S3 resume.
It seems Mengdong is not around today. While pm domain synchronisation and more advanced patches are discussed and written, I might try to deploy the attached patch in Ubuntu as an intermediate solution. Feel free to do the same if you wish.
For example, instead of adding codec->support_delay_resume, add a new codec ops, codec->patch_ops.delay_resume(). For HDMI codecs, this callback just calls
spec->ready_to_resume = 0;
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid; struct hda_jack_tbl *jack;
per_pin = &spec->pins[pin_idx]; pin_nid = per_pin->pin_nid; jack = snd_hda_jack_tbl_get(codec, pin_nid); if (jack) snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN | jack->tag);}
and returns. The rest would work almost as is. The pin-detection itself should work even in D3 for this chip.
Takashi
Takashi
> Also, a different thought: what are the possibilities that > Mengdong can fix the gfx driver side not to destroy the settings in the
first place?
The serialization with the graphics side is mandatory more or less. But it'd be good if the pm domain implementation would suffice...
Takashi
> -- > David Henningsson, Canonical Ltd. > https://launchpad.net/~diwic > [2 hsw_d3_workaround.patch <text/x-patch (7bit)>] diff --git > a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index > ede8215..edee301 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -1018,6 +1018,46 @@ static void hdmi_unsol_event(struct
hda_codec *codec, unsigned int res)
> hdmi_non_intrinsic_event(codec, res); > } > > +static void haswell_verify_pin_D0(struct hda_codec *codec, > +hda_nid_t nid) { > + int pwr, lamp, ramp; > + printk("Haswell: Verify pin D0 on pin 0x%x\n", nid); > + > + pwr = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_POWER_STATE, 0);
> + pwr = (pwr & AC_PWRST_ACTUAL) >> AC_PWRST_ACTUAL_SHIFT; > + printk("Haswell: Found pin in state D%d\n", pwr); > + if (pwr != AC_PWRST_D0) { > + printk("Haswell: Trying to set power to D0\n"); > + snd_hda_codec_write(codec, nid, 0,
AC_VERB_SET_POWER_STATE,
> + AC_PWRST_D0); > + msleep(40); > + pwr = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_POWER_STATE, 0);
> + pwr = (pwr & AC_PWRST_ACTUAL) >>
AC_PWRST_ACTUAL_SHIFT;
> + printk("Haswell: Found pin in state D%d\n", pwr); > + } > + > + lamp = snd_hda_codec_read(codec, nid, 0, > + AC_VERB_GET_AMP_GAIN_MUTE, > + AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT); > + ramp = snd_hda_codec_read(codec, nid, 0, > + AC_VERB_GET_AMP_GAIN_MUTE, > + AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT); > + printk("Haswell: Verify mute on pin 0x%x: [0x%x 0x%x]\n", nid, lamp,
ramp);
> + if (lamp != ramp) { > + printk("Haswell: Setting r amp mute to l amp mute\n"); > + snd_hda_codec_write(codec, nid, 0,
AC_VERB_SET_AMP_GAIN_MUTE,
> + AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT |
lamp);
> + > + lamp = snd_hda_codec_read(codec, nid, 0, > + AC_VERB_GET_AMP_GAIN_MUTE, > + AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT); > + ramp = snd_hda_codec_read(codec, nid, 0, > + AC_VERB_GET_AMP_GAIN_MUTE, > + AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT); > + printk("Haswell: Mute after set on pin 0x%x: [0x%x 0x%x]\n", nid,
lamp, ramp);
> + } > +} > + > /* > * Callbacks > */ > @@ -1032,6 +1072,9 @@ static int hdmi_setup_stream(struct
hda_codec *codec, hda_nid_t cvt_nid,
> int pinctl; > int new_pinctl = 0; > > + if (codec->vendor_id == 0x80862807) > + haswell_verify_pin_D0(codec, pin_nid); > + > if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR)
{
> pinctl = snd_hda_codec_read(codec, pin_nid, 0, > AC_VERB_GET_PIN_WIDGET_CONTROL,
0);
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 04/14/2013 03:48 PM, Lin, Mengdong wrote:
Hi David and Takashi,
I'm sorry for the late response. I was assigned other tasks this week
We don't have a better solution for this issue now, still trying. The delay_resume() ops for a codec can help not delaying the unsol event. So our patch can solve the problem after system suspend/resume if the cable is connected. But I'm afraid that if the HDMI/DP cable removed during system suspend and connected again sometime after system is resumed, and if the codec access happens before the cable is connected, waiting for unsol event can be time out and codec cannot be properly resumed. And if runtime power saving is also disabled, the audio driver has no chance to resume the codec again. I cannot verify such hot-plug case during system/suspend now because my Haswell machines cannot reach a stable S3 and then resume. But we do observed a similar bug: if cable is connected after boot, the pin is in D3 with right channel muted, as the codec is initialized before unsol event comes. Audio driver cannot find a suitable time out to wait for the usnsol event as we don't know when the cable will be connected.
Takashi's latest suggestion was to enable unsol events, and nothing else, directly on S3 resume. See suggested patch below.
Will that not help here? Then we would at least get some unsol events on hotplug I assume?
We'll try if we can fix this dependency issue in Gfx driver side, and by sync Gfx and audio driver processing.
And we cannot implement pm domain atm. Last Thursday, we have a meeting with Gfx team, for Gfx power well support and audio dependency on Gfx. Linux PM maintainer Rafael also attended the meeting. He suggested us not use pm domain now because it cannot fully support PCI devices (PM domains ops will override PCI bus ops). We will use and extend the existing private gfx driver API to control the powerwell and sequence the initialization/suspend/resume events between gfx and audio for internal releases. Once this is working well with the private API, we will then look at either implementing this functionality as PM domain or PM runtime depending on the best fit and upstream.
I'm working on HD-A RTD3 for legacy audio in one or two weeks. After that, I'll continue to work on this issue.
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
Thanks Mengdong
I'm going to make one more try to explain what I think won't work with this scheme.
- System wakes up from S3 suspend/resume.
- No initialization is performed because resume is delayed.
- HDMI/DP cable is plugged into the system.
- Because unsol events are not enabled (due to the lack of
initialization), userspace is not notified that HDMI/DP cable has been plugged in. 5. Userspace now has the wrong idea about whether HDMI/DP cable is plugged in or not.
Well, we can enable only unsol event without setting to D0.
Yes, this can work. My point is that we can't just do nothing on S3 resume.
It seems Mengdong is not around today. While pm domain synchronisation and more advanced patches are discussed and written, I might try to deploy the attached patch in Ubuntu as an intermediate solution. Feel free to do the same if you wish.
For example, instead of adding codec->support_delay_resume, add a new codec ops, codec->patch_ops.delay_resume(). For HDMI codecs, this callback just calls
spec->ready_to_resume = 0;
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin; hda_nid_t pin_nid; struct hda_jack_tbl *jack;
per_pin = &spec->pins[pin_idx]; pin_nid = per_pin->pin_nid; jack = snd_hda_jack_tbl_get(codec, pin_nid); if (jack) snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN | jack->tag);}
and returns. The rest would work almost as is. The pin-detection itself should work even in D3 for this chip.
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, April 15, 2013 3:03 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 04/14/2013 03:48 PM, Lin, Mengdong wrote:
Hi David and Takashi,
I'm sorry for the late response. I was assigned other tasks this week
We don't have a better solution for this issue now, still trying. The delay_resume() ops for a codec can help not delaying the unsol event. So
our patch can solve the problem after system suspend/resume if the cable is connected.
But I'm afraid that if the HDMI/DP cable removed during system suspend and connected again sometime after system is resumed, and if the codec
access happens before the cable is connected, waiting for unsol event can be time out and codec cannot be properly resumed.
And if runtime power saving is also disabled, the audio driver has no chance to
resume the codec again. I cannot verify such hot-plug case during system/suspend now because my Haswell machines cannot reach a stable S3 and then resume.
But we do observed a similar bug: if cable is connected after boot, the pin is in
D3 with right channel muted, as the codec is initialized before unsol event comes.
Audio driver cannot find a suitable time out to wait for the usnsol event as we
don't know when the cable will be connected.
Takashi's latest suggestion was to enable unsol events, and nothing else, directly on S3 resume. See suggested patch below.
Will that not help here? Then we would at least get some unsol events on hotplug I assume?
I think Takashi's patch can help when cable is connected during suspend/resume cycles. This patch enables unsol event, so make sure unsol event won't be delayed.
So shall I improve the patch as Takashi suggested, to solve current suspend/resume issue?
We'll try if we can fix this dependency issue in Gfx driver side, and by sync Gfx
and audio driver processing.
And we cannot implement pm domain atm. Last Thursday, we have a meeting
with Gfx team, for Gfx power well support and audio dependency on Gfx.
Linux PM maintainer Rafael also attended the meeting. He suggested us not
use pm domain now because it cannot fully support PCI devices (PM domains ops will override PCI bus ops). We will use and extend the existing private gfx driver API to control the powerwell and sequence the initialization/suspend/resume events between gfx and audio for internal releases. Once this is working well with the private API, we will then look at either implementing this functionality as PM domain or PM runtime depending on the best fit and upstream.
I'm working on HD-A RTD3 for legacy audio in one or two weeks. After that,
I'll continue to work on this issue.
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
We're still investigating the Gfx driver sequence. In fact, not every hot plug will cause trouble. If the system is booted with cable connected, later hot-plug does not cause error, including switch between DP and HDMI cable. The dependency issue only happens on boot and resume. Since the hot-plug will also make Gfx reconfig the display pipeline, so we want to compare the difference and locate the accurate dependency.
Thanks Mengdong
On 04/15/2013 09:48 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, April 15, 2013 3:03 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 04/14/2013 03:48 PM, Lin, Mengdong wrote:
Hi David and Takashi,
I'm sorry for the late response. I was assigned other tasks this week
We don't have a better solution for this issue now, still trying. The delay_resume() ops for a codec can help not delaying the unsol event. So
our patch can solve the problem after system suspend/resume if the cable is connected.
But I'm afraid that if the HDMI/DP cable removed during system suspend and connected again sometime after system is resumed, and if the codec
access happens before the cable is connected, waiting for unsol event can be time out and codec cannot be properly resumed.
And if runtime power saving is also disabled, the audio driver has no chance to
resume the codec again. I cannot verify such hot-plug case during system/suspend now because my Haswell machines cannot reach a stable S3 and then resume.
But we do observed a similar bug: if cable is connected after boot, the pin is in
D3 with right channel muted, as the codec is initialized before unsol event comes.
Audio driver cannot find a suitable time out to wait for the usnsol event as we
don't know when the cable will be connected.
Takashi's latest suggestion was to enable unsol events, and nothing else, directly on S3 resume. See suggested patch below.
Will that not help here? Then we would at least get some unsol events on hotplug I assume?
I think Takashi's patch can help when cable is connected during suspend/resume cycles. This patch enables unsol event, so make sure unsol event won't be delayed.
So shall I improve the patch as Takashi suggested, to solve current suspend/resume issue?
If you do, in what scenarios will this not be enough? I e, what will still require more advanced synchronisation between gfx and audio driver (as outlined below)?
We'll try if we can fix this dependency issue in Gfx driver side, and by sync Gfx
and audio driver processing.
And we cannot implement pm domain atm. Last Thursday, we have a meeting
with Gfx team, for Gfx power well support and audio dependency on Gfx.
Linux PM maintainer Rafael also attended the meeting. He suggested us not
use pm domain now because it cannot fully support PCI devices (PM domains ops will override PCI bus ops). We will use and extend the existing private gfx driver API to control the powerwell and sequence the initialization/suspend/resume events between gfx and audio for internal releases. Once this is working well with the private API, we will then look at either implementing this functionality as PM domain or PM runtime depending on the best fit and upstream.
I'm working on HD-A RTD3 for legacy audio in one or two weeks. After that,
I'll continue to work on this issue.
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
We're still investigating the Gfx driver sequence. In fact, not every hot plug will cause trouble. If the system is booted with cable connected, later hot-plug does not cause error, including switch between DP and HDMI cable. The dependency issue only happens on boot and resume. Since the hot-plug will also make Gfx reconfig the display pipeline, so we want to compare the difference and locate the accurate dependency.
It looks like this will take some time to implement, and the 3.10 merge window will soon open. It also sounds like the complexity makes it possible that a full fix can not be applied later during 3.10 rc cycle.
Takashi, may I ask you to apply the attached workaround in the mean time? I've confirmed it to resolve the problem on at least two different machines.
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
On 04/15/2013 09:48 AM, Lin, Mengdong wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, April 15, 2013 3:03 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 04/14/2013 03:48 PM, Lin, Mengdong wrote:
Hi David and Takashi,
I'm sorry for the late response. I was assigned other tasks this week
We don't have a better solution for this issue now, still trying. The delay_resume() ops for a codec can help not delaying the unsol event. So
our patch can solve the problem after system suspend/resume if the cable is connected.
But I'm afraid that if the HDMI/DP cable removed during system suspend and connected again sometime after system is resumed, and if the codec
access happens before the cable is connected, waiting for unsol event can be time out and codec cannot be properly resumed.
And if runtime power saving is also disabled, the audio driver has no chance to
resume the codec again. I cannot verify such hot-plug case during system/suspend now because my Haswell machines cannot reach a stable S3 and then resume.
But we do observed a similar bug: if cable is connected after boot, the pin is in
D3 with right channel muted, as the codec is initialized before unsol event comes.
Audio driver cannot find a suitable time out to wait for the usnsol event as we
don't know when the cable will be connected.
Takashi's latest suggestion was to enable unsol events, and nothing else, directly on S3 resume. See suggested patch below.
Will that not help here? Then we would at least get some unsol events on hotplug I assume?
I think Takashi's patch can help when cable is connected during suspend/resume cycles. This patch enables unsol event, so make sure unsol event won't be delayed.
So shall I improve the patch as Takashi suggested, to solve current suspend/resume issue?
If you do, in what scenarios will this not be enough? I e, what will still require more advanced synchronisation between gfx and audio driver (as outlined below)?
We'll try if we can fix this dependency issue in Gfx driver side, and by sync Gfx
and audio driver processing.
And we cannot implement pm domain atm. Last Thursday, we have a meeting
with Gfx team, for Gfx power well support and audio dependency on Gfx.
Linux PM maintainer Rafael also attended the meeting. He suggested us not
use pm domain now because it cannot fully support PCI devices (PM domains ops will override PCI bus ops). We will use and extend the existing private gfx driver API to control the powerwell and sequence the initialization/suspend/resume events between gfx and audio for internal releases. Once this is working well with the private API, we will then look at either implementing this functionality as PM domain or PM runtime depending on the best fit and upstream.
I'm working on HD-A RTD3 for legacy audio in one or two weeks. After that,
I'll continue to work on this issue.
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
We're still investigating the Gfx driver sequence. In fact, not every hot plug will cause trouble. If the system is booted with cable connected, later hot-plug does not cause error, including switch between DP and HDMI cable. The dependency issue only happens on boot and resume. Since the hot-plug will also make Gfx reconfig the display pipeline, so we want to compare the difference and locate the accurate dependency.
It looks like this will take some time to implement, and the 3.10 merge window will soon open. It also sounds like the complexity makes it possible that a full fix can not be applied later during 3.10 rc cycle.
Takashi, may I ask you to apply the attached workaround in the mean time? I've confirmed it to resolve the problem on at least two different machines.
Yeah, looking through the development, I'm inclined to postpone the delayed resume scheme for 3.10. As a bandaid fix, I'm going to apply David's patch for now.
IMO, the biggest concern in the current delayed resume implementation is that we don't know how reliable the unsol event generation and handling. In other words, if the graphics driver can _always_ sets and/or wakes up for the hardware readiness state, it'd be a much better way to control; it's more deterministic.
Then we can implement, e.g. a platform device shared by both graphics and audio drivers and let pm operations sync there, as Liam once suggested. This would work even w/o pm domain implementation (it's a kind of own pm domain implementation, after all).
Takashi
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 17, 2013 2:13 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
It was confirmed that this a per pin dependency, not codec-wide.
For Haswell, every pin is in D3 with amplifier muted by default. The audio driver must program the pin to D0 and unmute the pin, after the gfx driver connect the GPU pipe and port, and enable the transcoder (The transcoder is the component where audio is mixed with video). Otherwise, the pin's default power and mute state will not change, by checking the Gfx side audio register, although audio codec does not report error.
In addition, HDMI/DP cable hot plug will trigger Gfx driver to disconnect/connect the pipeline, and port may be connected to a new transcoder. If the transcoder has changed, the pin can return to D3 and muted again. There are 3 pipelines in GPU, a pipe/port connection only affects one pin, not the other pins and convertors. So our previous patch to delay powering up the whole codec once is not enough and not suitable.
Since audio driver does not and should not have any knowledge how GfX maniplute the display pipeline. The unsolicited event can be used to as flag that the pin widget is ready to power up.
And because the unsol event can happen at D3 and it not necessarily to power up the pin at once on the unsol event, we prefer to check and fix the pin state when a PCM stream is setting up. This can also avoid race in PM.
We also checked the info frame. Current mechanism to check and refresh audio info frame works well.
So David's patch is a reliable fix to solve the display audio dependency issue. There is no need to write a new patch atm.
Thanks Mengdong
It looks like this will take some time to implement, and the 3.10 merge window will soon open. It also sounds like the complexity makes it possible that a full fix can not be applied later during 3.10 rc cycle.
Takashi, may I ask you to apply the attached workaround in the mean time? I've confirmed it to resolve the problem on at least two different machines.
Yeah, looking through the development, I'm inclined to postpone the delayed resume scheme for 3.10. As a bandaid fix, I'm going to apply David's patch for now.
IMO, the biggest concern in the current delayed resume implementation is that we don't know how reliable the unsol event generation and handling. In other words, if the graphics driver can _always_ sets and/or wakes up for the hardware readiness state, it'd be a much better way to control; it's more deterministic.
Then we can implement, e.g. a platform device shared by both graphics and audio drivers and let pm operations sync there, as Liam once suggested. This would work even w/o pm domain implementation (it's a kind of own pm domain implementation, after all).
On 05/03/2013 08:56 AM, Lin, Mengdong wrote:
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 17, 2013 2:13 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
It was confirmed that this a per pin dependency, not codec-wide.
For Haswell, every pin is in D3 with amplifier muted by default. The audio driver must program the pin to D0 and unmute the pin, after the gfx driver connect the GPU pipe and port, and enable the transcoder (The transcoder is the component where audio is mixed with video). Otherwise, the pin's default power and mute state will not change, by checking the Gfx side audio register, although audio codec does not report error.
In addition, HDMI/DP cable hot plug will trigger Gfx driver to disconnect/connect the pipeline, and port may be connected to a new transcoder. If the transcoder has changed, the pin can return to D3 and muted again. There are 3 pipelines in GPU, a pipe/port connection only affects one pin, not the other pins and convertors. So our previous patch to delay powering up the whole codec once is not enough and not suitable.
This makes me a bit worried actually. If the transcoder to port connection can be changed at runtime, how does this affect the default pin config on the audio codec pin node?
E g, if the machine only has one physical HDMI output, it would be a good thing to mark only one of the pin nodes as a "jack" and the other two as "not connected".
Now, if the transcoder used for this HDMI output is varying, it can be sometimes not matching the audio codec pin node set to "jack", and then we have a problem...!
Or is the gfx engine taking the default pin config of the audio codec into account somehow?
Since audio driver does not and should not have any knowledge how GfX maniplute the display pipeline. The unsolicited event can be used to as flag that the pin widget is ready to power up.
And because the unsol event can happen at D3 and it not necessarily to power up the pin at once on the unsol event, we prefer to check and fix the pin state when a PCM stream is setting up. This can also avoid race in PM.
We also checked the info frame. Current mechanism to check and refresh audio info frame works well.
So David's patch is a reliable fix to solve the display audio dependency issue. There is no need to write a new patch atm.
Thanks Mengdong
It looks like this will take some time to implement, and the 3.10 merge window will soon open. It also sounds like the complexity makes it possible that a full fix can not be applied later during 3.10 rc cycle.
Takashi, may I ask you to apply the attached workaround in the mean time? I've confirmed it to resolve the problem on at least two different machines.
Yeah, looking through the development, I'm inclined to postpone the delayed resume scheme for 3.10. As a bandaid fix, I'm going to apply David's patch for now.
IMO, the biggest concern in the current delayed resume implementation is that we don't know how reliable the unsol event generation and handling. In other words, if the graphics driver can _always_ sets and/or wakes up for the hardware readiness state, it'd be a much better way to control; it's more deterministic.
Then we can implement, e.g. a platform device shared by both graphics and audio drivers and let pm operations sync there, as Liam once suggested. This would work even w/o pm domain implementation (it's a kind of own pm domain implementation, after all).
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, May 03, 2013 3:20 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R; Wang, Xingchao; Li, Jocelyn Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 05/03/2013 08:56 AM, Lin, Mengdong wrote:
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 17, 2013 2:13 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
The dependency on the Gfx driver, is it both codec-wide and per pin? It seems to be at least per pin, which means that whenever we plug something in, we need to redo our initialization of that pin after the Gfx driver has finished, is that correct?
It was confirmed that this a per pin dependency, not codec-wide.
For Haswell, every pin is in D3 with amplifier muted by default. The audio driver must program the pin to D0 and unmute the pin, after the gfx driver connect the GPU pipe and port, and enable the transcoder (The
transcoder is the component where audio is mixed with video).
Otherwise, the pin's default power and mute state will not change, by
checking the Gfx side audio register, although audio codec does not report error.
In addition, HDMI/DP cable hot plug will trigger Gfx driver to
disconnect/connect the pipeline, and port may be connected to a new transcoder.
If the transcoder has changed, the pin can return to D3 and muted again.
There are 3 pipelines in GPU, a pipe/port connection only affects one pin, not the other pins and convertors.
So our previous patch to delay powering up the whole codec once is not
enough and not suitable.
This makes me a bit worried actually. If the transcoder to port connection can be changed at runtime, how does this affect the default pin config on the audio codec pin node?
E g, if the machine only has one physical HDMI output, it would be a good thing to mark only one of the pin nodes as a "jack" and the other two as "not connected".
Now, if the transcoder used for this HDMI output is varying, it can be sometimes not matching the audio codec pin node set to "jack", and then we have a problem...!
Or is the gfx engine taking the default pin config of the audio codec into account somehow?
I think the pin configuration default should not be affected, as it's for BIOS programming.
The DDI port is bound to a HDMI or DP connector on the board. And although the transcoder can be changed at runtime, the pin report the eld info does not change.
Anyway, we'll double check this issue.
Thanks for reminding! Mengdong
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Lin, Mengdong Sent: Friday, May 03, 2013 3:30 PM To: David Henningsson Cc: Takashi Iwai; Li, Jocelyn; alsa-devel@alsa-project.org; Wang, Xingchao; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, May 03, 2013 3:20 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R; Wang, Xingchao; Li, Jocelyn Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 05/03/2013 08:56 AM, Lin, Mengdong wrote:
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 17, 2013 2:13 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
> The dependency on the Gfx driver, is it both codec-wide and per > pin? It seems to be at least per pin, which means that whenever > we plug something in, we need to redo our initialization of that > pin after the Gfx driver has finished, is that correct?
It was confirmed that this a per pin dependency, not codec-wide.
For Haswell, every pin is in D3 with amplifier muted by default. The audio driver must program the pin to D0 and unmute the pin, after the gfx driver connect the GPU pipe and port, and enable the transcoder (The
transcoder is the component where audio is mixed with video).
Otherwise, the pin's default power and mute state will not change, by
checking the Gfx side audio register, although audio codec does not report error.
In addition, HDMI/DP cable hot plug will trigger Gfx driver to
disconnect/connect the pipeline, and port may be connected to a new transcoder.
If the transcoder has changed, the pin can return to D3 and muted again.
There are 3 pipelines in GPU, a pipe/port connection only affects one pin, not the other pins and convertors.
So our previous patch to delay powering up the whole codec once is not
enough and not suitable.
This makes me a bit worried actually. If the transcoder to port connection can be changed at runtime, how does this affect the default pin config on the audio codec pin node?
E g, if the machine only has one physical HDMI output, it would be a good thing to mark only one of the pin nodes as a "jack" and the other two as "not connected".
Now, if the transcoder used for this HDMI output is varying, it can be sometimes not matching the audio codec pin node set to "jack", and then we have a problem...!
Or is the gfx engine taking the default pin config of the audio codec into account somehow?
I think the pin configuration default should not be affected, as it's for BIOS programming.
The DDI port is bound to a HDMI or DP connector on the board. And although the transcoder can be changed at runtime, the pin report the eld info does not change.
Anyway, we'll double check this issue.
The pin configuration default will not change after the DDI port is connected to a different transcoder. I verified this on my haswell machine.
Thanks Mengdong
On 05/06/2013 08:27 AM, Lin, Mengdong wrote:
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Lin, Mengdong Sent: Friday, May 03, 2013 3:30 PM To: David Henningsson Cc: Takashi Iwai; Li, Jocelyn; alsa-devel@alsa-project.org; Wang, Xingchao; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, May 03, 2013 3:20 PM To: Lin, Mengdong Cc: Takashi Iwai; alsa-devel@alsa-project.org; Girdwood, Liam R; Wang, Xingchao; Li, Jocelyn Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
On 05/03/2013 08:56 AM, Lin, Mengdong wrote:
Hi Takashi and David,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 17, 2013 2:13 PM To: David Henningsson Cc: Lin, Mengdong; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
At Wed, 17 Apr 2013 07:51:47 +0200, David Henningsson wrote:
>> The dependency on the Gfx driver, is it both codec-wide and per >> pin? It seems to be at least per pin, which means that whenever >> we plug something in, we need to redo our initialization of that >> pin after the Gfx driver has finished, is that correct?
It was confirmed that this a per pin dependency, not codec-wide.
For Haswell, every pin is in D3 with amplifier muted by default. The audio driver must program the pin to D0 and unmute the pin, after the gfx driver connect the GPU pipe and port, and enable the transcoder (The
transcoder is the component where audio is mixed with video).
Otherwise, the pin's default power and mute state will not change, by
checking the Gfx side audio register, although audio codec does not report error.
In addition, HDMI/DP cable hot plug will trigger Gfx driver to
disconnect/connect the pipeline, and port may be connected to a new transcoder.
If the transcoder has changed, the pin can return to D3 and muted again.
There are 3 pipelines in GPU, a pipe/port connection only affects one pin, not the other pins and convertors.
So our previous patch to delay powering up the whole codec once is not
enough and not suitable.
This makes me a bit worried actually. If the transcoder to port connection can be changed at runtime, how does this affect the default pin config on the audio codec pin node?
E g, if the machine only has one physical HDMI output, it would be a good thing to mark only one of the pin nodes as a "jack" and the other two as "not connected".
Now, if the transcoder used for this HDMI output is varying, it can be sometimes not matching the audio codec pin node set to "jack", and then we have a problem...!
Or is the gfx engine taking the default pin config of the audio codec into account somehow?
I think the pin configuration default should not be affected, as it's for BIOS programming.
The DDI port is bound to a HDMI or DP connector on the board. And although the transcoder can be changed at runtime, the pin report the eld info does not change.
Anyway, we'll double check this issue.
The pin configuration default will not change after the DDI port is connected to a different transcoder. I verified this on my haswell machine.
The core of the problem is that the DDI port can sometimes be connected to a transcoder that in turn is connected to an audio codec pin which default pin config is set to "not connected". In this case, audio will not work correctly. (It might sometimes work by accident.)
The solution is that DDI to transcoder connection cannot change, it must always be connected to the audio codec pin which is marked as "jack".
Btw, even if all audio codec pins are marked as "jack", it would be a bad habit to change the connection, because people will be confused when we show these pins to the user as e g "HDMI 1", "HDMI 2" and "HDMI 3", and which one is corresponding to the physical output is varying.
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Monday, May 06, 2013 3:08 PM To: Lin, Mengdong Cc: Takashi Iwai; Li, Jocelyn; alsa-devel@alsa-project.org; Girdwood, Liam R; Wang, Xingchao Subject: Re: [alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
The core of the problem is that the DDI port can sometimes be connected to a transcoder that in turn is connected to an audio codec pin which default pin config is set to "not connected". In this case, audio will not work correctly. (It might sometimes work by accident.)
The solution is that DDI to transcoder connection cannot change, it must always be connected to the audio codec pin which is marked as "jack".
Hi David,
The gfx driver will decide which pipe is used for a DDI port. And the connection can change by hot plug events. The gfx driver will always try to use the 1st pipe if it's free, because the 1st pipe is not affected by the power down well. Thus the user can get display while the other parts are turned off for power saving.
I think if everything is properly configured, the pin to DDI port mapping should not change even if the port is connected to a different transcoder. There may be something wrong or missing in current audio or gfx driver.
Btw, even if all audio codec pins are marked as "jack", it would be a bad habit to change the connection, because people will be confused when we show these pins to the user as e g "HDMI 1", "HDMI 2" and "HDMI 3", and which one is corresponding to the physical output is varying.
Yes. This would confuse users.
Thanks Mengdong
At Tue, 26 Mar 2013 14:12:52 -0400, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
In system resume, Haswell codec cannot be programmed to D0 before Gfx driver initializes the display pipeline and audio, which will trigger an unsol event on the pin with HDMI/DP cable connected. Otherwise, the connected pin will stay in D3 with right channel muted and thus no sound can be heard.
This patch
- adds a codec flag to delay resuming a codec. System resume will skip the codecs if this flag is set, and these codecs will be resumed on later codec access.
- adds a set_power_state ops for Haswell HDMI codec. In a delayed resume, this ops will enable and wait for the unsol event, and then resume the codec. A 300ms timeout is set in case unsol event is lost.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
I tried the patch on my HSW test machine, but this doesn't seem to work well. The machine shows the dead codec communication after S3. And I hoped that the problem could be fixed by this, but it wasn't.
It seems that the graphics doesn't give any unsol event after S3. And it doesn't power up correctly as well. So still something is wrong around graphics, maybe some initialization bits are missing.
FWIW, I tested the patch on vanilla 3.9-rc5+, with and without my for-linus/for-next branch merges, and also with Daniel's drm-intel-next branch.
Which machine did you test it?
I don't mind to merge the revised patch, though, since it doesn't break things (it's already broken), though.
thanks,
Takashi
At Tue, 26 Mar 2013 14:12:52 -0400, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
In system resume, Haswell codec cannot be programmed to D0 before Gfx driver initializes the display pipeline and audio, which will trigger an unsol event on the pin with HDMI/DP cable connected. Otherwise, the connected pin will stay in D3 with right channel muted and thus no sound can be heard.
This patch
- adds a codec flag to delay resuming a codec. System resume will skip the codecs if this flag is set, and these codecs will be resumed on later codec access.
- adds a set_power_state ops for Haswell HDMI codec. In a delayed resume, this ops will enable and wait for the unsol event, and then resume the codec. A 300ms timeout is set in case unsol event is lost.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
Some other comments:
- Please rebase your patch to for-next branch of sound git tree. It can't be applied cleanly due to recent other changes.
- Fix some strange indentations and spaces (e.g. in intel_haswell_set_power_state)
thanks,
Takashi
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 04b5738..bcb7205 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -5508,11 +5508,15 @@ int snd_hda_resume(struct hda_bus *bus) struct hda_codec *codec;
list_for_each_entry(codec, &bus->codec_list, list) {
hda_call_codec_resume(codec);
if (codec->support_delay_resume)codec->resume_delayed = 1;elsehda_call_codec_resume(codec);} EXPORT_SYMBOL_HDA(snd_hda_resume);
#endif /* CONFIG_PM */
/* diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 23ca172..5b5e5f4 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -886,6 +886,8 @@ struct hda_codec { unsigned int d3_stop_clk:1; /* support D3 operation without BCLK */ unsigned int pm_down_notified:1; /* PM notified to controller */ unsigned int in_pm:1; /* suspend/resume being performed */
- unsigned int support_delay_resume:1; /* codec support delay resume */
- unsigned int resume_delayed:1; /* resume delayed by PM */ int power_transition; /* power-state in transition */ int power_count; /* current (global) power refcount */ struct delayed_work power_work; /* delayed task for powerdown */
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 78e1827..d116908 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -98,6 +98,14 @@ struct hdmi_spec { */ struct hda_multi_out multiout; struct hda_pcm_stream pcm_playback;
+#ifdef CONFIG_PM
- /*
* Non-generic Intel Haswell specific*/- unsigned int ready_to_resume:1;
- wait_queue_head_t resume_wq;
+#endif };
@@ -977,6 +985,13 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) if (pin_idx < 0) return;
+#ifdef CONFIG_PM
- if (codec->resume_delayed) {
spec->ready_to_resume = 1;wake_up(&spec->resume_wq);- }
+#endif
- hdmi_present_sense(&spec->pins[pin_idx], 1); snd_hda_jack_report_sync(codec);
} @@ -1846,6 +1861,63 @@ static const struct hda_fixup hdmi_fixups[] = { };
+#ifdef CONFIG_PM +static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) +{
- struct hdmi_spec *spec = codec->spec;
- int pin_idx;
- spec->ready_to_resume = 0;
- for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
struct hdmi_spec_per_pin *per_pin;hda_nid_t pin_nid;struct hda_jack_tbl *jack;per_pin = &spec->pins[pin_idx];pin_nid = per_pin->pin_nid;jack = snd_hda_jack_tbl_get(codec, pin_nid);if (jack)snd_hda_codec_write(codec, pin_nid, 0,AC_VERB_SET_UNSOLICITED_ENABLE,AC_USRSP_EN | jack->tag);- }
- wait_event_timeout(spec->resume_wq,
spec->ready_to_resume, msecs_to_jiffies(300));- if (!spec->ready_to_resume)
snd_printd(KERN_WARNING "HDMI: Haswell not ready to resume\n");+}
+static void intel_haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
unsigned int power_state)+{
- if (codec->resume_delayed && power_state == AC_PWRST_D0) {
intel_haswell_wait_ready_to_resume(codec);codec->resume_delayed = 0;- }
- snd_hda_codec_read(codec, fg, 0,
AC_VERB_SET_POWER_STATE,power_state);- snd_hda_codec_set_power_to_all(codec, fg, power_state);
+}
+static inline void intel_haswell_allow_delay_resume(struct hda_codec *codec) +{
- struct hdmi_spec *spec = codec->spec;
- init_waitqueue_head(&spec->resume_wq);
- codec->support_delay_resume = 1;
- codec->patch_ops.set_power_state =
intel_haswell_set_power_state;+} +#else +define intel_haswell_allow_delay_resume NULL +#endif
static int patch_generic_hdmi(struct hda_codec *codec) { struct hdmi_spec *spec; @@ -1868,6 +1940,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) return -EINVAL; } codec->patch_ops = generic_hdmi_patch_ops;
if (codec->vendor_id == 0x80862807)
intel_haswell_allow_delay_resume(codec);generic_hdmi_init_per_pins(codec);
init_channel_allocations();
-- 1.7.10.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (4)
-
David Henningsson -
Lin, Mengdong -
mengdong.lin＠intel.com
-
Takashi Iwai