audio: Fix playback noise with 24/32 bit sample size on BXT

PATCH Applied [email protected] 2 participants 5 messages Dec 7, 2015 – Dec 8, 2015 Sound

lore

Share & tools

lore

[email protected]

author

2015-12-07 07:59 UTC #0 lore raw

PATCH v2 1/1 audio: Fix playback noise with 24/32 bit sample size on BXT

From: "Lu, Han" <[email protected]>

In BXT-P A0, HD-Audio DMA requests is later than expected, and makes an audio stream sensitive to system latencies when 24/32 bits are playing.
Adjusting threshold of DMA fifo to force the DMA request sooner to improve latency tolerance at the expense of power.

v2: move Intel specific code to hda_intel.c

Signed-off-by Lu, Han <[email protected]>

include/sound/hda_register.h +3 -0

index 2ae8812..94dc6a9 100644
@@ -93,6 +93,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_REG_HSW_EM4			0x100c
 #define AZX_REG_HSW_EM5			0x1010
 
+/* Skylake/Broxton display HD-A controller Extended Mode registers */
+#define AZX_REG_SKL_EM4L		0x1040
+
 /* PCI space */
 #define AZX_PCIREG_TCSEL		0x44

sound/pci/hda/hda_intel.c +23 -0

index 963f824..bff5c8b 100644
@@ -355,6 +355,8 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))
 
+#define IS_BROXTON(pci)	((pci)->device == 0x5a98)
+
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH",

@@ -506,15 +508,36 @@ static void azx_init_pci(struct azx *chip)
         }
 }
 
+/*
+ * In BXT-P A0, HD-Audio DMA requests is later than expected,
+ * and makes an audio stream sensitive to system latencies when
+ * 24/32 bits are playing.
+ * Adjusting threshold of DMA fifo to force the DMA request
+ * sooner to improve latency tolerance at the expense of power.
+ */
+static void bxt_reduce_dma_latency(struct azx *chip)
+{
+	u32 val;
+
+	val = azx_readl(chip, SKL_EM4L);
+	val &= (0x3 << 20);
+	azx_writel(chip, SKL_EM4L, val);
+}
+
 static void hda_intel_init_chip(struct azx *chip, bool full_reset)
 {
 	struct hdac_bus *bus = azx_bus(chip);
+	struct pci_dev *pci = chip->pci;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
 	azx_init_chip(chip, full_reset);
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, false);
+
+	/* reduce dma latency to avoid noise */
+	if (IS_BROXTON(pci))
+		bxt_reduce_dma_latency(chip);
 }
 
 /* calculate runtime delay from LPIB */

Signature · 1 line

2.5.0

Takashi Iwai

Applied

2015-12-07 08:05 UTC #1 lore raw

Quoted 9 lines · On Mon, 07 Dec 2015 08:59:13 +0100, [email protected] wrote:

From: "Lu, Han" <[email protected]>
In BXT-P A0, HD-Audio DMA requests is later than expected, and makes an audio stream sensitive to system latencies when 24/32 bits are playing.
Adjusting threshold of DMA fifo to force the DMA request sooner to improve latency tolerance at the expense of power.

Quoted 3 lines

v2: move Intel specific code to hda_intel.c
Signed-off-by Lu, Han <[email protected]>

Applied, thanks.

Takashi

Quoted 68 lines

include/sound/hda_register.h +3 -0

index 2ae8812..94dc6a9 100644
@@ -93,6 +93,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_REG_HSW_EM4			0x100c
 #define AZX_REG_HSW_EM5			0x1010
 
+/* Skylake/Broxton display HD-A controller Extended Mode registers */
+#define AZX_REG_SKL_EM4L		0x1040
+
 /* PCI space */
 #define AZX_PCIREG_TCSEL		0x44

sound/pci/hda/hda_intel.c +23 -0

index 963f824..bff5c8b 100644
@@ -355,6 +355,8 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))
 
+#define IS_BROXTON(pci)	((pci)->device == 0x5a98)
+
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH",

@@ -506,15 +508,36 @@ static void azx_init_pci(struct azx *chip)
         }
 }
 
+/*
+ * In BXT-P A0, HD-Audio DMA requests is later than expected,
+ * and makes an audio stream sensitive to system latencies when
+ * 24/32 bits are playing.
+ * Adjusting threshold of DMA fifo to force the DMA request
+ * sooner to improve latency tolerance at the expense of power.
+ */
+static void bxt_reduce_dma_latency(struct azx *chip)
+{
+	u32 val;
+
+	val = azx_readl(chip, SKL_EM4L);
+	val &= (0x3 << 20);
+	azx_writel(chip, SKL_EM4L, val);
+}
+
 static void hda_intel_init_chip(struct azx *chip, bool full_reset)
 {
 	struct hdac_bus *bus = azx_bus(chip);
+	struct pci_dev *pci = chip->pci;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
 	azx_init_chip(chip, full_reset);
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, false);
+
+	/* reduce dma latency to avoid noise */
+	if (IS_BROXTON(pci))
+		bxt_reduce_dma_latency(chip);
 }
 
 /* calculate runtime delay from LPIB */

Signature · 1 line

2.5.0

Lu, Han

author

2015-12-08 00:35 UTC #2 lore raw

Quoted 20 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, December 7, 2015 4:06 PM
To: Lu, Han
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V2 1/1] audio: Fix playback noise with 24/32 bit sample size on BXT
Quoted 9 lines · On Mon, 07 Dec 2015 08:59:13 +0100, [email protected] wrote:
From: "Lu, Han" <[email protected]>
In BXT-P A0, HD-Audio DMA requests is later than expected, and makes an audio stream sensitive to system latencies when
24/32 bits are playing.
Adjusting threshold of DMA fifo to force the DMA request sooner to improve latency tolerance at the expense of power.

Quoted 9 lines

Quoted 3 lines
v2: move Intel specific code to hda_intel.c
Signed-off-by Lu, Han <[email protected]>
Applied, thanks.
Takashi

Thanks!

One more thing, I was tried to implement this patch by define a macro AZX_DCAPS_XXX instead of IS_BROXTON(pci), but it looks like no spare bit left. (bit 22 and bit 31 are reserved bits, right?) Is there a plan to extend the caps structure?

BR,
Han

Quoted 67 lines

include/sound/hda_register.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

Quoted 6 lines

(quoted diff) +0 -0
b/include/sound/hda_register.h index 2ae8812..94dc6a9 100644
include/sound/hda_register.h +0 -0
@@ -93,6 +93,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

SDO3 };

Quoted 60 lines

include/sound/hda_register.h +3 -0

 #define AZX_REG_HSW_EM4			0x100c
 #define AZX_REG_HSW_EM5			0x1010
 
+/* Skylake/Broxton display HD-A controller Extended Mode registers */
+#define AZX_REG_SKL_EM4L		0x1040
+
 /* PCI space */
 #define AZX_PCIREG_TCSEL		0x44

sound/pci/hda/hda_intel.c +22 -1

index 963f824..bff5c8b 100644
@@ -355,6 +355,8 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))
 
+#define IS_BROXTON(pci)	((pci)->device == 0x5a98)
+
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH",

@@ -506,15 +508,36 @@ static void azx_init_pci(struct azx *chip)
         }
 }
 
+/*
+ * In BXT-P A0, HD-Audio DMA requests is later than expected,
+ * and makes an audio stream sensitive to system latencies when
+ * 24/32 bits are playing.
+ * Adjusting threshold of DMA fifo to force the DMA request
+ * sooner to improve latency tolerance at the expense of power.
+ */
+static void bxt_reduce_dma_latency(struct azx *chip) {
+	u32 val;
+
+	val = azx_readl(chip, SKL_EM4L);
+	val &= (0x3 << 20);
+	azx_writel(chip, SKL_EM4L, val);
+}
+
 static void hda_intel_init_chip(struct azx *chip, bool full_reset)  {
 	struct hdac_bus *bus = azx_bus(chip);
+	struct pci_dev *pci = chip->pci;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
 	azx_init_chip(chip, full_reset);
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, false);
+
+	/* reduce dma latency to avoid noise */
+	if (IS_BROXTON(pci))
+		bxt_reduce_dma_latency(chip);
 }
 
 /* calculate runtime delay from LPIB */
--

2.5.0

Takashi Iwai

2015-12-08 06:24 UTC #3 lore raw

Quoted 31 lines · Han

Quoted 21 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, December 7, 2015 4:06 PM
To: Lu, Han
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V2 1/1] audio: Fix playback noise with 24/32 bit sample size on BXT
Quoted 9 lines · On Mon, 07 Dec 2015 08:59:13 +0100, [email protected] wrote:
From: "Lu, Han" <[email protected]>
In BXT-P A0, HD-Audio DMA requests is later than expected, and makes an audio stream sensitive to system latencies when
24/32 bits are playing.
Adjusting threshold of DMA fifo to force the DMA request sooner to improve latency tolerance at the expense of power.
Quoted 9 lines
Quoted 3 lines
v2: move Intel specific code to hda_intel.c
Signed-off-by Lu, Han <[email protected]>
Applied, thanks.
Takashi

Quoted 6 lines

Thanks!
One more thing, I was tried to implement this patch by define a macro AZX_DCAPS_XXX instead of IS_BROXTON(pci), but it looks like no spare bit left. (bit 22 and bit 31 are reserved bits, right?) Is there a plan to extend the caps structure?

Currently not. If the check is once-off and be in hda_intel.c, extending DCAPS is too much.

thanks,

Takashi

Quoted 72 lines

BR,
Han

Quoted 67 lines

include/sound/hda_register.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

Quoted 6 lines

(quoted diff) +0 -0
b/include/sound/hda_register.h index 2ae8812..94dc6a9 100644
include/sound/hda_register.h +0 -0
@@ -93,6 +93,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

SDO3 };

Quoted 60 lines

include/sound/hda_register.h +3 -0

 #define AZX_REG_HSW_EM4			0x100c
 #define AZX_REG_HSW_EM5			0x1010
 
+/* Skylake/Broxton display HD-A controller Extended Mode registers */
+#define AZX_REG_SKL_EM4L		0x1040
+
 /* PCI space */
 #define AZX_PCIREG_TCSEL		0x44

sound/pci/hda/hda_intel.c +22 -1

index 963f824..bff5c8b 100644
@@ -355,6 +355,8 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))
 
+#define IS_BROXTON(pci)	((pci)->device == 0x5a98)
+
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH",

@@ -506,15 +508,36 @@ static void azx_init_pci(struct azx *chip)
         }
 }
 
+/*
+ * In BXT-P A0, HD-Audio DMA requests is later than expected,
+ * and makes an audio stream sensitive to system latencies when
+ * 24/32 bits are playing.
+ * Adjusting threshold of DMA fifo to force the DMA request
+ * sooner to improve latency tolerance at the expense of power.
+ */
+static void bxt_reduce_dma_latency(struct azx *chip) {
+	u32 val;
+
+	val = azx_readl(chip, SKL_EM4L);
+	val &= (0x3 << 20);
+	azx_writel(chip, SKL_EM4L, val);
+}
+
 static void hda_intel_init_chip(struct azx *chip, bool full_reset)  {
 	struct hdac_bus *bus = azx_bus(chip);
+	struct pci_dev *pci = chip->pci;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
 	azx_init_chip(chip, full_reset);
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, false);
+
+	/* reduce dma latency to avoid noise */
+	if (IS_BROXTON(pci))
+		bxt_reduce_dma_latency(chip);
 }
 
 /* calculate runtime delay from LPIB */
--

2.5.0

Lu, Han

author

2015-12-08 06:27 UTC #4 lore raw

Quoted 44 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Tuesday, December 8, 2015 2:25 PM
To: Lu, Han
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V2 1/1] audio: Fix playback noise with 24/32 bit sample size on BXT
Quoted 33 lines · Han
Quoted 21 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Monday, December 7, 2015 4:06 PM
To: Lu, Han
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V2 1/1] audio: Fix playback noise with 24/32 bit sample size on BXT
Quoted 9 lines · On Mon, 07 Dec 2015 08:59:13 +0100, [email protected] wrote:
From: "Lu, Han" <[email protected]>
In BXT-P A0, HD-Audio DMA requests is later than expected, and makes an audio stream sensitive to system latencies when 24/32 bits are playing.
Adjusting threshold of DMA fifo to force the DMA request sooner to improve latency tolerance at the expense of power.
Quoted 9 lines
Quoted 3 lines
v2: move Intel specific code to hda_intel.c
Signed-off-by Lu, Han <[email protected]>
Applied, thanks.
Takashi
Thanks!

Quoted 8 lines

Quoted 4 lines
One more thing, I was tried to implement this patch by define a macro AZX_DCAPS_XXX instead of IS_BROXTON(pci), but it looks like no spare bit left. (bit 22 and bit 31 are reserved bits, right?) Is there a plan to extend the caps structure?
Currently not. If the check is once-off and be in hda_intel.c, extending DCAPS is too much.

Understood. Thanks.

BR,
Han

Quoted 78 lines

thanks,

Takashi

Quoted 15 lines

BR,
Han
Quoted 11 lines
include/sound/hda_register.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
Quoted 6 lines
(quoted diff) +0 -0
b/include/sound/hda_register.h index 2ae8812..94dc6a9 100644
include/sound/hda_register.h +0 -0
@@ -93,6 +93,9 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,
SDO3 };
Quoted 4 lines
include/sound/hda_register.h +1 -0
 #define AZX_REG_HSW_EM4			0x100c
 #define AZX_REG_HSW_EM5			0x1010
 
+/* Skylake/Broxton display HD-A controller Extended Mode registers

Quoted 13 lines

include/sound/hda_register.h +2 -0

+#define AZX_REG_SKL_EM4L		0x1040
+
 /* PCI space */
 #define AZX_PCIREG_TCSEL		0x44

sound/pci/hda/hda_intel.c +0 -0

index 963f824..bff5c8b 100644
@@ -355,6 +355,8 @@ enum {
 					((pci)->device == 0x0d0c) || \
 					((pci)->device == 0x160c))

Quoted 5 lines

sound/pci/hda/hda_intel.c enum {

Quoted 5 lines

sound/pci/hda/hda_intel.c +2 -0

+#define IS_BROXTON(pci)	((pci)->device == 0x5a98)
+
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
 	[AZX_DRIVER_PCH] = "HDA Intel PCH", @@ -506,15 +508,36 @@

static

Quoted 38 lines

Quoted 37 lines

void azx_init_pci(struct azx *chip)

}

sound/pci/hda/hda_intel.c +19 -1

+/*
+ * In BXT-P A0, HD-Audio DMA requests is later than expected,
+ * and makes an audio stream sensitive to system latencies when
+ * 24/32 bits are playing.
+ * Adjusting threshold of DMA fifo to force the DMA request
+ * sooner to improve latency tolerance at the expense of power.
+ */
+static void bxt_reduce_dma_latency(struct azx *chip) {
+	u32 val;
+
+	val = azx_readl(chip, SKL_EM4L);
+	val &= (0x3 << 20);
+	azx_writel(chip, SKL_EM4L, val); }
+
 static void hda_intel_init_chip(struct azx *chip, bool full_reset)  {
 	struct hdac_bus *bus = azx_bus(chip);
+	struct pci_dev *pci = chip->pci;
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, true);
 	azx_init_chip(chip, full_reset);
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_hdac_set_codec_wakeup(bus, false);
+
+	/* reduce dma latency to avoid noise */
+	if (IS_BROXTON(pci))
+		bxt_reduce_dma_latency(chip);
 }
 
 /* calculate runtime delay from LPIB */
--

2.5.0