[PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[mengdong.lin]

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);
 }
 
-- 
1.9.1

Re: [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[Takashi Iwai]

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);

Re: [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[Lin, Mengdong]

> -----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

[PATCH v2] ALSA: hda - add Intel get vendor verb 0xf81 to regmap of the HDMI codec

[mengdong.lin]

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);
 }
-- 
1.9.1

[PATCH v3] ALSA: hda - set GET bit when adding a vendor verb to the codec regmap

[mengdong.lin]

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);
-- 
1.9.1

Re: [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[Lin, 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

Re: [PATCH] ALSA: hda - not use regmap for vendor verb 0x781 of Intel HDMI codec

[Takashi Iwai]

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