[alsa-devel] [PATCH v2] ALSA: hda - re-apply fixup when resuming haswell hdmi codec
From: Mengdong Lin mengdong.lin@intel.com
This patch is a supplement to a previous patch: "ALSA: hda - Add fixup for Haswell to enable all pin and convertor widgets".
Some Haswell BIOS will disable the 2nd and 3rd pin/covertor widgets when the HD-A controller changes state from D3 to D0. So when the controller resumes after a system or runtime suspend, these widgets are disabled and programming these widgets to D0 will cause H/W error and codec will not respond.
So this patch adds a "set_power_state" ops for Haswell codec. Before setting the codec to D0, this function will apply a BIOS-specific fixup to enable the 2nd & 3rd pins/cvts if need.
And since BIOS will also disable DP1.2, so this function will check and enable DP1.2 mode if need.
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 adb5dce..080398e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1842,7 +1842,7 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, { unsigned int vendor_param;
- if (action != HDA_FIXUP_ACT_PRE_PROBE) + if (action != HDA_FIXUP_ACT_PRE_PROBE && action != HDA_FIXUP_ACT_INIT) return; vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, INTEL_GET_VENDOR_VERB, 0); @@ -1855,7 +1855,8 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, if (vendor_param == -1) return;
- snd_hda_codec_update_widgets(codec); + if (action == HDA_FIXUP_ACT_PRE_PROBE) + snd_hda_codec_update_widgets(codec); return; }
@@ -1874,7 +1875,24 @@ static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) INTEL_SET_VENDOR_VERB, vendor_param); }
+static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg, + unsigned int power_state) +{ + if (AC_PWRST_D0 == power_state) { + snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT); + + intel_haswell_fixup_enable_dp12(codec); + + } + + 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); + + return; +}
/* available models for fixup */ enum { @@ -1922,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) + codec->patch_ops.set_power_state = haswell_set_power_state; + generic_hdmi_init_per_pins(codec);
init_channel_allocations();
At Tue, 7 May 2013 12:34:21 -0400, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
This patch is a supplement to a previous patch: "ALSA: hda - Add fixup for Haswell to enable all pin and convertor widgets".
Some Haswell BIOS will disable the 2nd and 3rd pin/covertor widgets when the HD-A controller changes state from D3 to D0. So when the controller resumes after a system or runtime suspend, these widgets are disabled and programming these widgets to D0 will cause H/W error and codec will not respond.
So this patch adds a "set_power_state" ops for Haswell codec. Before setting the codec to D0, this function will apply a BIOS-specific fixup to enable the 2nd & 3rd pins/cvts if need.
And since BIOS will also disable DP1.2, so this function will check and enable DP1.2 mode if need.
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 adb5dce..080398e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1842,7 +1842,7 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, { unsigned int vendor_param;
- if (action != HDA_FIXUP_ACT_PRE_PROBE)
- if (action != HDA_FIXUP_ACT_PRE_PROBE && action != HDA_FIXUP_ACT_INIT) return; vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, INTEL_GET_VENDOR_VERB, 0);
@@ -1855,7 +1855,8 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, if (vendor_param == -1) return;
- snd_hda_codec_update_widgets(codec);
- if (action == HDA_FIXUP_ACT_PRE_PROBE)
snd_hda_codec_update_widgets(codec);}
@@ -1874,7 +1875,24 @@ static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) INTEL_SET_VENDOR_VERB, vendor_param); }
+static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
unsigned int power_state)+{
- if (AC_PWRST_D0 == power_state) {
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);
Here is no right place to apply the fixup. If needed, call it generic_hdmi_init(), for example.
Takashi
intel_haswell_fixup_enable_dp12(codec);}
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);
return;
+}
/* available models for fixup */ enum { @@ -1922,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)
codec->patch_ops.set_power_state = haswell_set_power_state;generic_hdmi_init_per_pins(codec);
init_channel_allocations();
-- 1.7.10.4
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, May 07, 2013 7:21 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH v2] ALSA: hda - re-apply fixup when resuming haswell hdmi codec
At Tue, 7 May 2013 12:34:21 -0400, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
This patch is a supplement to a previous patch: "ALSA: hda - Add fixup for Haswell to enable all pin and convertor widgets".
Some Haswell BIOS will disable the 2nd and 3rd pin/covertor widgets when the HD-A controller changes state from D3 to D0. So when the controller resumes after a system or runtime suspend, these widgets are disabled and programming these widgets to D0 will cause H/W error and
codec will not respond.
So this patch adds a "set_power_state" ops for Haswell codec. Before setting the codec to D0, this function will apply a BIOS-specific fixup to enable the 2nd & 3rd pins/cvts if need.
And since BIOS will also disable DP1.2, so this function will check and enable DP1.2 mode if need.
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 adb5dce..080398e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1842,7 +1842,7 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, { unsigned int vendor_param;
- if (action != HDA_FIXUP_ACT_PRE_PROBE)
- if (action != HDA_FIXUP_ACT_PRE_PROBE && action !=
+HDA_FIXUP_ACT_INIT) return; vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, INTEL_GET_VENDOR_VERB, 0); @@ -1855,7 +1855,8 @@ static void intel_haswell_enable_all_pins(struct
hda_codec *codec,
if (vendor_param == -1) return;
- snd_hda_codec_update_widgets(codec);
- if (action == HDA_FIXUP_ACT_PRE_PROBE)
snd_hda_codec_update_widgets(codec);}
@@ -1874,7 +1875,24 @@ static void
intel_haswell_fixup_enable_dp12(struct hda_codec *codec)
INTEL_SET_VENDOR_VERB, vendor_param); }+static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
unsigned int power_state)+{
- if (AC_PWRST_D0 == power_state) {
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);Here is no right place to apply the fixup. If needed, call it generic_hdmi_init(), for example.
Hi Takashi,
These widgets need to be enabled before hda_set_power_state(codec, AC_PWRST_D0) programs these widgets to D0. Otherwise, audio driver would program these disabled widgets to D0. The codec will not respond when audio driver tries to sync power state. And later verb execution will fail.
In hda_call_codec_resume(), generic_hdmi_init() is called from codec->patch_ops.init, after setting power state to D0. It would be too late. Is it okay to add a new ops like "pre_resume" to apply the fixup?
Thanks Mengdong
At Tue, 7 May 2013 14:10:08 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, May 07, 2013 7:21 PM To: Lin, Mengdong Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH v2] ALSA: hda - re-apply fixup when resuming haswell hdmi codec
At Tue, 7 May 2013 12:34:21 -0400, mengdong.lin@intel.com wrote:
From: Mengdong Lin mengdong.lin@intel.com
This patch is a supplement to a previous patch: "ALSA: hda - Add fixup for Haswell to enable all pin and convertor widgets".
Some Haswell BIOS will disable the 2nd and 3rd pin/covertor widgets when the HD-A controller changes state from D3 to D0. So when the controller resumes after a system or runtime suspend, these widgets are disabled and programming these widgets to D0 will cause H/W error and
codec will not respond.
So this patch adds a "set_power_state" ops for Haswell codec. Before setting the codec to D0, this function will apply a BIOS-specific fixup to enable the 2nd & 3rd pins/cvts if need.
And since BIOS will also disable DP1.2, so this function will check and enable DP1.2 mode if need.
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 adb5dce..080398e 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1842,7 +1842,7 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, { unsigned int vendor_param;
- if (action != HDA_FIXUP_ACT_PRE_PROBE)
- if (action != HDA_FIXUP_ACT_PRE_PROBE && action !=
+HDA_FIXUP_ACT_INIT) return; vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, INTEL_GET_VENDOR_VERB, 0); @@ -1855,7 +1855,8 @@ static void intel_haswell_enable_all_pins(struct
hda_codec *codec,
if (vendor_param == -1) return;
- snd_hda_codec_update_widgets(codec);
- if (action == HDA_FIXUP_ACT_PRE_PROBE)
snd_hda_codec_update_widgets(codec);}
@@ -1874,7 +1875,24 @@ static void
intel_haswell_fixup_enable_dp12(struct hda_codec *codec)
INTEL_SET_VENDOR_VERB, vendor_param); }+static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
unsigned int power_state)+{
- if (AC_PWRST_D0 == power_state) {
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);Here is no right place to apply the fixup. If needed, call it generic_hdmi_init(), for example.
Hi Takashi,
These widgets need to be enabled before hda_set_power_state(codec, AC_PWRST_D0) programs these widgets to D0. Otherwise, audio driver would program these disabled widgets to D0. The codec will not respond when audio driver tries to sync power state. And later verb execution will fail.
Hm, but this is needed only for the machines with PCI SSID 8086:2010, right? If so, this will be never in market, and I wonder the importance of this fix.
So, the question is -- in which situation do we need this fix at all? Isn't it needed for all Haswell, or only certain Haswell variants, or only certain setups?
In hda_call_codec_resume(), generic_hdmi_init() is called from codec->patch_ops.init, after setting power state to D0. It would be too late. Is it okay to add a new ops like "pre_resume" to apply the fixup?
No. Using the standard fixup doesn't look correct in this case.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, May 07, 2013 10:16 PM
These widgets need to be enabled before hda_set_power_state(codec,
AC_PWRST_D0) programs these widgets to D0.
Otherwise, audio driver would program these disabled widgets to D0. The
codec will not respond when audio driver tries to sync power state. And later verb execution will fail.
Hm, but this is needed only for the machines with PCI SSID 8086:2010, right? If so, this will be never in market, and I wonder the importance of this fix.
So, the question is -- in which situation do we need this fix at all? Isn't it needed for all Haswell, or only certain Haswell variants, or only certain setups?
This fixup is needed for all Intel Haswell test machines on our hand, including desktop, mobile and ultrabook machines. All of these machines has both HDMI and DP connectors and so we need to enable these widgets to use HDMI on the 2nd pin. Without this fixup, we observed communication failure between controller and codec after a system suspend/resume.
The BIOS team told us they will no longer change BIOS for Haswell, and they expect the Linux audio driver to enable the widgets, just like Windows driver does. Now we hope to have a BIOS specific fixup.
Other OEM BIOS may enable the these widgets according to their machine design, or may not if they follow Intel BIOS. This need to be check case by case.
Thanks Mengdong
In hda_call_codec_resume(), generic_hdmi_init() is called from
codec->patch_ops.init, after setting power state to D0. It would be too late.
Is it okay to add a new ops like "pre_resume" to apply the fixup?
No. Using the standard fixup doesn't look correct in this case.
Takashi
At Tue, 7 May 2013 15:33:34 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, May 07, 2013 10:16 PM
These widgets need to be enabled before hda_set_power_state(codec,
AC_PWRST_D0) programs these widgets to D0.
Otherwise, audio driver would program these disabled widgets to D0. The
codec will not respond when audio driver tries to sync power state. And later verb execution will fail.
Hm, but this is needed only for the machines with PCI SSID 8086:2010, right? If so, this will be never in market, and I wonder the importance of this fix.
So, the question is -- in which situation do we need this fix at all? Isn't it needed for all Haswell, or only certain Haswell variants, or only certain setups?
This fixup is needed for all Intel Haswell test machines on our hand, including desktop, mobile and ultrabook machines.
But obviously this doesn't match with other vendors (e.g. HP).
All of these machines has both HDMI and DP connectors and so we need to enable these widgets to use HDMI on the 2nd pin. Without this fixup, we observed communication failure between controller and codec after a system suspend/resume.
... and I still see the problem with other vendors. So I suspect the fix may be really a band-aid over Intel-devel machines but doesn't help others in the current form.
The BIOS team told us they will no longer change BIOS for Haswell, and they expect the Linux audio driver to enable the widgets, just like Windows driver does. Now we hope to have a BIOS specific fixup.
Other OEM BIOS may enable the these widgets according to their machine design, or may not if they follow Intel BIOS. This need to be check case by case.
Hmm... your statement worries me pretty much. It sounds like that the driver won't work as is unless we know what BIOS on each machine does and how to paper over it. Can't we have a more generic solution?
Takashi
Thanks Mengdong
In hda_call_codec_resume(), generic_hdmi_init() is called from
codec->patch_ops.init, after setting power state to D0. It would be too late.
Is it okay to add a new ops like "pre_resume" to apply the fixup?
No. Using the standard fixup doesn't look correct in this case.
Takashi
participants (3)
-
Lin, Mengdong -
mengdong.lin＠intel.com
-
Takashi Iwai