* [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins @ 2012-12-18 21:59 mengdong.lin 2012-12-18 10:06 ` Takashi Iwai 2013-01-15 10:51 ` David Henningsson 0 siblings, 2 replies; 6+ messages in thread From: mengdong.lin @ 2012-12-18 21:59 UTC (permalink / raw) To: alsa-devel, tiwai; +Cc: Mengdong Lin, Xingchao Wang From: Mengdong Lin <mengdong.lin@intel.com> Haswell HDMI codec pins may report invalid connection list entries, which will cause failure to play audio via HDMI or Display Port. So this patch adds fixup for Haswell to workaround this hardware issue: enable DP1.2 mode and override the pins' connection list entries with proper value. Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> Signed-off-by: Xingchao Wang <xingchao.wang@intel.com> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 71555cc..59abe73 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1687,6 +1687,30 @@ static const struct hda_codec_ops generic_hdmi_patch_ops = { .unsol_event = hdmi_unsol_event, }; +static void intel_haswell_fixup_connect_list(struct hda_codec *codec) +{ + unsigned int vendor_param; + hda_nid_t list[3] = {0x2, 0x3, 0x4}; + + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); + if (vendor_param == -1 || vendor_param & 0x02) + return; + + /* enable DP1.2 mode */ + vendor_param |= 0x02; + snd_hda_codec_read(codec, 0x08, 0, 0x781, vendor_param); + + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); + if (vendor_param == -1 || !(vendor_param & 0x02)) + return; + + /* override 3 pins connection list */ + snd_hda_override_conn_list(codec, 0x05, 3, list); + snd_hda_override_conn_list(codec, 0x06, 3, list); + snd_hda_override_conn_list(codec, 0x07, 3, list); +} + + static int patch_generic_hdmi(struct hda_codec *codec) { struct hdmi_spec *spec; @@ -1696,6 +1720,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) return -ENOMEM; codec->spec = spec; + + if (codec->vendor_id == 0x80862807) + intel_haswell_fixup_connect_list(codec); + if (hdmi_parse_codec(codec) < 0) { codec->spec = NULL; kfree(spec); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins 2012-12-18 21:59 [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins mengdong.lin @ 2012-12-18 10:06 ` Takashi Iwai 2013-01-15 10:51 ` David Henningsson 1 sibling, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2012-12-18 10:06 UTC (permalink / raw) To: mengdong.lin; +Cc: alsa-devel, Xingchao Wang At Tue, 18 Dec 2012 16:59:15 -0500, mengdong.lin@intel.com wrote: > > From: Mengdong Lin <mengdong.lin@intel.com> > > Haswell HDMI codec pins may report invalid connection list entries, which > will cause failure to play audio via HDMI or Display Port. > > So this patch adds fixup for Haswell to workaround this hardware issue: > enable DP1.2 mode and override the pins' connection list entries with proper > value. > > Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> > Signed-off-by: Xingchao Wang <xingchao.wang@intel.com> Thanks, applied now. Takashi > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 71555cc..59abe73 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -1687,6 +1687,30 @@ static const struct hda_codec_ops generic_hdmi_patch_ops = { > .unsol_event = hdmi_unsol_event, > }; > > +static void intel_haswell_fixup_connect_list(struct hda_codec *codec) > +{ > + unsigned int vendor_param; > + hda_nid_t list[3] = {0x2, 0x3, 0x4}; > + > + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); > + if (vendor_param == -1 || vendor_param & 0x02) > + return; > + > + /* enable DP1.2 mode */ > + vendor_param |= 0x02; > + snd_hda_codec_read(codec, 0x08, 0, 0x781, vendor_param); > + > + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); > + if (vendor_param == -1 || !(vendor_param & 0x02)) > + return; > + > + /* override 3 pins connection list */ > + snd_hda_override_conn_list(codec, 0x05, 3, list); > + snd_hda_override_conn_list(codec, 0x06, 3, list); > + snd_hda_override_conn_list(codec, 0x07, 3, list); > +} > + > + > static int patch_generic_hdmi(struct hda_codec *codec) > { > struct hdmi_spec *spec; > @@ -1696,6 +1720,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) > return -ENOMEM; > > codec->spec = spec; > + > + if (codec->vendor_id == 0x80862807) > + intel_haswell_fixup_connect_list(codec); > + > if (hdmi_parse_codec(codec) < 0) { > codec->spec = NULL; > kfree(spec); > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins 2012-12-18 21:59 [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins mengdong.lin 2012-12-18 10:06 ` Takashi Iwai @ 2013-01-15 10:51 ` David Henningsson 2013-01-15 11:33 ` Takashi Iwai 2013-01-17 3:23 ` Lin, Mengdong 1 sibling, 2 replies; 6+ messages in thread From: David Henningsson @ 2013-01-15 10:51 UTC (permalink / raw) To: mengdong.lin; +Cc: tiwai, alsa-devel, Xingchao Wang On 12/18/2012 10:59 PM, mengdong.lin@intel.com wrote: > From: Mengdong Lin <mengdong.lin@intel.com> > > Haswell HDMI codec pins may report invalid connection list entries, which > will cause failure to play audio via HDMI or Display Port. > > So this patch adds fixup for Haswell to workaround this hardware issue: > enable DP1.2 mode and override the pins' connection list entries with proper > value. > > Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> > Signed-off-by: Xingchao Wang <xingchao.wang@intel.com> > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 71555cc..59abe73 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -1687,6 +1687,30 @@ static const struct hda_codec_ops generic_hdmi_patch_ops = { > .unsol_event = hdmi_unsol_event, > }; > > +static void intel_haswell_fixup_connect_list(struct hda_codec *codec) > +{ > + unsigned int vendor_param; > + hda_nid_t list[3] = {0x2, 0x3, 0x4}; > + > + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); > + if (vendor_param == -1 || vendor_param & 0x02) > + return; > + > + /* enable DP1.2 mode */ > + vendor_param |= 0x02; > + snd_hda_codec_read(codec, 0x08, 0, 0x781, vendor_param); Hi, When trying to get Haswell HDMI audio working, I discovered that this verb when executed can get pins to change state from D0 to D3. As the fixup is executed after hda_call_codec_resume this means that the pins will remain in D3. I'm not entirely sure of this, but I think we have no runtime power transitions if we are on AC power, so this could potentially be for a very long time. > + /* override 3 pins connection list */ > + snd_hda_override_conn_list(codec, 0x05, 3, list); > + snd_hda_override_conn_list(codec, 0x06, 3, list); > + snd_hda_override_conn_list(codec, 0x07, 3, list); So before the DP 1.2 verb is executed, the connections are just 5 -> 2, 6 -> 3, 7 -> 4, but afterwards, every pin node can connect to every cvt node, and connection select verbs must change as a result? -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins 2013-01-15 10:51 ` David Henningsson @ 2013-01-15 11:33 ` Takashi Iwai 2013-01-17 5:32 ` Lin, Mengdong 2013-01-17 3:23 ` Lin, Mengdong 1 sibling, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2013-01-15 11:33 UTC (permalink / raw) To: David Henningsson; +Cc: mengdong.lin, alsa-devel, Xingchao Wang At Tue, 15 Jan 2013 11:51:23 +0100, David Henningsson wrote: > > On 12/18/2012 10:59 PM, mengdong.lin@intel.com wrote: > > From: Mengdong Lin <mengdong.lin@intel.com> > > > > Haswell HDMI codec pins may report invalid connection list entries, which > > will cause failure to play audio via HDMI or Display Port. > > > > So this patch adds fixup for Haswell to workaround this hardware issue: > > enable DP1.2 mode and override the pins' connection list entries with proper > > value. > > > > Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> > > Signed-off-by: Xingchao Wang <xingchao.wang@intel.com> > > > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > > index 71555cc..59abe73 100644 > > --- a/sound/pci/hda/patch_hdmi.c > > +++ b/sound/pci/hda/patch_hdmi.c > > @@ -1687,6 +1687,30 @@ static const struct hda_codec_ops generic_hdmi_patch_ops = { > > .unsol_event = hdmi_unsol_event, > > }; > > > > +static void intel_haswell_fixup_connect_list(struct hda_codec *codec) > > +{ > > + unsigned int vendor_param; > > + hda_nid_t list[3] = {0x2, 0x3, 0x4}; > > + > > + vendor_param = snd_hda_codec_read(codec, 0x08, 0, 0xf81, 0); > > + if (vendor_param == -1 || vendor_param & 0x02) > > + return; > > + > > + /* enable DP1.2 mode */ > > + vendor_param |= 0x02; > > + snd_hda_codec_read(codec, 0x08, 0, 0x781, vendor_param); > > Hi, > > When trying to get Haswell HDMI audio working, I discovered that this > verb when executed can get pins to change state from D0 to D3. Oh, does this verb do it? It's bad. Also, Mengdong, could you define these verbs (0x781 & 0xf81) in hda_codec.h to understand and read the code better? > As the fixup is executed after hda_call_codec_resume this means that the > pins will remain in D3. I'm not entirely sure of this, but I think we > have no runtime power transitions if we are on AC power, so this could > potentially be for a very long time. Actually you spotted a potential bug -- the code isn't called in the resume callback. This is a code called directly from parse_generic_hdmi(), so it's called only once to override the connections. But the verb 0x781 isn't set. I thought we can simply replace the call snd_hda_codec_read(codec, 0x08, 0, 0x781, ...) with snd_hda_codec_write_cache(), so that the same verb will be executed automatically on resume. But, if this verb has any bad side effect (like touching D state), it's no good idea to do it in the cache resume. > > + /* override 3 pins connection list */ > > + snd_hda_override_conn_list(codec, 0x05, 3, list); > > + snd_hda_override_conn_list(codec, 0x06, 3, list); > > + snd_hda_override_conn_list(codec, 0x07, 3, list); > > So before the DP 1.2 verb is executed, the connections are > just 5 -> 2, 6 -> 3, 7 -> 4, but afterwards, every pin node can connect > to every cvt node, and connection select verbs must change as a result? This is my understanding, but maybe a bit more comments would be helpful... thanks, Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins 2013-01-15 11:33 ` Takashi Iwai @ 2013-01-17 5:32 ` Lin, Mengdong 0 siblings, 0 replies; 6+ messages in thread From: Lin, Mengdong @ 2013-01-17 5:32 UTC (permalink / raw) To: Takashi Iwai, David Henningsson Cc: alsa-devel@alsa-project.org, Wang, Xingchao > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, January 15, 2013 7:33 PM > > When trying to get Haswell HDMI audio working, I discovered that this > > verb when executed can get pins to change state from D0 to D3. > > Oh, does this verb do it? It's bad. No, I think these verbs ((0x781 & 0xf81) don't change pin state. So I'm asking David to share more Information. > Also, Mengdong, could you define these verbs (0x781 & 0xf81) in > hda_codec.h to understand and read the code better? Okay, I will. > > As the fixup is executed after hda_call_codec_resume this means that > > the pins will remain in D3. I'm not entirely sure of this, but I think > > we have no runtime power transitions if we are on AC power, so this > > could potentially be for a very long time. > > Actually you spotted a potential bug -- the code isn't called in the resume > callback. This is a code called directly from parse_generic_hdmi(), so it's > called only once to override the connections. But the verb 0x781 isn't set. Yes. intel_haswell_fixup_connect_list() is called only once from parse_generic_hdmi(). > I thought we can simply replace the call snd_hda_codec_read(codec, 0x08, > 0, 0x781, ...) with snd_hda_codec_write_cache(), so that the same verb will > be executed automatically on resume. But, if this verb has any bad side > effect (like touching D state), it's no good idea to do it in the cache resume. > But at first, we need to find out whether snd_hda_codec_read (codec, 0x08, 0, 0x781, ...) cannot bring the codec out of D3 before executing 0x781. I suspect it's not the executing the verb but before that that the whole codec (including the pins) enters D3. Thanks Mengdong ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins 2013-01-15 10:51 ` David Henningsson 2013-01-15 11:33 ` Takashi Iwai @ 2013-01-17 3:23 ` Lin, Mengdong 1 sibling, 0 replies; 6+ messages in thread From: Lin, Mengdong @ 2013-01-17 3:23 UTC (permalink / raw) To: David Henningsson Cc: tiwai@suse.de, alsa-devel@alsa-project.org, Wang, Xingchao > -----Original Message----- > From: David Henningsson [mailto:david.henningsson@canonical.com] > Sent: Tuesday, January 15, 2013 6:51 PM > When trying to get Haswell HDMI audio working, I discovered that this verb > when executed can get pins to change state from D0 to D3. > > As the fixup is executed after hda_call_codec_resume this means that the > pins will remain in D3. I'm not entirely sure of this, but I think we have no > runtime power transitions if we are on AC power, so this could potentially be > for a very long time. Hi David, Is the "power save" enabled for the codec, by setting a non-zero value to the "power_save" parameter under /sys/module/snd_hda_intel/parametesrs? The verb 0x0f81/0x0781/ is a vendor verb to query/configure HDMI, but it will not touch the pin's D-state. I suspect the codec is suspended in D3 during the interval between commands execution, since codec_exec_verb() will call snd_hda_power_down() after a command is done and so the codec will be suspended if 'power save' is enabled. And snd_hda_codec_read() just tries to resume the codec when executing a new command, the calling sequence is snd_hda_codec_read-> codec_exec_verb -> snd_hda_power_up. > > > + /* override 3 pins connection list */ > > + snd_hda_override_conn_list(codec, 0x05, 3, list); > > + snd_hda_override_conn_list(codec, 0x06, 3, list); > > + snd_hda_override_conn_list(codec, 0x07, 3, list); > > So before the DP 1.2 verb is executed, the connections are just 5 -> 2, 6 -> 3, > 7 -> 4, but afterwards, every pin node can connect to every cvt node, and > connection select verbs must change as a result? Yes. Without DP 1.2 there is a 1:1 mapping between pins and convertors. But HDMI cannot play sound. And even DP 1.2 is enabled, HDMI codec can still report invalid empty connect list during initialization. This is a HW issue for Haswell B0 processor. Which version of Haswell processor is on your machine? Could you share the 'cpu stepping' in BIOS under platform information menu. We'll verify this issue and side effect of the patch after we got a processor upgrade. Sorry for the late reply. Thanks Mengdong ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-17 5:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-18 21:59 [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins mengdong.lin 2012-12-18 10:06 ` Takashi Iwai 2013-01-15 10:51 ` David Henningsson 2013-01-15 11:33 ` Takashi Iwai 2013-01-17 5:32 ` Lin, Mengdong 2013-01-17 3:23 ` Lin, Mengdong
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.