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

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

[libin.yang]

From: rfredzim <rafal.f.redzimski@intel.com>

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 <rafal.f.redzimski@intel.com>
Signed-off-by: Jayachandran B <jayachandran.b@intel.com>
Signed-off-by: Libin Yang <libin.yang@intel.com>
Reviewed-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
 sound/pci/hda/hda_priv.h       |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 8337645..dcca1ef 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -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;
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index aa484fd..166e3e8 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -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 ,
-- 
1.9.1

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

[libin.yang]

From: Libin Yang <libin.yang@intel.com>

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 <libin.yang@intel.com>
Reviewed-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/pci/hda/hda_intel.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 5ac0d39..927daf4 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -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 },
-- 
1.9.1

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

[Takashi Iwai]

At Thu, 18 Dec 2014 14:13:50 +0800,
libin.yang@intel.com wrote:
> 
> From: rfredzim <rafal.f.redzimski@intel.com>
> 
> 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 <rafal.f.redzimski@intel.com>
> Signed-off-by: Jayachandran B <jayachandran.b@intel.com>

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


thanks,

Takashi

> Signed-off-by: Libin Yang <libin.yang@intel.com>
> Reviewed-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
>  sound/pci/hda/hda_priv.h       |  1 +
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
> index 8337645..dcca1ef 100644
> --- a/sound/pci/hda/hda_controller.c
> +++ b/sound/pci/hda/hda_controller.c
> @@ -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;
> diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
> index aa484fd..166e3e8 100644
> --- a/sound/pci/hda/hda_priv.h
> +++ b/sound/pci/hda/hda_priv.h
> @@ -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 ,
> -- 
> 1.9.1
> 

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

[Yang, Libin]

Hi Takashi,

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

Regards,
Libin


> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, December 18, 2014 4:05 PM
> To: Yang, Libin
> Cc: alsa-devel@alsa-project.org; 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,
> libin.yang@intel.com wrote:
> >
> > From: rfredzim <rafal.f.redzimski@intel.com>
> >
> > 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 <rafal.f.redzimski@intel.com>
> > Signed-off-by: Jayachandran B <jayachandran.b@intel.com>
> 
> The from and sign-off tags must contain the real names.
> The code change itself looks OK.
> 
> 
> thanks,
> 
> Takashi
> 
> > Signed-off-by: Libin Yang <libin.yang@intel.com>
> > Reviewed-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
> >  sound/pci/hda/hda_priv.h       |  1 +
> >  2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_controller.c
> > b/sound/pci/hda/hda_controller.c index 8337645..dcca1ef 100644
> > --- a/sound/pci/hda/hda_controller.c
> > +++ b/sound/pci/hda/hda_controller.c
> > @@ -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;
> > diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h index
> > aa484fd..166e3e8 100644
> > --- a/sound/pci/hda/hda_priv.h
> > +++ b/sound/pci/hda/hda_priv.h
> > @@ -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 ,
> > --
> > 1.9.1
> >

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

[libin.yang]

From: Rafal Redzimski <rafal.f.redzimski@intel.com>

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: Rafal Redzimski <rafal.f.redzimski@intel.com>
Signed-off-by: Jayachandran B <jayachandran.b@intel.com>
Signed-off-by: Libin Yang <libin.yang@intel.com>
Reviewed-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/pci/hda/hda_controller.c | 24 ++++++++++++++++++++++--
 sound/pci/hda/hda_priv.h       |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 8337645..dcca1ef 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -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;
diff --git a/sound/pci/hda/hda_priv.h b/sound/pci/hda/hda_priv.h
index aa484fd..166e3e8 100644
--- a/sound/pci/hda/hda_priv.h
+++ b/sound/pci/hda/hda_priv.h
@@ -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 ,
-- 
1.9.1

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

[libin.yang]

From: Libin Yang <libin.yang@intel.com>

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 <libin.yang@intel.com>
Reviewed-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/pci/hda/hda_intel.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 5ac0d39..927daf4 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -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 },
-- 
1.9.1