[PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Takashi Iwai]

ASUS Phoebus with CMI8888 HD-audio chip (PCI id 13f6:5011) doesn't
work with HD-audio driver as is because of some weird nature.  For
making DMA properly working, we need to disable MSI.  The position
report buffer doesn't work, thus we need to force reading LPIB
instead.  And yet, the codec CORB/RIRB communication gives errors
unless we disable the snooping (caching).

In this patch, all these workarounds are added as a quirk for the
device.  The HD-audio *codec* chip needs yet another workaround, but
it'll be provided in the succeeding patch.

Reported-and-tested-by: Vincent Lejeune <vljn@ovi.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_intel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 5db1948699d8..aa302fb03fc5 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -265,6 +265,7 @@ enum {
 	AZX_DRIVER_TERA,
 	AZX_DRIVER_CTX,
 	AZX_DRIVER_CTHDA,
+	AZX_DRIVER_CMEDIA,
 	AZX_DRIVER_GENERIC,
 	AZX_NUM_DRIVERS, /* keep this as last entry */
 };
@@ -330,6 +331,7 @@ static char *driver_short_names[] = {
 	[AZX_DRIVER_TERA] = "HDA Teradici", 
 	[AZX_DRIVER_CTX] = "HDA Creative", 
 	[AZX_DRIVER_CTHDA] = "HDA Creative",
+	[AZX_DRIVER_CMEDIA] = "HDA C-Media",
 	[AZX_DRIVER_GENERIC] = "HD-Audio Generic",
 };
 
@@ -1373,6 +1375,7 @@ static void azx_check_snoop_available(struct azx *chip)
 		snoop = false;
 		break;
 	case AZX_DRIVER_CTHDA:
+	case AZX_DRIVER_CMEDIA:
 		snoop = false;
 		break;
 	}
@@ -2154,6 +2157,10 @@ static const struct pci_device_id azx_ids[] = {
 	  .driver_data = AZX_DRIVER_CTX | AZX_DCAPS_CTX_WORKAROUND |
 	  AZX_DCAPS_RIRB_PRE_DELAY | AZX_DCAPS_POSFIX_LPIB },
 #endif
+	/* CM8888 */
+	{ PCI_DEVICE(0x13f6, 0x5011),
+	  .driver_data = AZX_DRIVER_CMEDIA |
+	  AZX_DCAPS_NO_MSI | AZX_DCAPS_POSFIX_LPIB },
 	/* Vortex86MX */
 	{ PCI_DEVICE(0x17f3, 0x3010), .driver_data = AZX_DRIVER_GENERIC },
 	/* VMware HDAudio */
-- 
2.0.3

[PATCH 2/2] ALSA: hda - Fix parsing of CMI8888 codec

[Takashi Iwai]

CMI8888 codec chip has a boost amp (only) on the headphone pin, and
this confuses the generic parser, which tends to pick up the most
outside amp.  This results in the wrong volume setup, as the driver
complains like:
  hda_codec: Mismatching dB step for vmaster slave (-100!=1000)

For avoiding this problem, rule out the amp on NID 0x10 and create
"Headphone Amp" volume control manually instead.

Note that this patch still doesn't fix all problems yet.  The sound
output from the line out seems still too low.  It will be fixed in
another patch (hopefully).

Reported-and-tested-by: Vincent Lejeune <vljn@ovi.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_cmedia.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/sound/pci/hda/patch_cmedia.c b/sound/pci/hda/patch_cmedia.c
index ed3d133ffbb6..c895a8f21192 100644
--- a/sound/pci/hda/patch_cmedia.c
+++ b/sound/pci/hda/patch_cmedia.c
@@ -75,15 +75,62 @@ static int patch_cmi9880(struct hda_codec *codec)
 	return err;
 }
 
+static int patch_cmi8888(struct hda_codec *codec)
+{
+	struct cmi_spec *spec;
+	struct auto_pin_cfg *cfg;
+	int err;
+
+	spec = kzalloc(sizeof(*spec), GFP_KERNEL);
+	if (!spec)
+		return -ENOMEM;
+
+	codec->spec = spec;
+	cfg = &spec->gen.autocfg;
+	snd_hda_gen_spec_init(&spec->gen);
+
+	/* mask NID 0x10 from the playback volume selection;
+	 * it's a headphone boost volume handled manually below
+	 */
+	spec->gen.out_vol_mask = (1ULL << 0x10);
+
+	err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
+	if (err < 0)
+		goto error;
+	err = snd_hda_gen_parse_auto_config(codec, cfg);
+	if (err < 0)
+		goto error;
+
+	if (get_defcfg_device(snd_hda_codec_get_pincfg(codec, 0x10)) ==
+	    AC_JACK_HP_OUT) {
+		static const struct snd_kcontrol_new amp_kctl =
+			HDA_CODEC_VOLUME("Headphone Amp Playback Volume",
+					 0x10, 0, HDA_OUTPUT);
+		if (!snd_hda_gen_add_kctl(&spec->gen, NULL, &amp_kctl)) {
+			err = -ENOMEM;
+			goto error;
+		}
+	}
+
+	codec->patch_ops = cmi_auto_patch_ops;
+	return 0;
+
+ error:
+	snd_hda_gen_free(codec);
+	return err;
+}
+
 /*
  * patch entries
  */
 static const struct hda_codec_preset snd_hda_preset_cmedia[] = {
+	{ .id = 0x13f68888, .name = "CMI8888", .patch = patch_cmi8888 },
 	{ .id = 0x13f69880, .name = "CMI9880", .patch = patch_cmi9880 },
  	{ .id = 0x434d4980, .name = "CMI9880", .patch = patch_cmi9880 },
 	{} /* terminator */
 };
 
+MODULE_ALIAS("snd-hda-codec-id:13f68888");
 MODULE_ALIAS("snd-hda-codec-id:13f69880");
 MODULE_ALIAS("snd-hda-codec-id:434d4980");
 
-- 
2.0.3

Re: [PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Geoffrey McRae]

I can confirm that disable of MSI is required as even Windows 7
64bit does not enable this for Intel HDA. Out of curiosity I altered
the driver to use MSI-X with the same results, interrupts are never
received even though the PCI caps state that MSI is supported.

Playback is very poor and stutters if I do not allow snoop by
commenting out "case AZX_DRIVER_CMEDIA:" in azx_check_snoop_available.

I can also confirm that AZX_DCAPS_POSFIX_LPIB is required on my
hardware also.

http://www.alsa-project.org/db/?f=512e0135e82a644bb9d2fe8f6b5e2be1cb10c53b

Things of note:

ASUS Sabertooth 990FX r1 with bugged IOMMU
Linux does not load correctly without IOMMU enabled, so disabling to
test is not an option.

On Wed,  6 Aug 2014 14:39:14 +0200
Takashi Iwai <tiwai@suse.de> wrote:

> ASUS Phoebus with CMI8888 HD-audio chip (PCI id 13f6:5011) doesn't
> work with HD-audio driver as is because of some weird nature.  For
> making DMA properly working, we need to disable MSI.  The position
> report buffer doesn't work, thus we need to force reading LPIB
> instead.  And yet, the codec CORB/RIRB communication gives errors
> unless we disable the snooping (caching).
> 
> In this patch, all these workarounds are added as a quirk for the
> device.  The HD-audio *codec* chip needs yet another workaround, but
> it'll be provided in the succeeding patch.
> 
> Reported-and-tested-by: Vincent Lejeune <vljn@ovi.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/pci/hda/hda_intel.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 5db1948699d8..aa302fb03fc5 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -265,6 +265,7 @@ enum {
>  	AZX_DRIVER_TERA,
>  	AZX_DRIVER_CTX,
>  	AZX_DRIVER_CTHDA,
> +	AZX_DRIVER_CMEDIA,
>  	AZX_DRIVER_GENERIC,
>  	AZX_NUM_DRIVERS, /* keep this as last entry */
>  };
> @@ -330,6 +331,7 @@ static char *driver_short_names[] = {
>  	[AZX_DRIVER_TERA] = "HDA Teradici", 
>  	[AZX_DRIVER_CTX] = "HDA Creative", 
>  	[AZX_DRIVER_CTHDA] = "HDA Creative",
> +	[AZX_DRIVER_CMEDIA] = "HDA C-Media",
>  	[AZX_DRIVER_GENERIC] = "HD-Audio Generic",
>  };
>  
> @@ -1373,6 +1375,7 @@ static void azx_check_snoop_available(struct
> azx *chip) snoop = false;
>  		break;
>  	case AZX_DRIVER_CTHDA:
> +	case AZX_DRIVER_CMEDIA:
>  		snoop = false;
>  		break;
>  	}
> @@ -2154,6 +2157,10 @@ static const struct pci_device_id azx_ids[] = {
>  	  .driver_data = AZX_DRIVER_CTX | AZX_DCAPS_CTX_WORKAROUND |
>  	  AZX_DCAPS_RIRB_PRE_DELAY | AZX_DCAPS_POSFIX_LPIB },
>  #endif
> +	/* CM8888 */
> +	{ PCI_DEVICE(0x13f6, 0x5011),
> +	  .driver_data = AZX_DRIVER_CMEDIA |
> +	  AZX_DCAPS_NO_MSI | AZX_DCAPS_POSFIX_LPIB },
>  	/* Vortex86MX */
>  	{ PCI_DEVICE(0x17f3, 0x3010), .driver_data =
> AZX_DRIVER_GENERIC }, /* VMware HDAudio */

Re: [PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Takashi Iwai]

At Wed, 29 Oct 2014 07:21:24 +1100,
Geoffrey McRae wrote:
> 
> I can confirm that disable of MSI is required as even Windows 7
> 64bit does not enable this for Intel HDA. Out of curiosity I altered
> the driver to use MSI-X with the same results, interrupts are never
> received even though the PCI caps state that MSI is supported.
> 
> Playback is very poor and stutters if I do not allow snoop by
> commenting out "case AZX_DRIVER_CMEDIA:" in azx_check_snoop_available.

OK, so we have some conflicting results.  Possibly we need non-cached
pages only for CORB/RIRB but leave the stream buffers?  Could you
check the patch below?


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index cfcca4c30d4d..118029d7c6de 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -366,14 +366,15 @@ static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool
 {
 	int pages;
 
-	if (azx_snoop(chip))
+	if (azx_snoop(chip) && chip->driver_type != AZX_DRIVER_CMEDIA)
 		return;
 	if (!dmab || !dmab->area || !dmab->bytes)
 		return;
-
 #ifdef CONFIG_SND_DMA_SGBUF
 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG) {
 		struct snd_sg_buf *sgbuf = dmab->private_data;
+		if (chip->driver_type == AZX_DRIVER_CMEDIA)
+			return;
 		if (on)
 			set_pages_array_wc(sgbuf->page_table, sgbuf->pages);
 		else
@@ -1376,7 +1377,6 @@ static void azx_check_snoop_available(struct azx *chip)
 		snoop = false;
 		break;
 	case AZX_DRIVER_CTHDA:
-	case AZX_DRIVER_CMEDIA:
 		snoop = false;
 		break;
 	}

Re: [PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Geoffrey McRae]

On Wed, 29 Oct 2014 08:50:53 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Wed, 29 Oct 2014 07:21:24 +1100,
> Geoffrey McRae wrote:
> > 
> > I can confirm that disable of MSI is required as even Windows 7
> > 64bit does not enable this for Intel HDA. Out of curiosity I altered
> > the driver to use MSI-X with the same results, interrupts are never
> > received even though the PCI caps state that MSI is supported.
> > 
> > Playback is very poor and stutters if I do not allow snoop by
> > commenting out "case AZX_DRIVER_CMEDIA:" in
> > azx_check_snoop_available.
> 
> OK, so we have some conflicting results.  Possibly we need non-cached
> pages only for CORB/RIRB but leave the stream buffers?  Could you
> check the patch below?
> 

The patch works fine, no errors reported, if I however disable snoop it
continues to stutter.

Re: [PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Takashi Iwai]

At Wed, 29 Oct 2014 23:13:15 +1100,
Geoffrey McRae wrote:
> 
> On Wed, 29 Oct 2014 08:50:53 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
> 
> > At Wed, 29 Oct 2014 07:21:24 +1100,
> > Geoffrey McRae wrote:
> > > 
> > > I can confirm that disable of MSI is required as even Windows 7
> > > 64bit does not enable this for Intel HDA. Out of curiosity I altered
> > > the driver to use MSI-X with the same results, interrupts are never
> > > received even though the PCI caps state that MSI is supported.
> > > 
> > > Playback is very poor and stutters if I do not allow snoop by
> > > commenting out "case AZX_DRIVER_CMEDIA:" in
> > > azx_check_snoop_available.
> > 
> > OK, so we have some conflicting results.  Possibly we need non-cached
> > pages only for CORB/RIRB but leave the stream buffers?  Could you
> > check the patch below?
> > 
> 
> The patch works fine, no errors reported, if I however disable snoop it
> continues to stutter.

OK, so the stuttering comes from the noncached pages of stream
buffers, not CORB/RIRB coherency.  I'll cook it up as a proper patch,
then.  Once when we confirm that the snoop for CORB/RIRB is really
superfluous, we can drop that hack, too.


thanks,

Takashi

Re: [PATCH 1/2] ALSA: hda - Fix probing and stuttering on CMI8888 HD-audio controller

[Takashi Iwai]

At Wed, 29 Oct 2014 14:01:20 +0100,
Takashi Iwai wrote:
> 
> At Wed, 29 Oct 2014 23:13:15 +1100,
> Geoffrey McRae wrote:
> > 
> > On Wed, 29 Oct 2014 08:50:53 +0100
> > Takashi Iwai <tiwai@suse.de> wrote:
> > 
> > > At Wed, 29 Oct 2014 07:21:24 +1100,
> > > Geoffrey McRae wrote:
> > > > 
> > > > I can confirm that disable of MSI is required as even Windows 7
> > > > 64bit does not enable this for Intel HDA. Out of curiosity I altered
> > > > the driver to use MSI-X with the same results, interrupts are never
> > > > received even though the PCI caps state that MSI is supported.
> > > > 
> > > > Playback is very poor and stutters if I do not allow snoop by
> > > > commenting out "case AZX_DRIVER_CMEDIA:" in
> > > > azx_check_snoop_available.
> > > 
> > > OK, so we have some conflicting results.  Possibly we need non-cached
> > > pages only for CORB/RIRB but leave the stream buffers?  Could you
> > > check the patch below?
> > > 
> > 
> > The patch works fine, no errors reported, if I however disable snoop it
> > continues to stutter.
> 
> OK, so the stuttering comes from the noncached pages of stream
> buffers, not CORB/RIRB coherency.  I'll cook it up as a proper patch,
> then.  Once when we confirm that the snoop for CORB/RIRB is really
> superfluous, we can drop that hack, too.

Hold on.  The previous patch was wrong.  My intention was to set wc
for CORB/RIRB but not for buffers, but the patch enabled snoop
wrongly for all.  Below is the fixed one.  It also covers the pgprot
setup for mmap, which is also relevant.

Please check whether it results in stuttering, too.  If yes, we just
need to enable snoop again.


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index cfcca4c30d4d..9ab1e631cb32 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -374,6 +374,8 @@ static void __mark_pages_wc(struct azx *chip, struct snd_dma_buffer *dmab, bool
 #ifdef CONFIG_SND_DMA_SGBUF
 	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG) {
 		struct snd_sg_buf *sgbuf = dmab->private_data;
+		if (chip->driver_type == AZX_DRIVER_CMEDIA)
+			return; /* deal with only CORB/RIRB buffers */
 		if (on)
 			set_pages_array_wc(sgbuf->page_table, sgbuf->pages);
 		else
@@ -1769,7 +1771,7 @@ static void pcm_mmap_prepare(struct snd_pcm_substream *substream,
 #ifdef CONFIG_X86
 	struct azx_pcm *apcm = snd_pcm_substream_chip(substream);
 	struct azx *chip = apcm->chip;
-	if (!azx_snoop(chip))
+	if (!azx_snoop(chip) && chip->driver_type != AZX_DRIVER_CMEDIA)
 		area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
 #endif
 }