Skip to contentSkip to conversation

ALSA: hda_controller: Separate stream_tag for input and output streams.

PATCH In review [email protected] 2 participants 4 messages Dec 18, 2014 Sound

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/2 ALSA: hda_controller: Separate stream_tag for input and output streams.

From: rfredzim <[email protected]>

Implemented separate stream_tag assignment for input and output streams. According to hda specification stream tag must be unique throughout the input streams group, however an output stream might use a stream tag which is already in use by an input stream. This change is necessary to support HW which provides a total of more than 15 stream DMA engines which with legacy implementation causes an overflow on SDxCTL.STRM field (and the whole SDxCTL register) and as a result usage of Reserved value 0 in the SDxCTL.STRM field which confuses HDA controller.

Signed-off-by rfredzim <[email protected]>
Signed-off-by Jayachandran B <[email protected]>
Signed-off-by Libin Yang <[email protected]>
Reviewed-by Vinod Koul <[email protected]>
---
 sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
 sound/pci/hda/hda_priv.h       |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)
sound/pci/hda/hda_controller.c +22 -2
index 8337645..dcca1ef 100644
@@ -1922,10 +1922,18 @@ int azx_mixer_create(struct azx *chip)
 EXPORT_SYMBOL_GPL(azx_mixer_create);
 
 
+static bool is_input_stream(struct azx *chip, unsigned char index)
+{
+	return (index >= chip->capture_index_offset &&
+		index < chip->capture_index_offset + chip->capture_streams);
+}
+
 /* initialize SD streams */
 int azx_init_stream(struct azx *chip)
 {
 	int i;
+	int in_stream_tag = 0;
+	int out_stream_tag = 0;
 
 	/* initialize each stream (aka device)
 	 * assign the starting bdl address to each stream (device)

@@ -1938,9 +1946,21 @@ int azx_init_stream(struct azx *chip)
 		azx_dev->sd_addr = chip->remap_addr + (0x20 * i + 0x80);
 		/* int mask: SDI0=0x01, SDI1=0x02, ... SDO3=0x80 */
 		azx_dev->sd_int_sta_mask = 1 << i;
-		/* stream tag: must be non-zero and unique */
 		azx_dev->index = i;
-		azx_dev->stream_tag = i + 1;
+
+		/* stream tag must be unique throughout
+		 * the stream direction group,
+		 * valid values 1...15
+		 * use separate stream tag if the flag
+		 * AZX_DCAPS_SEPARATE_STREAM_TAG is used
+		 */
+		if (chip->driver_caps & AZX_DCAPS_SEPARATE_STREAM_TAG)
+			azx_dev->stream_tag =
+				is_input_stream(chip, i) ?
+				++in_stream_tag :
+				++out_stream_tag;
+		else
+			azx_dev->stream_tag = i + 1;
 	}
 
 	return 0;
sound/pci/hda/hda_priv.h +1 -0
index aa484fd..166e3e8 100644
@@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_DCAPS_I915_POWERWELL (1 << 27)	/* HSW i915 powerwell support */
 #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears itself after reset */
 #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */
+#define AZX_DCAPS_SEPARATE_STREAM_TAG	(1 << 30) /* capture and playback use separate stream tag */
 
 enum {
 	AZX_SNOOP_TYPE_NONE ,
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 2/2 ALSA: hda_intel: apply the Seperate stream_tag for Skylake

From: Libin Yang <[email protected]>

The total stream number of Skylake's input and output stream exceeds 15, which will cause some streams do not work because of the overflow on SDxCTL.STRM field if using the legacy stream tag allocation method.

This patch uses the new stream tag allocation method by add the flag AZX_DCAPS_SEPARATE_STREAM_TAG for Skylake platform.

Signed-off-by Libin Yang <[email protected]>
Reviewed-by Vinod Koul <[email protected]>
---
 sound/pci/hda/hda_intel.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
sound/pci/hda/hda_intel.c +4 -1
index 5ac0d39..927daf4 100644
@@ -299,6 +299,9 @@ enum {
 	 AZX_DCAPS_PM_RUNTIME | AZX_DCAPS_I915_POWERWELL |\
 	 AZX_DCAPS_SNOOP_TYPE(SCH))
 
+#define AZX_DCAPS_INTEL_SKYLAKE \
+	(AZX_DCAPS_INTEL_PCH | AZX_DCAPS_SEPARATE_STREAM_TAG)
+
 /* quirks for ATI SB / AMD Hudson */
 #define AZX_DCAPS_PRESET_ATI_SB \
 	(AZX_DCAPS_NO_TCSEL | AZX_DCAPS_SYNC_WRITE | AZX_DCAPS_POSFIX_LPIB |\

@@ -2030,7 +2033,7 @@ static const struct pci_device_id azx_ids[] = {
 	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH },
 	/* Sunrise Point-LP */
 	{ PCI_DEVICE(0x8086, 0x9d70),
-	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH },
+	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
 	/* Haswell */
 	{ PCI_DEVICE(0x8086, 0x0a0c),
 	  .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },
Signature · 1 line
1.9.1
#2 lore raw
In reply to [email protected]

At Thu, 18 Dec 2014 14:13:50 +0800,

Quoted 12 lines · [email protected] wrote:

From: rfredzim <[email protected]>

Implemented separate stream_tag assignment for input and output streams. According to hda specification stream tag must be unique throughout the input streams group, however an output stream might use a stream tag which is already in use by an input stream. This change is necessary to support HW which provides a total of more than 15 stream DMA engines which with legacy implementation causes an overflow on SDxCTL.STRM field (and the whole SDxCTL register) and as a result usage of Reserved value 0 in the SDxCTL.STRM field which confuses HDA controller.

Quoted 2 lines
Signed-off-by rfredzim <[email protected]>
Signed-off-by Jayachandran B <[email protected]>

The from and sign-off tags must contain the real names. The code change itself looks OK.

thanks,

Takashi

Quoted 69 lines
Signed-off-by Libin Yang <[email protected]>
Reviewed-by Vinod Koul <[email protected]>
---
 sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
 sound/pci/hda/hda_priv.h       |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)
sound/pci/hda/hda_controller.c +22 -2
index 8337645..dcca1ef 100644
@@ -1922,10 +1922,18 @@ int azx_mixer_create(struct azx *chip)
 EXPORT_SYMBOL_GPL(azx_mixer_create);
 
 
+static bool is_input_stream(struct azx *chip, unsigned char index)
+{
+	return (index >= chip->capture_index_offset &&
+		index < chip->capture_index_offset + chip->capture_streams);
+}
+
 /* initialize SD streams */
 int azx_init_stream(struct azx *chip)
 {
 	int i;
+	int in_stream_tag = 0;
+	int out_stream_tag = 0;
 
 	/* initialize each stream (aka device)
 	 * assign the starting bdl address to each stream (device)

@@ -1938,9 +1946,21 @@ int azx_init_stream(struct azx *chip)
 		azx_dev->sd_addr = chip->remap_addr + (0x20 * i + 0x80);
 		/* int mask: SDI0=0x01, SDI1=0x02, ... SDO3=0x80 */
 		azx_dev->sd_int_sta_mask = 1 << i;
-		/* stream tag: must be non-zero and unique */
 		azx_dev->index = i;
-		azx_dev->stream_tag = i + 1;
+
+		/* stream tag must be unique throughout
+		 * the stream direction group,
+		 * valid values 1...15
+		 * use separate stream tag if the flag
+		 * AZX_DCAPS_SEPARATE_STREAM_TAG is used
+		 */
+		if (chip->driver_caps & AZX_DCAPS_SEPARATE_STREAM_TAG)
+			azx_dev->stream_tag =
+				is_input_stream(chip, i) ?
+				++in_stream_tag :
+				++out_stream_tag;
+		else
+			azx_dev->stream_tag = i + 1;
 	}
 
 	return 0;
sound/pci/hda/hda_priv.h +1 -0
index aa484fd..166e3e8 100644
@@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_DCAPS_I915_POWERWELL (1 << 27)	/* HSW i915 powerwell support */
 #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears itself after reset */
 #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */
+#define AZX_DCAPS_SEPARATE_STREAM_TAG	(1 << 30) /* capture and playback use separate stream tag */
 
 enum {
 	AZX_SNOOP_TYPE_NONE ,
Signature · 1 line
1.9.1
author
#3 lore raw

Hi Takashi,

Thanks for review. I will send another email to update the signed-off name.

Regards,
Libin

Quoted 108 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, December 18, 2014 4:05 PM
To: Yang, Libin
Cc: [email protected]; Redzimski, Rafal F; B, Jayachandran; Koul, Vinod
Subject: Re: [PATCH 1/2] ALSA: hda_controller: Separate stream_tag for input and output streams.

At Thu, 18 Dec 2014 14:13:50 +0800,

Quoted 4 lines · [email protected] wrote:

From: rfredzim <[email protected]>

Implemented separate stream_tag assignment for input and output

streams.

Quoted 7 lines

According to hda specification stream tag must be unique throughout the input streams group, however an output stream might use a stream tag which is already in use by an input stream. This change is necessary to support HW which provides a total of more than 15 stream DMA engines which with legacy implementation causes an overflow on SDxCTL.STRM field (and the whole SDxCTL register) and as a result usage of Reserved value 0 in the SDxCTL.STRM field which confuses HDA

controller.

Quoted 3 lines
Signed-off-by rfredzim <[email protected]>
Signed-off-by Jayachandran B <[email protected]>

The from and sign-off tags must contain the real names. The code change itself looks OK.

thanks,

Takashi

Quoted 33 lines
Signed-off-by Libin Yang <[email protected]>
Reviewed-by Vinod Koul <[email protected]>
---
 sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
 sound/pci/hda/hda_priv.h       |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/sound/pci/hda/hda_controller.c index 8337645..dcca1ef 100644

sound/pci/hda/hda_controller.c +6 -2
@@ -1922,10 +1922,18 @@ int azx_mixer_create(struct azx *chip)
 EXPORT_SYMBOL_GPL(azx_mixer_create);
 
 
+static bool is_input_stream(struct azx *chip, unsigned char index) {
+	return (index >= chip->capture_index_offset &&
+		index < chip->capture_index_offset + chip-
 capture_streams); }
+
 /* initialize SD streams */
 int azx_init_stream(struct azx *chip)  {
 	int i;
+	int in_stream_tag = 0;
+	int out_stream_tag = 0;
 
 	/* initialize each stream (aka device)
 	 * assign the starting bdl address to each stream (device) @@
-1938,9 +1946,21 @@ int azx_init_stream(struct azx *chip)
 		azx_dev->sd_addr = chip->remap_addr + (0x20 * i + 0x80);
 		/* int mask: SDI0=0x01, SDI1=0x02, ... SDO3=0x80 */
 		azx_dev->sd_int_sta_mask = 1 << i;
-		/* stream tag: must be non-zero and unique */

sound/pci/hda/hda_controller.c int azx_mixer_create(struct azx *chip)

Quoted 10 lines
sound/pci/hda/hda_controller.c +8 -1
 		azx_dev->index = i;
-		azx_dev->stream_tag = i + 1;
+
+		/* stream tag must be unique throughout
+		 * the stream direction group,
+		 * valid values 1...15
+		 * use separate stream tag if the flag
+		 * AZX_DCAPS_SEPARATE_STREAM_TAG is used
+		 */
+		if (chip->driver_caps &

AZX_DCAPS_SEPARATE_STREAM_TAG)

sound/pci/hda/hda_priv.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

Quoted 14 lines
sound/pci/hda/hda_controller.c +6 -0
+			azx_dev->stream_tag =
+				is_input_stream(chip, i) ?
+				++in_stream_tag :
+				++out_stream_tag;
+		else
+			azx_dev->stream_tag = i + 1;
 	}
 
 	return 0;
sound/pci/hda/hda_priv.h index renamed from sound/pci/hda/hda_priv.h +0 -0

aa484fd..166e3e8 100644

sound/pci/hda/hda_priv.h +0 -0
@@ -171,6 +171,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2,

SDO3 };

sound/pci/hda/hda_priv.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, · line 171

Quoted 1 line
sound/pci/hda/hda_priv.h +0 -0
 #define AZX_DCAPS_I915_POWERWELL (1 << 27)	/* HSW i915

powerwell support */

sound/pci/hda/hda_priv.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, · line 172

Quoted 1 line
sound/pci/hda/hda_priv.h +0 -0
 #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears

itself after reset */

sound/pci/hda/hda_priv.h enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, · line 174

Quoted 2 lines
sound/pci/hda/hda_priv.h +1 -0
 #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */
+#define AZX_DCAPS_SEPARATE_STREAM_TAG	(1 << 30) /* capture

and playback use separate stream tag */

Quoted 6 lines
sound/pci/hda/hda_priv.h +0 -1
 enum {
 	AZX_SNOOP_TYPE_NONE ,
--

1.9.1