Message ID | s5hpp78p94w.wl-tiwai@suse.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Monday, April 13, 2015 7:21 PM > To: Lin, Mengdong > Cc: alsa-devel@alsa-project.org > Subject: Re: [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel > HDMI codec > > At Mon, 13 Apr 2015 17:50:43 +0800, > mengdong.lin@intel.com wrote: > > > > From: Mengdong Lin <mengdong.lin@intel.com> > > > > For HDMI codec on Haswell/Broadwell/Skylake platforms, the vendor verb > > 0x781 is used enable DP 1.2 mode as a fixup if BIOS has not done this, > > in function intel_haswell_fixup_enable_dp12(). Otherwise, the display > > audio playback will be silent. > > > > Although the verb 0x781 is added to vendor verbs array, but > > snd_hdac_regmap_ > > encode_verb() will translate it to verb 0xf81 and cause regmap_write > > IO failure because 0xf81 is not in the vendor verb array and so will > > not be taken as a writable register. > > > > So this patch no longer uses regmap for verb 0x781 but directly send > > this command to enable DP1.2 mode. > > What if just add INTEL_GET_VENDOR_VERB as a vendor verb? Yes, it should work. hda_reg_write will drop the GET bit. I'll have a test tomorrow. Thanks Mengdong
> -----Original Message----- > From: Lin, Mengdong > Sent: Monday, April 13, 2015 9:30 PM > > > For HDMI codec on Haswell/Broadwell/Skylake platforms, the vendor > > > verb > > > 0x781 is used enable DP 1.2 mode as a fixup if BIOS has not done > > > this, in function intel_haswell_fixup_enable_dp12(). Otherwise, the > > > display audio playback will be silent. > > > > > > Although the verb 0x781 is added to vendor verbs array, but > > > snd_hdac_regmap_ > > > encode_verb() will translate it to verb 0xf81 and cause regmap_write > > > IO failure because 0xf81 is not in the vendor verb array and so will > > > not be taken as a writable register. > > > > > > So this patch no longer uses regmap for verb 0x781 but directly send > > > this command to enable DP1.2 mode. > > > > What if just add INTEL_GET_VENDOR_VERB as a vendor verb? > > Yes, it should work. hda_reg_write will drop the GET bit. > I'll have a test tomorrow. > > What if just add INTEL_GET_VENDOR_VERB as a vendor verb? > > Yes, it should work. hda_reg_write will drop the GET bit. > I'll have a test tomorrow. Hi Takashi, I've submitted v2 and v3 patches, both can work one my SKL. Please review. V2: add INTEL_GET_VENDOR_VERB as a vendor verb as you suggested. I verified this on a SKL desktop, on which BIOS does not enable DP1.2 mode by default and so we observed the HDMI/DP audio failure. V3: set GET bit when adding a vendor verb to the codec regmap It also works on my SKL. It's more generic than v2, because I observed other HD-A codecs (Conexant, si3054, sigmatel) also add their own vendor SET verbs and may have the same problem. However, I have no chance to verify these codecs since I cannot find platforms with them. Thanks Mengdong
At Tue, 14 Apr 2015 03:26:10 +0000, Lin, Mengdong wrote: > > > -----Original Message----- > > From: Lin, Mengdong > > Sent: Monday, April 13, 2015 9:30 PM > > > > > For HDMI codec on Haswell/Broadwell/Skylake platforms, the vendor > > > > verb > > > > 0x781 is used enable DP 1.2 mode as a fixup if BIOS has not done > > > > this, in function intel_haswell_fixup_enable_dp12(). Otherwise, the > > > > display audio playback will be silent. > > > > > > > > Although the verb 0x781 is added to vendor verbs array, but > > > > snd_hdac_regmap_ > > > > encode_verb() will translate it to verb 0xf81 and cause regmap_write > > > > IO failure because 0xf81 is not in the vendor verb array and so will > > > > not be taken as a writable register. > > > > > > > > So this patch no longer uses regmap for verb 0x781 but directly send > > > > this command to enable DP1.2 mode. > > > > > > What if just add INTEL_GET_VENDOR_VERB as a vendor verb? > > > > Yes, it should work. hda_reg_write will drop the GET bit. > > I'll have a test tomorrow. > > > > What if just add INTEL_GET_VENDOR_VERB as a vendor verb? > > > > Yes, it should work. hda_reg_write will drop the GET bit. > > I'll have a test tomorrow. > > Hi Takashi, > > I've submitted v2 and v3 patches, both can work one my SKL. Please review. > > V2: add INTEL_GET_VENDOR_VERB as a vendor verb as you suggested. > I verified this on a SKL desktop, on which BIOS does not enable DP1.2 mode by default and so we observed the HDMI/DP audio failure. > > V3: set GET bit when adding a vendor verb to the codec regmap > It also works on my SKL. It's more generic than v2, because I observed other HD-A codecs (Conexant, si3054, sigmatel) also add their own vendor SET verbs and may have the same problem. > However, I have no chance to verify these codecs since I cannot find platforms with them. I like v3 better so I took it. Thanks! Takashi
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5f44f60a6389..a818bb1c5886 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2298,6 +2298,7 @@ static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) /* enable DP1.2 mode */ vendor_param |= INTEL_EN_DP12; + snd_hdac_regmap_add_vendor_verb(&codec->core, INTEL_GET_VENDOR_VERB); snd_hdac_regmap_add_vendor_verb(&codec->core, INTEL_SET_VENDOR_VERB); snd_hda_codec_write_cache(codec, INTEL_VENDOR_NID, 0, INTEL_SET_VENDOR_VERB, vendor_param);