From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH v3 1/1] ALSA: hda - bug fix for invalid connection list of Haswell HDMI codec pins Date: Tue, 15 Jan 2013 12:33:27 +0100 Message-ID: References: <1355867955-2391-1-git-send-email-mengdong.lin@intel.com> <50F534AB.3010207@canonical.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id C7BDE2615F1 for ; Tue, 15 Jan 2013 12:33:27 +0100 (CET) In-Reply-To: <50F534AB.3010207@canonical.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: David Henningsson Cc: mengdong.lin@intel.com, alsa-devel@alsa-project.org, Xingchao Wang List-Id: alsa-devel@alsa-project.org 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 > > > > 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 > > Signed-off-by: Xingchao Wang > > > > 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