[alsa-devel] [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins
Takashi Iwai
tiwai at suse.de
Tue Jan 15 12:33:27 CET 2013
At Tue, 15 Jan 2013 11:51:23 +0100,
David Henningsson wrote:
>
> On 12/18/2012 10:59 PM, mengdong.lin at intel.com wrote:
> > From: Mengdong Lin <mengdong.lin at 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 at intel.com>
> > Signed-off-by: Xingchao Wang <xingchao.wang at 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
More information about the Alsa-devel
mailing list