[alsa-devel] [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec
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.
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 ca0c05e..1849c94 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2298,8 +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_SET_VENDOR_VERB); - snd_hda_codec_write_cache(codec, INTEL_VENDOR_NID, 0, + snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, INTEL_SET_VENDOR_VERB, vendor_param); }
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?
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);
-----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
From: Mengdong Lin mengdong.lin@intel.com
For HDMI codec on Haswell/Broadwell/Skylake, the SET 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.
So this patch adds its peer GET vendor verb 0xf81 to the codec's regmap, to avoid I/O error when sending 0x781 thru remap.
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 ca0c05e..84ee5ff 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2298,7 +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_SET_VENDOR_VERB); + snd_hdac_regmap_add_vendor_verb(&codec->core, INTEL_GET_VENDOR_VERB); snd_hda_codec_write_cache(codec, INTEL_VENDOR_NID, 0, INTEL_SET_VENDOR_VERB, vendor_param); }
From: Mengdong Lin mengdong.lin@intel.com
Some HD-A codecs may add their own vendor 'set' verb to the regmap, thru func snd_hdac_add_vendor_verb(). This patch sets the GET bit (bit 11) when adding the verb so that its peer vendor 'get' verb is actually added. This can avoid I/O error when writing the 'set' verb thru remap, since HD-A regmap internally looks up a writable vendor verb with GET bit set at first.
Signed-off-by: Mengdong Lin mengdong.lin@intel.com
diff --git a/sound/hda/hdac_regmap.c b/sound/hda/hdac_regmap.c index 1eb4320..9e933c3 100644 --- a/sound/hda/hdac_regmap.c +++ b/sound/hda/hdac_regmap.c @@ -368,7 +368,7 @@ int snd_hdac_regmap_add_vendor_verb(struct hdac_device *codec,
if (!p) return -ENOMEM; - *p = verb; + *p = verb | 0x800; /* set GET bit */ return 0; } EXPORT_SYMBOL_GPL(snd_hdac_regmap_add_vendor_verb);
participants (3)
-
Lin, Mengdong -
mengdong.lin＠intel.com
-
Takashi Iwai