diff mbox

ALSA: hda: separate stream tag for playback and capture

Message ID 1416306410-25073-1-git-send-email-vinod.koul@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vinod Koul Nov. 18, 2014, 10:26 a.m. UTC
From: Rafal Redzimski <rafal.f.redzimski@intel.com>

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.

Newer Intel HW has the capability to provides a total of more than
15 stream DMA engines which with the current implementation causes an overflow on
STRM field (and the whole register) which results in usage of
reserved value 0 in the STRM field which confuses HDA controller.
So make stream_tag assignment separate for input and output streams

Signed-off-by: Rafal Redzimski <rafal.f.redzimski@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/pci/hda/hda_controller.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

Comments

Takashi Iwai Nov. 18, 2014, 10:36 a.m. UTC | #1
At Tue, 18 Nov 2014 15:56:50 +0530,
Vinod Koul wrote:
> 
> From: Rafal Redzimski <rafal.f.redzimski@intel.com>
> 
> 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.

It's "might" and it has never been clear in the old specification,
AFAIK.

> Newer Intel HW has the capability to provides a total of more than
> 15 stream DMA engines which with the current implementation causes an overflow on
> STRM field (and the whole register) which results in usage of
> reserved value 0 in the STRM field which confuses HDA controller.
> So make stream_tag assignment separate for input and output streams

I'm afraid this will break the old controllers.  Even the order of
stream assignment (whether playback is first or capture is first)
broke the driver in the past.

So, please make this change somehow dependent on the chip, e.g. adding
a new flag in struct azx.

Also...

> Signed-off-by: Rafal Redzimski <rafal.f.redzimski@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/pci/hda/hda_controller.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
> index 8337645..703ac6e 100644
> --- a/sound/pci/hda/hda_controller.c
> +++ b/sound/pci/hda/hda_controller.c
> @@ -1921,11 +1921,17 @@ 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->playback_index_offset;
> +}

We don't define which direction comes first.
The code must be a range check like:
	return index >= chip->capture_index_offset &&
	       index < chip->capture_index_offset + chip->capture_streams;


Takashi

>  /* 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 +1944,13 @@ 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
> +		 */
> +		azx_dev->stream_tag = is_input_stream(chip, i) ?
> +			++in_stream_tag : ++out_stream_tag;
>  	}
>  
>  	return 0;
> -- 
> 1.7.0.4
>
diff mbox

Patch

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 8337645..703ac6e 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -1921,11 +1921,17 @@  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->playback_index_offset;
+}
 
 /* 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 +1944,13 @@  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
+		 */
+		azx_dev->stream_tag = is_input_stream(chip, i) ?
+			++in_stream_tag : ++out_stream_tag;
 	}
 
 	return 0;