[alsa-devel] [PATCH 3/3] ALSA: hda - delay resuming 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.
In this patch, Haswell codec will claim it needs "delaying resume" by setting flag "support_delay_resume", so system resume will skip it and mark it as "delay_resumed". On later codec access, Haswell codec will be resumed and hda_call_codec_resume() will call its set_power_states ops intel_haswell_set_ power_state(). For 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/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..f39e374 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -89,6 +89,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 };
@@ -975,6 +983,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(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec); } @@ -1725,6 +1740,11 @@ 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); } + +#ifdef CONFIG_PM + spec->ready_to_resume = 0; +#endif + return 0; }
@@ -1855,6 +1875,61 @@ 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; + + 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 = get_pin(spec, 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; @@ -1878,6 +1953,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();
At Mon, 8 Apr 2013 13:54:55 -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.
In this patch, Haswell codec will claim it needs "delaying resume" by setting flag "support_delay_resume", so system resume will skip it and mark it as "delay_resumed". On later codec access, Haswell codec will be resumed and hda_call_codec_resume() will call its set_power_states ops intel_haswell_set_ power_state(). For 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/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ede8215..f39e374 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -89,6 +89,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 };
@@ -975,6 +983,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(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec);
} @@ -1725,6 +1740,11 @@ 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); }
+#ifdef CONFIG_PM
- spec->ready_to_resume = 0;
+#endif
- return 0;
}
@@ -1855,6 +1875,61 @@ 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;
- 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 = get_pin(spec, 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");
+}
IMO, this should be shown no matter whether with or w/o debug option, i.e. use snd_printk().
+}
+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) {
A superfluous space after '=='
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)
You don't have to put inline for such a case. Rely more on the compiler. It shall do that anyway.
+{
- 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;
No need for a new line...
+} +#else +define intel_haswell_allow_delay_resume NULL
This should be an empty function instead
#define intel_haswell_allow_delay_resume() do { } while (0)
Takashi
+#endif
static int patch_generic_hdmi(struct hda_codec *codec) { struct hdmi_spec *spec; @@ -1878,6 +1953,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
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 08, 2013 10:27 PM
- 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"); }
IMO, this should be shown no matter whether with or w/o debug option, i.e. use snd_printk().
Yes. Time out would be a serious error.
+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) {
A superfluous space after '=='
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)
You don't have to put inline for such a case. Rely more on the compiler. It shall do that anyway.
+{
- 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;
No need for a new line...
+} +#else +define intel_haswell_allow_delay_resume NULL
This should be an empty function instead
#define intel_haswell_allow_delay_resume() do { } while (0)
I made an mistake here. Sorry!
I've revised the 1st patch comments and 3rd patch as you suggested. Please have a review.
Many thanks Mengdong
participants (3)
-
Lin, Mengdong
-
mengdong.lin@intel.com
-
Takashi Iwai